Merge branch 'webengine-hints'

qutebrowser/browser/browsertab.py

@@ -21,9 +21,9 @@
 
 import itertools
 
-from PyQt5.QtCore import pyqtSignal, pyqtSlot, QUrl, QObject, QSizeF
+from PyQt5.QtCore import pyqtSignal, pyqtSlot, QUrl, QObject, QSizeF, QTimer
 from PyQt5.QtGui import QIcon
-from PyQt5.QtWidgets import QWidget, QApplication
+from PyQt5.QtWidgets import QWidget
 
 from qutebrowser.keyinput import modeman
 from qutebrowser.config import config
@@ -60,7 +60,7 @@ class WebTabError(Exception):
     """Base class for various errors."""
 
 
-class TabData(QObject):
+class TabData:
 
     """A simple namespace with a fixed set of attributes.
 
@@ -73,8 +73,7 @@ class TabData(QObject):
         hint_target: Override for open_target for hints.
     """
 
-    def __init__(self, parent=None):
+    def __init__(self):
-        super().__init__(parent)
         self.keep_icon = False
         self.viewing_source = False
         self.inspector = None
@@ -87,21 +86,6 @@ class TabData(QObject):
         else:
             return self.open_target
 
-    @pyqtSlot(usertypes.ClickTarget)
-    def _on_start_hinting(self, hint_target):
-        """Emitted before a hinting-click takes place.
-
-        Args:
-            hint_target: A ClickTarget member to set self.hint_target to.
-        """
-        log.webview.debug("Setting force target to {}".format(hint_target))
-        self.hint_target = hint_target
-
-    @pyqtSlot()
-    def _on_stop_hinting(self):
-        log.webview.debug("Finishing hinting.")
-        self.hint_target = None
-
 
 class AbstractPrinting:
 
@@ -489,7 +473,7 @@ class AbstractTab(QWidget):
         # self.search = AbstractSearch(parent=self)
         # self.printing = AbstractPrinting()
 
-        self.data = TabData(parent=self)
+        self.data = TabData()
         self._layout = miscwidgets.WrapperLayout(self)
         self._widget = None
         self._progress = 0
@@ -501,11 +485,7 @@ class AbstractTab(QWidget):
         # FIXME:qtwebengine  Should this be public api via self.hints?
         #                    Also, should we get it out of objreg?
         hintmanager = hints.HintManager(win_id, self.tab_id, parent=self)
-        hintmanager.mouse_event.connect(self._on_hint_mouse_event)
+        hintmanager.hint_events.connect(self._on_hint_events)
-        # pylint: disable=protected-access
-        hintmanager.start_hinting.connect(self.data._on_start_hinting)
-        hintmanager.stop_hinting.connect(self.data._on_stop_hinting)
-        # pylint: enable=protected-access
         objreg.register('hintmanager', hintmanager, scope='tab',
                         window=self.win_id, tab=self.tab_id)
 
@@ -532,15 +512,24 @@ class AbstractTab(QWidget):
         self._load_status = val
         self.load_status_changed.emit(val.name)
 
-    @pyqtSlot('QMouseEvent')
+    def post_event(self, evt):
-    def _on_hint_mouse_event(self, evt):
+        """Send the given event to the underlying widget."""
+        raise NotImplementedError
+
+    @pyqtSlot(usertypes.ClickTarget, list)
+    def _on_hint_events(self, target, events):
         """Post a new mouse event from a hintmanager."""
-        # FIXME:qtwebengine Will this implementation work for QtWebEngine?
+        log.modes.debug("Sending hint events to {!r} with target {}".format(
-        #                   We probably need to send the event to the
+            self, target))
-        #                   focusProxy()?
-        log.modes.debug("Hint triggered, focusing {!r}".format(self))
         self._widget.setFocus()
-        QApplication.postEvent(self._widget, evt)
+        self.data.hint_target = target
+
+        for evt in events:
+            self.post_event(evt)
+
+        def reset_target():
+            self.data.hint_target = None
+        QTimer.singleShot(0, reset_target)
 
     @pyqtSlot(QUrl)
     def _on_link_clicked(self, url):

qutebrowser/browser/hints.py

@@ -30,7 +30,6 @@ 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, style
 from qutebrowser.keyinput import modeman, modeparsers
