Set the ' mark after following a link.

This moves mark storage from CommandDispatcher to TabbedBrowser, so it can also
be accessed by the HintManager.

qutebrowser/browser/commands.py

@@ -46,6 +46,7 @@ from qutebrowser.utils import (message, usertypes, log, qtutils, urlutils,
 from qutebrowser.utils.usertypes import KeyMode
 from qutebrowser.misc import editor, guiprocess
 from qutebrowser.completion.models import instances, sortfilter
+from qutebrowser.mainwindow.tabbedbrowser import MarkNotSetError
 
 
 class CommandDispatcher:
@@ -62,15 +63,11 @@ class CommandDispatcher:
         _editor: The ExternalEditor object.
         _win_id: The window ID the CommandDispatcher is associated with.
         _tabbed_browser: The TabbedBrowser used.
-        _local_marks: Jump markers local to each page
-        _global_marks: Jump markers used across all pages
     """
 
     def __init__(self, win_id, tabbed_browser):
         self._win_id = win_id
         self._tabbed_browser = tabbed_browser
-        self._local_marks = {}
-        self._global_marks = {}
 
     def __repr__(self):
         return utils.get_repr(self)
@@ -1896,18 +1893,7 @@ class CommandDispatcher:
             key: mark identifier; capital indicates a global mark
         """
         # consider urls that differ only in fragment to be identical
-        url = self._current_url().adjusted(QUrl.RemoveFragment)
+        self._tabbed_browser.set_mark(key)
-        y = self._current_y_px()
-
-        if key.isupper():
-            # this is a global mark, so store the position and url
-            # since we are already storing the scroll, strip the fragment as it
-            # might interfere with our scrolling
-            self._global_marks[key] = y, url
-        else:
-            if url not in self._local_marks:
-                self._local_marks[url] = {}
-            self._local_marks[url][key] = y
 
     @cmdutils.register(instance='command-dispatcher', scope='window')
     def jump_mark(self, key):
@@ -1916,13 +1902,19 @@ class CommandDispatcher:
         Args:
             key: mark identifier; capital indicates a global mark
         """
-        # consider urls that differ only in fragment to be identical
+        try:
-        urlkey = self._current_url().adjusted(QUrl.RemoveFragment)
+            y, url = self._tabbed_browser.get_mark(key)
-
+        except MarkNotSetError as e:
-        if key.isupper() and key in self._global_marks:
+            message.error(self._win_id, str(e))
-            # y is a pixel position relative to the top of the page
+            return
-            y, url = self._global_marks[key]
 
+        if url is None:
+            # save the pre-jump position in the special ' mark
+            # this has to happen after we read the mark, otherwise jump_mark
+            # "'" would just jump to the current position every time
+            self.set_mark("'")
+            self._scroll_px_absolute(y)
+        else:
             def callback(ok):
                 if ok:
                     self._tabbed_browser.cur_load_finished.disconnect(callback)
@@ -1930,22 +1922,6 @@ class CommandDispatcher:
 
             self.openurl(url.toString())
             self._tabbed_browser.cur_load_finished.connect(callback)
-        elif urlkey in self._local_marks and key in self._local_marks[urlkey]:
-            y = self._local_marks[urlkey][key]
-
-            # save the pre-jump position in the special ' mark
-            # this has to happen after we read the mark, otherwise jump_mark
-            # "'" would just jump to the current position every time
-            self.set_mark("'")
-
-            self._scroll_px_absolute(y)
-        else:
-            message.error(self._win_id, "Mark {} is not set".format(key))
-
-    def _current_y_px(self):
-        """Return the current y scroll position in pixels from the top."""
-        frame = self._current_widget().page().currentFrame()
-        return frame.scrollPosition().y()
 
     def _scroll_px_absolute(self, y):
         """Scroll to the position y pixels from the top of the page."""

qutebrowser/browser/hints.py

@@ -921,6 +921,10 @@ class HintManager(QObject):
                           immediately=True)
             return
         if self._context.target in elem_handlers:
+            # Set the pre-jump mark ', so we can jump back here after following
+            tabbed_browser = objreg.get('tabbed-browser', scope='window',
+                                        window=self._win_id)
+            tabbed_browser.set_mark("'")
             handler = functools.partial(
                 elem_handlers[self._context.target], elem, self._context)
         elif self._context.target in url_handlers:

qutebrowser/mainwindow/tabbedbrowser.py

@@ -41,6 +41,11 @@ class TabDeletedError(Exception):
     """Exception raised when _tab_index is called for a deleted tab."""
 
 
+class MarkNotSetError(Exception):
+
+    """Exception raised when _tab_index is called for a deleted tab."""
+
+
 class TabbedBrowser(tabwidget.TabWidget):
 
     """A TabWidget with QWebViews inside.
@@ -64,6 +69,8 @@ class TabbedBrowser(tabwidget.TabWidget):
         _tab_insert_idx_right: Same as above, for 'right'.
         _undo_stack: List of UndoEntry namedtuples of closed tabs.
         shutting_down: Whether we're currently shutting down.
+        _local_marks: Jump markers local to each page
+        _global_marks: Jump markers used across all pages
 
     Signals:
         cur_progress: Progress of the current tab changed (loadProgress).
@@ -114,6 +121,8 @@ class TabbedBrowser(tabwidget.TabWidget):
         self._now_focused = None
         self.search_text = None
         self.search_flags = 0
+        self._local_marks = {}
+        self._global_marks = {}
         objreg.get('config').changed.connect(self.update_favicons)
         objreg.get('config').changed.connect(self.update_window_title)
         objreg.get('config').changed.connect(self.update_tab_titles)
@@ -637,3 +646,41 @@ class TabbedBrowser(tabwidget.TabWidget):
             self._now_focused.wheelEvent(e)
         else:
             e.ignore()
+
+    def set_mark(self, key):
+        """Set a mark at the current scroll position in the current tab.
+
+        Args:
+            key: mark identifier; capital indicates a global mark
+        """
+        # strip the fragment as it may interfere with scrolling
+        url = self.current_url().adjusted(QUrl.RemoveFragment)
+        frame = self.currentWidget().page().currentFrame()
+        y = frame.scrollPosition().y()
+
+        if key.isupper():
+            self._global_marks[key] = y, url
+        else:
+            if url not in self._local_marks:
+                self._local_marks[url] = {}
+            self._local_marks[url][key] = y
+
+    def get_mark(self, key):
+        """Retrieve the mark named by `key` as (y, url).
+
+        For a local mark, url is None
+        Returns None if the mark is not set.
+
+        Args:
+            key: mark identifier; capital indicates a global mark
+        """
+        # consider urls that differ only in fragment to be identical
+        urlkey = self.current_url().adjusted(QUrl.RemoveFragment)
+
+        if key.isupper() and key in self._global_marks:
+            # y is a pixel position relative to the top of the page
+            return self._global_marks[key]
+        elif urlkey in self._local_marks and key in self._local_marks[urlkey]:
+            return self._local_marks[urlkey][key], None
+        else:
+            raise MarkNotSetError("Mark {} is not set".format(key))

tests/integration/features/marks.feature

@@ -63,3 +63,9 @@ Feature: Setting positional marks
         When I open data/marks.html#bottom
         And I run :jump-mark 'a'
         Then the page should be scrolled to 10
+
+    Scenario: Jumping back after following a link
+        When I run :hint links normal
+        And I run :follow-hint s
+        And I run :jump-mark "'"
+        Then the page should be scrolled to 0