From c76473347256fc7737c5aabb2875d462a948c481 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 27 Jul 2016 15:11:06 +0200 Subject: [PATCH 01/15] Refactor QWebElement API, part 1 --- qutebrowser/browser/commands.py | 10 +-- qutebrowser/browser/hints.py | 44 +++++----- qutebrowser/browser/webkit/webelem.py | 114 +++++++++++++++++--------- 3 files changed, 94 insertions(+), 74 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index c91cfca2b..4ba5c657f 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1460,15 +1460,7 @@ class CommandDispatcher: text: The new text to insert. """ try: - if elem.is_content_editable(): - log.misc.debug("Filling element {} via setPlainText.".format( - elem.debug_text())) - elem.setPlainText(text) - else: - log.misc.debug("Filling element {} via javascript.".format( - elem.debug_text())) - text = webelem.javascript_escape(text) - elem.evaluateJavaScript("this.value='{}'".format(text)) + elem.set_text(text) except webelem.IsNullError: raise cmdexc.CommandError("Element vanished while editing!") diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index be2b1bcef..a17d03972 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -335,16 +335,16 @@ class HintManager(QObject): def _is_hidden(self, elem): """Check if the element is hidden via display=none.""" - display = elem.styleProperty('display', QWebElement.InlineStyle) + display = elem.style_property('display', QWebElement.InlineStyle) return display == 'none' def _show_elem(self, elem): """Show a given element.""" - elem.setStyleProperty('display', 'inline !important') + elem.set_style_property('display', 'inline !important') def _hide_elem(self, elem): """Hide a given element.""" - elem.setStyleProperty('display', 'none !important') + elem.set_style_property('display', 'none !important') def _set_style_properties(self, elem, label): """Set the hint CSS on the element given. @@ -373,7 +373,7 @@ class HintManager(QObject): attrs.append(('text-transform', 'none !important')) for k, v in attrs: - label.setStyleProperty(k, v) + label.set_style_property(k, v) self._set_style_position(elem, label) def _set_style_position(self, elem, label): @@ -389,8 +389,8 @@ class HintManager(QObject): top = rect.y() log.hints.vdebug("Drawing label '{!r}' at {}/{} for element '{!r}' " "(no_js: {})".format(label, left, top, elem, no_js)) - label.setStyleProperty('left', '{}px !important'.format(left)) - label.setStyleProperty('top', '{}px !important'.format(top)) + 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. @@ -402,22 +402,16 @@ class HintManager(QObject): Return: The newly created label element """ - doc = elem.webFrame().documentElement() - # 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 + doc = elem.document_element() body = doc.findFirst('body') - if not body.isNull(): - parent = body - else: + if body is None: parent = doc - parent.appendInside('') - label = webelem.WebElementWrapper(parent.lastChild()) + else: + parent = body + label = parent.create_inside('span') label['class'] = 'qutehint' self._set_style_properties(elem, label) - label.setPlainText(string) + label.set_text(string) return label def _show_url_error(self): @@ -490,7 +484,7 @@ class HintManager(QObject): self.mouse_event.emit(evt) if elem.is_text_input() and elem.is_editable(): QTimer.singleShot(0, functools.partial( - elem.webFrame().page().triggerAction, + elem.frame().page().triggerAction, QWebPage.MoveToEndOfDocument)) QTimer.singleShot(0, self.stop_hinting.emit) @@ -559,7 +553,7 @@ class HintManager(QObject): download_manager = objreg.get('download-manager', scope='window', window=self._win_id) - download_manager.get(url, page=elem.webFrame().page(), + download_manager.get(url, page=elem.frame().page(), prompt_download_directory=prompt) def _call_userscript(self, elem, context): @@ -878,7 +872,7 @@ class HintManager(QObject): matched = string[:len(keystr)] rest = string[len(keystr):] match_color = config.get('colors', 'hints.fg.match') - elem.label.setInnerXml( + elem.label.set_inner_xml( '{}{}'.format( match_color, matched, rest)) if self._is_hidden(elem.label): @@ -913,7 +907,7 @@ class HintManager(QObject): strings = self._hint_strings(elems) self._context.elems = {} for elem, string in zip(elems, strings): - elem.label.setInnerXml(string) + elem.label.set_inner_xml(string) self._context.elems[string] = elem keyparsers = objreg.get('keyparsers', scope='window', window=self._win_id) @@ -1017,7 +1011,7 @@ class HintManager(QObject): Target.spawn: self._spawn, } elem = self._context.elems[keystr].elem - if elem.webFrame() is None: + if elem.frame() is None: message.error(self._win_id, "This element has no webframe.", immediately=True) @@ -1042,7 +1036,7 @@ class HintManager(QObject): self.filter_hints(None) # Undo keystring highlighting for string, elem in self._context.elems.items(): - elem.label.setInnerXml(string) + elem.label.set_inner_xml(string) handler() @cmdutils.register(instance='hintmanager', scope='tab', hide=True, @@ -1068,7 +1062,7 @@ class HintManager(QObject): log.hints.debug("Contents size changed...!") for e in self._context.all_elems: try: - if e.elem.webFrame() is None: + if e.elem.frame() is None: # This sometimes happens for some reason... e.label.removeFromDocument() continue diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webelem.py index 97210bebd..9cacada2d 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webelem.py @@ -28,7 +28,6 @@ Module attributes: """ import collections.abc -import functools from PyQt5.QtCore import QRect, QUrl from PyQt5.QtWebKit import QWebElement @@ -83,40 +82,6 @@ class WebElementWrapper(collections.abc.MutableMapping): if elem.isNull(): raise IsNullError('{} is a null element!'.format(elem)) self._elem = elem - for name in ['addClass', 'appendInside', 'appendOutside', - 'attributeNS', 'classes', 'clone', 'document', - 'encloseContentsWith', 'encloseWith', - 'evaluateJavaScript', 'findAll', 'findFirst', - 'firstChild', 'geometry', 'hasAttributeNS', - 'hasAttributes', 'hasClass', 'hasFocus', 'lastChild', - 'localName', 'namespaceUri', 'nextSibling', 'parent', - 'prefix', 'prependInside', 'prependOutside', - 'previousSibling', 'removeAllChildren', - 'removeAttributeNS', 'removeClass', 'removeFromDocument', - 'render', 'replace', 'setAttributeNS', 'setFocus', - 'setInnerXml', 'setOuterXml', 'setPlainText', - 'setStyleProperty', 'styleProperty', 'tagName', - 'takeFromDocument', 'toInnerXml', 'toOuterXml', - 'toggleClass', 'webFrame', '__eq__', '__ne__']: - # We don't wrap some methods for which we have better alternatives: - # - Mapping access for attributeNames/hasAttribute/setAttribute/ - # attribute/removeAttribute. - # - isNull is checked automagically. - # - str(...) instead of toPlainText - # For the rest, we create a wrapper which checks if the element is - # null. - - method = getattr(self._elem, name) - - def _wrapper(meth, *args, **kwargs): - self._check_vanished() - return meth(*args, **kwargs) - - wrapper = functools.partial(_wrapper, method) - # We used to do functools.update_wrapper here, but for some reason - # when using hints with many links, this accounted for nearly 50% - # of the time when profiling, which is unacceptable. - setattr(self, name, wrapper) def __str__(self): self._check_vanished() @@ -162,6 +127,75 @@ class WebElementWrapper(collections.abc.MutableMapping): if self._elem.isNull(): raise IsNullError('Element {} vanished!'.format(self._elem)) + def frame(self): + """Get the main frame of this element.""" + # FIXME:qtwebengine how to get rid of this? + self._check_vanished() + return self._elem.webFrame() + + def geometry(self): + """Get the geometry for this element.""" + self._check_vanished() + return self._elem.geometry() + + def document_element(self): + """Get the document element of this element.""" + self._check_vanished() + elem = self._elem.webFrame().documentElement() + return WebElementWrapper(elem) + + def create_inside(self, tagname): + """Append the given element inside the current one.""" + # 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 WebElementWrapper(self._elem.lastChild()) + + def find_first(self, selector): + """Find the first child based on the given CSS selector.""" + self._check_vanished() + elem = self._elem.findFirst(selector) + if elem.isNull(): + return None + return WebElementWrapper(elem) + + def style_property(self, name, strategy): + """Get the element style resolved with the given strategy.""" + self._check_vanished() + return self._elem.styleProperty(name, strategy) + + def set_text(self, text): + """Set the given plain text.""" + self._check_vanished() + if self.is_content_editable(): + log.misc.debug("Filling element {} via set_text.".format( + self.debug_text())) + self._elem.setPlainText(text) + else: + log.misc.debug("Filling element {} via javascript.".format( + self.debug_text())) + text = javascript_escape(text) + self._elem.evaluateJavaScript("this.value='{}'".format(text)) + + def set_inner_xml(self, xml): + """Set the given inner XML.""" + self._check_vanished() + self._elem.setInnerXml(text) + + def remove(self): + """Remove the node from the document.""" + self._check_vanished() + self._elem.removeFromDocument() + + def set_style_property(self, name, value): + """Set the element style.""" + self._check_vanished() + return self._elem.setStyleProperty(name, value) + def is_visible(self, mainframe): """Check whether the element is currently visible on the screen. @@ -404,14 +438,14 @@ def rect_on_view(elem, *, elem_geometry=None, adjust_zoom=True, no_js=False): height = rect.get("height", 0) if width > 1 and height > 1: # fix coordinates according to zoom level - zoom = elem.webFrame().zoomFactor() + zoom = elem.frame().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() + frame = elem.frame() while frame is not None: # Translate to parent frames' position # (scroll position is taken care of inside getClientRects) @@ -424,7 +458,7 @@ def rect_on_view(elem, *, elem_geometry=None, adjust_zoom=True, no_js=False): geometry = elem.geometry() else: geometry = elem_geometry - frame = elem.webFrame() + frame = elem.frame() rect = QRect(geometry) while frame is not None: rect.translate(frame.geometry().topLeft()) @@ -432,7 +466,7 @@ def rect_on_view(elem, *, elem_geometry=None, adjust_zoom=True, no_js=False): frame = frame.parentFrame() # We deliberately always adjust the zoom here, even with adjust_zoom=False if elem_geometry is None: - zoom = elem.webFrame().zoomFactor() + zoom = elem.frame().zoomFactor() if not config.get('ui', 'zoom-text-only'): rect.moveTo(rect.left() / zoom, rect.top() / zoom) rect.setWidth(rect.width() / zoom) @@ -476,7 +510,7 @@ def is_visible(elem, mainframe): visible_on_screen = mainframe_geometry.contains(elem_rect.topLeft()) # Then check if it's visible in its frame if it's not in the main # frame. - elem_frame = elem.webFrame() + elem_frame = elem.frame() framegeom = QRect(elem_frame.geometry()) if not framegeom.isValid(): visible_in_frame = False From c5a91004f5fb7ffaa610f9458b1c8a5912d727ae Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 27 Jul 2016 15:34:28 +0200 Subject: [PATCH 02/15] Add contents_size_changed to tab API --- qutebrowser/browser/browsertab.py | 3 +- qutebrowser/browser/hints.py | 30 ------------------- qutebrowser/browser/webengine/webenginetab.py | 4 +++ qutebrowser/browser/webkit/webkittab.py | 10 ++++++- 4 files changed, 15 insertions(+), 32 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 5886d851e..8ca437e2d 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -21,7 +21,7 @@ import itertools -from PyQt5.QtCore import pyqtSignal, pyqtSlot, QUrl, QObject, QPoint +from PyQt5.QtCore import pyqtSignal, pyqtSlot, QUrl, QObject, QPoint, QSizeF from PyQt5.QtGui import QIcon from PyQt5.QtWidgets import QWidget, QLayout @@ -482,6 +482,7 @@ class AbstractTab(QWidget): new_tab_requested = pyqtSignal(QUrl) url_changed = pyqtSignal(QUrl) shutting_down = pyqtSignal() + contents_size_changed = pyqtSignal(QSizeF) def __init__(self, win_id, parent=None): self.win_id = win_id diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index a17d03972..63b6ec031 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -64,8 +64,6 @@ class HintContext: Attributes: frames: The QWebFrames to use. - destroyed_frames: id()'s of QWebFrames which have been destroyed. - (Workaround for https://github.com/The-Compiler/qutebrowser/issues/152) 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. @@ -95,7 +93,6 @@ class HintContext: self.to_follow = None self.rapid = False self.frames = [] - self.destroyed_frames = [] self.args = [] self.mainframe = None self.tab = None @@ -179,22 +176,6 @@ class HintManager(QObject): elem.label.removeFromDocument() except webelem.IsNullError: pass - for f in self._context.frames: - log.hints.debug("Disconnecting frame {}".format(f)) - if id(f) in self._context.destroyed_frames: - # WORKAROUND for - # https://github.com/The-Compiler/qutebrowser/issues/152 - log.hints.debug("Frame has been destroyed, ignoring.") - continue - try: - f.contentsSizeChanged.disconnect(self.on_contents_size_changed) - except TypeError: - # It seems we can get this here: - # TypeError: disconnect() failed between - # 'contentsSizeChanged' and 'on_contents_size_changed' - # See # https://github.com/The-Compiler/qutebrowser/issues/263 - pass - log.hints.debug("Disconnected.") text = self._get_text() message_bridge = objreg.get('message-bridge', scope='window', window=self._win_id) @@ -656,12 +637,6 @@ class HintManager(QObject): log.hints.vdebug("No match on '{}'!".format(text)) return None - def _connect_frame_signals(self): - """Connect the contentsSizeChanged signals to all frames.""" - for f in self._context.frames: - log.hints.debug("Connecting frame {}".format(f)) - f.contentsSizeChanged.connect(self.on_contents_size_changed) - def _check_args(self, target, *args): """Check the arguments passed to start() and raise if they're wrong. @@ -847,11 +822,6 @@ class HintManager(QObject): except qtutils.QtValueError: raise cmdexc.CommandError("No URL set for this page yet!") self._context.frames = webelem.get_child_frames(mainframe) - for frame in self._context.frames: - # WORKAROUND for - # https://github.com/The-Compiler/qutebrowser/issues/152 - frame.destroyed.connect(functools.partial( - self._context.destroyed_frames.append, id(frame))) self._context.args = args self._context.mainframe = mainframe self._context.group = group diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 96c7e562e..5a8025a19 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -425,3 +425,7 @@ class WebEngineTab(browsertab.AbstractTab): view.iconChanged.connect(self.icon_changed) except AttributeError: log.stub('iconChanged, on Qt < 5.7') + try: + page.contentsSizeChanged.connect(self.contents_size_changed) + except AttributeError: + log.stub('contentsSizeChanged, on Qt < 5.7') diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 899a46573..7cc127e16 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -25,7 +25,7 @@ import xml.etree.ElementTree from PyQt5.QtCore import pyqtSlot, Qt, QEvent, QUrl, QPoint, QTimer from PyQt5.QtGui import QKeyEvent -from PyQt5.QtWebKitWidgets import QWebPage +from PyQt5.QtWebKitWidgets import QWebPage, QWebFrame from PyQt5.QtWebKit import QWebSettings from PyQt5.QtPrintSupport import QPrinter @@ -572,6 +572,14 @@ class WebKitTab(browsertab.AbstractTab): """Emit iconChanged with a QIcon like QWebEngineView does.""" self.icon_changed.emit(self._widget.icon()) + @pyqtSlot(QWebFrame) + def _on_frame_created(self, frame): + """Connect the contentsSizeChanged signal of each frame.""" + # FIXME:qtwebengine those could theoretically regress: + # https://github.com/The-Compiler/qutebrowser/issues/152 + # https://github.com/The-Compiler/qutebrowser/issues/263 + frame.contentsSizeChanged.connect(self.contents_size_changed) + def _connect_signals(self): view = self._widget page = view.page() From 193c75563737e873a08d857ac96431df941da0b3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 27 Jul 2016 16:27:38 +0200 Subject: [PATCH 03/15] Refactor WebElement API, part 2 Now we don't get a crash, but not any hints either... --- qutebrowser/browser/browsertab.py | 4 +++ qutebrowser/browser/commands.py | 12 ++----- qutebrowser/browser/hints.py | 42 ++++++++----------------- qutebrowser/browser/webkit/webelem.py | 25 +++++++-------- qutebrowser/browser/webkit/webkittab.py | 14 ++++++++- 5 files changed, 43 insertions(+), 54 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 8ca437e2d..cadc6a31d 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -633,6 +633,10 @@ class AbstractTab(QWidget): def set_html(self, html, base_url): raise NotImplementedError + def find_all_elements(self, selector): + """Find all HTML elements matching a given selector.""" + raise NotImplementedError + def __repr__(self): try: url = utils.elide(self.url().toDisplayString(QUrl.EncodeUnicode), diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 4ba5c657f..5a81cce5a 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -503,21 +503,13 @@ class CommandDispatcher: if widget.backend == usertypes.Backend.QtWebEngine: raise cmdexc.CommandError(":navigate prev/next is not " "supported yet with QtWebEngine") - page = widget._widget.page() # pylint: disable=protected-access - frame = page.currentFrame() - if frame is None: - raise cmdexc.CommandError("No frame focused!") - else: - frame = None hintmanager = objreg.get('hintmanager', scope='tab', tab='current') if where == 'prev': - assert frame is not None - hintmanager.follow_prevnext(frame, url, prev=True, tab=tab, + hintmanager.follow_prevnext(widget, url, prev=True, tab=tab, background=bg, window=window) elif where == 'next': - assert frame is not None - hintmanager.follow_prevnext(frame, url, prev=False, tab=tab, + hintmanager.follow_prevnext(widget, url, prev=False, tab=tab, background=bg, window=window) elif where == 'up': self._navigate_up(url, tab, bg, window) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 63b6ec031..2a1716bcf 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -80,7 +80,6 @@ class HintContext: to_follow: The link to follow when enter is pressed. args: Custom arguments for userscript/spawn rapid: Whether to do rapid hinting. - mainframe: The main QWebFrame where we started hinting in. tab: The WebTab object we started hinting in. group: The group of web elements to hint. """ @@ -94,7 +93,6 @@ class HintContext: self.rapid = False self.frames = [] self.args = [] - self.mainframe = None self.tab = None self.group = None @@ -173,7 +171,7 @@ class HintManager(QObject): """Clean up after hinting.""" for elem in self._context.all_elems: try: - elem.label.removeFromDocument() + elem.label.remove_from_document() except webelem.IsNullError: pass text = self._get_text() @@ -384,7 +382,7 @@ class HintManager(QObject): The newly created label element """ doc = elem.document_element() - body = doc.findFirst('body') + body = doc.find_first('body') if body is None: parent = doc else: @@ -598,13 +596,12 @@ class HintManager(QObject): qtutils.ensure_valid(url) return url - def _find_prevnext(self, frame, prev=False): + def _find_prevnext(self, tab, prev=False): """Find a prev/next element in frame.""" # First check for - elems = frame.findAllElements(webelem.SELECTORS[webelem.Group.links]) + elems = tab.find_all_elements(webelem.SELECTORS[webelem.Group.links]) rel_values = ('prev', 'previous') if prev else ('next') for e in elems: - e = webelem.WebElementWrapper(e) try: rel_attr = e['rel'] except KeyError: @@ -614,9 +611,8 @@ class HintManager(QObject): e.debug_text(), rel_attr)) return e # Then check for regular links/buttons. - elems = frame.findAllElements( + elems = tab.find_all_elements( webelem.SELECTORS[webelem.Group.prevnext]) - elems = [webelem.WebElementWrapper(e) for e in elems] filterfunc = webelem.FILTERS[webelem.Group.prevnext] elems = [e for e in elems if filterfunc(e)] @@ -659,14 +655,9 @@ class HintManager(QObject): def _init_elements(self): """Initialize the elements and labels based on the context set.""" - elems = [] - for f in self._context.frames: - elems += f.findAllElements(webelem.SELECTORS[self._context.group]) - elems = [e for e in elems - if webelem.is_visible(e, self._context.mainframe)] - # We wrap the elements late for performance reasons, as wrapping 1000s - # of elements (with ~50 methods each) just takes too much time... - elems = [webelem.WebElementWrapper(e) for e in elems] + selector = webelem.SELECTORS[self._context.group] + elems = self._context.tab.find_all_elements(selector) + elems = [e for e in elems if e.is_visible()] filterfunc = webelem.FILTERS.get(self._context.group, lambda e: True) elems = [e for e in elems if filterfunc(e)] if not elems: @@ -693,12 +684,12 @@ class HintManager(QObject): # Do multi-word matching return all(word in elemstr for word in filterstr.split()) - def follow_prevnext(self, frame, baseurl, prev=False, tab=False, + def follow_prevnext(self, browsertab, baseurl, prev=False, tab=False, background=False, window=False): """Click a "previous"/"next" element on the page. Args: - frame: The frame where the element is in. + browsertab: The WebKitTab/WebEngineTab of the page. baseurl: The base URL of the current tab. prev: True to open a "previous" link, False to open a "next" link. tab: True to open in a new tab, False for the current tab. @@ -706,7 +697,7 @@ class HintManager(QObject): window: True to open in a new window, False for the current one. """ from qutebrowser.mainwindow import mainwindow - elem = self._find_prevnext(frame, prev) + elem = self._find_prevnext(browsertab, prev) if elem is None: raise cmdexc.CommandError("No {} links found!".format( "prev" if prev else "forward")) @@ -789,11 +780,6 @@ class HintManager(QObject): tab = tabbed_browser.currentWidget() if tab is None: raise cmdexc.CommandError("No WebView available yet!") - # FIXME:qtwebengine have a proper API for this - page = tab._widget.page() # pylint: disable=protected-access - mainframe = page.mainFrame() - if mainframe is None: - raise cmdexc.CommandError("No frame focused!") mode_manager = objreg.get('mode-manager', scope='window', window=self._win_id) if mode_manager.mode == usertypes.KeyMode.hint: @@ -821,15 +807,13 @@ class HintManager(QObject): self._context.baseurl = tabbed_browser.current_url() except qtutils.QtValueError: raise cmdexc.CommandError("No URL set for this page yet!") - self._context.frames = webelem.get_child_frames(mainframe) + self._context.tab = tab self._context.args = args - self._context.mainframe = mainframe self._context.group = group self._init_elements() message_bridge = objreg.get('message-bridge', scope='window', window=self._win_id) message_bridge.set_text(self._get_text()) - self._connect_frame_signals() modeman.enter(self._win_id, usertypes.KeyMode.hint, 'HintManager.start') @@ -1034,7 +1018,7 @@ class HintManager(QObject): try: if e.elem.frame() is None: # This sometimes happens for some reason... - e.label.removeFromDocument() + e.label.remove_from_document() continue self._set_style_position(e.elem, e.label) except webelem.IsNullError: diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webelem.py index 9cacada2d..3da6227ac 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webelem.py @@ -184,9 +184,9 @@ class WebElementWrapper(collections.abc.MutableMapping): def set_inner_xml(self, xml): """Set the given inner XML.""" self._check_vanished() - self._elem.setInnerXml(text) + self._elem.setInnerXml(xml) - def remove(self): + def remove_from_document(self): """Remove the node from the document.""" self._check_vanished() self._elem.removeFromDocument() @@ -196,16 +196,13 @@ class WebElementWrapper(collections.abc.MutableMapping): self._check_vanished() return self._elem.setStyleProperty(name, value) - def is_visible(self, mainframe): + def is_visible(self): """Check whether the element is currently visible on the screen. - Args: - mainframe: The main QWebFrame. - Return: True if the element is visible, False otherwise. """ - return is_visible(self._elem, mainframe) + return is_visible(self._elem) def rect_on_view(self, **kwargs): """Get the geometry of the element relative to the webview.""" @@ -438,14 +435,14 @@ def rect_on_view(elem, *, elem_geometry=None, adjust_zoom=True, no_js=False): height = rect.get("height", 0) if width > 1 and height > 1: # fix coordinates according to zoom level - zoom = elem.frame().zoomFactor() + 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.frame() + frame = elem.webFrame() while frame is not None: # Translate to parent frames' position # (scroll position is taken care of inside getClientRects) @@ -458,7 +455,7 @@ def rect_on_view(elem, *, elem_geometry=None, adjust_zoom=True, no_js=False): geometry = elem.geometry() else: geometry = elem_geometry - frame = elem.frame() + frame = elem.webFrame() rect = QRect(geometry) while frame is not None: rect.translate(frame.geometry().topLeft()) @@ -466,7 +463,7 @@ def rect_on_view(elem, *, elem_geometry=None, adjust_zoom=True, no_js=False): frame = frame.parentFrame() # We deliberately always adjust the zoom here, even with adjust_zoom=False if elem_geometry is None: - zoom = elem.frame().zoomFactor() + zoom = elem.webFrame().zoomFactor() if not config.get('ui', 'zoom-text-only'): rect.moveTo(rect.left() / zoom, rect.top() / zoom) rect.setWidth(rect.width() / zoom) @@ -474,7 +471,7 @@ def rect_on_view(elem, *, elem_geometry=None, adjust_zoom=True, no_js=False): return rect -def is_visible(elem, mainframe): +def is_visible(elem): """Check if the given element is visible in the frame. We need this as a standalone function (as opposed to a WebElementWrapper @@ -483,10 +480,10 @@ def is_visible(elem, mainframe): Args: elem: The QWebElement to check. - mainframe: The QWebFrame in which the element should be visible. """ if elem.isNull(): raise IsNullError("Got called on a null element!") + mainframe = elem.webFrame() # CSS attributes which hide an element hidden_attributes = { 'visibility': 'hidden', @@ -510,7 +507,7 @@ def is_visible(elem, mainframe): visible_on_screen = mainframe_geometry.contains(elem_rect.topLeft()) # Then check if it's visible in its frame if it's not in the main # frame. - elem_frame = elem.frame() + elem_frame = elem.webFrame() framegeom = QRect(elem_frame.geometry()) if not framegeom.isValid(): visible_in_frame = False diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 7cc127e16..dca126f91 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -30,7 +30,7 @@ from PyQt5.QtWebKit import QWebSettings from PyQt5.QtPrintSupport import QPrinter from qutebrowser.browser import browsertab -from qutebrowser.browser.webkit import webview, tabhistory +from qutebrowser.browser.webkit import webview, tabhistory, webelem from qutebrowser.utils import qtutils, objreg, usertypes, utils @@ -557,6 +557,18 @@ class WebKitTab(browsertab.AbstractTab): def set_html(self, html, base_url): self._widget.setHtml(html, base_url) + def find_all_elements(self, selector): + mainframe = self._widget.page().mainFrame() + if mainframe is None: + raise WebTabError("No frame focused!") + + elems = [] + frames = webelem.get_child_frames(mainframe) + for f in frames: + for elem in f.findAllElements(selector): + elems.append(webelem.WebElementWrapper(elem)) + return elems + @pyqtSlot() def _on_frame_load_finished(self): """Make sure we emit an appropriate status when loading finished. From 4318604c57d6605ef95dcc51524a5f91a1fa7e15 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 27 Jul 2016 16:45:44 +0200 Subject: [PATCH 04/15] Don't use javascript in webelem.set_text for hints --- qutebrowser/browser/commands.py | 2 +- qutebrowser/browser/webkit/webelem.py | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 5a81cce5a..ab464d304 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1452,7 +1452,7 @@ class CommandDispatcher: text: The new text to insert. """ try: - elem.set_text(text) + elem.set_text(text, use_js=True) except webelem.IsNullError: raise cmdexc.CommandError("Element vanished while editing!") diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webelem.py index 3da6227ac..b8ffa1f86 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webelem.py @@ -168,10 +168,14 @@ class WebElementWrapper(collections.abc.MutableMapping): self._check_vanished() return self._elem.styleProperty(name, strategy) - def set_text(self, text): - """Set the given plain text.""" + def set_text(self, text, *, use_js=False): + """Set the given plain text. + + Args: + use_js: Whether to use javascript if the element isn't content-editable. + """ self._check_vanished() - if self.is_content_editable(): + if self.is_content_editable() or not use_js: log.misc.debug("Filling element {} via set_text.".format( self.debug_text())) self._elem.setPlainText(text) From ffdaf8d470f075b8fc5c67609d13a5a8afacb115 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 27 Jul 2016 16:51:24 +0200 Subject: [PATCH 05/15] Move webelem.is_visible/.rect_on_view to methods --- qutebrowser/browser/webkit/webelem.py | 257 ++++++++++++-------------- 1 file changed, 116 insertions(+), 141 deletions(-) diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webelem.py index b8ffa1f86..5fd21397a 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webelem.py @@ -200,18 +200,6 @@ class WebElementWrapper(collections.abc.MutableMapping): self._check_vanished() return self._elem.setStyleProperty(name, value) - def is_visible(self): - """Check whether the element is currently visible on the screen. - - Return: - True if the element is visible, False otherwise. - """ - return is_visible(self._elem) - - def rect_on_view(self, **kwargs): - """Get the geometry of the element relative to the webview.""" - return rect_on_view(self._elem, **kwargs) - def is_writable(self): """Check whether an element is writable.""" self._check_vanished() @@ -339,6 +327,122 @@ class WebElementWrapper(collections.abc.MutableMapping): self._check_vanished() return utils.compact_text(self._elem.toOuterXml(), 500) + def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, 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 elements containing other + elements with "display:block" style, see + https://github.com/The-Compiler/qutebrowser/issues/1298 + + Args: + 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 + """ + 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: + rects = self._elem.evaluateJavaScript("this.getClientRects()") + text = utils.compact_text(self._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 = self._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 = 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 + + # No suitable rects found via JS, try via the QWebElement API + if elem_geometry is None: + geometry = self._elem.geometry() + else: + geometry = elem_geometry + frame = self._elem.webFrame() + rect = QRect(geometry) + while frame is not 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 + 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 is_visible(self): + """Check if the given element is visible in the frame.""" + self._check_vanished() + mainframe = self._elem.webFrame() + # CSS attributes which hide an element + hidden_attributes = { + 'visibility': 'hidden', + 'display': 'none', + } + for k, v in hidden_attributes.items(): + if self._elem.styleProperty(k, QWebElement.ComputedStyle) == v: + return False + elem_geometry = self._elem.geometry() + if not elem_geometry.isValid() and elem_geometry.x() == 0: + # Most likely an invisible link + return False + # First check if the element is visible on screen + 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) + else: + # We got an invalid rectangle (width/height 0/0 probably), but this + # can still be a valid link. + visible_on_screen = mainframe_geometry.contains(elem_rect.topLeft()) + # Then check if it's visible in its frame if it's not in the main + # frame. + elem_frame = self._elem.webFrame() + framegeom = QRect(elem_frame.geometry()) + if not framegeom.isValid(): + visible_in_frame = False + elif elem_frame.parentFrame() is not None: + framegeom.moveTo(0, 0) + framegeom.translate(elem_frame.scrollPosition()) + if elem_geometry.isValid(): + visible_in_frame = framegeom.intersects(elem_geometry) + else: + # We got an invalid rectangle (width/height 0/0 probably), but + # this can still be a valid link. + visible_in_frame = framegeom.contains(elem_geometry.topLeft()) + else: + visible_in_frame = visible_on_screen + return all([visible_on_screen, visible_in_frame]) + + + def javascript_escape(text): """Escape values special to javascript in strings. @@ -398,132 +502,3 @@ def focus_elem(frame): return WebElementWrapper(elem) -def rect_on_view(elem, *, elem_geometry=None, adjust_zoom=True, no_js=False): - """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. - no_js: Fall back to the Python implementation - """ - 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 and not no_js: - 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: - geometry = elem.geometry() - else: - geometry = elem_geometry - frame = elem.webFrame() - rect = QRect(geometry) - while frame is not 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 - if elem_geometry is None: - zoom = 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 is_visible(elem): - """Check if the given element is visible in the frame. - - We need this as a standalone function (as opposed to a WebElementWrapper - method) because we want to check this before wrapping when hinting for - performance reasons. - - Args: - elem: The QWebElement to check. - """ - if elem.isNull(): - raise IsNullError("Got called on a null element!") - mainframe = elem.webFrame() - # CSS attributes which hide an element - hidden_attributes = { - 'visibility': 'hidden', - 'display': 'none', - } - for k, v in hidden_attributes.items(): - if elem.styleProperty(k, QWebElement.ComputedStyle) == v: - return False - elem_geometry = elem.geometry() - if not elem_geometry.isValid() and elem_geometry.x() == 0: - # Most likely an invisible link - return False - # First check if the element is visible on screen - elem_rect = rect_on_view(elem, elem_geometry=elem_geometry) - mainframe_geometry = mainframe.geometry() - if elem_rect.isValid(): - visible_on_screen = mainframe_geometry.intersects(elem_rect) - else: - # We got an invalid rectangle (width/height 0/0 probably), but this - # can still be a valid link. - visible_on_screen = mainframe_geometry.contains(elem_rect.topLeft()) - # Then check if it's visible in its frame if it's not in the main - # frame. - elem_frame = elem.webFrame() - framegeom = QRect(elem_frame.geometry()) - if not framegeom.isValid(): - visible_in_frame = False - elif elem_frame.parentFrame() is not None: - framegeom.moveTo(0, 0) - framegeom.translate(elem_frame.scrollPosition()) - if elem_geometry.isValid(): - visible_in_frame = framegeom.intersects(elem_geometry) - else: - # We got an invalid rectangle (width/height 0/0 probably), but - # this can still be a valid link. - visible_in_frame = framegeom.contains(elem_geometry.topLeft()) - else: - visible_in_frame = visible_on_screen - return all([visible_on_screen, visible_in_frame]) From e051de4843674a629df0fc891a6443b4aed146f3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 27 Jul 2016 16:54:26 +0200 Subject: [PATCH 06/15] Fix word hinting --- qutebrowser/browser/hints.py | 2 +- qutebrowser/browser/webkit/webelem.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 2a1716bcf..143946309 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -1094,7 +1094,7 @@ class WordHinter: }) return (attr_extractors[attr](elem) - for attr in extractable_attrs[elem.tagName()] + for attr in extractable_attrs[elem.tag_name()] if attr in elem or attr == "text") def tag_words_to_hints(self, words): diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webelem.py index 5fd21397a..41e321821 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webelem.py @@ -327,6 +327,11 @@ class WebElementWrapper(collections.abc.MutableMapping): self._check_vanished() return utils.compact_text(self._elem.toOuterXml(), 500) + def tag_name(self): + """Get the tag name for the current element.""" + self._check_vanished() + return self._elem.tagName() + def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, no_js=False): """Get the geometry of the element relative to the webview. From 5eca6e11a58822a0204b662b6c62cb01f39df957 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 27 Jul 2016 16:58:44 +0200 Subject: [PATCH 07/15] Fix editor/paste-primary --- qutebrowser/browser/commands.py | 7 ++----- qutebrowser/browser/webkit/webelem.py | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index ab464d304..3e5581ff3 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1433,10 +1433,7 @@ class CommandDispatcher: raise cmdexc.CommandError("No element focused!") if not elem.is_editable(strict=True): raise cmdexc.CommandError("Focused element is not editable!") - if elem.is_content_editable(): - text = str(elem) - else: - text = elem.evaluateJavaScript('this.value') + text = elem.text(use_js=True) ed = editor.ExternalEditor(self._win_id, self._tabbed_browser) ed.editing_finished.connect(functools.partial( self.on_editing_finished, elem)) @@ -1478,7 +1475,7 @@ class CommandDispatcher: log.misc.debug("Pasting primary selection into element {}".format( elem.debug_text())) - elem.evaluateJavaScript(""" + elem.run_js_async(""" var sel = '{}'; var event = document.createEvent('TextEvent'); event.initTextEvent('textInput', true, true, null, sel); diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webelem.py index 41e321821..a94652669 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webelem.py @@ -168,6 +168,18 @@ class WebElementWrapper(collections.abc.MutableMapping): self._check_vanished() return self._elem.styleProperty(name, strategy) + def text(self, *, use_js=False): + """Get the plain text content for this element. + + Args: + use_js: Whether to use javascript if the element isn't content-editable. + """ + self._check_vanished() + if self._elem.is_content_editable() or not use_js: + return self._elem.toPlainText() + else: + return self._elem.evaluateJavaScript('this.value') + def set_text(self, text, *, use_js=False): """Set the given plain text. @@ -332,6 +344,13 @@ class WebElementWrapper(collections.abc.MutableMapping): self._check_vanished() return self._elem.tagName() + def run_js_async(self, code, callback=None): + """Run the given JS snippet async on the element.""" + self._check_vanished() + result = self._elem.evaluateJavaScript(code) + if callback is not None: + callback(result) + def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, no_js=False): """Get the geometry of the element relative to the webview. From 49f57a5d7eaa0209d7e9d0554fb3b6529e6e9ffd Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 27 Jul 2016 17:05:24 +0200 Subject: [PATCH 08/15] Fix userscripts --- qutebrowser/browser/hints.py | 2 +- qutebrowser/browser/webkit/webelem.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 143946309..4d4b590f6 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -547,7 +547,7 @@ class HintManager(QObject): env = { 'QUTE_MODE': 'hints', 'QUTE_SELECTED_TEXT': str(elem), - 'QUTE_SELECTED_HTML': elem.toOuterXml(), + 'QUTE_SELECTED_HTML': elem.outer_xml(), } url = self._resolve_url(elem, context.baseurl) if url is not None: diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webelem.py index a94652669..594237e6e 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webelem.py @@ -339,6 +339,11 @@ class WebElementWrapper(collections.abc.MutableMapping): self._check_vanished() return utils.compact_text(self._elem.toOuterXml(), 500) + def outer_xml(self): + """Get the full HTML representation of this element.""" + self._check_vanished() + return self._elem.toOuterXml() + def tag_name(self): """Get the tag name for the current element.""" self._check_vanished() From 5b943a431ee1cea5f6a1076169f30e09da66f13b Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 27 Jul 2016 17:07:12 +0200 Subject: [PATCH 09/15] Fix WebElementWrapper.text --- qutebrowser/browser/webkit/webelem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webelem.py index 594237e6e..7b2236cc9 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webelem.py @@ -175,7 +175,7 @@ class WebElementWrapper(collections.abc.MutableMapping): use_js: Whether to use javascript if the element isn't content-editable. """ self._check_vanished() - if self._elem.is_content_editable() or not use_js: + if self.is_content_editable() or not use_js: return self._elem.toPlainText() else: return self._elem.evaluateJavaScript('this.value') From 540c62c232a8644da58bf1e44e3c5e2c9f1ec656 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 28 Jul 2016 10:34:51 +0200 Subject: [PATCH 10/15] Add only_visible to find_all_elements --- qutebrowser/browser/browsertab.py | 9 +++++++-- qutebrowser/browser/hints.py | 4 ++-- qutebrowser/browser/webkit/webelem.py | 5 ++--- qutebrowser/browser/webkit/webkittab.py | 6 +++++- tests/unit/browser/webkit/test_webelem.py | 4 ++-- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index cadc6a31d..996edc842 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -633,8 +633,13 @@ class AbstractTab(QWidget): def set_html(self, html, base_url): raise NotImplementedError - def find_all_elements(self, selector): - """Find all HTML elements matching a given selector.""" + def find_all_elements(self, selector, *, only_visible=False): + """Find all HTML elements matching a given selector. + + Args: + selector: The CSS selector to search for. + only_visible: Only show elements which are visible on screen. + """ raise NotImplementedError def __repr__(self): diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 4d4b590f6..d59267c92 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -656,8 +656,8 @@ class HintManager(QObject): def _init_elements(self): """Initialize the elements and labels based on the context set.""" selector = webelem.SELECTORS[self._context.group] - elems = self._context.tab.find_all_elements(selector) - elems = [e for e in elems if e.is_visible()] + elems = self._context.tab.find_all_elements(selector, + only_visible=True) filterfunc = webelem.FILTERS.get(self._context.group, lambda e: True) elems = [e for e in elems if filterfunc(e)] if not elems: diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webelem.py index 7b2236cc9..3325130a2 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webelem.py @@ -426,10 +426,9 @@ class WebElementWrapper(collections.abc.MutableMapping): rect.setHeight(rect.height() / zoom) return rect - def is_visible(self): - """Check if the given element is visible in the frame.""" + def is_visible(self, mainframe): + """Check if the given element is visible in the given frame.""" self._check_vanished() - mainframe = self._elem.webFrame() # CSS attributes which hide an element hidden_attributes = { 'visibility': 'hidden', diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index dca126f91..0db50212b 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -557,7 +557,7 @@ class WebKitTab(browsertab.AbstractTab): def set_html(self, html, base_url): self._widget.setHtml(html, base_url) - def find_all_elements(self, selector): + def find_all_elements(self, selector, *, only_visible=False): mainframe = self._widget.page().mainFrame() if mainframe is None: raise WebTabError("No frame focused!") @@ -567,6 +567,10 @@ class WebKitTab(browsertab.AbstractTab): for f in frames: for elem in f.findAllElements(selector): elems.append(webelem.WebElementWrapper(elem)) + + if only_visible: + elems = [e for e in elems if e.is_visible(mainframe)] + return elems @pyqtSlot() diff --git a/tests/unit/browser/webkit/test_webelem.py b/tests/unit/browser/webkit/test_webelem.py index b87a2f529..bfe1db224 100644 --- a/tests/unit/browser/webkit/test_webelem.py +++ b/tests/unit/browser/webkit/test_webelem.py @@ -672,9 +672,9 @@ class TestRectOnView: 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)) - raw_elem = get_webelem(frame=frame, js_rect_return=js_rect)._elem + elem = get_webelem(frame=frame, js_rect_return=js_rect) rect = QRect(10, 20, 30, 40) - assert webelem.rect_on_view(raw_elem, elem_geometry=rect) == rect + assert elem.rect_on_view(elem_geometry=rect) == rect assert not raw_elem.geometry.called @pytest.mark.parametrize('js_rect', [None, {}]) From ebc72c9b5f34d4b17180a9e6936ba7edad030bdc Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 28 Jul 2016 10:50:14 +0200 Subject: [PATCH 11/15] Fix TestRemoveBlankTarget.test_no_link --- tests/unit/browser/webkit/test_webelem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/browser/webkit/test_webelem.py b/tests/unit/browser/webkit/test_webelem.py index bfe1db224..f4309bae0 100644 --- a/tests/unit/browser/webkit/test_webelem.py +++ b/tests/unit/browser/webkit/test_webelem.py @@ -402,7 +402,7 @@ class TestRemoveBlankTarget: elem = [None] * depth elem[0] = get_webelem(tagname='div') for i in range(1, depth): - elem[i] = get_webelem(tagname='div', parent=elem[i-1]) + elem[i] = get_webelem(tagname='div', parent=elem[i-1]._elem) elem[i]._elem.encloseWith(elem[i-1]._elem) elem[-1].remove_blank_target() for i in range(depth): From 8d7baea9ae277cb967ff3fb444992ac8e18f7fd8 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 28 Jul 2016 10:50:49 +0200 Subject: [PATCH 12/15] Fix test_passed_geometry --- tests/unit/browser/webkit/test_webelem.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/browser/webkit/test_webelem.py b/tests/unit/browser/webkit/test_webelem.py index f4309bae0..aaeb3c42e 100644 --- a/tests/unit/browser/webkit/test_webelem.py +++ b/tests/unit/browser/webkit/test_webelem.py @@ -675,7 +675,7 @@ class TestRectOnView: elem = get_webelem(frame=frame, js_rect_return=js_rect) rect = QRect(10, 20, 30, 40) assert elem.rect_on_view(elem_geometry=rect) == rect - assert not raw_elem.geometry.called + assert not elem._elem.geometry.called @pytest.mark.parametrize('js_rect', [None, {}]) @pytest.mark.parametrize('zoom_text_only', [True, False]) From d87840e25400c3b68c4dfd9b6cbf45dfa9f6a4a6 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 28 Jul 2016 11:13:07 +0200 Subject: [PATCH 13/15] Fix lint --- qutebrowser/browser/webkit/webelem.py | 35 +++++++++++++++------------ 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webelem.py index 3325130a2..0187171e2 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webelem.py @@ -172,7 +172,8 @@ class WebElementWrapper(collections.abc.MutableMapping): """Get the plain text content for this element. Args: - use_js: Whether to use javascript if the element isn't content-editable. + use_js: Whether to use javascript if the element isn't + content-editable. """ self._check_vanished() if self.is_content_editable() or not use_js: @@ -184,7 +185,8 @@ class WebElementWrapper(collections.abc.MutableMapping): """Set the given plain text. Args: - use_js: Whether to use javascript if the element isn't content-editable. + use_js: Whether to use javascript if the element isn't + content-editable. """ self._check_vanished() if self.is_content_editable() or not use_js: @@ -356,13 +358,14 @@ class WebElementWrapper(collections.abc.MutableMapping): if callback is not None: callback(result) - def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, no_js=False): + def rect_on_view(self, *, elem_geometry=None, adjust_zoom=True, + 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(). + 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 @@ -370,10 +373,10 @@ class WebElementWrapper(collections.abc.MutableMapping): Args: elem_geometry: The geometry of the element, or None. - Calling QWebElement::geometry is rather expensive so we - want to avoid doing it twice. + 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. + current zoom level. no_js: Fall back to the Python implementation """ self._check_vanished() @@ -383,8 +386,8 @@ class WebElementWrapper(collections.abc.MutableMapping): if elem_geometry is None and not no_js: rects = self._elem.evaluateJavaScript("this.getClientRects()") text = utils.compact_text(self._elem.toOuterXml(), 500) - log.hints.vdebug("Client rectangles of element '{}': {}".format(text, - rects)) + 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) @@ -400,8 +403,8 @@ class WebElementWrapper(collections.abc.MutableMapping): rect = QRect(rect["left"], rect["top"], width, height) frame = self._elem.webFrame() while frame is not None: - # Translate to parent frames' position - # (scroll position is taken care of inside getClientRects) + # Translate to parent frames' position (scroll position + # is taken care of inside getClientRects) rect.translate(frame.geometry().topLeft()) frame = frame.parentFrame() return rect @@ -417,7 +420,8 @@ class WebElementWrapper(collections.abc.MutableMapping): 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 + # 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'): @@ -449,7 +453,8 @@ class WebElementWrapper(collections.abc.MutableMapping): else: # We got an invalid rectangle (width/height 0/0 probably), but this # can still be a valid link. - visible_on_screen = mainframe_geometry.contains(elem_rect.topLeft()) + visible_on_screen = mainframe_geometry.contains( + elem_rect.topLeft()) # Then check if it's visible in its frame if it's not in the main # frame. elem_frame = self._elem.webFrame() From 631109e5eb760f5a26d6f377aa924473a4ac2c21 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 28 Jul 2016 11:13:24 +0200 Subject: [PATCH 14/15] Fix raising of WebTabError --- qutebrowser/browser/webkit/webelem.py | 2 -- qutebrowser/browser/webkit/webkittab.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/qutebrowser/browser/webkit/webelem.py b/qutebrowser/browser/webkit/webelem.py index 0187171e2..5de299f77 100644 --- a/qutebrowser/browser/webkit/webelem.py +++ b/qutebrowser/browser/webkit/webelem.py @@ -533,5 +533,3 @@ def focus_elem(frame): """ elem = frame.findFirstElement(SELECTORS[Group.focus]) return WebElementWrapper(elem) - - diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 0db50212b..15c670567 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -560,7 +560,7 @@ class WebKitTab(browsertab.AbstractTab): def find_all_elements(self, selector, *, only_visible=False): mainframe = self._widget.page().mainFrame() if mainframe is None: - raise WebTabError("No frame focused!") + raise browsertab.WebTabError("No frame focused!") elems = [] frames = webelem.get_child_frames(mainframe) From 41e6a2392ac8d2b2465279db5a36ea4eb0985a4d Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 28 Jul 2016 11:13:56 +0200 Subject: [PATCH 15/15] QtWebEngine: Add stub for find_all_elements --- qutebrowser/browser/webengine/webenginetab.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 5a8025a19..4e0a82066 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -410,6 +410,10 @@ class WebEngineTab(browsertab.AbstractTab): def clear_ssl_errors(self): log.stub() + def find_all_elements(self, selector, *, only_visible=False): + log.stub() + return [] + def _connect_signals(self): view = self._widget page = view.page()