@@ -118,7 +117,7 @@ class HintLabel(QLabel):
     @pyqtSlot()
     def _move_to_elem(self):
         """Reposition the label to its element."""
-        if self.elem.frame() is None:
+        if not self.elem.has_frame():
             # This sometimes happens for some reason...
             log.hints.debug("Frame for {!r} vanished!".format(self))
             self.hide()
@@ -186,16 +185,10 @@ class HintActions(QObject):
     """Actions which can be done after selecting a hint.
 
     Signals:
-        mouse_event: Mouse event to be posted in the web view.
+        hint_events: Emitted with a ClickTarget and a list of hint event.s
-                     arg: A QMouseEvent
-        start_hinting: Emitted when hinting starts, before a link is clicked.
-                       arg: The ClickTarget to use.
-        stop_hinting: Emitted after a link was clicked.
     """
 
-    mouse_event = pyqtSignal('QMouseEvent')
+    hint_events = pyqtSignal(usertypes.ClickTarget, list)  # QMouseEvent list
-    start_hinting = pyqtSignal(usertypes.ClickTarget)
-    stop_hinting = pyqtSignal()
 
     def __init__(self, win_id, parent=None):
         super().__init__(parent)
@@ -236,7 +229,6 @@ class HintActions(QObject):
         log.hints.debug("{} on '{}' at position {}".format(
             action, elem.debug_text(), pos))
 
-        self.start_hinting.emit(target_mapping[context.target])
         if context.target in [Target.tab, Target.tab_fg, Target.tab_bg,
                               Target.window]:
             modifiers = Qt.ControlModifier
@@ -262,13 +254,10 @@ class HintActions(QObject):
 
         if context.target == Target.current:
             elem.remove_blank_target()
-        for evt in events:
+
-            self.mouse_event.emit(evt)
+        self.hint_events.emit(target_mapping[context.target], events)
         if elem.is_text_input() and elem.is_editable():
-            QTimer.singleShot(0, functools.partial(
+            QTimer.singleShot(0, context.tab.caret.move_to_end_of_document)
-                elem.frame().page().triggerAction,
-                QWebPage.MoveToEndOfDocument))
-        QTimer.singleShot(0, self.stop_hinting.emit)
 
     def yank(self, url, context):
         """Yank an element to the clipboard or primary selection.
@@ -311,11 +300,8 @@ class HintActions(QObject):
         args = context.get_args(urlstr)
         text = ' '.join(args)
         if text[0] not in modeparsers.STARTCHARS:
-            message.error(self._win_id,
+            raise HintingError("Invalid command text '{}'.".format(text))
-                          "Invalid command text '{}'.".format(text),
+        message.set_cmd_text(self._win_id, text)
-                          immediately=True)
-        else:
-            message.set_cmd_text(self._win_id, text)
 
     def download(self, elem, context):
         """Download a hint URL.
@@ -326,16 +312,20 @@ class HintActions(QObject):
         """
         url = elem.resolve_url(context.baseurl)
         if url is None:
-            raise HintingError
+            raise HintingError("No suitable link found for this element.")
         if context.rapid:
             prompt = False
         else:
             prompt = None
 
+        # FIXME:qtwebengine get a proper API for this
+        # pylint: disable=protected-access
+        page = elem._elem.webFrame().page()
+        # pylint: enable=protected-access
+
         download_manager = objreg.get('download-manager', scope='window',
                                       window=self._win_id)
-        download_manager.get(url, page=elem.frame().page(),
+        download_manager.get(url, page=page, prompt_download_directory=prompt)
-                             prompt_download_directory=prompt)
 
     def call_userscript(self, elem, context):
         """Call a userscript from a hint.
@@ -359,7 +349,7 @@ class HintActions(QObject):
             userscripts.run_async(context.tab, cmd, *args, win_id=self._win_id,
                                   env=env)
         except userscripts.UnsupportedError as e:
-            message.error(self._win_id, str(e), immediately=True)
+            raise HintingError(str(e))
 
     def spawn(self, url, context):
         """Spawn a simple command from a hint.
@@ -407,9 +397,7 @@ class HintManager(QObject):
         Target.spawn: "Spawn command via hint",
     }
 
-    mouse_event = pyqtSignal('QMouseEvent')
+    hint_events = pyqtSignal(usertypes.ClickTarget, list)  # QMouseEvent list
-    start_hinting = pyqtSignal(usertypes.ClickTarget)
-    stop_hinting = pyqtSignal()
 
     def __init__(self, win_id, tab_id, parent=None):
         """Constructor."""
@@ -420,9 +408,7 @@ class HintManager(QObject):
         self._word_hinter = WordHinter()
 
         self._actions = HintActions(win_id)
-        self._actions.start_hinting.connect(self.start_hinting)
+        self._actions.hint_events.connect(self.hint_events)
-        self._actions.stop_hinting.connect(self.stop_hinting)
-        self._actions.mouse_event.connect(self.mouse_event)
 
         mode_manager = objreg.get('mode-manager', scope='window',
                                   window=win_id)
@@ -580,11 +566,6 @@ class HintManager(QObject):
             hintstr.insert(0, chars[0])
         return ''.join(hintstr)
 
-    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.",
-                      immediately=True)
-
     def _check_args(self, target, *args):
         """Check the arguments passed to start() and raise if they're wrong.
 
@@ -654,8 +635,7 @@ class HintManager(QObject):
         self._handle_auto_follow()
 
     @cmdutils.register(instance='hintmanager', scope='tab', name='hint',
-                       star_args_optional=True, maxsplit=2,
+                       star_args_optional=True, maxsplit=2)
-                       backend=usertypes.Backend.QtWebKit)
     @cmdutils.argument('win_id', win_id=True)
     def start(self, rapid=False, group=webelem.Group.all, target=Target.normal,
               *args, win_id, mode=None):
@@ -719,6 +699,12 @@ class HintManager(QObject):
         tab = tabbed_browser.currentWidget()
         if tab is None:
             raise cmdexc.CommandError("No WebView available yet!")
+        if (tab.backend == usertypes.Backend.QtWebEngine and
+                target == Target.download):
+            message.error(self._win_id, "The download target is not available "
+                          "yet with QtWebEngine.", immediately=True)
+            return
+
         mode_manager = objreg.get('mode-manager', scope='window',
                                   window=self._win_id)
         if mode_manager.mode == usertypes.KeyMode.hint:
@@ -901,7 +887,7 @@ class HintManager(QObject):
         }
         elem = self._context.labels[keystr].elem
 
-        if elem.frame() is None:
+        if not elem.has_frame():
             message.error(self._win_id,
                           "This element has no webframe.",
                           immediately=True)
@@ -913,7 +899,9 @@ class HintManager(QObject):
         elif self._context.target in url_handlers:
             url = elem.resolve_url(self._context.baseurl)
             if url is None:
-                self._show_url_error()
+                message.error(self._win_id,
+                              "No suitable link found for this element.",
+                              immediately=True)
                 return
             handler = functools.partial(url_handlers[self._context.target],
                                         url, self._context)
@@ -932,8 +920,8 @@ class HintManager(QObject):
 
         try:
             handler()
-        except HintingError:
+        except HintingError as e:
-            self._show_url_error()
+            message.error(self._win_id, str(e), immediately=True)
 
     @cmdutils.register(instance='hintmanager', scope='tab', hide=True,
                        modes=[usertypes.KeyMode.hint])

qutebrowser/browser/webelem.py

@@ -103,9 +103,8 @@ class AbstractWebElement(collections.abc.MutableMapping):
             html = None
         return utils.get_repr(self, html=html)
 
-    def frame(self):
+    def has_frame(self):
-        """Get the main frame of this element."""
+        """Check if this element has a valid frame attached."""
-        # FIXME:qtwebengine get rid of this?
         raise NotImplementedError
 
     def geometry(self):

qutebrowser/browser/webengine/webengineelem.py

@@ -32,10 +32,10 @@ class WebEngineElement(webelem.AbstractWebElement):
 
     """A web element for QtWebEngine, using JS under the hood."""
 
-    def __init__(self, js_dict, run_js_callable):
+    def __init__(self, js_dict, tab):
         self._id = js_dict['id']
         self._js_dict = js_dict
-        self._run_js = run_js_callable
+        self._tab = tab
 
     def __eq__(self, other):
         if not isinstance(other, WebEngineElement):
