Revert "Make webelem.rect_on_view work async"

This reverts commit 4e11613d2df064b138532c18f88bbf278c64f347.

We can actually make this synchronous just fine by collecting that
information when searching for the elements...

qutebrowser/browser/hints.py

@@ -124,8 +124,8 @@ class HintLabel(QLabel):
             self.hide()
             return
         no_js = config.get('hints', 'find-implementation') != 'javascript'
-        self.elem.rect_on_view(no_js=no_js,
+        rect = self.elem.rect_on_view(no_js=no_js)
-                               callback=lambda r: self.move(r.x(), r.y()))
+        self.move(rect.x(), rect.y())
 
     def cleanup(self):
         """Clean up this element and hide it."""
@@ -221,59 +221,54 @@ class HintActions(QObject):
         else:
             target_mapping[Target.tab] = usertypes.ClickTarget.tab
 
-        def click_cb(rect):
+        # Click the center of the largest square fitting into the top/left
-            """Actually click the element.
+        # corner of the rectangle, this will help if part of the <a> element
+        # is hidden behind other elements
+        # https://github.com/The-Compiler/qutebrowser/issues/1005
+        rect = elem.rect_on_view()
+        if rect.width() > rect.height():
+            rect.setWidth(rect.height())
+        else:
+            rect.setHeight(rect.width())
+        pos = rect.center()
 
-            Click the center of the largest square fitting into the top/left
+        action = "Hovering" if context.target == Target.hover else "Clicking"
-            corner of the rectangle, this will help if part of the <a>
+        log.hints.debug("{} on '{}' at position {}".format(
-            element is hidden behind other elements
+            action, elem.debug_text(), pos))
-            https://github.com/The-Compiler/qutebrowser/issues/1005
-            """
-            if rect.width() > rect.height():
-                rect.setWidth(rect.height())
-            else:
-                rect.setHeight(rect.width())
-            pos = rect.center()
 
-            action = "Hovering" if context.target == Target.hover else "Clicking"
+        self.start_hinting.emit(target_mapping[context.target])
-            log.hints.debug("{} on '{}' at position {}".format(
+        if context.target in [Target.tab, Target.tab_fg, Target.tab_bg,
-                action, elem.debug_text(), pos))
+                              Target.window]:
-
+            modifiers = Qt.ControlModifier
-            self.start_hinting.emit(target_mapping[context.target])
+        else:
-            if context.target in [Target.tab, Target.tab_fg, Target.tab_bg,
+            modifiers = Qt.NoModifier
-                                  Target.window]:
+        events = [
-                modifiers = Qt.ControlModifier
+            QMouseEvent(QEvent.MouseMove, pos, Qt.NoButton, Qt.NoButton,
-            else:
+                        Qt.NoModifier),
-                modifiers = Qt.NoModifier
+        ]
-            events = [
+        if context.target != Target.hover:
-                QMouseEvent(QEvent.MouseMove, pos, Qt.NoButton, Qt.NoButton,
+            events += [
-                            Qt.NoModifier),
+                QMouseEvent(QEvent.MouseButtonPress, pos, Qt.LeftButton,
+                            Qt.LeftButton, modifiers),
+                QMouseEvent(QEvent.MouseButtonRelease, pos, Qt.LeftButton,
+                            Qt.NoButton, modifiers),
             ]
-            if context.target != Target.hover:
-                events += [
-                    QMouseEvent(QEvent.MouseButtonPress, pos, Qt.LeftButton,
-                                Qt.LeftButton, modifiers),
-                    QMouseEvent(QEvent.MouseButtonRelease, pos, Qt.LeftButton,
-                                Qt.NoButton, modifiers),
-                ]
 
-            if context.target in [Target.normal, Target.current]:
+        if context.target in [Target.normal, Target.current]:
-                # Set the pre-jump mark ', so we can jump back here after following
+            # Set the pre-jump mark ', so we can jump back here after following
-                tabbed_browser = objreg.get('tabbed-browser', scope='window',
+            tabbed_browser = objreg.get('tabbed-browser', scope='window',
-                                            window=self._win_id)
+                                        window=self._win_id)
-                tabbed_browser.set_mark("'")
+            tabbed_browser.set_mark("'")
 
-            if context.target == Target.current:
+        if context.target == Target.current:
-                elem.remove_blank_target()
+            elem.remove_blank_target()
-            for evt in events:
+        for evt in events:
-                self.mouse_event.emit(evt)
+            self.mouse_event.emit(evt)
-            if elem.is_text_input() and elem.is_editable():
+        if elem.is_text_input() and elem.is_editable():
-                QTimer.singleShot(0, functools.partial(
+            QTimer.singleShot(0, functools.partial(
-                    elem.frame().page().triggerAction,
+                elem.frame().page().triggerAction,
-                    QWebPage.MoveToEndOfDocument))
+                QWebPage.MoveToEndOfDocument))
-            QTimer.singleShot(0, self.stop_hinting.emit)
+        QTimer.singleShot(0, self.stop_hinting.emit)
-
-        elem.rect_on_view(callback=click_cb)
 
     def yank(self, url, context):
         """Yank an element to the clipboard or primary selection.

qutebrowser/browser/webkit/webkitelem.py

@@ -199,22 +199,8 @@ class WebKitElement(webelem.AbstractWebElement):
             frame = frame.parentFrame()
         return rect
 
-    def _rect_on_view_sync(self, *, elem_geometry=None, no_js=False):
+    def rect_on_view(self, *, elem_geometry=None, no_js=False):
-        """Synchronous part of rect_on_view."""
+        """Get the geometry of the element relative to the webview.
-        self._check_vanished()
-
-        # 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()
-            if rect is not None:
-                return rect
-
-        # No suitable rects found via JS, try via the QWebElement API
-        return self._rect_on_view_python(elem_geometry)
-
-    def rect_on_view(self, *, callback, **kwargs):
-        """Get the geometry of the element relative to the webview (async).
 
         Uses the getClientRects() JavaScript method to obtain the collection of
         rectangles containing the element and returns the first rectangle which
@@ -230,15 +216,21 @@ class WebKitElement(webelem.AbstractWebElement):
                            Calling QWebElement::geometry is rather expensive so
                            we want to avoid doing it twice.
             no_js: Fall back to the Python implementation
-            callback: Gets called with the found QRect.
         """
         self._check_vanished()
-        callback(self._rect_on_view_sync(**kwargs))
+
+        # 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()
+            if rect is not None:
+                return rect
+
+        # No suitable rects found via JS, try via the QWebElement API
+        return self._rect_on_view_python(elem_geometry)
 
     def is_visible(self, mainframe):
         """Check if the given element is visible in the given frame."""
-        # FIXME:qtwebengine can we get rid of this with
-        # find_all_elements(only_visible=True)?
         self._check_vanished()
         # CSS attributes which hide an element
         hidden_attributes = {
@@ -253,7 +245,7 @@ class WebKitElement(webelem.AbstractWebElement):
             # Most likely an invisible link
             return False
         # First check if the element is visible on screen
-        elem_rect = self._rect_on_view_sync(elem_geometry=elem_geometry)
+        elem_rect = self.rect_on_view(elem_geometry=elem_geometry)
         mainframe_geometry = mainframe.geometry()
         if elem_rect.isValid():
             visible_on_screen = mainframe_geometry.intersects(elem_rect)

tests/unit/browser/webkit/test_webkitelem.py

@@ -269,7 +269,7 @@ class TestWebKitElement:
         lambda e: e.outer_xml(),
         lambda e: e.tag_name(),
         lambda e: e.run_js_async(''),
-        lambda e: e.rect_on_view(callback=None),
+        lambda e: e.rect_on_view(),
         lambda e: e.is_visible(None),
     ], ids=['str', 'getitem', 'setitem', 'delitem', 'contains', 'iter', 'len',
             'frame', 'geometry', 'style_property', 'text', 'set_text',
@@ -708,24 +708,22 @@ class TestRectOnView:
         # unusable geometry via getElementRects
         {'length': '1', '0': {'width': 0, 'height': 0, 'x': 0, 'y': 0}},
     ])
