Use QUrl with QWebHistory

Now that we have our own history implementation, we get the URLs as QUrl
and not as string - so no point in converting them to string and back.
This commit is contained in:
Florian Bruhin 2016-06-09 14:19:03 +02:00
parent 5f25ce69ec
commit 9510af9912
3 changed files with 30 additions and 27 deletions

View File

@ -26,7 +26,7 @@ from PyQt5.QtCore import pyqtSignal, QUrl, QObject
from PyQt5.QtWebKit import QWebHistoryInterface from PyQt5.QtWebKit import QWebHistoryInterface
from qutebrowser.commands import cmdutils from qutebrowser.commands import cmdutils
from qutebrowser.utils import utils, objreg, standarddir, log from qutebrowser.utils import utils, objreg, standarddir, log, qtutils
from qutebrowser.config import config from qutebrowser.config import config
from qutebrowser.misc import lineparser from qutebrowser.misc import lineparser
@ -38,31 +38,34 @@ class Entry:
Attributes: Attributes:
atime: The time the page was accessed. atime: The time the page was accessed.
url: The URL which was accessed as QUrl. url: The URL which was accessed as QUrl.
url_string: The URL which was accessed as string.
hidden: If True, don't save this entry to disk hidden: If True, don't save this entry to disk
""" """
def __init__(self, atime, url, title, hidden=False): def __init__(self, atime, url, title, hidden=False):
self.atime = float(atime) self.atime = float(atime)
self.url = QUrl(url) self.url = url
self.url_string = url
self.title = title self.title = title
self.hidden = hidden self.hidden = hidden
qtutils.ensure_valid(url)
def __repr__(self): def __repr__(self):
return utils.get_repr(self, constructor=True, atime=self.atime, return utils.get_repr(self, constructor=True, atime=self.atime,
url=self.url.toDisplayString(), title=self.title, url=self.url_str(), title=self.title,
hidden=self.hidden) hidden=self.hidden)
def __str__(self): def __str__(self):
return '{} {} {}'.format(int(self.atime), self.url_string, self.title) return '{} {} {}'.format(int(self.atime), self.url_str(), self.title)
def __eq__(self, other): def __eq__(self, other):
return (self.atime == other.atime and return (self.atime == other.atime and
self.title == other.title and self.title == other.title and
self.url_string == other.url_string and self.url == other.url and
self.hidden == other.hidden) self.hidden == other.hidden)
def url_str(self):
"""Get the URL as a lossless string."""
return self.url.toString(QUrl.FullyEncoded | QUrl.RemovePassword)
@classmethod @classmethod
def from_str(cls, line): def from_str(cls, line):
data = line.split(maxsplit=2) data = line.split(maxsplit=2)
@ -73,6 +76,11 @@ class Entry:
atime, url, title = data atime, url, title = data
else: else:
raise ValueError("2 or 3 fields expected") raise ValueError("2 or 3 fields expected")
url = QUrl(url)
if not url.isValid():
raise ValueError("Invalid URL: {}".format(url.errorString()))
if atime.startswith('\0'): if atime.startswith('\0'):
log.init.debug( log.init.debug(
"Removing NUL bytes from entry {!r} - see " "Removing NUL bytes from entry {!r} - see "
@ -213,8 +221,9 @@ class WebHistory(QObject):
"""Add an entry to self.history_dict or another given OrderedDict.""" """Add an entry to self.history_dict or another given OrderedDict."""
if target is None: if target is None:
target = self.history_dict target = self.history_dict
target[entry.url_string] = entry url_str = entry.url_str()
target.move_to_end(entry.url_string) target[url_str] = entry
target.move_to_end(url_str)
def get_recent(self): def get_recent(self):
"""Get the most recent history entries.""" """Get the most recent history entries."""
@ -243,18 +252,16 @@ class WebHistory(QObject):
self._saved_count = 0 self._saved_count = 0
self.cleared.emit() self.cleared.emit()
def add_url(self, url_string, title="", hidden=False): def add_url(self, url, title="", hidden=False):
"""Called by WebKit when an URL should be added to the history. """Called by WebKit when an URL should be added to the history.
Args: Args:
url_string: An url as string to add to the history. url: An url (as QUrl) to add to the history.
hidden: Whether to hide the entry from the on-disk history hidden: Whether to hide the entry from the on-disk history
""" """
if not url_string:
return
if config.get('general', 'private-browsing'): if config.get('general', 'private-browsing'):
return return
entry = Entry(time.time(), url_string, title, hidden=hidden) entry = Entry(time.time(), url, title, hidden=hidden)
if self._initial_read_done: if self._initial_read_done:
self._add_entry(entry) self._add_entry(entry)
if not entry.hidden: if not entry.hidden:

View File

@ -154,12 +154,8 @@ class WebView(QWebView):
QUrl.UrlFormattingOption(0)): QUrl.UrlFormattingOption(0)):
# If the url of the page is different than the url of the link # If the url of the page is different than the url of the link
# originally clicked, save them both. # originally clicked, save them both.
url = self._orig_url.toString(QUrl.FullyEncoded | history.add_url(self._orig_url, self.title(), hidden=True)
QUrl.RemovePassword) history.add_url(self.cur_url, self.title())
history.add_url(url, self.title(), hidden=True)
url = self.cur_url.toString(QUrl.FullyEncoded | QUrl.RemovePassword)
history.add_url(url, self.title())
def _init_page(self): def _init_page(self):
"""Initialize the QWebPage used by this view.""" """Initialize the QWebPage used by this view."""

View File

@ -41,7 +41,7 @@ def test_adding_item_during_async_read(qtbot, hist):
with qtbot.assertNotEmitted(hist.add_completion_item), \ with qtbot.assertNotEmitted(hist.add_completion_item), \
qtbot.assertNotEmitted(hist.item_added): qtbot.assertNotEmitted(hist.item_added):
hist.add_url('http://www.example.com/') hist.add_url(QUrl('http://www.example.com/'))
with qtbot.waitSignals([hist.add_completion_item, with qtbot.waitSignals([hist.add_completion_item,
hist.async_read_done]): hist.async_read_done]):
@ -61,7 +61,7 @@ def test_private_browsing(qtbot, tmpdir, fake_save_manager, config_stub):
# Before initial read # Before initial read
with qtbot.assertNotEmitted(private_hist.add_completion_item), \ with qtbot.assertNotEmitted(private_hist.add_completion_item), \
qtbot.assertNotEmitted(private_hist.item_added): qtbot.assertNotEmitted(private_hist.item_added):
private_hist.add_url('http://www.example.com/') private_hist.add_url(QUrl('http://www.example.com/'))
assert not private_hist._temp_history assert not private_hist._temp_history
# read # read
@ -73,7 +73,7 @@ def test_private_browsing(qtbot, tmpdir, fake_save_manager, config_stub):
# after read # after read
with qtbot.assertNotEmitted(private_hist.add_completion_item), \ with qtbot.assertNotEmitted(private_hist.add_completion_item), \
qtbot.assertNotEmitted(private_hist.item_added): qtbot.assertNotEmitted(private_hist.item_added):
private_hist.add_url('http://www.example.com/') private_hist.add_url(QUrl('http://www.example.com/'))
assert not private_hist._temp_history assert not private_hist._temp_history
assert not private_hist._new_history assert not private_hist._new_history
@ -84,18 +84,18 @@ def test_private_browsing(qtbot, tmpdir, fake_save_manager, config_stub):
( (
# old format without title # old format without title
'12345 http://example.com/', '12345 http://example.com/',
history.Entry(atime=12345, url='http://example.com/', title='',) history.Entry(atime=12345, url=QUrl('http://example.com/'), title='',)
), ),
( (
# new format with title # new format with title
'12345 http://example.com/ this is a title', '12345 http://example.com/ this is a title',
history.Entry(atime=12345, url='http://example.com/', history.Entry(atime=12345, url=QUrl('http://example.com/'),
title='this is a title') title='this is a title')
), ),
( (
# weird NUL bytes # weird NUL bytes
'\x0012345 http://example.com/', '\x0012345 http://example.com/',
history.Entry(atime=12345, url='http://example.com/', title=''), history.Entry(atime=12345, url=QUrl('http://example.com/'), title=''),
), ),
]) ])
def test_entry_parse_valid(line, expected): def test_entry_parse_valid(line, expected):
@ -105,7 +105,7 @@ def test_entry_parse_valid(line, expected):
@pytest.mark.parametrize('line', [ @pytest.mark.parametrize('line', [
'12345', # one field '12345', # one field
'12345 onlyscheme:', # invalid URL '12345 ::', # invalid URL
'xyz http://www.example.com/', # invalid timestamp 'xyz http://www.example.com/', # invalid timestamp
]) ])
def test_entry_parse_invalid(line): def test_entry_parse_invalid(line):