From 50f31ca7cbba5cb4c2f479be32e0bddcc8d96631 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 12 May 2014 10:04:27 +0200 Subject: [PATCH] Handle visibility of elements on screen correctly --- qutebrowser/browser/hints.py | 15 +++++---- qutebrowser/utils/webelem.py | 59 +++++++++++++++--------------------- 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 047c3f9d8..c204346bc 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -244,7 +244,10 @@ class HintManager(QObject): else: target = self._target self.set_open_target.emit(Target[target]) - pos = webelem.pos_on_screen(elem) + # FIXME Instead of clicking the center, we could have nicer heuristics. + # e.g. parse (-webkit-)border-radius correctly and click text fields at + # the bottom right, and everything else on the top left or so. + pos = webelem.rect_on_screen(elem).center() logging.debug("Clicking on \"{}\" at {}/{}".format( elem.toPlainText(), pos.x(), pos.y())) events = [ @@ -346,12 +349,12 @@ class HintManager(QObject): return self.openurl.emit(link, newtab) - def start(self, frame, baseurl, group=webelem.Group.all, + def start(self, mainframe, baseurl, group=webelem.Group.all, target=Target.normal): """Start hinting. Args: - frame: The QWebFrame to place hints in. + mainframe: The main QWebFrame. baseurl: URL of the current page. group: Which group of elements to hint. target: What to do with the link. See attribute docstring. @@ -361,19 +364,19 @@ class HintManager(QObject): """ self._target = target self._baseurl = baseurl - if frame is None: + if mainframe is None: # This should never happen since we check frame before calling # start. But since we had a bug where frame is None in # on_mode_left, we are extra careful here. raise ValueError("start() was called with frame=None") - self._frames = webelem.get_child_frames(frame) + self._frames = webelem.get_child_frames(mainframe) elems = [] for f in self._frames: elems += f.findAllElements(webelem.SELECTORS[group]) filterfunc = webelem.FILTERS.get(group, lambda e: True) visible_elems = [] for e in elems: - if filterfunc(e) and webelem.is_visible(e): + if filterfunc(e) and webelem.is_visible(e, mainframe): visible_elems.append(e) if not visible_elems: message.error("No elements found.") diff --git a/qutebrowser/utils/webelem.py b/qutebrowser/utils/webelem.py index 978dfbc8a..2557a23e8 100644 --- a/qutebrowser/utils/webelem.py +++ b/qutebrowser/utils/webelem.py @@ -58,56 +58,47 @@ FILTERS = { } -def is_visible(elem): - """Check whether the element is currently visible in its frame. +def is_visible(elem, mainframe): + """Check whether the element is currently visible on the screen. Args: elem: The QWebElement to check. + mainframe: The main QWebFrame. Return: True if the element is visible, False otherwise. """ if elem.isNull(): raise ValueError("Element is a null-element!") - ## We're starting in the innermost (element) frame and check if every frame - ## is visible in its parent. - base = elem.webFrame() - parent = base.parentFrame() - while parent is not None: - parentgeom = parent.geometry() - parentgeom.translate(parent.scrollPosition()) - if not parentgeom.intersects(base.geometry()): - return False - base = parent - parent = parent.parentFrame() - rect = elem.geometry() - ## Now check if the element is visible in the frame. - if (not rect.isValid()) and rect.x() == 0: + if (not elem.geometry().isValid()) and elem.geometry().x() == 0: # Most likely an invisible link return False - frame = elem.webFrame() - framegeom = frame.geometry() - framegeom.moveTo(0, 0) - framegeom.translate(frame.scrollPosition()) - if framegeom.intersects(rect): - return True - return False + # First check if the element is visible on screen + elem_rect = rect_on_screen(elem) + visible_on_screen = mainframe.geometry().intersects(elem_rect) + # Then check if it's visible in its frame if it's not in the main frame. + elem_frame = elem.webFrame() + if elem_frame.parentFrame() is not None: + framegeom = elem_frame.geometry() + framegeom.moveTo(0, 0) + framegeom.translate(elem_frame.scrollPosition()) + visible_in_frame = framegeom.intersects(elem.geometry()) + else: + visible_in_frame = visible_on_screen + return all([visible_on_screen, visible_in_frame]) -def pos_on_screen(elem): - """Get the position of the element on the screen.""" - # FIXME Instead of clicking the center, we could have nicer heuristics. - # e.g. parse (-webkit-)border-radius correctly and click text fields at - # the bottom right, and everything else on the top left or so. +def rect_on_screen(elem): + """Get the geometry of the element relative to the screen.""" frame = elem.webFrame() - pos = elem.geometry().center() + rect = elem.geometry() while frame is not None: - pos += frame.geometry().topLeft() - logging.debug("After adding frame pos: {}".format(pos)) - pos -= frame.scrollPosition() - logging.debug("After removing frame scrollpos: {}".format(pos)) + rect.translate(frame.geometry().topLeft()) + logging.debug("After adding frame pos: {}".format(rect)) + rect.translate(frame.scrollPosition() * -1) + logging.debug("After removing frame scrollpos: {}".format(rect)) frame = frame.parentFrame() - return pos + return rect def javascript_escape(text):