diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py
index 32e0786be..7b098fd25 100644
--- a/qutebrowser/browser/hints.py
+++ b/qutebrowser/browser/hints.py
@@ -26,7 +26,7 @@ import re
import string
from PyQt5.QtCore import (pyqtSignal, pyqtSlot, QObject, QEvent, Qt, QUrl,
- QTimer, QRect)
+ QTimer)
from PyQt5.QtGui import QMouseEvent
from PyQt5.QtWebKit import QWebElement
from PyQt5.QtWebKitWidgets import QWebPage
@@ -376,7 +376,7 @@ class HintManager(QObject):
elem: The QWebElement to set the style attributes for.
label: The label QWebElement.
"""
- rect = self._get_first_rectangle(elem, adjust_zoom=False)
+ rect = elem.rect_on_view(adjust_zoom=False)
left = rect.x()
top = rect.y()
log.hints.vdebug("Drawing label '{!r}' at {}/{} for element '{!r}'"
@@ -417,55 +417,6 @@ class HintManager(QObject):
message.error(self._win_id, "No suitable link found for this element.",
immediately=True)
- def _get_first_rectangle(self, elem, *, adjust_zoom=True):
- """Return the element's first client rectangle with positive size.
-
- Uses the getClientRects() JavaScript method to obtain the collection of
- rectangles containing the element and returns the first rectangle which
- is large enough (larger than 1px times 1px). If all rectangles returned
- by getClientRects() are too small, falls back to elem.rect_on_view().
-
- Skipping of small rectangles is due to elements containing other
- elements with "display:block" style, see
- https://github.com/The-Compiler/qutebrowser/issues/1298
-
- Args:
- elem: The QWebElement of interest.
- adjust_zoom: Whether to adjust the element position based on the
- current zoom level.
- """
- rects = elem.evaluateJavaScript("this.getClientRects()")
- log.hints.vdebug("Client rectangles of element '{}': {}"
- .format(elem.debug_text(), rects))
- for i in range(int(rects.get("length", 0))):
- rect = rects[str(i)]
- width = rect.get("width", 0)
- height = rect.get("height", 0)
- if width > 1 and height > 1:
- # fix coordinates according to zoom level
- zoom = elem.webFrame().zoomFactor()
- if not config.get('ui', 'zoom-text-only') and adjust_zoom:
- rect["left"] *= zoom
- rect["top"] *= zoom
- width *= zoom
- height *= zoom
- rect = QRect(rect["left"], rect["top"], width, height)
- frame = elem.webFrame()
- while frame is not None:
- # Translate to parent frames' position
- # (scroll position is taken care of inside getClientRects)
- rect.translate(frame.geometry().topLeft())
- frame = frame.parentFrame()
- return rect
-
- # No suitable rects found via JS, try via the QWebElement API
- rect = elem.rect_on_view()
- zoom = elem.webFrame().zoomFactor()
- if not config.get('ui', 'zoom-text-only'):
- rect.setLeft(rect.left() / zoom)
- rect.setTop(rect.top() / zoom)
- return rect
-
def _click(self, elem, context):
"""Click an element.
@@ -490,7 +441,7 @@ class HintManager(QObject):
# corner of the rectangle, this will help if part of the element
# is hidden behind other elements
# https://github.com/The-Compiler/qutebrowser/issues/1005
- rect = self._get_first_rectangle(elem)
+ rect = elem.rect_on_view()
if rect.width() > rect.height():
rect.setWidth(rect.height())
else:
diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py
index cc387bc9a..cac214102 100644
--- a/qutebrowser/browser/webelem.py
+++ b/qutebrowser/browser/webelem.py
@@ -173,9 +173,14 @@ class WebElementWrapper(collections.abc.MutableMapping):
"""
return is_visible(self._elem, mainframe)
- def rect_on_view(self):
- """Get the geometry of the element relative to the webview."""
- return rect_on_view(self._elem)
+ def rect_on_view(self, *, adjust_zoom=True):
+ """Get the geometry of the element relative to the webview.
+
+ Args:
+ adjust_zoom: Whether to adjust the element position based on the
+ current zoom level.
+ """
+ return rect_on_view(self._elem, adjust_zoom=adjust_zoom)
def is_writable(self):
"""Check whether an element is writable."""
@@ -363,21 +368,62 @@ def focus_elem(frame):
return WebElementWrapper(elem)
-def rect_on_view(elem, elem_geometry=None):
+def rect_on_view(elem, *, elem_geometry=None, adjust_zoom=True):
"""Get the geometry of the element relative to the webview.
We need this as a standalone function (as opposed to a WebElementWrapper
method) because we want to run is_visible before wrapping when hinting for
performance reasons.
+ Uses the getClientRects() JavaScript method to obtain the collection of
+ rectangles containing the element and returns the first rectangle which is
+ large enough (larger than 1px times 1px). If all rectangles returned by
+ getClientRects() are too small, falls back to elem.rect_on_view().
+
+ Skipping of small rectangles is due to elements containing other
+ elements with "display:block" style, see
+ https://github.com/The-Compiler/qutebrowser/issues/1298
+
Args:
elem: The QWebElement to get the rect for.
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.
"""
if elem.isNull():
raise IsNullError("Got called on a null element!")
+
+ # First try getting the element rect via JS, as that's usually more
+ # accurate
+ if elem_geometry is None:
+ rects = elem.evaluateJavaScript("this.getClientRects()")
+ text = utils.compact_text(elem.toOuterXml(), 500)
+ log.hints.vdebug("Client rectangles of element '{}': {}".format(text,
+ rects))
+ for i in range(int(rects.get("length", 0))):
+ rect = rects[str(i)]
+ width = rect.get("width", 0)
+ height = rect.get("height", 0)
+ if width > 1 and height > 1:
+ # fix coordinates according to zoom level
+ zoom = elem.webFrame().zoomFactor()
+ if not config.get('ui', 'zoom-text-only') and adjust_zoom:
+ rect["left"] *= zoom
+ rect["top"] *= zoom
+ width *= zoom
+ height *= zoom
+ rect = QRect(rect["left"], rect["top"], width, height)
+ frame = elem.webFrame()
+ while frame is not None:
+ # Translate to parent frames' position
+ # (scroll position is taken care of inside getClientRects)
+ rect.translate(frame.geometry().topLeft())
+ frame = frame.parentFrame()
+ return rect
+
+ # No suitable rects found via JS, try via the QWebElement API
if elem_geometry is None:
elem_geometry = elem.geometry()
frame = elem.webFrame()
@@ -386,6 +432,11 @@ def rect_on_view(elem, elem_geometry=None):
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
+ zoom = elem.webFrame().zoomFactor()
+ if not config.get('ui', 'zoom-text-only'):
+ rect.setLeft(rect.left() / zoom)
+ rect.setTop(rect.top() / zoom)
return rect
diff --git a/tests/helpers/stubs.py b/tests/helpers/stubs.py
index adaebd168..91519d558 100644
--- a/tests/helpers/stubs.py
+++ b/tests/helpers/stubs.py
@@ -77,7 +77,7 @@ class FakeWebFrame:
"""
def __init__(self, geometry=None, *, scroll=None, plaintext=None,
- html=None, parent=None):
+ html=None, parent=None, zoom=1.0):
"""Constructor.
Args:
@@ -85,6 +85,7 @@ class FakeWebFrame:
scroll: The scroll position as QPoint.
plaintext: Return value of toPlainText
html: Return value of tohtml.
+ zoom: The zoom factor.
parent: The parent frame.
"""
if scroll is None:
@@ -95,6 +96,7 @@ class FakeWebFrame:
self.focus_elem = None
self.toPlainText = mock.Mock(return_value=plaintext)
self.toHtml = mock.Mock(return_value=html)
+ self.zoomFactor = mock.Mock(return_value=zoom)
def findFirstElement(self, selector):
if selector == '*:focus':
diff --git a/tests/unit/browser/test_webelem.py b/tests/unit/browser/test_webelem.py
index 8ab0eaa53..10917e4e5 100644
--- a/tests/unit/browser/test_webelem.py
+++ b/tests/unit/browser/test_webelem.py
@@ -49,6 +49,7 @@ def get_webelem(geometry=None, frame=None, null=False, style=None,
tagname: The tag name.
classes: HTML classes to be added.
"""
+ # pylint: disable=too-many-locals
elem = mock.Mock()
elem.isNull.return_value = null
elem.geometry.return_value = geometry
@@ -58,6 +59,25 @@ def get_webelem(geometry=None, frame=None, null=False, style=None,
elem.toPlainText.return_value = 'text'
elem.parent.return_value = parent
+ if geometry is not None:
+ if frame is None:
+ scroll_x = 0
+ scroll_y = 0
+ else:
+ scroll_x = frame.scrollPosition().x()
+ scroll_y = frame.scrollPosition().y()
+ elem.evaluateJavaScript.return_value = {
+ "length": 1,
+ "0": {
+ "left": geometry.left() - scroll_x,
+ "top": geometry.top() - scroll_y,
+ "right": geometry.right() - scroll_x,
+ "bottom": geometry.bottom() - scroll_y,
+ "width": geometry.width(),
+ "height": geometry.height(),
+ }
+ }
+
attribute_dict = {}
if attributes is None:
pass
@@ -94,6 +114,17 @@ def get_webelem(geometry=None, frame=None, null=False, style=None,
return wrapped
+@pytest.fixture(autouse=True)
+def stubbed_config(config_stub, monkeypatch):
+ """Add a zoom-text-only fake config value.
+
+ This is needed for all the tests calling rect_on_view or is_visible.
+ """
+ config_stub.data = {'ui': {'zoom-text-only': 'true'}}
+ monkeypatch.setattr('qutebrowser.browser.webelem.config', config_stub)
+ return config_stub
+
+
class SelectionAndFilterTests:
"""Generator for tests for TestSelectionsAndFilters."""
@@ -618,9 +649,10 @@ class TestRectOnView:
def test_passed_geometry(self, stubs):
"""Make sure geometry isn't called when a geometry is passed."""
- raw_elem = get_webelem()._elem
+ frame = stubs.FakeWebFrame(QRect(0, 0, 200, 200))
+ raw_elem = get_webelem(frame=frame)._elem
rect = QRect(10, 20, 30, 40)
- assert webelem.rect_on_view(raw_elem, rect) == rect
+ assert webelem.rect_on_view(raw_elem, elem_geometry=rect) == rect
assert not raw_elem.geometry.called