-    def test_simple(self, callback_checker, stubs, js_rect):
+    def test_simple(self, stubs, js_rect):
         geometry = QRect(5, 5, 4, 4)
         frame = stubs.FakeWebFrame(QRect(0, 0, 100, 100))
         elem = get_webelem(geometry, frame, js_rect_return=js_rect)
-        elem.rect_on_view(callback=callback_checker.callback)
+        assert elem.rect_on_view() == QRect(5, 5, 4, 4)
-        callback_checker.check(QRect(5, 5, 4, 4))
 
     @pytest.mark.parametrize('js_rect', [None, {}])
-    def test_scrolled(self, callback_checker, stubs, js_rect):
+    def test_scrolled(self, stubs, js_rect):
         geometry = QRect(20, 20, 4, 4)
         frame = stubs.FakeWebFrame(QRect(0, 0, 100, 100),
                                    scroll=QPoint(10, 10))
         elem = get_webelem(geometry, frame, js_rect_return=js_rect)
-        elem.rect_on_view(callback=callback_checker.callback)
+        assert elem.rect_on_view() == QRect(20 - 10, 20 - 10, 4, 4)
-        callback_checker.check(QRect(20 - 10, 20 - 10, 4, 4))
 
     @pytest.mark.parametrize('js_rect', [None, {}])
-    def test_iframe(self, callback_checker, stubs, js_rect):
+    def test_iframe(self, stubs, js_rect):
         """Test an element in an iframe.
 
              0, 0                         200, 0
@@ -746,32 +744,27 @@ class TestRectOnView:
         assert frame.geometry().contains(iframe.geometry())
         elem = get_webelem(QRect(20, 90, 10, 10), iframe,
                            js_rect_return=js_rect)
-        elem.rect_on_view(callback=callback_checker.callback)
+        assert elem.rect_on_view() == QRect(20, 10 + 90, 10, 10)
-        callback_checker.check(QRect(20, 10 + 90, 10, 10))
 
     @pytest.mark.parametrize('js_rect', [None, {}])
-    def test_passed_geometry(self, callback_checker, stubs, js_rect):
+    def test_passed_geometry(self, stubs, js_rect):
         """Make sure geometry isn't called when a geometry is passed."""
         frame = stubs.FakeWebFrame(QRect(0, 0, 200, 200))
         elem = get_webelem(frame=frame, js_rect_return=js_rect)
         rect = QRect(10, 20, 30, 40)
-        elem.rect_on_view(elem_geometry=rect,
+        assert elem.rect_on_view(elem_geometry=rect) == rect
-                          callback=callback_checker.callback)
-        callback_checker.check(rect)
         assert not elem._elem.geometry.called
 
     @pytest.mark.parametrize('js_rect', [None, {}])
     @pytest.mark.parametrize('zoom_text_only', [True, False])
-    def test_zoomed(self, callback_checker, stubs, config_stub, js_rect,
+    def test_zoomed(self, stubs, config_stub, js_rect, zoom_text_only):
-                    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)
-        elem.rect_on_view(callback=callback_checker.callback)
+        assert elem.rect_on_view() == QRect(10, 10, 4, 4)
-        callback_checker.check(QRect(10, 10, 4, 4))
 
 
 class TestGetChildFrames: