diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 903752f3e..f8e408522 100644 --- a/CHANGELOG.asciidoc +++ b/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 diff --git a/README.asciidoc b/README.asciidoc index 4b9c2f1d0..f8fe1b31b 100644 --- a/README.asciidoc +++ b/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 diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 3c4cc84af..69252f106 100644 --- a/qutebrowser/browser/hints.py +++ b/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 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( - action, elem, pos.x(), pos.y())) + 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]: diff --git a/tests/integration/data/hints/html/nested_block_style.html b/tests/integration/data/hints/html/nested_block_style.html new file mode 100644 index 000000000..9ee1ff603 --- /dev/null +++ b/tests/integration/data/hints/html/nested_block_style.html @@ -0,0 +1,17 @@ + + + + + + + + Link containing an element with "display: block" style + + + + + link + + + + diff --git a/tests/integration/data/hints/html/nested_formatting_tags.html b/tests/integration/data/hints/html/nested_formatting_tags.html new file mode 100644 index 000000000..66c3d6e04 --- /dev/null +++ b/tests/integration/data/hints/html/nested_formatting_tags.html @@ -0,0 +1,20 @@ + + + + + + + + Link containing formatting tags + + + + link
+ link
+ link
+ link
+ link
+ link +
+ + diff --git a/tests/integration/data/hints/html/nested_table_style.html b/tests/integration/data/hints/html/nested_table_style.html new file mode 100644 index 000000000..a6a1a2b8f --- /dev/null +++ b/tests/integration/data/hints/html/nested_table_style.html @@ -0,0 +1,17 @@ + + + + + + + + Link containing an element with "display: table" style + + + + + link + + + + diff --git a/tests/integration/data/hints/html/wrapped.html b/tests/integration/data/hints/html/wrapped.html new file mode 100644 index 000000000..dcc05c8c7 --- /dev/null +++ b/tests/integration/data/hints/html/wrapped.html @@ -0,0 +1,15 @@ + + + + + + + + Link wrapped across multiple lines + + +
+ 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 id est laborum. +
+ + diff --git a/tests/integration/data/hints/html/zoom_precision.html b/tests/integration/data/hints/html/zoom_precision.html new file mode 100644 index 000000000..5b9b73f99 --- /dev/null +++ b/tests/integration/data/hints/html/zoom_precision.html @@ -0,0 +1,15 @@ + + + + + + + + Test hinting precision with different zoom levels + + +
+ 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 id est laborum. +
+ + diff --git a/tests/integration/data/hints/iframe.html b/tests/integration/data/hints/iframe.html new file mode 100644 index 000000000..269b689a2 --- /dev/null +++ b/tests/integration/data/hints/iframe.html @@ -0,0 +1,11 @@ + + + + + + Hinting inside an iframe + + + + + diff --git a/tests/integration/data/hints/iframe_scroll.html b/tests/integration/data/hints/iframe_scroll.html new file mode 100644 index 000000000..4c3f56453 --- /dev/null +++ b/tests/integration/data/hints/iframe_scroll.html @@ -0,0 +1,11 @@ + + + + + + Scrolling inside an iframe + + + + + diff --git a/tests/integration/data/hints/iframe_target.html b/tests/integration/data/hints/iframe_target.html new file mode 100644 index 000000000..db8afad27 --- /dev/null +++ b/tests/integration/data/hints/iframe_target.html @@ -0,0 +1,14 @@ + + + + + + Opening links in a specific iframe + + + +

+ A link to be opened in the iframe above. +

+ + diff --git a/tests/integration/data/hints/link.html b/tests/integration/data/hints/link.html deleted file mode 100644 index ec4f9f38c..000000000 --- a/tests/integration/data/hints/link.html +++ /dev/null @@ -1,10 +0,0 @@ - - - - - A link to use hints on - - - Follow me! - - diff --git a/tests/integration/features/hints.feature b/tests/integration/features/hints.feature index c2c057b60..4d4519fdd 100644 --- a/tests/integration/features/hints.feature +++ b/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 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) diff --git a/tests/integration/features/tabs.feature b/tests/integration/features/tabs.feature index f8c3941bb..53476bbb8 100644 --- a/tests/integration/features/tabs.feature +++ b/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) diff --git a/tests/integration/test_hints_html.py b/tests/integration/test_hints_html.py index 2a1566c1f..f8451c741 100644 --- a/tests/integration/test_hints_html.py +++ b/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):