@@ -58,9 +58,8 @@ class WebEngineElement(webelem.AbstractWebElement):
     def __len__(self):
         return len(self._js_dict['attributes'])
 
-    def frame(self):
+    def has_frame(self):
-        log.stub()
+        return True
-        return None
 
     def geometry(self):
         log.stub()
@@ -107,7 +106,7 @@ class WebEngineElement(webelem.AbstractWebElement):
         """
         # FIXME:qtwebengine what to do about use_js with WebEngine?
         js_code = javascript.assemble('webelem', 'set_text', self._id, text)
-        self._run_js(js_code)
+        self._tab.run_js_async(js_code)
 
     def run_js_async(self, code, callback=None):
         """Run the given JS snippet async on the element."""
@@ -123,11 +122,6 @@ class WebEngineElement(webelem.AbstractWebElement):
     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
-        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 <a> elements containing other
         elements with "display:block" style, see
         https://github.com/The-Compiler/qutebrowser/issues/1298
@@ -138,7 +132,33 @@ class WebEngineElement(webelem.AbstractWebElement):
                            we want to avoid doing it twice.
             no_js: Fall back to the Python implementation
         """
-        log.stub()
+        rects = self._js_dict['rects']
+        for rect in rects:
+            # FIXME:qtwebengine
+            # width = rect.get("width", 0)
+            # height = rect.get("height", 0)
+            width = rect['width']
+            height = rect['height']
+            if width > 1 and height > 1:
+                # Fix coordinates according to zoom level
+                # We're not checking for zoom-text-only here as that doesn't
+                # exist for QtWebEngine.
+                zoom = self._tab.zoom.factor()
+                rect["left"] *= zoom
+                rect["top"] *= zoom
+                width *= zoom
+                height *= zoom
+                rect = QRect(rect["left"], rect["top"], width, height)
+                # FIXME:qtwebengine
+                # frame = self._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
+        log.webview.debug("Couldn't find rectangle for {!r} ({})".format(
+            self, rects))
         return QRect()
 
     def is_visible(self, mainframe):

qutebrowser/browser/webengine/webenginetab.py

@@ -200,12 +200,9 @@ class WebEngineScroller(browsertab.AbstractScroller):
         # FIXME:qtwebengine Abort scrolling if the minimum/maximum was reached.
         press_evt = QKeyEvent(QEvent.KeyPress, key, Qt.NoModifier, 0, 0, 0)
         release_evt = QKeyEvent(QEvent.KeyRelease, key, Qt.NoModifier, 0, 0, 0)
-        recipient = self._widget.focusProxy()
         for _ in range(count):
-            # If we get a segfault here, we might want to try sendEvent
+            self._tab.post_event(press_evt)
-            # instead.
+            self._tab.post_event(release_evt)
-            QApplication.postEvent(recipient, press_evt)
-            QApplication.postEvent(recipient, release_evt)
 
     @pyqtSlot()
     def _update_pos(self):
@@ -458,7 +455,7 @@ class WebEngineTab(browsertab.AbstractTab):
         """
         elems = []
         for js_elem in js_elems:
-            elem = webengineelem.WebEngineElement(js_elem, self.run_js_async)
+            elem = webengineelem.WebEngineElement(js_elem, tab=self)
             elems.append(elem)
         callback(elems)
 
@@ -474,11 +471,12 @@ class WebEngineTab(browsertab.AbstractTab):
         if js_elem is None:
             callback(None)
         else:
-            elem = webengineelem.WebEngineElement(js_elem, self.run_js_async)
+            elem = webengineelem.WebEngineElement(js_elem, tab=self)
             callback(elem)
 
     def find_all_elements(self, selector, callback, *, only_visible=False):
-        js_code = javascript.assemble('webelem', 'find_all', selector)
+        js_code = javascript.assemble('webelem', 'find_all', selector,
+                                      only_visible)
         js_cb = functools.partial(self._js_element_cb_multiple, callback)
         self.run_js_async(js_code, js_cb)
 
@@ -516,3 +514,9 @@ class WebEngineTab(browsertab.AbstractTab):
             page.contentsSizeChanged.connect(self.contents_size_changed)
         except AttributeError:
             log.stub('contentsSizeChanged, on Qt < 5.7')
+
+    def post_event(self, evt):
+        # If we get a segfault here, we might want to try sendEvent
+        # instead.
+        recipient = self._widget.focusProxy()
+        QApplication.postEvent(recipient, evt)

qutebrowser/browser/webkit/webkitelem.py

@@ -83,9 +83,9 @@ class WebKitElement(webelem.AbstractWebElement):
         if self._elem.isNull():
             raise IsNullError('Element {} vanished!'.format(self._elem))
 
-    def frame(self):
+    def has_frame(self):
         self._check_vanished()
-        return self._elem.webFrame()
+        return self._elem.webFrame() is not None
 
     def geometry(self):
         self._check_vanished()
@@ -217,8 +217,6 @@ class WebKitElement(webelem.AbstractWebElement):
                            we want to avoid doing it twice.
             no_js: Fall back to the Python implementation
         """
