From 1753d3507c4642182287ba5382b824af4ce174d3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 16 Aug 2016 17:07:38 +0200 Subject: [PATCH 01/17] Use native QLabels for hints This will make labels work easily with QtWebEngine, and make sure they're not affected by the page's contents. Fixes #925. Fixes #1126. --- qutebrowser/browser/hints.py | 221 ++++++++++++++--------------------- 1 file changed, 86 insertions(+), 135 deletions(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 8dac93e03..c74a6dff7 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -23,11 +23,13 @@ import collections import functools import math import re +import html from string import ascii_lowercase from PyQt5.QtCore import (pyqtSignal, pyqtSlot, QObject, QEvent, Qt, QUrl, QTimer) from PyQt5.QtGui import QMouseEvent +from PyQt5.QtWidgets import QLabel from PyQt5.QtWebKitWidgets import QWebPage from qutebrowser.config import config @@ -57,6 +59,72 @@ def on_mode_entered(mode, win_id): modeman.maybe_leave(win_id, usertypes.KeyMode.hint, 'insert mode') +class HintLabel(QLabel): + + """A label for a link. + + Attributes: + elem: The element this label belongs to. + _context: The current hinting context. + """ + + def __init__(self, elem, context): + super().__init__(parent=context.tab) + self._context = context + self.elem = elem + self._context.tab.contents_size_changed.connect( + self._on_contents_size_changed) + self._move_to_elem() + self.show() + + # FIXME styling + # ('color', config.get('colors', 'hints.fg') + ' !important'), + # ('background', config.get('colors', 'hints.bg') + ' !important'), + # ('font', config.get('fonts', 'hints') + ' !important'), + # ('border', config.get('hints', 'border') + ' !important'), + # ('opacity', str(config.get('hints', 'opacity')) + ' !important'), + + def update_text(self, matched, unmatched): + """Set the text for the hint. + + Args: + matched: The part of the text which was typed. + unmatched: The part of the text which was not typed yet. + """ + if (config.get('hints', 'uppercase') and + self._context.hint_mode == 'letter'): + matched = html.escape(matched.upper()) + unmatched = html.escape(unmatched.upper()) + else: + matched = html.escape(matched) + unmatched = html.escape(unmatched) + + match_color = html.escape(config.get('colors', 'hints.fg.match')) + self.setText('{}{}'.format( + match_color, matched, unmatched)) + + def _move_to_elem(self): + """Reposition the label to its element.""" + no_js = config.get('hints', 'find-implementation') != 'javascript' + rect = self.elem.rect_on_view(adjust_zoom=False, no_js=no_js) + self.move(rect.x(), rect.y()) + + @pyqtSlot() + def _on_contents_size_changed(self): + """Reposition hints if contents size changed.""" + log.hints.debug("Contents size changed...!") + if self.elem.frame() is None: + # This sometimes happens for some reason... + self.cleanup() + else: + self._move_to_elem() + + def cleanup(self): + """Clean up this element and hide it.""" + self.hide() + self.deleteLater() + + class HintContext: """Context namespace used for hinting. @@ -362,18 +430,9 @@ class HintManager(QObject): def _cleanup(self): """Clean up after hinting.""" - try: - self._context.tab.contents_size_changed.disconnect( - self.on_contents_size_changed) - except TypeError: - # For some reason, this can fail sometimes... - pass - for elem in self._context.all_elems: - try: - elem.label.remove_from_document() - except webelem.Error: - pass + elem.label.cleanup() + text = self._get_text() message_bridge = objreg.get('message-bridge', scope='window', window=self._win_id) @@ -513,87 +572,6 @@ class HintManager(QObject): hintstr.insert(0, chars[0]) return ''.join(hintstr) - def _is_hidden(self, elem): - """Check if the element is hidden via display=none.""" - display = elem.style_property('display', strategy='inline') - return display == 'none' - - def _show_elem(self, elem): - """Show a given element.""" - elem.set_style_property('display', 'inline !important') - - def _hide_elem(self, elem): - """Hide a given element.""" - elem.set_style_property('display', 'none !important') - - def _set_style_properties(self, elem, label): - """Set the hint CSS on the element given. - - Args: - elem: The QWebElement to set the style attributes for. - label: The label QWebElement. - """ - attrs = [ - ('display', 'inline !important'), - ('z-index', '{} !important'.format(int(2 ** 32 / 2 - 1))), - ('pointer-events', 'none !important'), - ('position', 'fixed !important'), - ('color', config.get('colors', 'hints.fg') + ' !important'), - ('background', config.get('colors', 'hints.bg') + ' !important'), - ('font', config.get('fonts', 'hints') + ' !important'), - ('border', config.get('hints', 'border') + ' !important'), - ('opacity', str(config.get('hints', 'opacity')) + ' !important'), - ] - - # Make text uppercase if set in config - if (config.get('hints', 'uppercase') and - self._context.hint_mode == 'letter'): - attrs.append(('text-transform', 'uppercase !important')) - else: - attrs.append(('text-transform', 'none !important')) - - for k, v in attrs: - label.set_style_property(k, v) - self._set_style_position(elem, label) - - def _set_style_position(self, elem, label): - """Set the CSS position of the label element. - - Args: - elem: The QWebElement to set the style attributes for. - label: The label QWebElement. - """ - no_js = config.get('hints', 'find-implementation') != 'javascript' - rect = elem.rect_on_view(adjust_zoom=False, no_js=no_js) - left = rect.x() - top = rect.y() - log.hints.vdebug("Drawing label '{!r}' at {}/{} for element '{!r}' " - "(no_js: {})".format(label, left, top, elem, no_js)) - label.set_style_property('left', '{}px !important'.format(left)) - label.set_style_property('top', '{}px !important'.format(top)) - - def _draw_label(self, elem, string): - """Draw a hint label over an element. - - Args: - elem: The QWebElement to use. - string: The hint string to print. - - Return: - The newly created label element - """ - doc = elem.document_element() - body = doc.find_first('body') - if body is None: - parent = doc - else: - parent = body - label = parent.create_inside('span') - label['class'] = 'qutehint' - self._set_style_properties(elem, label) - label.set_text(string) - return label - def _show_url_error(self): """Show an error because no link was found.""" message.error(self._win_id, "No suitable link found for this element.", @@ -646,18 +624,19 @@ class HintManager(QObject): raise cmdexc.CommandError("No elements found.") strings = self._hint_strings(elems) log.hints.debug("hints: {}".format(', '.join(strings))) + for e, string in zip(elems, strings): - label = self._draw_label(e, string) + label = HintLabel(e, self._context) + label.update_text('', string) elem = ElemTuple(e, label) self._context.all_elems.append(elem) self._context.elems[string] = elem + keyparsers = objreg.get('keyparsers', scope='window', window=self._win_id) keyparser = keyparsers[usertypes.KeyMode.hint] keyparser.update_bindings(strings) - self._context.tab.contents_size_changed.connect( - self.on_contents_size_changed) message_bridge = objreg.get('message-bridge', scope='window', window=self._win_id) message_bridge.set_text(self._get_text()) @@ -777,21 +756,12 @@ class HintManager(QObject): return self._context.hint_mode - def _get_visible_hints(self): - """Get elements which are currently visible.""" - visible = {} - for string, elem in self._context.elems.items(): - try: - if not self._is_hidden(elem.label): - visible[string] = elem - except webelem.Error: - pass - return visible - def _handle_auto_follow(self, keystr="", filterstr="", visible=None): """Handle the auto-follow option.""" if visible is None: - visible = self._get_visible_hints() + visible = {string: elem + for string, elem in self._context.elems.items() + if elem.label.isVisible()} if len(visible) != 1: return @@ -830,19 +800,15 @@ class HintManager(QObject): if string.startswith(keystr): matched = string[:len(keystr)] rest = string[len(keystr):] - match_color = config.get('colors', 'hints.fg.match') - elem.label.set_inner_xml( - '{}{}'.format( - match_color, matched, rest)) - if self._is_hidden(elem.label): - # hidden element which matches again -> show it - self._show_elem(elem.label) + elem.label.update_text(matched, rest) + # Show label again if it was hidden before + elem.label.show() else: # element doesn't match anymore -> hide it, unless in rapid # mode and hide-unmatched-rapid-hints is false (see #1799) if (not self._context.rapid or config.get('hints', 'hide-unmatched-rapid-hints')): - self._hide_elem(elem.label) + elem.label.hide() except webelem.Error: pass self._handle_auto_follow(keystr=keystr) @@ -866,12 +832,11 @@ class HintManager(QObject): try: if self._filter_matches(filterstr, str(elem.elem)): visible.append(elem) - if self._is_hidden(elem.label): - # hidden element which matches again -> show it - self._show_elem(elem.label) + # Show label again if it was hidden before + elem.label.show() else: # element doesn't match anymore -> hide it - self._hide_elem(elem.label) + elem.label.hide() except webelem.Error: pass @@ -886,7 +851,7 @@ class HintManager(QObject): strings = self._hint_strings(visible) self._context.elems = {} for elem, string in zip(visible, strings): - elem.label.set_inner_xml(string) + elem.label.update_text('', string) self._context.elems[string] = elem keyparsers = objreg.get('keyparsers', scope='window', window=self._win_id) @@ -956,7 +921,7 @@ class HintManager(QObject): self.filter_hints(None) # Undo keystring highlighting for string, elem in self._context.elems.items(): - elem.label.set_inner_xml(string) + elem.label.update_text('', string) try: handler() @@ -980,20 +945,6 @@ class HintManager(QObject): raise cmdexc.CommandError("No hint {}!".format(keystring)) self._fire(keystring) - @pyqtSlot() - def on_contents_size_changed(self): - """Reposition hints if contents size changed.""" - log.hints.debug("Contents size changed...!") - for e in self._context.all_elems: - try: - if e.elem.frame() is None: - # This sometimes happens for some reason... - e.label.remove_from_document() - continue - self._set_style_position(e.elem, e.label) - except webelem.Error: - pass - @pyqtSlot(usertypes.KeyMode) def on_mode_left(self, mode): """Stop hinting when hinting mode was left.""" From cf7170a33b40f18b51948e4781eab9a0e1e77589 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 16 Aug 2016 17:15:44 +0200 Subject: [PATCH 02/17] Remove no longer needed webelem methods The following methods were only used for hint labels and thus removed now: - document_element - create_inside - find_first - set_inner_xml - remove_from_document - set_style_property --- qutebrowser/browser/webelem.py | 30 ------------ .../browser/webengine/webengineelem.py | 27 ---------- qutebrowser/browser/webkit/webkitelem.py | 34 ------------- tests/helpers/stubs.py | 4 +- tests/unit/browser/webkit/test_webkitelem.py | 49 +------------------ 5 files changed, 2 insertions(+), 142 deletions(-) diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py index 749cb151f..5a0dacf6d 100644 --- a/qutebrowser/browser/webelem.py +++ b/qutebrowser/browser/webelem.py @@ -112,21 +112,6 @@ class AbstractWebElement(collections.abc.MutableMapping): """Get the geometry for this element.""" raise NotImplementedError - def document_element(self): - """Get the document element of this element.""" - # FIXME:qtwebengine get rid of this? - raise NotImplementedError - - def create_inside(self, tagname): - """Append the given element inside the current one.""" - # FIXME:qtwebengine get rid of this? - raise NotImplementedError - - def find_first(self, selector): - """Find the first child based on the given CSS selector.""" - # FIXME:qtwebengine get rid of this? - raise NotImplementedError - def style_property(self, name, *, strategy): """Get the element style resolved with the given strategy.""" raise NotImplementedError @@ -166,21 +151,6 @@ class AbstractWebElement(collections.abc.MutableMapping): # FIXME:qtwebengine what to do about use_js with WebEngine? raise NotImplementedError - def set_inner_xml(self, xml): - """Set the given inner XML.""" - # FIXME:qtwebengine get rid of this? - raise NotImplementedError - - def remove_from_document(self): - """Remove the node from the document.""" - # FIXME:qtwebengine get rid of this? - raise NotImplementedError - - def set_style_property(self, name, value): - """Set the element style.""" - # FIXME:qtwebengine get rid of this? - raise NotImplementedError - def run_js_async(self, code, callback=None): """Run the given JS snippet async on the element.""" # FIXME:qtwebengine get rid of this? diff --git a/qutebrowser/browser/webengine/webengineelem.py b/qutebrowser/browser/webengine/webengineelem.py index afc12a1bc..982aa898a 100644 --- a/qutebrowser/browser/webengine/webengineelem.py +++ b/qutebrowser/browser/webengine/webengineelem.py @@ -66,18 +66,6 @@ class WebEngineElement(webelem.AbstractWebElement): log.stub() return QRect() - def document_element(self): - log.stub() - return None - - def create_inside(self, tagname): - log.stub() - return None - - def find_first(self, selector): - log.stub() - return None - def style_property(self, name, *, strategy): log.stub() return '' @@ -121,21 +109,6 @@ class WebEngineElement(webelem.AbstractWebElement): js_code = javascript.assemble('webelem', 'set_text', self._id, text) self._run_js(js_code) - def set_inner_xml(self, xml): - """Set the given inner XML.""" - # FIXME:qtwebengine get rid of this? - log.stub() - - def remove_from_document(self): - """Remove the node from the document.""" - # FIXME:qtwebengine get rid of this? - log.stub() - - def set_style_property(self, name, value): - """Set the element style.""" - # FIXME:qtwebengine get rid of this? - log.stub() - def run_js_async(self, code, callback=None): """Run the given JS snippet async on the element.""" # FIXME:qtwebengine get rid of this? diff --git a/qutebrowser/browser/webkit/webkitelem.py b/qutebrowser/browser/webkit/webkitelem.py index 895c48e59..e320a7e12 100644 --- a/qutebrowser/browser/webkit/webkitelem.py +++ b/qutebrowser/browser/webkit/webkitelem.py @@ -91,28 +91,6 @@ class WebKitElement(webelem.AbstractWebElement): self._check_vanished() return self._elem.geometry() - def document_element(self): - self._check_vanished() - elem = self._elem.webFrame().documentElement() - return WebKitElement(elem) - - def create_inside(self, tagname): - # It seems impossible to create an empty QWebElement for which isNull() - # is false so we can work with it. - # As a workaround, we use appendInside() with markup as argument, and - # then use lastChild() to get a reference to it. - # See: http://stackoverflow.com/q/7364852/2085149 - self._check_vanished() - self._elem.appendInside('<{}>'.format(tagname, tagname)) - return WebKitElement(self._elem.lastChild()) - - def find_first(self, selector): - self._check_vanished() - elem = self._elem.findFirst(selector) - if elem.isNull(): - return None - return WebKitElement(elem) - def style_property(self, name, *, strategy): self._check_vanished() strategies = { @@ -156,18 +134,6 @@ class WebKitElement(webelem.AbstractWebElement): text = javascript.string_escape(text) self._elem.evaluateJavaScript("this.value='{}'".format(text)) - def set_inner_xml(self, xml): - self._check_vanished() - self._elem.setInnerXml(xml) - - def remove_from_document(self): - self._check_vanished() - self._elem.removeFromDocument() - - def set_style_property(self, name, value): - self._check_vanished() - return self._elem.setStyleProperty(name, value) - def run_js_async(self, code, callback=None): """Run the given JS snippet async on the element.""" self._check_vanished() diff --git a/tests/helpers/stubs.py b/tests/helpers/stubs.py index e79ed0b74..169190853 100644 --- a/tests/helpers/stubs.py +++ b/tests/helpers/stubs.py @@ -80,7 +80,7 @@ class FakeWebFrame: """ def __init__(self, geometry=None, *, scroll=None, plaintext=None, - html=None, parent=None, zoom=1.0, document_element=None): + html=None, parent=None, zoom=1.0): """Constructor. Args: @@ -89,7 +89,6 @@ class FakeWebFrame: plaintext: Return value of toPlainText html: Return value of tohtml. zoom: The zoom factor. - document_element: The documentElement() to return parent: The parent frame. """ if scroll is None: @@ -101,7 +100,6 @@ class FakeWebFrame: self.toPlainText = mock.Mock(return_value=plaintext) self.toHtml = mock.Mock(return_value=html) self.zoomFactor = mock.Mock(return_value=zoom) - self.documentElement = mock.Mock(return_value=document_element) def findFirstElement(self, selector): if selector == '*:focus': diff --git a/tests/unit/browser/webkit/test_webkitelem.py b/tests/unit/browser/webkit/test_webkitelem.py index 278f7834d..9c88b35fe 100644 --- a/tests/unit/browser/webkit/test_webkitelem.py +++ b/tests/unit/browser/webkit/test_webkitelem.py @@ -255,15 +255,9 @@ class TestWebKitElement: len, lambda e: e.frame(), lambda e: e.geometry(), - lambda e: e.document_element(), - lambda e: e.create_inside('span'), - lambda e: e.find_first('span'), lambda e: e.style_property('visibility', strategy='computed'), lambda e: e.text(), lambda e: e.set_text('foo'), - lambda e: e.set_inner_xml(''), - lambda e: e.remove_from_document(), - lambda e: e.set_style_property('visibility', 'hidden'), lambda e: e.is_writable(), lambda e: e.is_content_editable(), lambda e: e.is_editable(), @@ -276,9 +270,7 @@ class TestWebKitElement: lambda e: e.rect_on_view(), lambda e: e.is_visible(None), ], ids=['str', 'getitem', 'setitem', 'delitem', 'contains', 'iter', 'len', - 'frame', 'geometry', 'document_element', 'create_inside', - 'find_first', 'style_property', 'text', 'set_text', - 'set_inner_xml', 'remove_from_document', 'set_style_property', + 'frame', 'geometry', 'style_property', 'text', 'set_text', 'is_writable', 'is_content_editable', 'is_editable', 'is_text_input', 'remove_blank_target', 'debug_text', 'outer_xml', 'tag_name', 'run_js_async', 'rect_on_view', 'is_visible']) @@ -410,17 +402,6 @@ class TestWebKitElement: setattr(mock, 'return_value', sentinel) assert code(elem) is sentinel - @pytest.mark.parametrize('code, method, args', [ - (lambda e: e.set_inner_xml('foo'), 'setInnerXml', ['foo']), - (lambda e: e.set_style_property('foo', 'bar'), 'setStyleProperty', - ['foo', 'bar']), - (lambda e: e.remove_from_document(), 'removeFromDocument', []), - ]) - def test_simple_setters(self, elem, code, method, args): - code(elem) - mock = getattr(elem._elem, method) - mock.assert_called_with(*args) - def test_tag_name(self, elem): elem._elem.tagName.return_value = 'SPAN' assert elem.tag_name() == 'span' @@ -428,34 +409,6 @@ class TestWebKitElement: def test_style_property(self, elem): assert elem.style_property('foo', strategy='computed') == 'bar' - def test_document_element(self, stubs): - doc_elem = get_webelem() - frame = stubs.FakeWebFrame(document_element=doc_elem._elem) - elem = get_webelem(frame=frame) - - doc_elem_ret = elem.document_element() - assert isinstance(doc_elem_ret, webkitelem.WebKitElement) - assert doc_elem_ret == doc_elem - - def test_find_first(self, elem): - result = get_webelem() - elem._elem.findFirst.return_value = result._elem - find_result = elem.find_first('') - assert isinstance(find_result, webkitelem.WebKitElement) - assert find_result == result - - def test_create_inside(self, elem): - child = get_webelem() - elem._elem.lastChild.return_value = child._elem - assert elem.create_inside('span')._elem is child._elem - elem._elem.appendInside.assert_called_with('') - - def test_find_first_null(self, elem): - nullelem = get_webelem() - nullelem._elem.isNull.return_value = True - elem._elem.findFirst.return_value = nullelem._elem - assert elem.find_first('foo') is None - @pytest.mark.parametrize('use_js, editable, expected', [ (True, 'false', 'js'), (True, 'true', 'nojs'), From 9c3807c117df66cdacfe5ccf53cf2b61a3f7846d Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 16 Aug 2016 17:22:27 +0200 Subject: [PATCH 03/17] Store HintLabels (not ElemTuples) in HintManager --- qutebrowser/browser/hints.py | 70 +++++++++++++++++------------------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index c74a6dff7..1c30f80c8 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -39,9 +39,6 @@ from qutebrowser.commands import userscripts, cmdexc, cmdutils, runners from qutebrowser.utils import usertypes, log, qtutils, message, objreg, utils -ElemTuple = collections.namedtuple('ElemTuple', ['elem', 'label']) - - Target = usertypes.enum('Target', ['normal', 'current', 'tab', 'tab_fg', 'tab_bg', 'window', 'yank', 'yank_primary', 'run', 'fill', 'hover', 'download', @@ -130,9 +127,9 @@ class HintContext: """Context namespace used for hinting. Attributes: - all_elems: A list of all (elem, label) namedtuples ever created. - elems: A mapping from key strings to (elem, label) namedtuples. - May contain less elements than `all_elems` due to filtering. + all_labels: A list of all HintLabel objects ever created. + labels: A mapping from key strings to HintLabel objects. + May contain less elements than `all_labels` due to filtering. baseurl: The URL of the current page. target: What to do with the opened links. normal/current/tab/tab_fg/tab_bg/window: Get passed to @@ -152,8 +149,8 @@ class HintContext: """ def __init__(self): - self.all_elems = [] - self.elems = {} + self.all_labels = [] + self.labels = {} self.target = None self.baseurl = None self.to_follow = None @@ -430,8 +427,8 @@ class HintManager(QObject): def _cleanup(self): """Clean up after hinting.""" - for elem in self._context.all_elems: - elem.label.cleanup() + for label in self._context.all_labels: + label.cleanup() text = self._get_text() message_bridge = objreg.get('message-bridge', scope='window', @@ -625,12 +622,11 @@ class HintManager(QObject): strings = self._hint_strings(elems) log.hints.debug("hints: {}".format(', '.join(strings))) - for e, string in zip(elems, strings): - label = HintLabel(e, self._context) + for elem, string in zip(elems, strings): + label = HintLabel(elem, self._context) label.update_text('', string) - elem = ElemTuple(e, label) - self._context.all_elems.append(elem) - self._context.elems[string] = elem + self._context.all_labels.append(label) + self._context.labels[string] = label keyparsers = objreg.get('keyparsers', scope='window', window=self._win_id) @@ -759,9 +755,9 @@ class HintManager(QObject): def _handle_auto_follow(self, keystr="", filterstr="", visible=None): """Handle the auto-follow option.""" if visible is None: - visible = {string: elem - for string, elem in self._context.elems.items() - if elem.label.isVisible()} + visible = {string: label + for string, label in self._context.labels.items() + if label.isVisible()} if len(visible) != 1: return @@ -795,20 +791,20 @@ class HintManager(QObject): def handle_partial_key(self, keystr): """Handle a new partial keypress.""" log.hints.debug("Handling new keystring: '{}'".format(keystr)) - for string, elem in self._context.elems.items(): + for string, label in self._context.labels.items(): try: if string.startswith(keystr): matched = string[:len(keystr)] rest = string[len(keystr):] - elem.label.update_text(matched, rest) + label.update_text(matched, rest) # Show label again if it was hidden before - elem.label.show() + label.show() else: # element doesn't match anymore -> hide it, unless in rapid # mode and hide-unmatched-rapid-hints is false (see #1799) if (not self._context.rapid or config.get('hints', 'hide-unmatched-rapid-hints')): - elem.label.hide() + label.hide() except webelem.Error: pass self._handle_auto_follow(keystr=keystr) @@ -828,15 +824,15 @@ class HintManager(QObject): self._context.filterstr = filterstr visible = [] - for elem in self._context.all_elems: + for label in self._context.all_labels: try: - if self._filter_matches(filterstr, str(elem.elem)): - visible.append(elem) + if self._filter_matches(filterstr, str(label.elem)): + visible.append(label) # Show label again if it was hidden before - elem.label.show() + label.show() else: # element doesn't match anymore -> hide it - elem.label.hide() + label.hide() except webelem.Error: pass @@ -849,10 +845,10 @@ class HintManager(QObject): if self._context.hint_mode == 'number': # renumber filtered hints strings = self._hint_strings(visible) - self._context.elems = {} - for elem, string in zip(visible, strings): - elem.label.update_text('', string) - self._context.elems[string] = elem + self._context.labels = {} + for label, string in zip(visible, strings): + label.update_text('', string) + self._context.labels[string] = label keyparsers = objreg.get('keyparsers', scope='window', window=self._win_id) keyparser = keyparsers[usertypes.KeyMode.hint] @@ -861,9 +857,9 @@ class HintManager(QObject): # Note: filter_hints can be called with non-None filterstr only # when number mode is active if filterstr is not None: - # pass self._context.elems as the dict of visible hints + # pass self._context.labels as the dict of visible hints self._handle_auto_follow(filterstr=filterstr, - visible=self._context.elems) + visible=self._context.labels) def _fire(self, keystr): """Fire a completed hint. @@ -892,7 +888,7 @@ class HintManager(QObject): Target.fill: self._actions.preset_cmd_text, Target.spawn: self._actions.spawn, } - elem = self._context.elems[keystr].elem + elem = self._context.labels[keystr].elem if elem.frame() is None: message.error(self._win_id, @@ -920,8 +916,8 @@ class HintManager(QObject): # Reset filtering self.filter_hints(None) # Undo keystring highlighting - for string, elem in self._context.elems.items(): - elem.label.update_text('', string) + for string, label in self._context.labels.items(): + label.update_text('', string) try: handler() @@ -941,7 +937,7 @@ class HintManager(QObject): raise cmdexc.CommandError("No hint to follow") else: keystring = self._context.to_follow - elif keystring not in self._context.elems: + elif keystring not in self._context.labels: raise cmdexc.CommandError("No hint {}!".format(keystring)) self._fire(keystring) From c033ed9edbfac5ff195fab63990704f1ecb014e9 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 16 Aug 2016 18:38:45 +0200 Subject: [PATCH 04/17] Allow config value transformers to give up --- qutebrowser/config/config.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index df7ce6b96..d53e02fdb 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -525,7 +525,15 @@ class ConfigManager(QObject): k = self.RENAMED_OPTIONS[sectname, k] if (sectname, k) in self.CHANGED_OPTIONS: func = self.CHANGED_OPTIONS[(sectname, k)] - v = func(v) + new_v = func(v) + if new_v is None: + exc = configexc.ValidationError( + v, "Could not automatically migrate the given value") + exc.section = sectname + exc.option = k + raise exc + + v = new_v try: self.set('conf', sectname, k, v, validate=False) From eac30fc84b297abecdb761a66e313d24b9b8d631 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 16 Aug 2016 17:44:48 +0200 Subject: [PATCH 05/17] Add styling of new hints This also removes the hints -> opacity setting, as this is now set by using an rgba(...) color. --- doc/help/settings.asciidoc | 13 +++-------- qutebrowser/browser/hints.py | 25 +++++++++++++------- qutebrowser/config/config.py | 40 ++++++++++++++++++++++++++++++++ qutebrowser/config/configdata.py | 18 ++++++-------- 4 files changed, 67 insertions(+), 29 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index e70f06b6b..9c346a249 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -178,7 +178,6 @@ |============== |Setting|Description |<>|CSS border value for hints. -|<>|Opacity for hints. |<>|Mode to use for hints. |<>|Chars used for hint strings. |<>|Minimum number of chars used for hint strings. @@ -248,7 +247,7 @@ |<>|Color for the tab indicator on errors.. |<>|Color gradient interpolation system for the tab indicator. |<>|Font color for hints. -|<>|Background color for hints. +|<>|Background color for hints. Note that you can use a `rgba(...)` value for transparency. |<>|Font color for the matched part of hints. |<>|Background color for the download bar. |<>|Color gradient start for download text. @@ -1601,12 +1600,6 @@ CSS border value for hints. Default: +pass:[1px solid #E3BE23]+ -[[hints-opacity]] -=== opacity -Opacity for hints. - -Default: +pass:[0.7]+ - [[hints-mode]] === mode Mode to use for hints. @@ -2052,9 +2045,9 @@ Default: +pass:[black]+ [[colors-hints.bg]] === hints.bg -Background color for hints. +Background color for hints. Note that you can use a `rgba(...)` value for transparency. -Default: +pass:[-webkit-gradient(linear, left top, left bottom, color-stop(0%,#FFF785), color-stop(100%,#FFC542))]+ +Default: +pass:[qlineargradient(x1:0, y1:0, x2:1, y2:1, stop:0 rgba(255, 247, 133, 0.8), stop:1 rgba(255, 197, 66, 0.8))]+ [[colors-hints.fg.match]] === hints.fg.match diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 1c30f80c8..86af0162e 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -32,7 +32,7 @@ from PyQt5.QtGui import QMouseEvent from PyQt5.QtWidgets import QLabel from PyQt5.QtWebKitWidgets import QWebPage -from qutebrowser.config import config +from qutebrowser.config import config, style from qutebrowser.keyinput import modeman, modeparsers from qutebrowser.browser import webelem from qutebrowser.commands import userscripts, cmdexc, cmdutils, runners @@ -65,22 +65,30 @@ class HintLabel(QLabel): _context: The current hinting context. """ + STYLESHEET = """ + QLabel { + background-color: {{ color['hints.bg'] }}; + color: {{ color['hints.fg'] }}; + font: {{ font['hints'] }}; + border: {{ config.get('hints', 'border') }}; + padding-left: -3px; + padding-right: -3px; + } + """ + def __init__(self, elem, context): super().__init__(parent=context.tab) self._context = context self.elem = elem + + self.setAttribute(Qt.WA_StyledBackground, True) + style.set_register_stylesheet(self) + self._context.tab.contents_size_changed.connect( self._on_contents_size_changed) self._move_to_elem() self.show() - # FIXME styling - # ('color', config.get('colors', 'hints.fg') + ' !important'), - # ('background', config.get('colors', 'hints.bg') + ' !important'), - # ('font', config.get('fonts', 'hints') + ' !important'), - # ('border', config.get('hints', 'border') + ' !important'), - # ('opacity', str(config.get('hints', 'opacity')) + ' !important'), - def update_text(self, matched, unmatched): """Set the text for the hint. @@ -99,6 +107,7 @@ class HintLabel(QLabel): match_color = html.escape(config.get('colors', 'hints.fg.match')) self.setText('{}{}'.format( match_color, matched, unmatched)) + self.adjustSize() def _move_to_elem(self): """Reposition the label to its element.""" diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index d53e02fdb..f5ce60d3f 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -24,6 +24,7 @@ are fundamentally different. This is why nothing inherits from configparser, but we borrow some methods and classes from there where it makes sense. """ +import re import os import sys import os.path @@ -34,6 +35,7 @@ import collections import collections.abc from PyQt5.QtCore import pyqtSignal, QObject, QUrl, QSettings +from PyQt5.QtGui import QColor from qutebrowser.config import configdata, configexc, textwrapper from qutebrowser.config.parsers import keyconf @@ -286,6 +288,40 @@ def _transform_position(val): return val +def _transform_hint_color(val): + """Transformer for hint colors.""" + log.config.debug("Transforming hint value {}".format(val)) + + def to_rgba(qcolor): + """Convert a QColor to a rgba() value.""" + return 'rgba({}, {}, {}, 0.8)'.format(qcolor.red(), qcolor.green(), + qcolor.blue()) + + if val.startswith('-webkit-gradient'): + pattern = re.compile(r'-webkit-gradient\(linear, left top, ' + r'left bottom, ' + r'color-stop\(0%, *(#[a-fA-F0-9]{3,6})\), ' + r'color-stop\(100%, *(#[a-fA-F0-9]{3,6})\)') + + match = pattern.match(val) + if match: + log.config.debug('Color groups: {}'.format(match.groups())) + start_color = QColor(match.group(1)) + stop_color = QColor(match.group(2)) + qtutils.ensure_valid(start_color) + qtutils.ensure_valid(stop_color) + + return ('qlineargradient(x1:0, y1:0, x2:1, y2:1, stop:0 {}, ' + 'stop:1 {})'.format(to_rgba(start_color), + to_rgba(stop_color))) + else: + return None + elif val.startswith('-'): # Custom CSS stuff? + return None + else: # Already transformed or a named color. + return val + + class ConfigManager(QObject): """Configuration manager for qutebrowser. @@ -353,6 +389,7 @@ class ConfigManager(QObject): ('ui', 'display-statusbar-messages'), ('ui', 'hide-mouse-cursor'), ('general', 'wrap-search'), + ('hints', 'opacity'), ] CHANGED_OPTIONS = { ('content', 'cookies-accept'): @@ -367,6 +404,9 @@ class ConfigManager(QObject): _get_value_transformer({'false': '*', 'true': ''}), ('hints', 'auto-follow'): _get_value_transformer({'false': 'never', 'true': 'unique-match'}), + ('colors', 'hints.bg'): _transform_hint_color, + ('colors', 'hints.fg'): _transform_hint_color, + ('colors', 'hints.fg.match'): _transform_hint_color, } changed = pyqtSignal(str, str) diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 9b8779037..f7a5474b6 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -897,10 +897,6 @@ def data(readonly=False): SettingValue(typ.String(), '1px solid #E3BE23'), "CSS border value for hints."), - ('opacity', - SettingValue(typ.Float(minval=0.0, maxval=1.0), '0.7'), - "Opacity for hints."), - ('mode', SettingValue(typ.String( valid_values=typ.ValidValues( @@ -1209,18 +1205,18 @@ def data(readonly=False): "Color gradient interpolation system for the tab indicator."), ('hints.fg', - SettingValue(typ.CssColor(), 'black'), + SettingValue(typ.QssColor(), 'black'), "Font color for hints."), ('hints.bg', - SettingValue( - typ.CssColor(), '-webkit-gradient(linear, left top, ' - 'left bottom, color-stop(0%,#FFF785), ' - 'color-stop(100%,#FFC542))'), - "Background color for hints."), + SettingValue(typ.QssColor(), 'qlineargradient(x1:0, y1:0, x2:1, ' + 'y2:1, stop:0 rgba(255, 247, 133, 0.8), ' + 'stop:1 rgba(255, 197, 66, 0.8))'), + "Background color for hints. Note that you can use a `rgba(...)` " + "value for transparency."), ('hints.fg.match', - SettingValue(typ.CssColor(), 'green'), + SettingValue(typ.QssColor(), 'green'), "Font color for the matched part of hints."), ('downloads.bg.bar', From 373eca1ad26af4d8f3ef4fc3a690e3a0c51edfdb Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 16 Aug 2016 19:38:37 +0200 Subject: [PATCH 06/17] Fix some corner cases in _transform_hint_color --- qutebrowser/config/config.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index f5ce60d3f..43d439bac 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -300,16 +300,16 @@ def _transform_hint_color(val): if val.startswith('-webkit-gradient'): pattern = re.compile(r'-webkit-gradient\(linear, left top, ' r'left bottom, ' - r'color-stop\(0%, *(#[a-fA-F0-9]{3,6})\), ' - r'color-stop\(100%, *(#[a-fA-F0-9]{3,6})\)') + r'color-stop\(0%, *([^)]*)\), ' + r'color-stop\(100%, *([^)]*)\)\)') - match = pattern.match(val) + match = pattern.fullmatch(val) if match: log.config.debug('Color groups: {}'.format(match.groups())) start_color = QColor(match.group(1)) stop_color = QColor(match.group(2)) - qtutils.ensure_valid(start_color) - qtutils.ensure_valid(stop_color) + if not start_color.isValid() or not stop_color.isValid(): + return None return ('qlineargradient(x1:0, y1:0, x2:1, y2:1, stop:0 {}, ' 'stop:1 {})'.format(to_rgba(start_color), From 4860ad5487deceab8ddd376c47d6981666a48b69 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 16 Aug 2016 19:38:46 +0200 Subject: [PATCH 07/17] Add some tests for config transformers --- tests/unit/config/test_config.py | 47 ++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index eb8aaa9e8..e6205412f 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -210,6 +210,53 @@ class TestConfigParser: assert objects.cfg.get('general', 'save-session') +class TestTransformers: + + """Test value transformers in CHANGED_OPTIONS.""" + + @pytest.mark.parametrize('val, expected', [('a', 'b'), ('c', 'c')]) + def test_get_value_transformer(self, val, expected): + func = config._get_value_transformer({'a': 'b'}) + assert func(val) == expected + + @pytest.mark.parametrize('val, expected', [ + ('top', 'top'), + ('north', 'top'), + ('south', 'bottom'), + ('west', 'left'), + ('east', 'right'), + ]) + def test_position(self, val, expected): + func = config._transform_position + assert func(val) == expected + + OLD_GRADIENT = ('-webkit-gradient(linear, left top, left bottom, ' + 'color-stop(0%,{}), color-stop(100%,{}))') + NEW_GRADIENT = ('qlineargradient(x1:0, y1:0, x2:1, y2:1, stop:0 {}, ' + 'stop:1 {})') + + @pytest.mark.parametrize('val, expected', [ + ('-unknown-stuff', None), + ('blue', 'blue'), + ('rgba(1, 2, 3, 4)', 'rgba(1, 2, 3, 4)'), + ('-webkit-gradient(unknown)', None), + (OLD_GRADIENT.format('blah', 'blah'), None), + (OLD_GRADIENT.format('red', 'green'), + NEW_GRADIENT.format('rgba(255, 0, 0, 0.8)', 'rgba(0, 128, 0, 0.8)')), + (OLD_GRADIENT.format(' red', ' green'), + NEW_GRADIENT.format('rgba(255, 0, 0, 0.8)', 'rgba(0, 128, 0, 0.8)')), + (OLD_GRADIENT.format('#101010', ' #202020'), + NEW_GRADIENT.format('rgba(16, 16, 16, 0.8)', + 'rgba(32, 32, 32, 0.8)')), + (OLD_GRADIENT.format('#666', ' #777'), + NEW_GRADIENT.format('rgba(102, 102, 102, 0.8)', + 'rgba(119, 119, 119, 0.8)')), + (OLD_GRADIENT.format('red', 'green') + 'more stuff', None), + ]) + def test_hint_color(self, val, expected): + assert config._transform_hint_color(val) == expected + + class TestKeyConfigParser: """Test config.parsers.keyconf.KeyConfigParser.""" From 016349941b8f1df7851756198a3701b605dd928f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 16 Aug 2016 20:02:28 +0200 Subject: [PATCH 08/17] Update vulture whitelist --- scripts/dev/run_vulture.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/dev/run_vulture.py b/scripts/dev/run_vulture.py index 184b711e1..240258297 100755 --- a/scripts/dev/run_vulture.py +++ b/scripts/dev/run_vulture.py @@ -61,6 +61,8 @@ def whitelist_generator(): yield 'qutebrowser.utils.debug.qflags_key' yield 'qutebrowser.utils.qtutils.QtOSError.qt_errno' yield 'scripts.utils.bg_colors' + yield 'qutebrowser.browser.webelem.AbstractWebElement.style_property' + yield 'qutebrowser.config.configtypes.Float' # Qt attributes yield 'PyQt5.QtWebKit.QWebPage.ErrorPageExtensionReturn().baseUrl' From 7c17af3889b15845fbccf9af07c224ca4c4dbe29 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 17 Aug 2016 12:42:24 +0200 Subject: [PATCH 09/17] Use ${_monospace} for default hints font --- doc/help/settings.asciidoc | 2 +- qutebrowser/config/config.py | 11 +++++++++++ qutebrowser/config/configdata.py | 2 +- tests/unit/config/test_config.py | 9 +++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 9c346a249..7f2041fcc 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -2194,7 +2194,7 @@ Default: +pass:[8pt ${_monospace}]+ === hints Font used for the hints. -Default: +pass:[bold 13px Monospace]+ +Default: +pass:[bold 13px ${_monospace}]+ [[fonts-debug-console]] === debug-console diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 43d439bac..931fbdabe 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -322,6 +322,16 @@ def _transform_hint_color(val): return val +def _transform_hint_font(val): + """Transformer for fonts -> hints.""" + match = re.fullmatch(r'(.*\d+p[xt]) Monospace', val) + if match: + # Close enough to the old default: + return match.group(1) + ' ${_monospace}' + else: + return val + + class ConfigManager(QObject): """Configuration manager for qutebrowser. @@ -407,6 +417,7 @@ class ConfigManager(QObject): ('colors', 'hints.bg'): _transform_hint_color, ('colors', 'hints.fg'): _transform_hint_color, ('colors', 'hints.fg.match'): _transform_hint_color, + ('fonts', 'hints'): _transform_hint_font, } changed = pyqtSignal(str, str) diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index f7a5474b6..14f98e97a 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -1305,7 +1305,7 @@ def data(readonly=False): "Font used for the downloadbar."), ('hints', - SettingValue(typ.Font(), 'bold 13px Monospace'), + SettingValue(typ.Font(), 'bold 13px ${_monospace}'), "Font used for the hints."), ('debug-console', diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index e6205412f..e05f91f15 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -256,6 +256,15 @@ class TestTransformers: def test_hint_color(self, val, expected): assert config._transform_hint_color(val) == expected + @pytest.mark.parametrize('val, expected', [ + ('bold 12pt Monospace', 'bold 12pt ${_monospace}'), + ('23pt Monospace', '23pt ${_monospace}'), + ('bold 12pt ${_monospace}', 'bold 12pt ${_monospace}'), + ('bold 12pt Comic Sans MS', 'bold 12pt Comic Sans MS'), + ]) + def test_hint_font(self, val, expected): + assert config._transform_hint_font(val) == expected + class TestKeyConfigParser: From 0293307d61833e3e1b217ebb1c6e9b55a48a5c1b Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 17 Aug 2016 12:56:54 +0200 Subject: [PATCH 10/17] Remove adjust_zoom for webelem.rect_on_view Previously, the drawn hint labels were affected by the zoom, i.e., they were stretched out by QtWebKit and actually had to be drawn at the unzoomed position. The Python/C++ API gives us coordinated adjusted for zoom, so we always *negatively* adjusted them to get the unzoomed coordinates. JS gave us the original coordinates, so we stretched them out according to the zoom if adjust_zoom was given (which means only when clicking a link). Now we always operate in term of display coordinates: The point where we draw the hint label is equal to the point we're clicking. Thus, the zoom level for javascript is always adjusted, and the Python zoom level is never (negatively) adjusted. --- qutebrowser/browser/hints.py | 2 +- qutebrowser/browser/webelem.py | 5 +---- .../browser/webengine/webengineelem.py | 5 +---- qutebrowser/browser/webkit/webkitelem.py | 19 ++++--------------- tests/unit/browser/webkit/test_webkitelem.py | 10 ++-------- 5 files changed, 9 insertions(+), 32 deletions(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 86af0162e..93c3d00d1 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -112,7 +112,7 @@ class HintLabel(QLabel): def _move_to_elem(self): """Reposition the label to its element.""" no_js = config.get('hints', 'find-implementation') != 'javascript' - rect = self.elem.rect_on_view(adjust_zoom=False, no_js=no_js) + rect = self.elem.rect_on_view(no_js=no_js) self.move(rect.x(), rect.y()) @pyqtSlot() diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py index 5a0dacf6d..766c1d9e1 100644 --- a/qutebrowser/browser/webelem.py +++ b/qutebrowser/browser/webelem.py @@ -161,8 +161,7 @@ class AbstractWebElement(collections.abc.MutableMapping): # FIXME:qtwebengine get rid of this? raise NotImplementedError - def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, - no_js=False): + def rect_on_view(self, *, elem_geometry=None, no_js=False): """Get the geometry of the element relative to the webview. Uses the getClientRects() JavaScript method to obtain the collection of @@ -178,8 +177,6 @@ class AbstractWebElement(collections.abc.MutableMapping): elem_geometry: The geometry of the element, or None. Calling QWebElement::geometry is rather expensive so we want to avoid doing it twice. - adjust_zoom: Whether to adjust the element position based on the - current zoom level. no_js: Fall back to the Python implementation """ raise NotImplementedError diff --git a/qutebrowser/browser/webengine/webengineelem.py b/qutebrowser/browser/webengine/webengineelem.py index 982aa898a..0e586ce10 100644 --- a/qutebrowser/browser/webengine/webengineelem.py +++ b/qutebrowser/browser/webengine/webengineelem.py @@ -120,8 +120,7 @@ class WebEngineElement(webelem.AbstractWebElement): log.stub() return None - def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, - no_js=False): + def rect_on_view(self, *, elem_geometry=None, no_js=False): """Get the geometry of the element relative to the webview. Uses the getClientRects() JavaScript method to obtain the collection of @@ -137,8 +136,6 @@ class WebEngineElement(webelem.AbstractWebElement): elem_geometry: The geometry of the element, or None. Calling QWebElement::geometry is rather expensive so we want to avoid doing it twice. - adjust_zoom: Whether to adjust the element position based on the - current zoom level. no_js: Fall back to the Python implementation """ log.stub() diff --git a/qutebrowser/browser/webkit/webkitelem.py b/qutebrowser/browser/webkit/webkitelem.py index e320a7e12..667636f90 100644 --- a/qutebrowser/browser/webkit/webkitelem.py +++ b/qutebrowser/browser/webkit/webkitelem.py @@ -148,7 +148,7 @@ class WebKitElement(webelem.AbstractWebElement): return None return WebKitElement(elem) - def _rect_on_view_js(self, adjust_zoom): + def _rect_on_view_js(self): """Javascript implementation for rect_on_view.""" # FIXME:qtwebengine maybe we can reuse this? rects = self._elem.evaluateJavaScript("this.getClientRects()") @@ -169,7 +169,7 @@ class WebKitElement(webelem.AbstractWebElement): if width > 1 and height > 1: # fix coordinates according to zoom level zoom = self._elem.webFrame().zoomFactor() - if not config.get('ui', 'zoom-text-only') and adjust_zoom: + if not config.get('ui', 'zoom-text-only'): rect["left"] *= zoom rect["top"] *= zoom width *= zoom @@ -197,18 +197,9 @@ class WebKitElement(webelem.AbstractWebElement): rect.translate(frame.geometry().topLeft()) rect.translate(frame.scrollPosition() * -1) frame = frame.parentFrame() - # We deliberately always adjust the zoom here, even with - # adjust_zoom=False - if elem_geometry is None: - zoom = self._elem.webFrame().zoomFactor() - if not config.get('ui', 'zoom-text-only'): - rect.moveTo(rect.left() / zoom, rect.top() / zoom) - rect.setWidth(rect.width() / zoom) - rect.setHeight(rect.height() / zoom) return rect - def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, - no_js=False): + def rect_on_view(self, *, elem_geometry=None, no_js=False): """Get the geometry of the element relative to the webview. Uses the getClientRects() JavaScript method to obtain the collection of @@ -224,8 +215,6 @@ class WebKitElement(webelem.AbstractWebElement): elem_geometry: The geometry of the element, or None. Calling QWebElement::geometry is rather expensive so we want to avoid doing it twice. - adjust_zoom: Whether to adjust the element position based on the - current zoom level. no_js: Fall back to the Python implementation """ # FIXME:qtwebengine can we get rid of this with @@ -235,7 +224,7 @@ class WebKitElement(webelem.AbstractWebElement): # First try getting the element rect via JS, as that's usually more # accurate if elem_geometry is None and not no_js: - rect = self._rect_on_view_js(adjust_zoom) + rect = self._rect_on_view_js() if rect is not None: return rect diff --git a/tests/unit/browser/webkit/test_webkitelem.py b/tests/unit/browser/webkit/test_webkitelem.py index 9c88b35fe..7ca961f16 100644 --- a/tests/unit/browser/webkit/test_webkitelem.py +++ b/tests/unit/browser/webkit/test_webkitelem.py @@ -755,20 +755,14 @@ class TestRectOnView: @pytest.mark.parametrize('js_rect', [None, {}]) @pytest.mark.parametrize('zoom_text_only', [True, False]) - @pytest.mark.parametrize('adjust_zoom', [True, False]) - def test_zoomed(self, stubs, config_stub, js_rect, zoom_text_only, - adjust_zoom): + def test_zoomed(self, stubs, config_stub, js_rect, zoom_text_only): """Make sure the coordinates are adjusted when zoomed.""" config_stub.data = {'ui': {'zoom-text-only': zoom_text_only}} geometry = QRect(10, 10, 4, 4) frame = stubs.FakeWebFrame(QRect(0, 0, 100, 100), zoom=0.5) elem = get_webelem(geometry, frame, js_rect_return=js_rect, zoom_text_only=zoom_text_only) - rect = elem.rect_on_view(adjust_zoom=adjust_zoom) - if zoom_text_only or (js_rect is None and adjust_zoom): - assert rect == QRect(10, 10, 4, 4) - else: - assert rect == QRect(20, 20, 8, 8) + assert elem.rect_on_view() == QRect(10, 10, 4, 4) class TestGetChildFrames: From 8cd822c7db6b917ca817bc8ced43cedc6f52cf14 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 17 Aug 2016 13:54:36 +0200 Subject: [PATCH 11/17] Add blank lines --- tests/unit/browser/webkit/test_webkitelem.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/browser/webkit/test_webkitelem.py b/tests/unit/browser/webkit/test_webkitelem.py index 7ca961f16..fb179c590 100644 --- a/tests/unit/browser/webkit/test_webkitelem.py +++ b/tests/unit/browser/webkit/test_webkitelem.py @@ -67,11 +67,13 @@ def get_webelem(geometry=None, frame=None, *, null=False, style=None, else: scroll_x = frame.scrollPosition().x() scroll_y = frame.scrollPosition().y() + if js_rect_return is None: if frame is None or zoom_text_only: zoom = 1.0 else: zoom = frame.zoomFactor() + elem.evaluateJavaScript.return_value = { "length": 1, "0": { From f42f54f403a20f31a6ead4579a41c8938c5f7207 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 17 Aug 2016 15:17:13 +0200 Subject: [PATCH 12/17] Update changelog --- CHANGELOG.asciidoc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 9cbfea251..c450e4d6b 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -42,6 +42,20 @@ Added Changed ~~~~~~~ +- Hints are now drawn natively in Qt instead of using web elements. This has a + few implications for users: + * The `hints -> opacity` setting does not exist anymore, but you can use + `rgba(r, g, b, alpha)` colors instead for `colors -> hints.bg`. + * The `hints -> font` setting is not affected by + `fonts -> web-family-fixed` anymore. Thus, a transformer got added to + change `Monospace` to `${_monospace}`. + * Gradients in hint colors can now be configured by using `qlineargradient` + and friends instead of `-webkit-gradient`. The most common cases get + migrated automatically, but if you drastically changed the defaults, + you'll need to manually adjust your config. + * Styling hints by styling `qutehint` elements in `user-stylesheet` was + never officially supported and does not work anymore. + * Hints are now not affected by the page's stylesheet or zoom anymore. - `:bookmark-add` now has a `--toggle` flag which deletes the bookmark if it already exists. - `:bookmark-load` now has a `--delete` flag which deletes the bookmark after @@ -99,6 +113,7 @@ Removed into a new `:completion-focus {prev,next}` command and thus removed. - The `ui -> hide-mouse-cursor` setting since it was completely broken and nobody seemed to care. +- The `hints -> opacity` setting - see the "Changed" section for details. Fixed ~~~~~ From d87a255c0a0d0fa3c782f209b9816b09d7b8ab91 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 17 Aug 2016 15:24:10 +0200 Subject: [PATCH 13/17] Don't fully clean up labels if their frame is gone Otherwise they're invalid but still in the HintContext, so calling .hide() on it later (e.g. because the user pressed another key) would give us a RuntimeError from PyQt. --- qutebrowser/browser/hints.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 93c3d00d1..6f575bf2c 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -121,7 +121,7 @@ class HintLabel(QLabel): log.hints.debug("Contents size changed...!") if self.elem.frame() is None: # This sometimes happens for some reason... - self.cleanup() + self.hide() else: self._move_to_elem() From 1d82ea87407b0c998c36f39b247d3c9fc49b9310 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 17 Aug 2016 15:25:38 +0200 Subject: [PATCH 14/17] Remove logging in _on_contents_size_changed Otherwise this will be logged once for every element. --- qutebrowser/browser/hints.py | 1 - 1 file changed, 1 deletion(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 6f575bf2c..e07fdbef7 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -118,7 +118,6 @@ class HintLabel(QLabel): @pyqtSlot() def _on_contents_size_changed(self): """Reposition hints if contents size changed.""" - log.hints.debug("Contents size changed...!") if self.elem.frame() is None: # This sometimes happens for some reason... self.hide() From 955ed2f52dfb6e1276be5c2e639ed3e0a42ddafb Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 17 Aug 2016 15:27:31 +0200 Subject: [PATCH 15/17] Simply connect HintLabel._move_to_elem to signal --- qutebrowser/browser/hints.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index e07fdbef7..eada11ca4 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -84,8 +84,7 @@ class HintLabel(QLabel): self.setAttribute(Qt.WA_StyledBackground, True) style.set_register_stylesheet(self) - self._context.tab.contents_size_changed.connect( - self._on_contents_size_changed) + self._context.tab.contents_size_changed.connect(self._move_to_elem) self._move_to_elem() self.show() @@ -109,21 +108,18 @@ class HintLabel(QLabel): match_color, matched, unmatched)) self.adjustSize() + @pyqtSlot() def _move_to_elem(self): """Reposition the label to its element.""" + if self.elem.frame() is None: + # This sometimes happens for some reason... + log.hints.debug("Frame for {!r} vanished!".format(self)) + self.hide() + return no_js = config.get('hints', 'find-implementation') != 'javascript' rect = self.elem.rect_on_view(no_js=no_js) self.move(rect.x(), rect.y()) - @pyqtSlot() - def _on_contents_size_changed(self): - """Reposition hints if contents size changed.""" - if self.elem.frame() is None: - # This sometimes happens for some reason... - self.hide() - else: - self._move_to_elem() - def cleanup(self): """Clean up this element and hide it.""" self.hide() From 52e47e0c3db537aa7aac76a07be34efb3c6acfe3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 17 Aug 2016 15:31:29 +0200 Subject: [PATCH 16/17] Add HintLabel.__repr__ --- qutebrowser/browser/hints.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index eada11ca4..5f3dff63d 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -88,6 +88,13 @@ class HintLabel(QLabel): self._move_to_elem() self.show() + def __repr__(self): + try: + text = self.text() + except RuntimeError: + text = '' + return utils.get_repr(self, elem=self.elem, text=text) + def update_text(self, matched, unmatched): """Set the text for the hint. From 8eaa387f21c582f8f64c1655dd096a24f97e7598 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 17 Aug 2016 20:17:17 +0200 Subject: [PATCH 17/17] Adjust default hint.bg gradient orientation --- doc/help/settings.asciidoc | 2 +- qutebrowser/config/config.py | 2 +- qutebrowser/config/configdata.py | 2 +- tests/unit/config/test_config.py | 2 +- tests/unit/config/test_configtypes.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 7f2041fcc..d121dd885 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -2047,7 +2047,7 @@ Default: +pass:[black]+ === hints.bg Background color for hints. Note that you can use a `rgba(...)` value for transparency. -Default: +pass:[qlineargradient(x1:0, y1:0, x2:1, y2:1, stop:0 rgba(255, 247, 133, 0.8), stop:1 rgba(255, 197, 66, 0.8))]+ +Default: +pass:[qlineargradient(x1:0, y1:0, x2:0, y2:1, stop:0 rgba(255, 247, 133, 0.8), stop:1 rgba(255, 197, 66, 0.8))]+ [[colors-hints.fg.match]] === hints.fg.match diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 931fbdabe..524a7ce7e 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -311,7 +311,7 @@ def _transform_hint_color(val): if not start_color.isValid() or not stop_color.isValid(): return None - return ('qlineargradient(x1:0, y1:0, x2:1, y2:1, stop:0 {}, ' + return ('qlineargradient(x1:0, y1:0, x2:0, y2:1, stop:0 {}, ' 'stop:1 {})'.format(to_rgba(start_color), to_rgba(stop_color))) else: diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 14f98e97a..ab2638ec4 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -1209,7 +1209,7 @@ def data(readonly=False): "Font color for hints."), ('hints.bg', - SettingValue(typ.QssColor(), 'qlineargradient(x1:0, y1:0, x2:1, ' + SettingValue(typ.QssColor(), 'qlineargradient(x1:0, y1:0, x2:0, ' 'y2:1, stop:0 rgba(255, 247, 133, 0.8), ' 'stop:1 rgba(255, 197, 66, 0.8))'), "Background color for hints. Note that you can use a `rgba(...)` " diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index e05f91f15..0194e4480 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -232,7 +232,7 @@ class TestTransformers: OLD_GRADIENT = ('-webkit-gradient(linear, left top, left bottom, ' 'color-stop(0%,{}), color-stop(100%,{}))') - NEW_GRADIENT = ('qlineargradient(x1:0, y1:0, x2:1, y2:1, stop:0 {}, ' + NEW_GRADIENT = ('qlineargradient(x1:0, y1:0, x2:0, y2:1, stop:0 {}, ' 'stop:1 {})') @pytest.mark.parametrize('val, expected', [ diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index 391ae757a..3ad579266 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -871,7 +871,7 @@ class ColorTests: ('hsva(359, 255, 255, 255)', [configtypes.QssColor]), ('hsv(10%, 10%, 10%)', [configtypes.QssColor]), ('hsv(10%,10%,10%)', [configtypes.QssColor]), - ('qlineargradient(x1:0, y1:0, x2:1, y2:1, stop:0 white, ' + ('qlineargradient(x1:0, y1:0, x2:0, y2:1, stop:0 white, ' 'stop: 0.4 gray, stop:1 green)', [configtypes.QssColor]), ('qconicalgradient(cx:0.5, cy:0.5, angle:30, stop:0 white, ' 'stop:1 #00FF00)', [configtypes.QssColor]),