Merge branch 'toofar-history-title'

CHANGELOG.asciidoc

@@ -67,6 +67,7 @@ Changed
 - Number hints are now renumbered after filtering
 - Number hints can now be filtered with multiple space-separated search terms
 - `hints -> scatter` is now ignored for number hints
+- Better history implementation which also stores titles
 
 Fixed
 -----

qutebrowser/browser/history.py

@@ -41,18 +41,18 @@ class HistoryEntry:
         url_string: The URL which was accessed as string.
     """
 
-    def __init__(self, atime, url):
+    def __init__(self, atime, url, title):
         self.atime = float(atime)
         self.url = QUrl(url)
         self.url_string = url
+        self.title = title
 
     def __repr__(self):
         return utils.get_repr(self, constructor=True, atime=self.atime,
-                              url=self.url.toDisplayString())
+                              url=self.url.toDisplayString(), title=self.title)
 
     def __str__(self):
-        return '{} {}'.format(int(self.atime), self.url_string)
-
+        return '{} {} {}'.format(int(self.atime), self.url_string, self.title)
 
 class WebHistory(QWebHistoryInterface):
 
@@ -121,16 +121,20 @@ class WebHistory(QWebHistoryInterface):
         with self._lineparser.open():
             for line in self._lineparser:
                 yield
-                data = line.rstrip().split(maxsplit=1)
+                data = line.rstrip().split(maxsplit=2)
                 if not data:
                     # empty line
                     continue
-                elif len(data) != 2:
+                elif len(data) == 2:
+                    atime, url = data
+                    title = ""
+                elif len(data) == 3:
+                    atime, url, title = data
+                else:
                     # other malformed line
                     log.init.warning("Invalid history entry {!r}!".format(
                         line))
                     continue
-                atime, url = data
                 if atime.startswith('\0'):
                     log.init.debug(
                         "Removing NUL bytes from entry {!r} - see "
@@ -142,7 +146,7 @@ class WebHistory(QWebHistoryInterface):
                 # information about previous hits change the items in
                 # old_urls to be lists or change HistoryEntry to have a
                 # list of atimes.
-                entry = HistoryEntry(atime, url)
+                entry = HistoryEntry(atime, url, title)
                 self._add_entry(entry)
 
         self._initial_read_done = True
@@ -183,6 +187,10 @@ class WebHistory(QWebHistoryInterface):
         self.cleared.emit()
 
     def addHistoryEntry(self, url_string):
+        """Required for a QWebHistoryInterface impl, obsoleted by add_url."""
+        pass
+
+    def add_url(self, url_string, title=""):
         """Called by WebKit when an URL should be added to the history.
 
         Args:
@@ -192,7 +200,7 @@ class WebHistory(QWebHistoryInterface):
             return
         if config.get('general', 'private-browsing'):
             return
-        entry = HistoryEntry(time.time(), url_string)
+        entry = HistoryEntry(time.time(), url_string, title)
         if self._initial_read_done:
             self.add_completion_item.emit(entry)
             self._new_history.append(entry)

qutebrowser/browser/webview.py

@@ -119,6 +119,7 @@ class WebView(QWebView):
         self.destroyed.connect(functools.partial(
             cfg.changed.disconnect, self.init_neighborlist))
         self.cur_url = QUrl()
+        self._orig_url = QUrl()
         self.progress = 0
         self.registry = objreg.ObjectRegistry()
         self.tab_id = next(tab_id_gen)
@@ -145,6 +146,21 @@ class WebView(QWebView):
         self.loadProgress.connect(lambda p: setattr(self, 'progress', p))
         objreg.get('config').changed.connect(self.on_config_changed)
 
+    @pyqtSlot()
+    def on_initial_layout_completed(self):
+        """Add url to history now that we have displayed something."""
+        history = objreg.get('web-history')
+        if not self._orig_url.matches(self.cur_url,
+                                      QUrl.UrlFormattingOption(0)):
+            # If the url of the page is different than the url of the link
+            # originally clicked, save them both.
+            url = self._orig_url.toString(QUrl.FullyEncoded |
+                                          QUrl.RemovePassword)
+            history.add_url(url, self.title())
+        url = self.cur_url.toString(QUrl.FullyEncoded | QUrl.RemovePassword)
+
+        history.add_url(url, self.title())
+
     def _init_page(self):
         """Initialize the QWebPage used by this view."""
         page = webpage.BrowserPage(self.win_id, self.tab_id, self)
@@ -152,6 +168,8 @@ class WebView(QWebView):
         page.linkHovered.connect(self.linkHovered)
         page.mainFrame().loadStarted.connect(self.on_load_started)
         page.mainFrame().loadFinished.connect(self.on_load_finished)