-        # FIXME:qtwebengine can we get rid of this with
-        # find_all_elements(only_visible=True)?
         self._check_vanished()
 
         # First try getting the element rect via JS, as that's usually more

qutebrowser/browser/webkit/webkittab.py

@@ -28,6 +28,7 @@ from PyQt5.QtCore import (pyqtSlot, Qt, QEvent, QUrl, QPoint, QTimer, QSizeF,
 from PyQt5.QtGui import QKeyEvent
 from PyQt5.QtWebKitWidgets import QWebPage, QWebFrame
 from PyQt5.QtWebKit import QWebSettings
+from PyQt5.QtWidgets import QApplication
 from PyQt5.QtPrintSupport import QPrinter
 
 from qutebrowser.browser import browsertab
@@ -677,3 +678,8 @@ class WebKitTab(browsertab.AbstractTab):
         frame.contentsSizeChanged.connect(self._on_contents_size_changed)
         frame.initialLayoutCompleted.connect(self._on_history_trigger)
         page.link_clicked.connect(self._on_link_clicked)
+
+    def post_event(self, evt):
+        # If we get a segfault here, we might want to try sendEvent
+        # instead.
+        QApplication.postEvent(self._widget, evt)

qutebrowser/javascript/.eslintrc.yaml

@@ -23,7 +23,7 @@ rules:
      init-declarations: "off"
      no-plusplus: "off"
      no-extra-parens: off
-     id-length: ["error", {"exceptions": ["i", "x", "y"]}]
+     id-length: ["error", {"exceptions": ["i", "k", "x", "y"]}]
      object-shorthand: "off"
      max-statements: ["error", {"max": 30}]
      quotes: ["error", "double", {"avoidEscape": true}]
@@ -35,3 +35,4 @@ rules:
      func-names: "off"
      sort-keys: "off"
      no-warning-comments: "off"
+     max-len: ["error", {"ignoreUrls": true}]

qutebrowser/javascript/webelem.js

@@ -23,12 +23,20 @@ window._qutebrowser.webelem = (function() {
     var funcs = {};
     var elements = [];
 
-    function serialize_elem(elem, id) {
+    function serialize_elem(elem) {
+        if (!elem) {
+            return null;
+        }
+
+        var id = elements.length;
+        elements[id] = elem;
+
         var out = {
             "id": id,
             "text": elem.text,
             "tag_name": elem.tagName,
             "outer_xml": elem.outerHTML,
+            "rects": [],  // Gets filled up later
         };
 
         var attributes = {};
@@ -38,21 +46,71 @@ window._qutebrowser.webelem = (function() {
         }
         out.attributes = attributes;
 
+        var client_rects = elem.getClientRects();
+        for (var k = 0; k < client_rects.length; ++k) {
+            var rect = client_rects[k];
+            out.rects.push({
+                "top": rect.top,
+                "right": rect.right,
+                "bottom": rect.bottom,
+                "left": rect.left,
+                "height": rect.height,
+                "width": rect.width,
+            });
+        }
+
         // console.log(JSON.stringify(out));
 
         return out;
     }
 
-    funcs.find_all = function(selector) {
+    function is_visible(elem) {
+        // FIXME:qtwebengine Handle frames and iframes
+
+        // Adopted from vimperator:
+        // https://github.com/vimperator/vimperator-labs/blob/vimperator-3.14.0/common/content/hints.js#L259-L285
+        // FIXME:qtwebengine we might need something more sophisticated like
+        // the cVim implementation here?
+        // https://github.com/1995eaton/chromium-vim/blob/1.2.85/content_scripts/dom.js#L74-L134
+
+        var win = elem.ownerDocument.defaultView;
+        var rect = elem.getBoundingClientRect();
+
+        if (!rect ||
+                rect.top > window.innerHeight ||
+                rect.bottom < 0 ||
+                rect.left > window.innerWidth ||
+                rect.right < 0) {
+            return false;
+        }
+
+        rect = elem.getClientRects()[0];
+        if (!rect) {
+            return false;
+        }
+
+        var style = win.getComputedStyle(elem, null);
+        // FIXME:qtwebengine do we need this <area> handling?
+        // visibility and display style are misleading for area tags and they
+        // get "display: none" by default.
+        // See https://github.com/vimperator/vimperator-labs/issues/236
+        if (elem.nodeName.toLowerCase() !== "area" && (
+                style.getPropertyValue("visibility") !== "visible" ||
+                style.getPropertyValue("display") === "none")) {
+            return false;
+        }
+
+        return true;
+    }
+
+    funcs.find_all = function(selector, only_visible) {
         var elems = document.querySelectorAll(selector);
         var out = [];
-        var id = elements.length;
 
         for (var i = 0; i < elems.length; ++i) {
-            var elem = elems[i];
+            if (!only_visible || is_visible(elems[i])) {
-            out.push(serialize_elem(elem, id));
+                out.push(serialize_elem(elems[i]));
-            elements[id] = elem;
+            }
-            id++;
         }
 
         return out;
@@ -67,9 +125,7 @@ window._qutebrowser.webelem = (function() {
             return null;
         }
 
-        var id = elements.length;
+        return serialize_elem(elem);
-        elements[id] = elem;
-        return serialize_elem(elem, id);
     };
 
     funcs.set_text = function(id, text) {
@@ -83,13 +139,7 @@ window._qutebrowser.webelem = (function() {
         // element is returned (the iframe itself).
 
         var elem = document.elementFromPoint(x, y);
-        if (!elem) {
+        return serialize_elem(elem);
-            return null;
-        }
-
-        var id = elements.length;
-        elements[id] = elem;
-        return serialize_elem(elem, id);
     };
 
     return funcs;

qutebrowser/utils/javascript.py

@@ -52,6 +52,8 @@ def _convert_js_arg(arg):
         return 'undefined'
     elif isinstance(arg, str):
         return '"{}"'.format(string_escape(arg))
+    elif isinstance(arg, bool):
+        return str(arg).lower()
     elif isinstance(arg, (int, float)):
         return str(arg)
     else:

tests/helpers/fixtures.py

@@ -41,7 +41,7 @@ from qutebrowser.browser.webkit import cookies
 from qutebrowser.misc import savemanager
 from qutebrowser.keyinput import modeman
 
-from PyQt5.QtCore import PYQT_VERSION, QEvent, QSize, Qt
+from PyQt5.QtCore import PYQT_VERSION, pyqtSignal, QEvent, QSize, Qt, QObject
 from PyQt5.QtGui import QKeyEvent
 from PyQt5.QtWidgets import QWidget, QHBoxLayout, QVBoxLayout
 from PyQt5.QtNetwork import QNetworkCookieJar
@@ -72,6 +72,36 @@ class WinRegistryHelper:
             del objreg.window_registry[win_id]
 
 
+class CallbackChecker(QObject):
+
+    """Check if a value provided by a callback is the expected one."""
+
+    got_result = pyqtSignal(object)
+    UNSET = object()
+
+    def __init__(self, qtbot, parent=None):
+        super().__init__(parent)
+        self._qtbot = qtbot
+        self._result = self.UNSET
+
+    def callback(self, result):
+        """Callback which can be passed to runJavaScript."""
+        self._result = result
+        self.got_result.emit(result)
+
+    def check(self, expected):
+        """Wait until the JS result arrived and compare it."""
+        if self._result is self.UNSET:
+            with self._qtbot.waitSignal(self.got_result):
+                pass
+        assert self._result == expected
+
+
+@pytest.fixture
+def callback_checker(qtbot):
+    return CallbackChecker(qtbot)
+
+
 class FakeStatusBar(QWidget):
 
     """Fake statusbar to test progressbar sizing."""

tests/unit/browser/webkit/test_qt_javascript.py

@@ -22,31 +22,9 @@
 
 import pytest
 
-from PyQt5.QtCore import QObject, pyqtSignal
 from PyQt5.QtWebKit import QWebSettings
 
 
-class WebEngineJSChecker(QObject):
-
-    """Check if a JS value provided by a callback is the expected one."""
-
-    got_result = pyqtSignal(object)
-
-    def __init__(self, qtbot, parent=None):
-        super().__init__(parent)
-        self._qtbot = qtbot
-
-    def callback(self, result):
-        """Callback which can be passed to runJavaScript."""
-        self.got_result.emit(result)
-
-    def check(self, expected):
-        """Wait until the JS result arrived and compare it."""
-        with self._qtbot.waitSignal(self.got_result) as blocker:
-            pass
-        assert blocker.args == [expected]
-
-
 @pytest.mark.parametrize('js_enabled, expected', [(True, 2.0), (False, None)])
 def test_simple_js_webkit(webview, js_enabled, expected):
     """With QtWebKit, evaluateJavaScript works when JS is on."""
@@ -66,7 +44,8 @@ def test_element_js_webkit(webview, js_enabled, expected):
 
 @pytest.mark.usefixtures('redirect_xdg_data')
 @pytest.mark.parametrize('js_enabled, expected', [(True, 2.0), (False, 2.0)])
-def test_simple_js_webengine(qtbot, webengineview, js_enabled, expected):
+def test_simple_js_webengine(callback_checker, webengineview, js_enabled,
+                             expected):
     """With QtWebEngine, runJavaScript works even when JS is off."""
     # pylint: disable=no-name-in-module,useless-suppression
     # If we get there (because of the webengineview fixture) we can be certain
@@ -75,6 +54,5 @@ def test_simple_js_webengine(qtbot, webengineview, js_enabled, expected):
     webengineview.settings().setAttribute(QWebEngineSettings.JavascriptEnabled,
                                           js_enabled)
 
-    checker = WebEngineJSChecker(qtbot)
+    webengineview.page().runJavaScript('1 + 1', callback_checker.callback)
-    webengineview.page().runJavaScript('1 + 1', checker.callback)
+    callback_checker.check(expected)
-    checker.check(expected)

tests/unit/browser/webkit/test_webkitelem.py

@@ -255,7 +255,7 @@ class TestWebKitElement:
         lambda e: None in e,
         list,  # __iter__
         len,
-        lambda e: e.frame(),
+        lambda e: e.has_frame(),
         lambda e: e.geometry(),
         lambda e: e.style_property('visibility', strategy='computed'),
         lambda e: e.text(),
@@ -394,7 +394,6 @@ class TestWebKitElement:
         assert elem.debug_text() == expected
 
     @pytest.mark.parametrize('attribute, code', [
-        ('webFrame', lambda e: e.frame()),
         ('geometry', lambda e: e.geometry()),
         ('toOuterXml', lambda e: e.outer_xml()),
     ])
@@ -404,6 +403,12 @@ class TestWebKitElement:
         setattr(mock, 'return_value', sentinel)
         assert code(elem) is sentinel
 
+    @pytest.mark.parametrize('frame, expected', [
+        (object(), True), (None, False)])
+    def test_has_frame(self, elem, frame, expected):
+        elem._elem.webFrame.return_value = frame
+        assert elem.has_frame() == expected
+
     def test_tag_name(self, elem):
         elem._elem.tagName.return_value = 'SPAN'
         assert elem.tag_name() == 'span'

tests/unit/utils/test_javascript.py

@@ -127,8 +127,10 @@ class TestStringEscape:
     ('foo\\bar', r'"foo\\bar"'),
     (42, '42'),
     (23.42, '23.42'),
+    (False, 'false'),
     (None, 'undefined'),
     (object(), TypeError),
+    (True, 'true'),
 ])
 def test_convert_js_arg(arg, expected):
     if expected is TypeError: