From c60753731973946c4e8a565970fcd8af24ac44cf Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 14 Aug 2017 21:37:43 -0400 Subject: [PATCH] Consistently format urls in history. Encode urls that are inserted into the history, but do not encode urls for completion (other than removing passwords). Also ensure that urls read from the history text file are formatted consistenly with those added while browsing. Fixes #2903. --- qutebrowser/browser/history.py | 16 ++++++++++----- tests/unit/browser/test_history.py | 32 +++++++++++++++++------------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/qutebrowser/browser/history.py b/qutebrowser/browser/history.py index 86e597bcd..60c3f3e36 100644 --- a/qutebrowser/browser/history.py +++ b/qutebrowser/browser/history.py @@ -159,13 +159,12 @@ class WebHistory(sql.SqlTable): return atime = int(atime) if (atime is not None) else int(time.time()) - url_str = url.toString(QUrl.FullyEncoded | QUrl.RemovePassword) - self.insert({'url': url_str, + self.insert({'url': self._format_url(url), 'title': title, 'atime': atime, 'redirect': redirect}) if not redirect: - self.completion.insert({'url': url_str, + self.completion.insert({'url': self._format_completion_url(url), 'title': title, 'last_atime': atime}, replace=True) @@ -249,12 +248,13 @@ class WebHistory(sql.SqlTable): if parsed is None: continue url, title, atime, redirect = parsed - data['url'].append(url) + data['url'].append(self._format_url(url)) data['title'].append(title) data['atime'].append(atime) data['redirect'].append(redirect) if not redirect: - completion_data['url'].append(url) + completion_data['url'].append( + self._format_completion_url(url)) completion_data['title'].append(title) completion_data['last_atime'].append(atime) except ValueError as ex: @@ -263,6 +263,12 @@ class WebHistory(sql.SqlTable): self.insert_batch(data) self.completion.insert_batch(completion_data, replace=True) + def _format_url(self, url): + return url.toString(QUrl.RemovePassword | QUrl.FullyEncoded) + + def _format_completion_url(self, url): + return url.toString(QUrl.RemovePassword) + @cmdutils.register(instance='web-history', debug=True) def debug_dump_history(self, dest): """Dump the history to a file in the old pre-SQL format. diff --git a/tests/unit/browser/test_history.py b/tests/unit/browser/test_history.py index c109f44eb..0c6f65683 100644 --- a/tests/unit/browser/test_history.py +++ b/tests/unit/browser/test_history.py @@ -143,23 +143,27 @@ def test_delete_url(hist): assert completion_diff == {('http://example.com/1', '', 0)} -@pytest.mark.parametrize('url, atime, title, redirect, expected_url', [ - ('http://www.example.com', 12346, 'the title', False, - 'http://www.example.com'), - ('http://www.example.com', 12346, 'the title', True, - 'http://www.example.com'), - ('http://www.example.com/spa ce', 12346, 'the title', False, - 'http://www.example.com/spa%20ce'), - ('https://user:pass@example.com', 12346, 'the title', False, - 'https://user@example.com'), -]) -def test_add_url(qtbot, hist, url, atime, title, redirect, expected_url): +@pytest.mark.parametrize( + 'url, atime, title, redirect, history_url, completion_url', [ + + ('http://www.example.com', 12346, 'the title', False, + 'http://www.example.com', 'http://www.example.com'), + ('http://www.example.com', 12346, 'the title', True, + 'http://www.example.com', None), + ('http://www.example.com/sp ce', 12346, 'the title', False, + 'http://www.example.com/sp%20ce', 'http://www.example.com/sp ce'), + ('https://user:pass@example.com', 12346, 'the title', False, + 'https://user@example.com', 'https://user@example.com'), + ] +) +def test_add_url(qtbot, hist, url, atime, title, redirect, history_url, + completion_url): hist.add_url(QUrl(url), atime=atime, title=title, redirect=redirect) - assert list(hist) == [(expected_url, title, atime, redirect)] - if redirect: + assert list(hist) == [(history_url, title, atime, redirect)] + if completion_url is None: assert not len(hist.completion) else: - assert list(hist.completion) == [(expected_url, title, atime)] + assert list(hist.completion) == [(completion_url, title, atime)] def test_add_url_invalid(qtbot, hist, caplog):