Merge branch 'lahwaacz-hints_clicking'

CHANGELOG.asciidoc

@@ -57,6 +57,7 @@ Changed
 Fixed
 -----
 
+- Various fixes for hinting corner-cases where following a link didn't work
 - Fixed crash when downloading from an URL with SSL errors
 - Close file handles correctly when a download failed
 - Fixed crash when using `;Y` (`:hint links yank-primary`) on a system without

README.asciidoc

@@ -152,6 +152,7 @@ Contributors, sorted by the number of commits in descending order:
 * Claude
 * Corentin Julé
 * meles5
+* Jakub Klinkovský
 * Tarcisio Fedrizzi
 * Philipp Hansch
 * Panagiotis Ktistakis
@@ -172,7 +173,6 @@ Contributors, sorted by the number of commits in descending order:
 * Jonas Schürmann
 * error800
 * Liam BEGUIN
-* Jakub Klinkovský
 * skinnay
 * Zach-Button
 * Halfwit

qutebrowser/browser/hints.py

@@ -26,7 +26,7 @@ import re
 import string
 
 from PyQt5.QtCore import (pyqtSignal, pyqtSlot, QObject, QEvent, Qt, QUrl,
-                          QTimer)
+                          QTimer, QRect)
 from PyQt5.QtGui import QMouseEvent
 from PyQt5.QtWebKit import QWebElement
 from PyQt5.QtWebKitWidgets import QWebPage
@@ -422,6 +422,46 @@ class HintManager(QObject):
         message.error(self._win_id, "No suitable link found for this element.",
                       immediately=True)
 
+    def _get_first_rectangle(self, elem):
+        """Return the element's first client rectangle with positive size.
+
+        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
+
+        Args:
+            elem: The QWebElement of interest.
+        """
+        rects = elem.evaluateJavaScript("this.getClientRects()")
+        log.hints.debug("Client rectangles of element '{}': {}"
+                .format(elem.debug_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'):
+                    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
+        return elem.rect_on_view()
+
     def _click(self, elem, context):
         """Click an element.
 
@@ -441,14 +481,18 @@ class HintManager(QObject):
             target_mapping[Target.tab] = usertypes.ClickTarget.tab_bg
         else:
             target_mapping[Target.tab] = usertypes.ClickTarget.tab
+
         # 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.
         # https://github.com/The-Compiler/qutebrowser/issues/70
-        pos = elem.rect_on_view().center()
+        rect = self._get_first_rectangle(elem)
+        pos = rect.center()
+
         action = "Hovering" if context.target == Target.hover else "Clicking"
-        log.hints.debug("{} on '{}' at {}/{}".format(
+        log.hints.debug("{} on '{}' at position {}".format(
-            action, elem, pos.x(), pos.y()))
+            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]:

tests/integration/data/hints/html/nested_block_style.html

@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+
+<!-- target: hello.txt -->
+
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Link containing an element with "display: block" style</title>
+    </head>
+    <body>
+        <a href="/data/hello.txt">
+            <span style="display: block;">
+                link
+            </span>
+        </a>
+    </body>
+</html>

tests/integration/data/hints/html/nested_formatting_tags.html

@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+
+<!-- target: hello.txt -->
+
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Link containing formatting tags</title>
+    </head>
+    <body>
+        <a href="/data/hello.txt">
+            link<br>
+            <em>link</em><br>
+            <strong>link</strong><br>
+            <i>link</i><br>
+            <b>link</b><br>
+            link
+        </a>
+    </body>
+</html>

tests/integration/data/hints/html/nested_table_style.html

@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+
+<!-- target: hello.txt -->
+
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Link containing an element with "display: table" style</title>
+    </head>
+    <body>
+        <a href="/data/hello.txt">
+            <span style="display: table;">
+                link
+            </span>
+        </a>
+    </body>
+</html>

tests/integration/data/hints/html/wrapped.html

@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+
+<!-- target: hello.txt -->
+
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Link wrapped across multiple lines</title>
+    </head>
+    <body>
+        <div style="width: 20em;">
+            Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis <a href="/data/hello.txt">nostrud exercitation</a> ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
+        </div>
+    </body>
+</html>

tests/integration/data/hints/html/zoom_precision.html

@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+
+<!-- target: hello.txt -->
+
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Test hinting precision with different zoom levels</title>
+    </head>
+    <body>
+        <div style="margin-top: 120px;">
+            Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim <a href="/data/hello.txt">id</a> est laborum.
+        </div>
+    </body>
+</html>

tests/integration/data/hints/iframe.html

@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Hinting inside an iframe</title>
+    </head>
+    <body>
+        <iframe style="margin: 50px;" src="/data/hints/html/wrapped.html"></iframe>
+    </body>
+</html>

tests/integration/data/hints/iframe_scroll.html

@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Scrolling inside an iframe</title>
+    </head>
+    <body>
+        <iframe style="margin: 50px;" src="/data/scroll.html"></iframe>
+    </body>
+</html>

tests/integration/data/hints/iframe_target.html

@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Opening links in a specific iframe</title>
+    </head>
+    <body>
+        <iframe id="my-iframe"></iframe>
+        <p>
+            A <a href="/data/hello.txt" target="my-iframe">link</a> to be opened in the iframe above.
+        </p>
+    </body>
+</html>

tests/integration/data/hints/link.html

@@ -1,10 +0,0 @@
-<!DOCTYPE html>
-<html>
-    <head>
-        <meta charset="utf-8">
-        <title>A link to use hints on</title>
-    </head>
-    <body>
-        <a href="/data/hello.txt">Follow me!</a>
-    </body>
-</html>

tests/integration/features/hints.feature

@@ -1,24 +1,17 @@
 Feature: Using hints
 
-    Scenario: Following a hint.
-        When I open data/hints/link.html
-        And I run :hint links normal
-        And I run :follow-hint a
-        And I wait until data/hello.txt is loaded
-        Then the requests should be:
-            data/hints/link.html
-            data/hello.txt
-
     Scenario: Using :follow-hint outside of hint mode (issue 1105)
         When I run :follow-hint
         Then the error "follow-hint: This command is only allowed in hint mode." should be shown
 
     Scenario: Using :follow-hint with an invalid index.
-        When I open data/hints/link.html
+        When I open data/hints/html/simple.html
         And I run :hint links normal
         And I run :follow-hint xyz
         Then the error "No hint xyz!" should be shown
 
+    ### Opening in current or new tab
+
     Scenario: Following a hint and force to open in current tab.
         When I open data/hints/link_blank.html
         And I run :hint links current
@@ -46,14 +39,14 @@ Feature: Using hints
             - data/hello.txt (active)
 
     Scenario: Entering and leaving hinting mode (issue 1464)
-      When I open data/hints/link.html
+      When I open data/hints/html/simple.html
       And I run :hint
       And I run :fake-key -g <Esc>
       Then no crash should happen
 
     @xfail
     Scenario: Using :hint spawn with flags (issue 797)
-        When I open data/hints/link.html
+        When I open data/hints/html/simple.html
         And I run :hint all spawn -v echo
         And I run :follow-hint a
         Then the message "Command exited successfully" should be shown
@@ -61,7 +54,40 @@ Feature: Using hints
     Scenario: Yanking to primary selection without it being supported (#1336)
         When selection is not supported
         And I run :debug-set-fake-clipboard
-        And I open data/hints/link.html
+        And I open data/hints/html/simple.html
         And I run :hint links yank-primary
         And I run :follow-hint a
         Then the clipboard should contain "http://localhost:(port)/data/hello.txt"
+
+    ### iframes
+
+    Scenario: Using :follow-hint inside an iframe
+        When I open data/hints/iframe.html
+        And I run :hint all normal
+        And I run :follow-hint a
+        And I run :hint links normal
+        And I run :follow-hint a
+        Then "acceptNavigationRequest, url http://localhost:*/data/hello.txt, type NavigationTypeLinkClicked, *" should be logged
+
+    Scenario: Using :follow-hint inside a scrolled iframe
+        When I open data/hints/iframe_scroll.html
+        And I run :hint all normal
+        And I run :follow-hint a
+        And I run :scroll bottom
+        And I run :hint links normal
+        And I run :follow-hint a
+        Then "acceptNavigationRequest, url http://localhost:*/data/hello2.txt, type NavigationTypeLinkClicked, *" should be logged
+
+    Scenario: Opening a link inside a specific iframe
+        When I open data/hints/iframe_target.html
+        And I run :hint links normal
+        And I run :follow-hint a
+        Then "acceptNavigationRequest, url http://localhost:*/data/hello.txt, type NavigationTypeLinkClicked, *" should be logged
+
+    Scenario: Opening a link with specific target frame in a new tab
+        When I open data/hints/iframe_target.html
+        And I run :hint links tab
+        And I run :follow-hint a
+        Then the following tabs should be open:
+            - data/hints/iframe_target.html
+            - data/hello.txt (active)

tests/integration/features/tabs.feature

@@ -687,64 +687,64 @@ Feature: Tab management
 
     Scenario: opening links with tabs->background-tabs true
         When I set tabs -> background-tabs to true
-        And I open data/hints/link.html
+        And I open data/hints/html/simple.html
         And I run :hint all tab
         And I run :follow-hint a
         And I wait until data/hello.txt is loaded
         Then the following tabs should be open:
-            - data/hints/link.html (active)
+            - data/hints/html/simple.html (active)
             - data/hello.txt
 
     Scenario: opening tab with tabs->new-tab-position left
         When I set tabs -> new-tab-position to left
         And I set tabs -> background-tabs to false
         And I open about:blank
-        And I open data/hints/link.html in a new tab
+        And I open data/hints/html/simple.html in a new tab
         And I run :hint all tab
         And I run :follow-hint a
         And I wait until data/hello.txt is loaded
         Then the following tabs should be open:
             - about:blank
             - data/hello.txt (active)
-            - data/hints/link.html
+            - data/hints/html/simple.html
 
     Scenario: opening tab with tabs->new-tab-position right
         When I set tabs -> new-tab-position to right
         And I set tabs -> background-tabs to false
         And I open about:blank
-        And I open data/hints/link.html in a new tab
+        And I open data/hints/html/simple.html in a new tab
         And I run :hint all tab
         And I run :follow-hint a
         And I wait until data/hello.txt is loaded
         Then the following tabs should be open:
             - about:blank
-            - data/hints/link.html
+            - data/hints/html/simple.html
             - data/hello.txt (active)
 
     Scenario: opening tab with tabs->new-tab-position first
         When I set tabs -> new-tab-position to first
         And I set tabs -> background-tabs to false
         And I open about:blank
-        And I open data/hints/link.html in a new tab
+        And I open data/hints/html/simple.html in a new tab
         And I run :hint all tab
         And I run :follow-hint a
         And I wait until data/hello.txt is loaded
         Then the following tabs should be open:
             - data/hello.txt (active)
             - about:blank
-            - data/hints/link.html
+            - data/hints/html/simple.html
 
     Scenario: opening tab with tabs->new-tab-position last
         When I set tabs -> new-tab-position to last
         And I set tabs -> background-tabs to false
-        And I open data/hints/link.html
+        And I open data/hints/html/simple.html
         And I open about:blank in a new tab
         And I run :tab-focus last
         And I run :hint all tab
         And I run :follow-hint a
         And I wait until data/hello.txt is loaded
         Then the following tabs should be open:
-            - data/hints/link.html
+            - data/hints/html/simple.html
             - about:blank
             - data/hello.txt (active)
 

tests/integration/test_hints_html.py

@@ -36,7 +36,9 @@ def collect_tests():
 
 
 @pytest.mark.parametrize('test_name', collect_tests())
-def test_hints(test_name, quteproc):
+@pytest.mark.parametrize('zoom_text_only', [True, False])
+@pytest.mark.parametrize('zoom_level', [100, 66, 33])
+def test_hints(test_name, zoom_text_only, zoom_level, quteproc):
     file_path = os.path.join(os.path.abspath(os.path.dirname(__file__)),
                              'data', 'hints', 'html', test_name)
     url_path = 'data/hints/html/{}'.format(test_name)
@@ -51,10 +53,17 @@ def test_hints(test_name, quteproc):
 
     assert set(parsed.keys()) == {'target'}
 
+    # setup
+    quteproc.send_cmd(':set ui zoom-text-only {}'.format(zoom_text_only))
+    quteproc.send_cmd(':zoom {}'.format(zoom_level))
+    # follow hint
     quteproc.send_cmd(':hint links normal')
     quteproc.wait_for(message='hints: a', category='hints')
     quteproc.send_cmd(':follow-hint a')
     quteproc.wait_for_load_finished('data/' + parsed['target'])
+    # reset
+    quteproc.send_cmd(':zoom 100')
+    quteproc.send_cmd(':set ui zoom-text-only false')
 
 
 def test_word_hints_issue1393(quteproc, tmpdir):