+        page.mainFrame().initialLayoutCompleted.connect(
+            self.on_initial_layout_completed)
         page.statusBarMessage.connect(
             lambda msg: setattr(self, 'statusbar_message', msg))
         page.networkAccessManager().sslErrors.connect(
@@ -182,6 +200,8 @@ class WebView(QWebView):
         log.webview.debug("load status for {}: {}".format(repr(self), val))
         self.load_status = val
         self.load_status_changed.emit(val.name)
+        if val == LoadStatus.loading:
+            self._orig_url = self.cur_url
 
     def _set_bg_color(self):
         """Set the webpage background color as configured."""

qutebrowser/completion/models/urlmodel.py

@@ -100,7 +100,8 @@ class UrlCompletionModel(base.BaseCompletionModel):
 
     def _add_history_entry(self, entry):
         """Add a new history entry to the completion."""
-        self.new_item(self._history_cat, entry.url.toDisplayString(), "",
+        self.new_item(self._history_cat, entry.url.toDisplayString(),
+                      entry.title,
                       self._fmt_atime(entry.atime), sort=int(entry.atime),
                       userdata=entry.url)
 
@@ -123,9 +124,11 @@ class UrlCompletionModel(base.BaseCompletionModel):
         for i in range(self._history_cat.rowCount()):
             url_item = self._history_cat.child(i, self.URL_COLUMN)
             atime_item = self._history_cat.child(i, self.TIME_COLUMN)
+            title_item = self._history_cat.child(i, self.TEXT_COLUMN)
             url = url_item.data(base.Role.userdata)
             if url == entry.url:
                 atime_item.setText(self._fmt_atime(entry.atime))
+                title_item.setText(entry.title)
                 url_item.setData(int(entry.atime), base.Role.sort)
                 break
         else:

tests/end2end/data/äöü.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Chäschüechli</title>
+    </head>
+    <body>
+        foo
+    </body>
+</html>

tests/end2end/features/history.feature

@@ -0,0 +1,47 @@
+Feature: Page history
+
+    Make sure the global page history is saved correctly.
+
+    Background:
+        Given I run :history-clear
+
+    Scenario: Simple history saving
+        When I open data/numbers/1.txt
+        And I open data/numbers/2.txt
+        Then the history file should contain:
+            http://localhost:(port)/data/numbers/1.txt
+            http://localhost:(port)/data/numbers/2.txt
+            
+    Scenario: History item with title
+        When I open data/title.html
+        Then the history file should contain:
+            http://localhost:(port)/data/title.html Test title
+
+    Scenario: History item with redirect
+        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:
+            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
+        When I open data/title with spaces.html
+        Then the history file should contain:
+            http://localhost:(port)/data/title%20with%20spaces.html Test title
+
+    Scenario: History item with umlauts
+        When I open data/äöü.html
+        Then the history file should contain:
+            http://localhost:(port)/data/%C3%A4%C3%B6%C3%BC.html Chäschüechli
+            
+    Scenario: History with an error
+        When I run :open file:///does/not/exist
+        And I wait for "Error while loading file:///does/not/exist: Error opening /does/not/exist: *" in the log
+        Then the history file should contain:
+            file:///does/not/exist Error loading page: file:///does/not/exist
+
+    Scenario: History with a 404
+        When I open status/404 without waiting
+        And I wait for "Error while loading http://localhost:*/status/404: NOT FOUND" in the log
+        Then the history file should contain:
+            http://localhost:(port)/status/404 Error loading page: http://localhost:(port)/status/404

tests/end2end/features/test_history_bdd.py

@@ -0,0 +1,44 @@
+# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
+
+# Copyright 2016 Florian Bruhin (The Compiler) <mail@qutebrowser.org>
+#
+# This file is part of qutebrowser.
+#
+# qutebrowser is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# qutebrowser is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
+
+import os.path
+
+import pytest_bdd as bdd
+bdd.scenarios('history.feature')
+
+
+@bdd.then(bdd.parsers.parse("the history file should contain:\n{expected}"))
+def check_history(quteproc, httpbin, expected):
+    history_file = os.path.join(quteproc.basedir, 'data', 'history')
+    quteproc.send_cmd(':save history')
+    quteproc.wait_for(message='Saved to *history')
+
+    expected = expected.replace('(port)', str(httpbin.port)).splitlines()
+
+    with open(history_file, 'r', encoding='utf-8') as f:
+        lines = []
+        for line in f:
+            if not line.strip():
+                continue
+            print('history line: ' + line)
+            line = line.split(' ', maxsplit=1)[1]  # Strip off timestamp
+            line = line.rstrip()
+            lines.append(line)
+
+    assert lines == expected

tests/end2end/fixtures/webserver.py

@@ -68,6 +68,8 @@ class Request(testprocess.Line):
             '/custom/redirect-later': [http.client.FOUND],
             '/basic-auth/user/password':
                 [http.client.UNAUTHORIZED, http.client.OK],
+            '/redirect-to': [http.client.FOUND],
+            '/status/404': [http.client.NOT_FOUND],
         }
 
         sanitized = QUrl('http://localhost' + self.path).path()  # Remove ?foo