Full redirect support for history

When a redirect occurs, the item is saved in history with a -r suffix
now. When opening qutebrowser that's picked up and the item is hidden
from completion.

qutebrowser/browser/history.py

@@ -38,23 +38,26 @@ class Entry:
     Attributes:
         atime: The time the page was accessed.
         url: The URL which was accessed as QUrl.
-        hidden: If True, don't save this entry to disk
+        redirect: If True, don't save this entry to disk
     """
 
-    def __init__(self, atime, url, title, hidden=False):
+    def __init__(self, atime, url, title, redirect=False):
         self.atime = float(atime)
         self.url = url
         self.title = title
-        self.hidden = hidden
+        self.redirect = redirect
         qtutils.ensure_valid(url)
 
     def __repr__(self):
         return utils.get_repr(self, constructor=True, atime=self.atime,
                               url=self.url_str(), title=self.title,
-                              hidden=self.hidden)
+                              redirect=self.redirect)
 
     def __str__(self):
-        elems = [str(int(self.atime)), self.url_str()]
+        atime = str(int(self.atime))
+        if self.redirect:
+            atime += '-r'  # redirect flag
+        elems = [atime, self.url_str()]
         if self.title:
             elems.append(self.title)
         return ' '.join(elems)
@@ -63,7 +66,7 @@ class Entry:
         return (self.atime == other.atime and
                 self.title == other.title and
                 self.url == other.url and
-                self.hidden == other.hidden)
+                self.redirect == other.redirect)
 
     def url_str(self):
         """Get the URL as a lossless string."""
@@ -91,7 +94,18 @@ class Entry:
                 "https://github.com/The-Compiler/qutebrowser/issues/"
                 "670".format(data))
             atime = atime.lstrip('\0')
-        return cls(atime, url, title)
+
+        if '-' in atime:
+            atime, flags = atime.split('-')
+        else:
+            flags = ''
+
+        if not set(flags).issubset('r'):
+            raise ValueError("Invalid flags {!r}".format(flags))
+
+        redirect = 'r' in flags
+
+        return cls(atime, url, title, redirect=redirect)
 
 
 class WebHistoryInterface(QWebHistoryInterface):
@@ -230,8 +244,8 @@ class WebHistory(QObject):
 
         for entry in self._temp_history.values():
             self._add_entry(entry)
-            if not entry.hidden:
+            self._new_history.append(entry)
-                self._new_history.append(entry)
+            if not entry.redirect:
                 self.add_completion_item.emit(entry)
         self._temp_history.clear()
 
@@ -270,25 +284,26 @@ class WebHistory(QObject):
         self._saved_count = 0
         self.cleared.emit()
 
-    def add_url(self, url, title="", *, hidden=False, atime=None):
+    def add_url(self, url, title="", *, redirect=False, atime=None):
         """Called by WebKit when an URL should be added to the history.
 
         Args:
             url: An url (as QUrl) to add to the history.
-            hidden: Whether to hide the entry from the on-disk history
+            redirect: Whether the entry was redirected to another URL
+                      (hidden in completion)
             atime: Override the atime used to add the entry
         """
         if config.get('general', 'private-browsing'):
             return
         if atime is None:
             atime = time.time()
-        entry = Entry(atime, url, title, hidden=hidden)
+        entry = Entry(atime, url, title, redirect=redirect)
         if self._initial_read_done:
             self._add_entry(entry)
-            if not entry.hidden:
+            self._new_history.append(entry)
+            self.item_added.emit(entry)
+            if not entry.redirect:
                 self.add_completion_item.emit(entry)
-                self._new_history.append(entry)
-                self.item_added.emit(entry)
         else:
             self._add_entry(entry, target=self._temp_history)
 

qutebrowser/browser/webview.py

@@ -155,7 +155,7 @@ class WebView(QWebView):
                 not self._orig_url.matches(self.cur_url, no_formatting)):
             # If the url of the page is different than the url of the link
             # originally clicked, save them both.
-            history.add_url(self._orig_url, self.title(), hidden=True)
+            history.add_url(self._orig_url, self.title(), redirect=True)
         history.add_url(self.cur_url, self.title())
 
     def _init_page(self):

qutebrowser/completion/models/urlmodel.py

@@ -74,7 +74,8 @@ class UrlCompletionModel(base.BaseCompletionModel):
         self._max_history = config.get('completion', 'web-history-max-items')
         history = utils.newest_slice(self._history, self._max_history)
         for entry in history:
-            self._add_history_entry(entry)
+            if not entry.redirect:
+                self._add_history_entry(entry)
         self._history.add_completion_item.connect(self.on_history_item_added)
         self._history.cleared.connect(self.on_history_cleared)
 

tests/end2end/features/history.feature

@@ -21,6 +21,7 @@ Feature: Page history
         When I open redirect-to?url=data/title.html without waiting
         And I wait until data/title.html is loaded
         Then the history file should contain:
+            r http://localhost:(port)/redirect-to?url=data/title.html Test title
             http://localhost:(port)/data/title.html Test title
             
     Scenario: History item with spaces in URL

tests/end2end/features/test_history_bdd.py

@@ -37,8 +37,11 @@ def check_history(quteproc, httpbin, expected):
             if not line.strip():
                 continue
             print('history line: ' + line)
-            line = line.split(' ', maxsplit=1)[1]  # Strip off timestamp
+            atime, line = line.split(' ', maxsplit=1)
             line = line.rstrip()
+            if '-' in atime:
+                flags = atime.split('-')[1]
+                line = '{} {}'.format(flags, line)
             lines.append(line)
 
     assert lines == expected

tests/manual/history/visited.html

@@ -5,8 +5,10 @@
         <title>Visited/Unvisited links</title>
     </head>
     <body>
-        <p>After visiting a link, it should turn purple.</p>
+      <ul>
-        <p>When closing/reopening qutebrowser, the link still should be purple.</p>
+        <li>After visiting a link, it should turn purple.</li>
+        <li>When closing/reopening qutebrowser, the link still should be purple.</li>
+        <li>The httpbin.org link (before redirect) should not be in the completion.</li>
         <ul>
           <li><a href="http://www.example.com/">normal link</a></li>
           <li><a href="http://httpbin.org/redirect-to?url=http://www.example.com/">link with redirect</a></li>

tests/unit/browser/test_history.py

@@ -73,16 +73,16 @@ def test_async_read_no_datadir(qtbot, config_stub, fake_save_manager):
         list(hist.async_read())
 
 
-@pytest.mark.parametrize('hidden', [True, False])
+@pytest.mark.parametrize('redirect', [True, False])
-def test_adding_item_during_async_read(qtbot, hist, hidden):
+def test_adding_item_during_async_read(qtbot, hist, redirect):
     """Check what happens when adding URL while reading the history."""
     url = QUrl('http://www.example.com/')
 
     with qtbot.assertNotEmitted(hist.add_completion_item), \
             qtbot.assertNotEmitted(hist.item_added):
-        hist.add_url(url, hidden=hidden, atime=12345)
+        hist.add_url(url, redirect=redirect, atime=12345)
 
-    if hidden:
+    if redirect:
         with qtbot.assertNotEmitted(hist.add_completion_item):
             with qtbot.waitSignal(hist.async_read_done):
                 list(hist.async_read())
@@ -93,7 +93,7 @@ def test_adding_item_during_async_read(qtbot, hist, hidden):
 
     assert not hist._temp_history
 
-    expected = history.Entry(url=url, atime=12345, hidden=hidden, title="")
+    expected = history.Entry(url=url, atime=12345, redirect=redirect, title="")
     assert list(hist.history_dict.values()) == [expected]
 
 
@@ -131,7 +131,7 @@ def test_iter(hist):
     url = QUrl('http://www.example.com/')
     hist.add_url(url, atime=12345)
 
-    entry = history.Entry(url=url, atime=12345, hidden=False, title="")
+    entry = history.Entry(url=url, atime=12345, redirect=False, title="")
     assert list(hist) == [entry]
 
 
@@ -249,19 +249,36 @@ def test_add_item(qtbot, hist):
     with qtbot.waitSignals([hist.add_completion_item, hist.item_added]):
         hist.add_url(QUrl(url), atime=12345, title="the title")
 
-    entry = history.Entry(url=QUrl(url), hidden=False, atime=12345,
+    entry = history.Entry(url=QUrl(url), redirect=False, atime=12345,
                           title="the title")
     assert hist.history_dict[url] == entry
 
 
-def test_add_item_hidden(qtbot, hist):
+def test_add_item_redirect(qtbot, hist):
     list(hist.async_read())
     url = 'http://www.example.com/'
-    with qtbot.assertNotEmitted(hist.add_completion_item), \
+    with qtbot.assertNotEmitted(hist.add_completion_item):
-            qtbot.assertNotEmitted(hist.item_added):
+        with qtbot.waitSignal(hist.item_added):
-        hist.add_url(QUrl(url), hidden=True, atime=12345)
+            hist.add_url(QUrl(url), redirect=True, atime=12345)
 
-    entry = history.Entry(url=QUrl(url), hidden=True, atime=12345, title="")
+    entry = history.Entry(url=QUrl(url), redirect=True, atime=12345, title="")
+    assert hist.history_dict[url] == entry
+
+
+def test_add_item_redirect_update(qtbot, tmpdir):
+    """A redirect update added should override a non-redirect one."""
+    url = 'http://www.example.com/'
+
+    hist_file = tmpdir / 'filled-history'
+    hist_file.write('12345 {}\n'.format(url))
+    hist = history.WebHistory(hist_dir=str(tmpdir), hist_name='filled-history')
+    list(hist.async_read())
+
+    with qtbot.assertNotEmitted(hist.add_completion_item):
+        with qtbot.waitSignal(hist.item_added):
+            hist.add_url(QUrl(url), redirect=True, atime=67890)
+
+    entry = history.Entry(url=QUrl(url), redirect=True, atime=67890, title="")
     assert hist.history_dict[url] == entry
 
 
@@ -287,6 +304,12 @@ def test_add_item_hidden(qtbot, hist):
         '\x0012345 http://example.com/',
         history.Entry(atime=12345, url=QUrl('http://example.com/'), title=''),
     ),
+    (
+        # redirect flag
+        '12345-r http://example.com/ this is a title',
+        history.Entry(atime=12345, url=QUrl('http://example.com/'),
+                      title='this is a title', redirect=True)
+    ),
 ])
 def test_entry_parse_valid(line, expected):
     entry = history.Entry.from_str(line)
@@ -297,6 +320,8 @@ def test_entry_parse_valid(line, expected):
     '12345',  # one field
     '12345 ::',  # invalid URL
     'xyz http://www.example.com/',  # invalid timestamp
+    '12345-x http://www.example.com/',  # invalid flags
+    '12345-r-r http://www.example.com/',  # double flags
 ])
 def test_entry_parse_invalid(line):
     with pytest.raises(ValueError):
@@ -328,6 +353,12 @@ def test_entry_parse_hypothesis(text):
         history.Entry(12345.678, QUrl('http://example.com/'), ""),
         "12345 http://example.com/",
     ),
+    # redirect flag
+    (
+        history.Entry(12345.678, QUrl('http://example.com/'), "",
+                      redirect=True),
+        "12345-r http://example.com/",
+    ),
 ])
 def test_entry_str(entry, expected):
     assert str(entry) == expected