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.
This commit is contained in:
Florian Bruhin 2016-08-17 12:56:54 +02:00
parent 7c17af3889
commit 0293307d61
5 changed files with 9 additions and 32 deletions

View File

@ -112,7 +112,7 @@ class HintLabel(QLabel):
def _move_to_elem(self): def _move_to_elem(self):
"""Reposition the label to its element.""" """Reposition the label to its element."""
no_js = config.get('hints', 'find-implementation') != 'javascript' 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()) self.move(rect.x(), rect.y())
@pyqtSlot() @pyqtSlot()

View File

@ -161,8 +161,7 @@ class AbstractWebElement(collections.abc.MutableMapping):
# FIXME:qtwebengine get rid of this? # FIXME:qtwebengine get rid of this?
raise NotImplementedError raise NotImplementedError
def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, def rect_on_view(self, *, elem_geometry=None, no_js=False):
no_js=False):
"""Get the geometry of the element relative to the webview. """Get the geometry of the element relative to the webview.
Uses the getClientRects() JavaScript method to obtain the collection of 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. elem_geometry: The geometry of the element, or None.
Calling QWebElement::geometry is rather expensive so Calling QWebElement::geometry is rather expensive so
we want to avoid doing it twice. 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 no_js: Fall back to the Python implementation
""" """
raise NotImplementedError raise NotImplementedError

View File

@ -120,8 +120,7 @@ class WebEngineElement(webelem.AbstractWebElement):
log.stub() log.stub()
return None return None
def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, def rect_on_view(self, *, elem_geometry=None, no_js=False):
no_js=False):
"""Get the geometry of the element relative to the webview. """Get the geometry of the element relative to the webview.
Uses the getClientRects() JavaScript method to obtain the collection of 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. elem_geometry: The geometry of the element, or None.
Calling QWebElement::geometry is rather expensive so Calling QWebElement::geometry is rather expensive so
we want to avoid doing it twice. 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 no_js: Fall back to the Python implementation
""" """
log.stub() log.stub()

View File

@ -148,7 +148,7 @@ class WebKitElement(webelem.AbstractWebElement):
return None return None
return WebKitElement(elem) return WebKitElement(elem)
def _rect_on_view_js(self, adjust_zoom): def _rect_on_view_js(self):
"""Javascript implementation for rect_on_view.""" """Javascript implementation for rect_on_view."""
# FIXME:qtwebengine maybe we can reuse this? # FIXME:qtwebengine maybe we can reuse this?
rects = self._elem.evaluateJavaScript("this.getClientRects()") rects = self._elem.evaluateJavaScript("this.getClientRects()")
@ -169,7 +169,7 @@ class WebKitElement(webelem.AbstractWebElement):
if width > 1 and height > 1: if width > 1 and height > 1:
# fix coordinates according to zoom level # fix coordinates according to zoom level
zoom = self._elem.webFrame().zoomFactor() 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["left"] *= zoom
rect["top"] *= zoom rect["top"] *= zoom
width *= zoom width *= zoom
@ -197,18 +197,9 @@ class WebKitElement(webelem.AbstractWebElement):
rect.translate(frame.geometry().topLeft()) rect.translate(frame.geometry().topLeft())
rect.translate(frame.scrollPosition() * -1) rect.translate(frame.scrollPosition() * -1)
frame = frame.parentFrame() 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 return rect
def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, def rect_on_view(self, *, elem_geometry=None, no_js=False):
no_js=False):
"""Get the geometry of the element relative to the webview. """Get the geometry of the element relative to the webview.
Uses the getClientRects() JavaScript method to obtain the collection of 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. elem_geometry: The geometry of the element, or None.
Calling QWebElement::geometry is rather expensive so Calling QWebElement::geometry is rather expensive so
we want to avoid doing it twice. 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 no_js: Fall back to the Python implementation
""" """
# FIXME:qtwebengine can we get rid of this with # 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 # First try getting the element rect via JS, as that's usually more
# accurate # accurate
if elem_geometry is None and not no_js: 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: if rect is not None:
return rect return rect

View File

@ -755,20 +755,14 @@ class TestRectOnView:
@pytest.mark.parametrize('js_rect', [None, {}]) @pytest.mark.parametrize('js_rect', [None, {}])
@pytest.mark.parametrize('zoom_text_only', [True, False]) @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):
def test_zoomed(self, stubs, config_stub, js_rect, zoom_text_only,
adjust_zoom):
"""Make sure the coordinates are adjusted when zoomed.""" """Make sure the coordinates are adjusted when zoomed."""
config_stub.data = {'ui': {'zoom-text-only': zoom_text_only}} config_stub.data = {'ui': {'zoom-text-only': zoom_text_only}}
geometry = QRect(10, 10, 4, 4) geometry = QRect(10, 10, 4, 4)
frame = stubs.FakeWebFrame(QRect(0, 0, 100, 100), zoom=0.5) frame = stubs.FakeWebFrame(QRect(0, 0, 100, 100), zoom=0.5)
elem = get_webelem(geometry, frame, js_rect_return=js_rect, elem = get_webelem(geometry, frame, js_rect_return=js_rect,
zoom_text_only=zoom_text_only) zoom_text_only=zoom_text_only)
rect = elem.rect_on_view(adjust_zoom=adjust_zoom) assert elem.rect_on_view() == QRect(10, 10, 4, 4)
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)
class TestGetChildFrames: class TestGetChildFrames: