From 65625a9dead9068f08315f7ff956797f844f6cbf Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 9 Nov 2016 21:49:36 +0100 Subject: [PATCH 01/27] webengine: Initial support for authentication and javascript prompts --- qutebrowser/browser/shared.py | 71 +++++++++++++++ qutebrowser/browser/webengine/webenginetab.py | 15 +++- qutebrowser/browser/webengine/webview.py | 47 ++++++++++ .../browser/webkit/network/networkmanager.py | 19 ++-- qutebrowser/browser/webkit/webpage.py | 62 +++++-------- tests/end2end/features/prompts.feature | 87 +++++++++++-------- tests/end2end/features/test_prompts_bdd.py | 3 - tests/end2end/fixtures/webserver.py | 2 +- 8 files changed, 215 insertions(+), 91 deletions(-) diff --git a/qutebrowser/browser/shared.py b/qutebrowser/browser/shared.py index ddc49e02e..fbd53327c 100644 --- a/qutebrowser/browser/shared.py +++ b/qutebrowser/browser/shared.py @@ -19,7 +19,15 @@ """Various utilities shared between webpage/webview subclasses.""" +import html + from qutebrowser.config import config +from qutebrowser.utils import usertypes, message, log + + +class CallSuper(Exception): + + """Raised when the caller should call the superclass instead.""" def custom_headers(): @@ -39,3 +47,66 @@ def custom_headers(): headers[b'Accept-Language'] = accept_language.encode('ascii') return sorted(headers.items()) + + +def authentication_required(url, authenticator, abort_on): + """Ask a prompt for an authentication question.""" + msg = '{} says:
{}'.format( + html.escape(url.toDisplayString()), + html.escape(authenticator.realm())) + answer = message.ask(title="Authentication required", text=msg, + mode=usertypes.PromptMode.user_pwd, + abort_on=abort_on) + if answer is not None: + authenticator.setUser(answer.user) + authenticator.setPassword(answer.password) + + +def javascript_confirm(url, js_msg, abort_on): + """Display a javascript confirm prompt.""" + log.js.debug("confirm: {}".format(js_msg)) + if config.get('ui', 'modal-js-dialog'): + raise CallSuper + + msg = 'From {}:
{}'.format(html.escape(url.toDisplayString()), + html.escape(js_msg)) + ans = message.ask('Javascript confirm', msg, + mode=usertypes.PromptMode.yesno, + abort_on=abort_on) + return bool(ans) + + +def javascript_prompt(url, js_msg, default, abort_on): + """Display a javascript prompt.""" + log.js.debug("prompt: {}".format(js_msg)) + if config.get('ui', 'modal-js-dialog'): + raise CallSuper + if config.get('content', 'ignore-javascript-prompt'): + return (False, "") + + msg = '{} asks:
{}'.format(html.escape(url.toDisplayString()), + html.escape(js_msg)) + answer = message.ask('Javascript prompt', msg, + mode=usertypes.PromptMode.text, + default=default, + abort_on=abort_on) + + if answer is None: + return (False, "") + else: + return (True, answer) + + +def javascript_alert(url, js_msg, abort_on): + """Display a javascript alert.""" + log.js.debug("alert: {}".format(js_msg)) + if config.get('ui', 'modal-js-dialog'): + raise CallSuper + + if config.get('content', 'ignore-javascript-alert'): + return + + msg = 'From {}:
{}'.format(html.escape(url.toDisplayString()), + html.escape(js_msg)) + message.ask('Javascript alert', msg, mode=usertypes.PromptMode.alert, + abort_on=abort_on) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index c66f3fe2e..95b570b38 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -22,6 +22,7 @@ """Wrapper over a QWebEngineView.""" +import html import functools from PyQt5.QtCore import pyqtSlot, Qt, QEvent, QPoint, QUrl, QTimer @@ -32,12 +33,12 @@ from PyQt5.QtWebEngineWidgets import (QWebEnginePage, QWebEngineScript, QWebEngineProfile) # pylint: enable=no-name-in-module,import-error,useless-suppression -from qutebrowser.browser import browsertab, mouse +from qutebrowser.browser import browsertab, mouse, shared from qutebrowser.browser.webengine import (webview, webengineelem, tabhistory, interceptor, webenginequtescheme, webenginedownloads) from qutebrowser.utils import (usertypes, qtutils, log, javascript, utils, - objreg) + objreg, message) _qute_scheme_handler = None @@ -538,7 +539,7 @@ class WebEngineTab(browsertab.AbstractTab): self._widget.page().runJavaScript(code, callback) def shutdown(self): - log.stub() + self._widget.shutdown() def reload(self, *, force=False): if force: @@ -590,6 +591,13 @@ class WebEngineTab(browsertab.AbstractTab): self.add_history_item.emit(url, requested_url, title) + @pyqtSlot(QUrl, 'QAuthenticator*') + def _on_authentication_required(self, url, authenticator): + # FIXME:qtwebengine support .netrc + shared.authentication_required(url, authenticator, + abort_on=[self.shutting_down, + self.load_started]) + def _connect_signals(self): view = self._widget page = view.page() @@ -603,6 +611,7 @@ class WebEngineTab(browsertab.AbstractTab): page.loadFinished.connect(self._on_load_finished) page.certificate_error.connect(self._on_ssl_errors) page.link_clicked.connect(self._on_link_clicked) + page.authenticationRequired.connect(self._on_authentication_required) try: view.iconChanged.connect(self.icon_changed) except AttributeError: diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index 18a7607a5..52ca6de97 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -26,6 +26,7 @@ from PyQt5.QtCore import pyqtSignal, QUrl from PyQt5.QtWebEngineWidgets import QWebEngineView, QWebEnginePage # pylint: enable=no-name-in-module,import-error,useless-suppression +from qutebrowser.browser import shared from qutebrowser.config import config from qutebrowser.utils import log, debug, usertypes, objreg, qtutils @@ -39,6 +40,9 @@ class WebEngineView(QWebEngineView): self._win_id = win_id self.setPage(WebEnginePage(tabdata, parent=self)) + def shutdown(self): + self.page().shutdown() + def createWindow(self, wintype): """Called by Qt when a page wants to create a new window. @@ -99,22 +103,65 @@ class WebEnginePage(QWebEnginePage): """Custom QWebEnginePage subclass with qutebrowser-specific features. + Attributes: + _is_shutting_down: Whether the page is currently shutting down. + Signals: certificate_error: FIXME:qtwebengine link_clicked: Emitted when a link was clicked on a page. + shutting_down: Emitted when the page is shutting down. """ certificate_error = pyqtSignal() link_clicked = pyqtSignal(QUrl) + shutting_down = pyqtSignal() def __init__(self, tabdata, parent=None): super().__init__(parent) self._tabdata = tabdata + self._is_shutting_down = False + + def shutdown(self): + self._is_shutting_down = True + self.shutting_down.emit() def certificateError(self, error): self.certificate_error.emit() return super().certificateError(error) + def javaScriptConfirm(self, url, js_msg): + if self._is_shutting_down: + return False + try: + return shared.javascript_confirm(url, js_msg, + abort_on=[self.loadStarted, + self.shutting_down]) + except shared.CallSuper: + return super().javaScriptConfirm(url, js_msg) + + # Can't override javaScriptPrompt currently + # https://www.riverbankcomputing.com/pipermail/pyqt/2016-November/038293.html + # def javaScriptPrompt(self, url, js_msg, default, result): + # if self._is_shutting_down: + # return (False, "") + # try: + # return shared.javascript_prompt(url, js_msg, default, + # abort_on=[self.loadStarted, + # self.shutting_down]) + # except shared.CallSuper: + # return super().javaScriptPrompt(url, js_msg, default) + + def javaScriptAlert(self, url, js_msg): + """Override javaScriptAlert to use the statusbar.""" + if self._is_shutting_down: + return + try: + shared.javascript_alert(url, js_msg, + abort_on=[self.loadStarted, + self.shutting_down]) + except shared.CallSuper: + super().javaScriptAlert(url, js_msg) + def javaScriptConsoleMessage(self, level, msg, line, source): """Log javascript messages to qutebrowser's log.""" # FIXME:qtwebengine maybe unify this in the tab api somehow? diff --git a/qutebrowser/browser/webkit/network/networkmanager.py b/qutebrowser/browser/webkit/network/networkmanager.py index 81ce0eb79..21f4f48bd 100644 --- a/qutebrowser/browser/webkit/network/networkmanager.py +++ b/qutebrowser/browser/webkit/network/networkmanager.py @@ -343,19 +343,18 @@ class NetworkManager(QNetworkAccessManager): except netrc.NetrcParseError: log.misc.exception("Error when parsing the netrc file") - if user is None: - # netrc check failed - msg = '{} says:
{}'.format( - html.escape(reply.url().toDisplayString()), - html.escape(authenticator.realm())) - answer = self._ask("Authentication required", - text=msg, mode=usertypes.PromptMode.user_pwd, - owner=reply) - if answer is not None: - user, password = answer.user, answer.password if user is not None: authenticator.setUser(user) authenticator.setPassword(password) + else: + abort_on = [self.shutting_down, reply.destroyed] + if self._tab_id is not None: + tab = objreg.get('tab', scope='tab', window=self._win_id, + tab=self._tab_id) + abort_on.append(tab.load_started) + + shared.authentication_required(reply.url(), authenticator, + abort_on=abort_on) @pyqtSlot('QNetworkProxy', 'QAuthenticator*') def on_proxy_authentication_required(self, proxy, authenticator): diff --git a/qutebrowser/browser/webkit/webpage.py b/qutebrowser/browser/webkit/webpage.py index 09feed3d5..c72e01faa 100644 --- a/qutebrowser/browser/webkit/webpage.py +++ b/qutebrowser/browser/webkit/webpage.py @@ -30,7 +30,7 @@ from PyQt5.QtPrintSupport import QPrintDialog from PyQt5.QtWebKitWidgets import QWebPage, QWebFrame from qutebrowser.config import config -from qutebrowser.browser import pdfjs +from qutebrowser.browser import pdfjs, shared from qutebrowser.browser.webkit import http from qutebrowser.browser.webkit.network import networkmanager from qutebrowser.utils import (message, usertypes, log, jinja, qtutils, utils, @@ -94,23 +94,16 @@ class BrowserPage(QWebPage): # of a bug in PyQt. # See http://www.riverbankcomputing.com/pipermail/pyqt/2014-June/034385.html - def javaScriptPrompt(self, _frame, js_msg, default): + def javaScriptPrompt(self, frame, js_msg, default): """Override javaScriptPrompt to use the statusbar.""" - if (self._is_shutting_down or - config.get('content', 'ignore-javascript-prompt')): + if self._is_shutting_down: return (False, "") - msg = '{} asks:
{}'.format( - html.escape(self.mainFrame().url().toDisplayString()), - html.escape(js_msg)) - answer = message.ask('Javascript prompt', msg, - mode=usertypes.PromptMode.text, - default=default, - abort_on=[self.loadStarted, - self.shutting_down]) - if answer is None: - return (False, "") - else: - return (True, answer) + try: + return shared.javascript_prompt(frame.url(), js_msg, default, + abort_on=[self.loadStarted, + self.shutting_down]) + except shared.CallSuper: + return super().javaScriptPrompt(frame, js_msg, default) def _handle_errorpage(self, info, errpage): """Display an error page if needed. @@ -442,36 +435,25 @@ class BrowserPage(QWebPage): def javaScriptAlert(self, frame, js_msg): """Override javaScriptAlert to use the statusbar.""" - log.js.debug("alert: {}".format(js_msg)) - if config.get('ui', 'modal-js-dialog'): - return super().javaScriptAlert(frame, js_msg) - - if (self._is_shutting_down or - config.get('content', 'ignore-javascript-alert')): + if self._is_shutting_down: return - - msg = 'From {}:
{}'.format( - html.escape(self.mainFrame().url().toDisplayString()), - html.escape(js_msg)) - message.ask('Javascript alert', msg, mode=usertypes.PromptMode.alert, - abort_on=[self.loadStarted, self.shutting_down]) + try: + shared.javascript_alert(frame.url(), js_msg, + abort_on=[self.loadStarted, + self.shutting_down]) + except shared.CallSuper: + super().javaScriptAlert(frame, js_msg) def javaScriptConfirm(self, frame, js_msg): """Override javaScriptConfirm to use the statusbar.""" - log.js.debug("confirm: {}".format(js_msg)) - if config.get('ui', 'modal-js-dialog'): - return super().javaScriptConfirm(frame, js_msg) - if self._is_shutting_down: return False - - msg = 'From {}:
{}'.format( - html.escape(self.mainFrame().url().toDisplayString()), - html.escape(js_msg)) - ans = message.ask('Javascript confirm', msg, - mode=usertypes.PromptMode.yesno, - abort_on=[self.loadStarted, self.shutting_down]) - return bool(ans) + try: + return shared.javascript_confirm(frame.url(), js_msg, + abort_on=[self.loadStarted, + self.shutting_down]) + except shared.CallSuper: + return super().javaScriptConfirm(frame, js_msg) def javaScriptConsoleMessage(self, msg, line, source): """Override javaScriptConsoleMessage to use debug log.""" diff --git a/tests/end2end/features/prompts.feature b/tests/end2end/features/prompts.feature index fe39a90be..12497857b 100644 --- a/tests/end2end/features/prompts.feature +++ b/tests/end2end/features/prompts.feature @@ -40,7 +40,7 @@ Feature: Prompts And I run :leave-mode Then the javascript message "confirm reply: false" should be logged - @pyqt>=5.3.1 + @pyqt>=5.3.1 @qtwebengine_skip Scenario: Javascript prompt When I open data/prompt/jsprompt.html And I run :click-element id button @@ -49,7 +49,7 @@ Feature: Prompts And I run :prompt-accept Then the javascript message "Prompt reply: prompt test" should be logged - @pyqt>=5.3.1 + @pyqt>=5.3.1 @qtwebengine_skip Scenario: Javascript prompt with default When I open data/prompt/jsprompt.html And I run :click-element id button-default @@ -57,7 +57,7 @@ Feature: Prompts And I run :prompt-accept Then the javascript message "Prompt reply: default" should be logged - @pyqt>=5.3.1 + @pyqt>=5.3.1 @qtwebengine_skip Scenario: Rejected javascript prompt When I open data/prompt/jsprompt.html And I run :click-element id button @@ -68,6 +68,7 @@ Feature: Prompts # Multiple prompts + @qtwebengine_skip: QtWebEngine refuses to load anything with a JS question Scenario: Blocking question interrupted by blocking one When I set content -> ignore-javascript-alert to false And I open data/prompt/jsalert.html @@ -83,6 +84,7 @@ Feature: Prompts Then the javascript message "confirm reply: true" should be logged And the javascript message "Alert done" should be logged + @qtwebengine_skip: QtWebEngine refuses to load anything with a JS question Scenario: Blocking question interrupted by async one When I set content -> ignore-javascript-alert to false And I set content -> notifications to ask @@ -99,6 +101,7 @@ Feature: Prompts Then the javascript message "Alert done" should be logged And the javascript message "notification permission granted" should be logged + @qtwebengine_todo: Permissions are not implemented yet Scenario: Async question interrupted by async one When I set content -> notifications to ask And I open data/prompt/notifications.html in a new tab @@ -113,6 +116,7 @@ Feature: Prompts Then the javascript message "notification permission granted" should be logged And "Added quickmark test for *" should be logged + @qtwebengine_todo: Permissions are not implemented yet Scenario: Async question interrupted by blocking one When I set content -> notifications to ask And I set content -> ignore-javascript-alert to false @@ -131,7 +135,7 @@ Feature: Prompts # Shift-Insert with prompt (issue 1299) - @pyqt>=5.3.1 + @pyqt>=5.3.1 @qtwebengine_skip Scenario: Pasting via shift-insert in prompt mode When selection is supported And I put "insert test" into the primary selection @@ -142,7 +146,7 @@ Feature: Prompts And I run :prompt-accept Then the javascript message "Prompt reply: insert test" should be logged - @pyqt>=5.3.1 + @pyqt>=5.3.1 @qtwebengine_skip Scenario: Pasting via shift-insert without it being supported When selection is not supported And I put "insert test" into the primary selection @@ -153,7 +157,7 @@ Feature: Prompts And I run :prompt-accept Then the javascript message "Prompt reply: " should be logged - @pyqt>=5.3.1 + @pyqt>=5.3.1 @qtwebengine_skip Scenario: Using content -> ignore-javascript-prompt When I set content -> ignore-javascript-prompt to true And I open data/prompt/jsprompt.html @@ -162,6 +166,7 @@ Feature: Prompts # SSL + @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = false When I run :debug-clear-ssl-errors And I set network -> ssl-strict to false @@ -170,6 +175,7 @@ Feature: Prompts Then the error "SSL error: *" should be shown And the page should contain the plaintext "Hello World via SSL!" + @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = true When I run :debug-clear-ssl-errors And I set network -> ssl-strict to true @@ -177,6 +183,7 @@ Feature: Prompts Then "Error while loading *: SSL handshake failed" should be logged And the page should contain the plaintext "Unable to load page" + @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = ask -> yes When I run :debug-clear-ssl-errors And I set network -> ssl-strict to ask @@ -186,6 +193,7 @@ Feature: Prompts And I wait until the SSL page finished loading Then the page should contain the plaintext "Hello World via SSL!" + @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = ask -> no When I run :debug-clear-ssl-errors And I set network -> ssl-strict to ask @@ -197,20 +205,21 @@ Feature: Prompts # Geolocation + @qtwebengine_todo: Permissions are not implemented yet Scenario: Always rejecting geolocation When I set content -> geolocation to false And I open data/prompt/geolocation.html in a new tab And I run :click-element id button Then the javascript message "geolocation permission denied" should be logged - @ci @not_osx + @ci @not_osx @qtwebengine_todo: Permissions are not implemented yet Scenario: Always accepting geolocation When I set content -> geolocation to true And I open data/prompt/geolocation.html in a new tab And I run :click-element id button Then the javascript message "geolocation permission denied" should not be logged - @ci @not_osx + @ci @not_osx @qtwebengine_todo: Permissions are not implemented yet Scenario: geolocation with ask -> true When I set content -> geolocation to ask And I open data/prompt/geolocation.html in a new tab @@ -219,6 +228,7 @@ Feature: Prompts And I run :prompt-accept yes Then the javascript message "geolocation permission denied" should not be logged + @qtwebengine_todo: Permissions are not implemented yet Scenario: geolocation with ask -> false When I set content -> geolocation to ask And I open data/prompt/geolocation.html in a new tab @@ -227,6 +237,7 @@ Feature: Prompts And I run :prompt-accept no Then the javascript message "geolocation permission denied" should be logged + @qtwebengine_todo: Permissions are not implemented yet Scenario: geolocation with ask -> abort When I set content -> geolocation to ask And I open data/prompt/geolocation.html in a new tab @@ -237,18 +248,21 @@ Feature: Prompts # Notifications + @qtwebengine_todo: Permissions are not implemented yet Scenario: Always rejecting notifications When I set content -> notifications to false And I open data/prompt/notifications.html in a new tab And I run :click-element id button Then the javascript message "notification permission denied" should be logged + @qtwebengine_todo: Permissions are not implemented yet Scenario: Always accepting notifications When I set content -> notifications to true And I open data/prompt/notifications.html in a new tab And I run :click-element id button Then the javascript message "notification permission granted" should be logged + @qtwebengine_todo: Permissions are not implemented yet Scenario: notifications with ask -> false When I set content -> notifications to ask And I open data/prompt/notifications.html in a new tab @@ -257,6 +271,7 @@ Feature: Prompts And I run :prompt-accept no Then the javascript message "notification permission denied" should be logged + @qtwebengine_todo: Permissions are not implemented yet Scenario: notifications with ask -> true When I set content -> notifications to ask And I open data/prompt/notifications.html in a new tab @@ -275,6 +290,7 @@ Feature: Prompts And I run :leave-mode Then the javascript message "notification permission aborted" should be logged + @qtwebengine_todo: Permissions are not implemented yet Scenario: answering notification after closing tab When I set content -> notifications to ask And I open data/prompt/notifications.html in a new tab @@ -287,55 +303,55 @@ Feature: Prompts # Page authentication Scenario: Successful webpage authentification - When I open basic-auth/user/password without waiting + When I open basic-auth/user1/password1 without waiting And I wait for a prompt - And I press the keys "user" + And I press the keys "user1" And I run :prompt-accept - And I press the keys "password" + And I press the keys "password1" And I run :prompt-accept - And I wait until basic-auth/user/password is loaded + And I wait until basic-auth/user1/password1 is loaded Then the json on the page should be: { "authenticated": true, - "user": "user" + "user": "user1" } Scenario: Authentication with :prompt-accept value When I open about:blank in a new tab - And I open basic-auth/user/password without waiting + And I open basic-auth/user2/password2 without waiting And I wait for a prompt - And I run :prompt-accept user:password - And I wait until basic-auth/user/password is loaded + And I run :prompt-accept user2:password2 + And I wait until basic-auth/user2/password2 is loaded Then the json on the page should be: { "authenticated": true, - "user": "user" + "user": "user2" } Scenario: Authentication with invalid :prompt-accept value When I open about:blank in a new tab - And I open basic-auth/user/password without waiting + And I open basic-auth/user3/password3 without waiting And I wait for a prompt And I run :prompt-accept foo - And I run :prompt-accept user:password + And I run :prompt-accept user3:password3 Then the error "Value needs to be in the format username:password, but foo was given" should be shown Scenario: Tabbing between username and password When I open about:blank in a new tab - And I open basic-auth/user/password without waiting + And I open basic-auth/user4/password4 without waiting And I wait for a prompt And I press the keys "us" And I run :prompt-item-focus next - And I press the keys "password" + And I press the keys "password4" And I run :prompt-item-focus prev - And I press the keys "er" + And I press the keys "er4" And I run :prompt-accept And I run :prompt-accept - And I wait until basic-auth/user/password is loaded + And I wait until basic-auth/user4/password4 is loaded Then the json on the page should be: { "authenticated": true, - "user": "user" + "user": "user4" } # :prompt-accept with value argument @@ -350,7 +366,7 @@ Feature: Prompts Then the javascript message "Alert done" should be logged And the error "No value is permitted with alert prompts!" should be shown - @pyqt>=5.3.1 + @pyqt>=5.3.1 @qtwebengine_skip Scenario: Javascript prompt with value When I set content -> ignore-javascript-prompt to false And I open data/prompt/jsprompt.html @@ -396,6 +412,7 @@ Feature: Prompts # Other + @qtwebengine_skip Scenario: Shutting down with a question When I open data/prompt/jsconfirm.html And I run :click-element id button @@ -429,32 +446,34 @@ Feature: Prompts Then "Added quickmark prompt-in-command-mode for *" should be logged # https://github.com/The-Compiler/qutebrowser/issues/1093 + @qtwebengine_skip: QtWebEngine doesn't open the second page/prompt Scenario: Keyboard focus with multiple auth prompts - When I open basic-auth/user1/password1 without waiting - And I open basic-auth/user2/password2 in a new tab without waiting + When I open basic-auth/user5/password5 without waiting + And I open basic-auth/user6/password6 in a new tab without waiting And I wait for a prompt And I wait for a prompt # Second prompt (showed first) - And I press the keys "user2" + And I press the keys "user6" And I press the key "" - And I press the keys "password2" + And I press the keys "password6" And I press the key "" - And I wait until basic-auth/user2/password2 is loaded + And I wait until basic-auth/user6/password6 is loaded # First prompt - And I press the keys "user1" + And I press the keys "user5" And I press the key "" - And I press the keys "password1" + And I press the keys "password5" And I press the key "" - And I wait until basic-auth/user1/password1 is loaded + And I wait until basic-auth/user5/password5 is loaded # We're on the second page Then the json on the page should be: { "authenticated": true, - "user": "user2" + "user": "user6" } # https://github.com/The-Compiler/qutebrowser/issues/1249#issuecomment-175205531 # https://github.com/The-Compiler/qutebrowser/pull/2054#issuecomment-258285544 + @qtwebengine_todo: SSL errors are not implemented yet Scenario: Interrupting SSL prompt during a notification prompt When I set content -> notifications to ask And I set network -> ssl-strict to ask diff --git a/tests/end2end/features/test_prompts_bdd.py b/tests/end2end/features/test_prompts_bdd.py index 8a66880b7..b5530bbea 100644 --- a/tests/end2end/features/test_prompts_bdd.py +++ b/tests/end2end/features/test_prompts_bdd.py @@ -22,9 +22,6 @@ import pytest_bdd as bdd bdd.scenarios('prompts.feature') -pytestmark = pytest.mark.qtwebengine_todo("Prompts are not implemented") - - @bdd.when("I load an SSL page") def load_ssl_page(quteproc, ssl_server): # We don't wait here as we can get an SSL question. diff --git a/tests/end2end/fixtures/webserver.py b/tests/end2end/fixtures/webserver.py index 2c5d23e65..f6e0f8ff2 100644 --- a/tests/end2end/fixtures/webserver.py +++ b/tests/end2end/fixtures/webserver.py @@ -81,7 +81,7 @@ class Request(testprocess.Line): http.client.FOUND] path_to_statuses['/absolute-redirect/{}'.format(i)] = [ http.client.FOUND] - for suffix in ['', '1', '2']: + for suffix in ['', '1', '2', '3', '4', '5', '6']: key = '/basic-auth/user{}/password{}'.format(suffix, suffix) path_to_statuses[key] = [http.client.UNAUTHORIZED, http.client.OK] From 6697d692e1580953aa0531c1a13a5f3d496c9af6 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 06:50:00 +0100 Subject: [PATCH 02/27] webengine: Initial SSL error support --- doc/help/settings.asciidoc | 2 - qutebrowser/browser/browsertab.py | 17 +++ qutebrowser/browser/shared.py | 47 ++++++++ qutebrowser/browser/webengine/webenginetab.py | 25 ++++- qutebrowser/browser/webengine/webview.py | 29 ++++- .../browser/webkit/network/networkmanager.py | 103 +++++------------- qutebrowser/browser/webkit/webkittab.py | 33 +++++- qutebrowser/config/configdata.py | 3 +- tests/end2end/features/prompts.feature | 7 +- tests/end2end/fixtures/quteprocess.py | 22 ++-- 10 files changed, 191 insertions(+), 97 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 9341676f5..159d173cb 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -797,8 +797,6 @@ Valid values: Default: +pass:[ask]+ -This setting is only available with the QtWebKit backend. - [[network-dns-prefetch]] === dns-prefetch Whether to try to pre-fetch DNS entries to speed up browsing. diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 22f1c7e51..2589154a5 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -75,6 +75,23 @@ class UnsupportedOperationError(WebTabError): """Raised when an operation is not supported with the given backend.""" +class AbstractCertificateErrorWrapper: + + """A wrapper over an SSL/certificate error.""" + + def __init__(self, error): + self._error = error + + def __str__(self): + raise NotImplementedError + + def __repr__(self): + raise NotImplementedError + + def is_overridable(self): + raise NotImplementedError + + class TabData: """A simple namespace with a fixed set of attributes. diff --git a/qutebrowser/browser/shared.py b/qutebrowser/browser/shared.py index fbd53327c..417b741c6 100644 --- a/qutebrowser/browser/shared.py +++ b/qutebrowser/browser/shared.py @@ -21,6 +21,8 @@ import html +import jinja2 + from qutebrowser.config import config from qutebrowser.utils import usertypes, message, log @@ -110,3 +112,48 @@ def javascript_alert(url, js_msg, abort_on): html.escape(js_msg)) message.ask('Javascript alert', msg, mode=usertypes.PromptMode.alert, abort_on=abort_on) + + +def ignore_certificate_errors(url, errors, abort_on): + """Display a certificate error question. + + Args: + url: The URL the errors happened in + errors: A list of QSslErrors or QWebEngineCertificateErrors + + Return: + True if the error should be ignored, False otherwise. + """ + ssl_strict = config.get('network', 'ssl-strict') + log.webview.debug("Certificate errors {!r}, strict {}".format( + errors, ssl_strict)) + + for error in errors: + assert error.is_overridable(), repr(error) + + if ssl_strict == 'ask': + err_template = jinja2.Template(""" + Errors while loading {{url.toDisplayString()}}:
+
    + {% for err in errors %} +
  • {{err}}
    • + {% endfor %} +
+ """.strip()) + msg = err_template.render(url=url, errors=errors) + + return message.ask(title="Certificate errors - continue?", text=msg, + mode=usertypes.PromptMode.yesno, default=False, + abort_on=abort_on) + elif ssl_strict is False: + log.webview.debug("ssl-strict is False, only warning about errors") + for err in errors: + # FIXME we might want to use warn here (non-fatal error) + # https://github.com/The-Compiler/qutebrowser/issues/114 + message.error('Certificate error: {}'.format(err)) + return True + elif ssl_strict is True: + return False + else: + raise ValueError("Invalid ssl_strict value {!r}".format(ssl_strict)) + raise AssertionError("Not reached") diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 95b570b38..9ec89836a 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -30,7 +30,8 @@ from PyQt5.QtGui import QKeyEvent, QIcon # pylint: disable=no-name-in-module,import-error,useless-suppression from PyQt5.QtWidgets import QOpenGLWidget, QApplication from PyQt5.QtWebEngineWidgets import (QWebEnginePage, QWebEngineScript, - QWebEngineProfile) + QWebEngineProfile, + QWebEngineCertificateError) # pylint: enable=no-name-in-module,import-error,useless-suppression from qutebrowser.browser import browsertab, mouse, shared @@ -38,7 +39,7 @@ from qutebrowser.browser.webengine import (webview, webengineelem, tabhistory, interceptor, webenginequtescheme, webenginedownloads) from qutebrowser.utils import (usertypes, qtutils, log, javascript, utils, - objreg, message) + objreg, message, debug) _qute_scheme_handler = None @@ -78,6 +79,26 @@ _JS_WORLD_MAP = { } +class CertificateErrorWrapper(browsertab.AbstractCertificateErrorWrapper): + + """A wrapper over a QWebEngineCertificateError.""" + + def __init__(self, error): + self._error = error + + def __str__(self): + return self._error.errorDescription() + + def __repr__(self): + return utils.get_repr( + self, error=debug.qenum_key(QWebEngineCertificateError, + self._error.error()), + string=str(self)) + + def is_overridable(self): + return self._error.isOverridable() + + class WebEnginePrinting(browsertab.AbstractPrinting): """QtWebEngine implementations related to printing.""" diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index 52ca6de97..c8488bb4a 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -27,8 +27,9 @@ from PyQt5.QtWebEngineWidgets import QWebEngineView, QWebEnginePage # pylint: enable=no-name-in-module,import-error,useless-suppression from qutebrowser.browser import shared +from qutebrowser.browser.webengine import webenginetab from qutebrowser.config import config -from qutebrowser.utils import log, debug, usertypes, objreg, qtutils +from qutebrowser.utils import log, debug, usertypes, objreg, qtutils, jinja class WebEngineView(QWebEngineView): @@ -107,7 +108,7 @@ class WebEnginePage(QWebEnginePage): _is_shutting_down: Whether the page is currently shutting down. Signals: - certificate_error: FIXME:qtwebengine + certificate_error: Emitted on certificate errors. link_clicked: Emitted when a link was clicked on a page. shutting_down: Emitted when the page is shutting down. """ @@ -127,7 +128,29 @@ class WebEnginePage(QWebEnginePage): def certificateError(self, error): self.certificate_error.emit() - return super().certificateError(error) + url = error.url() + error = webenginetab.CertificateErrorWrapper(error) + + # FIXME + error_page = jinja.render('error.html', + title="Error while loading page", + url=url.toDisplayString(), error=str(error), + icon='', qutescheme=False) + + if not error.is_overridable(): + log.webview.error("Non-overridable certificate error: " + "{}".format(error)) + self.setHtml(error_page) + return False + + ignore = shared.ignore_certificate_errors( + url, [error], abort_on=[self.loadStarted, self.shutting_down]) + + if not ignore: + log.webview.error("Certificate error: {}".format(error)) + self.setHtml(error_page) + + return ignore def javaScriptConfirm(self, url, js_msg): if self._is_shutting_down: diff --git a/qutebrowser/browser/webkit/network/networkmanager.py b/qutebrowser/browser/webkit/network/networkmanager.py index 21f4f48bd..d4655d04a 100644 --- a/qutebrowser/browser/webkit/network/networkmanager.py +++ b/qutebrowser/browser/webkit/network/networkmanager.py @@ -24,7 +24,6 @@ import collections import netrc import html -import jinja2 from PyQt5.QtCore import (pyqtSlot, pyqtSignal, PYQT_VERSION, QCoreApplication, QUrl, QByteArray) from PyQt5.QtNetwork import (QNetworkAccessManager, QNetworkReply, QSslError, @@ -34,8 +33,9 @@ from qutebrowser.config import config from qutebrowser.utils import (message, log, usertypes, utils, objreg, qtutils, urlutils, debug) from qutebrowser.browser import shared -from qutebrowser.browser.webkit.network import webkitqutescheme, networkreply -from qutebrowser.browser.webkit.network import filescheme +from qutebrowser.browser.webkit import webkittab +from qutebrowser.browser.webkit.network import (webkitqutescheme, networkreply, + filescheme) HOSTBLOCK_ERROR_STRING = '%HOSTBLOCK%' @@ -112,24 +112,6 @@ def init(): QSslSocket.setDefaultCiphers(good_ciphers) -class SslError(QSslError): - - """A QSslError subclass which provides __hash__ on Qt < 5.4.""" - - def __hash__(self): - try: - # Qt >= 5.4 - # pylint: disable=not-callable,useless-suppression - return super().__hash__() - except TypeError: - return hash((self.certificate().toDer(), self.error())) - - def __repr__(self): - return utils.get_repr( - self, error=debug.qenum_key(QSslError, self.error()), - string=self.errorString()) - - class NetworkManager(QNetworkAccessManager): """Our own QNetworkAccessManager. @@ -203,6 +185,19 @@ class NetworkManager(QNetworkAccessManager): self.setCache(cache) cache.setParent(app) + def _get_abort_signals(self, owner=None): + """Get a list of signals which should abort a question.""" + abort_on = [self.shutting_down] + if owner is not None: + abort_on.append(owner.destroyed) + # This might be a generic network manager, e.g. one belonging to a + # DownloadManager. In this case, just skip the webview thing. + if self._tab_id is not None: + tab = objreg.get('tab', scope='tab', window=self._win_id, + tab=self._tab_id) + abort_on.append(tab.load_started) + return abort_on + def _ask(self, title, text, mode, owner=None, default=None): """Ask a blocking question in the statusbar. @@ -216,17 +211,7 @@ class NetworkManager(QNetworkAccessManager): Return: The answer the user gave or None if the prompt was cancelled. """ - abort_on = [self.shutting_down] - if owner is not None: - abort_on.append(owner.destroyed) - - # This might be a generic network manager, e.g. one belonging to a - # DownloadManager. In this case, just skip the webview thing. - if self._tab_id is not None: - tab = objreg.get('tab', scope='tab', window=self._win_id, - tab=self._tab_id) - abort_on.append(tab.load_started) - + abort_on = self._get_abort_signals(owner) return message.ask(title=title, text=text, mode=mode, abort_on=abort_on, default=default) @@ -248,11 +233,8 @@ class NetworkManager(QNetworkAccessManager): reply: The QNetworkReply that is encountering the errors. errors: A list of errors. """ - errors = [SslError(e) for e in errors] - ssl_strict = config.get('network', 'ssl-strict') - log.webview.debug("SSL errors {!r}, strict {}".format( - errors, ssl_strict)) - + errors = [webkittab.CertificateErrorWrapper(e) for e in errors] + log.webview.debug("Certificate errors {!r}".format(errors)) try: host_tpl = urlutils.host_tuple(reply.url()) except ValueError: @@ -268,42 +250,22 @@ class NetworkManager(QNetworkAccessManager): log.webview.debug("Already accepted: {} / " "rejected {}".format(is_accepted, is_rejected)) - if (ssl_strict and ssl_strict != 'ask') or is_rejected: + if is_rejected: return elif is_accepted: reply.ignoreSslErrors() return - if ssl_strict == 'ask': - err_template = jinja2.Template(""" - Errors while loading {{url.toDisplayString()}}:
-
    - {% for err in errors %} -
  • {{err.errorString()}}
    • - {% endfor %} -
- """.strip()) - msg = err_template.render(url=reply.url(), errors=errors) - - answer = self._ask('SSL errors - continue?', msg, - mode=usertypes.PromptMode.yesno, owner=reply, - default=False) - log.webview.debug("Asked for SSL errors, answer {}".format(answer)) - if answer: - reply.ignoreSslErrors() - err_dict = self._accepted_ssl_errors - else: - err_dict = self._rejected_ssl_errors - if host_tpl is not None: - err_dict[host_tpl] += errors - else: - log.webview.debug("ssl-strict is False, only warning about errors") - for err in errors: - # FIXME we might want to use warn here (non-fatal error) - # https://github.com/The-Compiler/qutebrowser/issues/114 - message.error('SSL error: {}'.format(err.errorString())) + abort_on = self._get_abort_signals(reply) + ignore = shared.ignore_certificate_errors(reply.url(), errors, + abort_on=abort_on) + if ignore: reply.ignoreSslErrors() - self._accepted_ssl_errors[host_tpl] += errors + err_dict = self._accepted_ssl_errors + else: + err_dict = self._rejected_ssl_errors + if host_tpl is not None: + err_dict[host_tpl] += errors def clear_all_ssl_errors(self): """Clear all remembered SSL errors.""" @@ -347,12 +309,7 @@ class NetworkManager(QNetworkAccessManager): authenticator.setUser(user) authenticator.setPassword(password) else: - abort_on = [self.shutting_down, reply.destroyed] - if self._tab_id is not None: - tab = objreg.get('tab', scope='tab', window=self._win_id, - tab=self._tab_id) - abort_on.append(tab.load_started) - + abort_on = self._get_abort_signals(reply) shared.authentication_required(reply.url(), authenticator, abort_on=abort_on) diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 61de1bd54..a2bbb2b9a 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -30,11 +30,12 @@ from PyQt5.QtWidgets import QApplication from PyQt5.QtWebKitWidgets import QWebPage, QWebFrame from PyQt5.QtWebKit import QWebSettings from PyQt5.QtPrintSupport import QPrinter +from PyQt5.QtNetwork import QSslError from qutebrowser.browser import browsertab from qutebrowser.browser.webkit import webview, tabhistory, webkitelem from qutebrowser.browser.webkit.network import proxy, webkitqutescheme -from qutebrowser.utils import qtutils, objreg, usertypes, utils, log +from qutebrowser.utils import qtutils, objreg, usertypes, utils, log, debug def init(): @@ -49,6 +50,36 @@ def init(): objreg.register('js-bridge', js_bridge) +class CertificateErrorWrapper(browsertab.AbstractCertificateErrorWrapper): + + """A wrapper over a QSslError.""" + + def __init__(self, error): + self._error = error + + def __str__(self): + return self._error.errorString() + + def __repr__(self): + return utils.get_repr( + self, error=debug.qenum_key(QSslError, self._error.error()), + string=str(self)) + + def __hash__(self): + try: + # Qt >= 5.4 + return hash(self._error) + except TypeError: + return hash((self._error.certificate().toDer(), + self._error.error())) + + def __eq__(self, other): + return self._error == other._error + + def is_overridable(self): + return True + + class WebKitPrinting(browsertab.AbstractPrinting): """QtWebKit implementations related to printing.""" diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index d55830380..a841d0567 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -430,8 +430,7 @@ def data(readonly=False): "Whether to send DNS requests over the configured proxy."), ('ssl-strict', - SettingValue(typ.BoolAsk(), 'ask', - backends=[usertypes.Backend.QtWebKit]), + SettingValue(typ.BoolAsk(), 'ask'), "Whether to validate SSL handshakes."), ('dns-prefetch', diff --git a/tests/end2end/features/prompts.feature b/tests/end2end/features/prompts.feature index 12497857b..90e2e6d9c 100644 --- a/tests/end2end/features/prompts.feature +++ b/tests/end2end/features/prompts.feature @@ -166,16 +166,14 @@ Feature: Prompts # SSL - @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = false When I run :debug-clear-ssl-errors And I set network -> ssl-strict to false And I load an SSL page And I wait until the SSL page finished loading - Then the error "SSL error: *" should be shown + Then the error "Certificate error: *" should be shown And the page should contain the plaintext "Hello World via SSL!" - @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = true When I run :debug-clear-ssl-errors And I set network -> ssl-strict to true @@ -183,7 +181,6 @@ Feature: Prompts Then "Error while loading *: SSL handshake failed" should be logged And the page should contain the plaintext "Unable to load page" - @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = ask -> yes When I run :debug-clear-ssl-errors And I set network -> ssl-strict to ask @@ -193,7 +190,6 @@ Feature: Prompts And I wait until the SSL page finished loading Then the page should contain the plaintext "Hello World via SSL!" - @qtwebengine_todo: SSL errors are not implemented yet Scenario: SSL error with ssl-strict = ask -> no When I run :debug-clear-ssl-errors And I set network -> ssl-strict to ask @@ -473,7 +469,6 @@ Feature: Prompts # https://github.com/The-Compiler/qutebrowser/issues/1249#issuecomment-175205531 # https://github.com/The-Compiler/qutebrowser/pull/2054#issuecomment-258285544 - @qtwebengine_todo: SSL errors are not implemented yet Scenario: Interrupting SSL prompt during a notification prompt When I set content -> notifications to ask And I set network -> ssl-strict to ask diff --git a/tests/end2end/fixtures/quteprocess.py b/tests/end2end/fixtures/quteprocess.py index 46a0d2ff7..4db75e55b 100644 --- a/tests/end2end/fixtures/quteprocess.py +++ b/tests/end2end/fixtures/quteprocess.py @@ -53,6 +53,19 @@ def is_ignored_qt_message(message): return False +def is_ignored_lowlevel_message(message): + """Check if we want to ignore a lowlevel process output.""" + if 'Running without the SUID sandbox!' in message: + return True + elif message.startswith('Xlib: sequence lost'): + # https://travis-ci.org/The-Compiler/qutebrowser/jobs/157941720 + # ??? + return True + elif 'CERT_PKIXVerifyCert for localhost failed' in message: + return True + return False + + class LogLine(testprocess.Line): """A parsed line from the qutebrowser log output. @@ -222,14 +235,7 @@ class QuteProc(testprocess.Process): except testprocess.InvalidLine: if not line.strip(): return None - elif 'Running without the SUID sandbox!' in line: - # QtWebEngine error - return None - elif line.startswith('Xlib: sequence lost'): - # https://travis-ci.org/The-Compiler/qutebrowser/jobs/157941720 - # ??? - return None - elif is_ignored_qt_message(line): + elif is_ignored_qt_message(line) or is_ignored_lowlevel_message(line): return None else: raise From d93bc8b26b98de194fd2b028711dc3ac4b672185 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 10:26:27 +0100 Subject: [PATCH 03/27] Make prompt tests run --- qutebrowser/browser/webengine/webview.py | 11 +++++----- .../browser/webkit/network/networkmanager.py | 3 ++- tests/end2end/features/prompts.feature | 15 +++++++------ tests/end2end/features/test_prompts_bdd.py | 21 +++++++++++++++++++ 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index c8488bb4a..9b9741317 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -130,12 +130,12 @@ class WebEnginePage(QWebEnginePage): self.certificate_error.emit() url = error.url() error = webenginetab.CertificateErrorWrapper(error) + log.webview.debug("Certificate error: {}".format(error)) - # FIXME - error_page = jinja.render('error.html', - title="Error while loading page", - url=url.toDisplayString(), error=str(error), - icon='', qutescheme=False) + url_string = url.toDisplayString() + error_page = jinja.render( + 'error.html', title="Error loading page: {}".format(url_string), + url=url_string, error=str(error), icon='') if not error.is_overridable(): log.webview.error("Non-overridable certificate error: " @@ -147,7 +147,6 @@ class WebEnginePage(QWebEnginePage): url, [error], abort_on=[self.loadStarted, self.shutting_down]) if not ignore: - log.webview.error("Certificate error: {}".format(error)) self.setHtml(error_page) return ignore diff --git a/qutebrowser/browser/webkit/network/networkmanager.py b/qutebrowser/browser/webkit/network/networkmanager.py index d4655d04a..6294efc1e 100644 --- a/qutebrowser/browser/webkit/network/networkmanager.py +++ b/qutebrowser/browser/webkit/network/networkmanager.py @@ -234,7 +234,8 @@ class NetworkManager(QNetworkAccessManager): errors: A list of errors. """ errors = [webkittab.CertificateErrorWrapper(e) for e in errors] - log.webview.debug("Certificate errors {!r}".format(errors)) + log.webview.debug("Certificate errors: {!r}".format( + ' / '.join(str(err) for err in errors))) try: host_tpl = urlutils.host_tuple(reply.url()) except ValueError: diff --git a/tests/end2end/features/prompts.feature b/tests/end2end/features/prompts.feature index 90e2e6d9c..1b75bfd70 100644 --- a/tests/end2end/features/prompts.feature +++ b/tests/end2end/features/prompts.feature @@ -167,7 +167,7 @@ Feature: Prompts # SSL Scenario: SSL error with ssl-strict = false - When I run :debug-clear-ssl-errors + When I clear SSL errors And I set network -> ssl-strict to false And I load an SSL page And I wait until the SSL page finished loading @@ -175,14 +175,13 @@ Feature: Prompts And the page should contain the plaintext "Hello World via SSL!" Scenario: SSL error with ssl-strict = true - When I run :debug-clear-ssl-errors + When I clear SSL errors And I set network -> ssl-strict to true And I load an SSL page - Then "Error while loading *: SSL handshake failed" should be logged - And the page should contain the plaintext "Unable to load page" + Then a SSL error page should be shown Scenario: SSL error with ssl-strict = ask -> yes - When I run :debug-clear-ssl-errors + When I clear SSL errors And I set network -> ssl-strict to ask And I load an SSL page And I wait for a prompt @@ -191,13 +190,12 @@ Feature: Prompts Then the page should contain the plaintext "Hello World via SSL!" Scenario: SSL error with ssl-strict = ask -> no - When I run :debug-clear-ssl-errors + When I clear SSL errors And I set network -> ssl-strict to ask And I load an SSL page And I wait for a prompt And I run :prompt-accept no - Then "Error while loading *: SSL handshake failed" should be logged - And the page should contain the plaintext "Unable to load page" + Then a SSL error page should be shown # Geolocation @@ -469,6 +467,7 @@ Feature: Prompts # https://github.com/The-Compiler/qutebrowser/issues/1249#issuecomment-175205531 # https://github.com/The-Compiler/qutebrowser/pull/2054#issuecomment-258285544 + @qtwebengine_todo: Permissions are not implemented yet Scenario: Interrupting SSL prompt during a notification prompt When I set content -> notifications to ask And I set network -> ssl-strict to ask diff --git a/tests/end2end/features/test_prompts_bdd.py b/tests/end2end/features/test_prompts_bdd.py index b5530bbea..2711d3888 100644 --- a/tests/end2end/features/test_prompts_bdd.py +++ b/tests/end2end/features/test_prompts_bdd.py @@ -22,6 +22,15 @@ import pytest_bdd as bdd bdd.scenarios('prompts.feature') +@bdd.when("I clear SSL errors") +def clear_ssl_errors(request, quteproc): + if request.config.webengine: + quteproc.terminate() + quteproc.start() + else: + quteproc.send_cmd(':debug-clear-ssl-errors') + + @bdd.when("I load an SSL page") def load_ssl_page(quteproc, ssl_server): # We don't wait here as we can get an SSL question. @@ -43,3 +52,15 @@ def wait_for_prompt(quteproc): def no_prompt_shown(quteproc): quteproc.ensure_not_logged(message='Entering mode KeyMode.* (reason: ' 'question asked)') + + +@bdd.then("a SSL error page should be shown") +def ssl_error_page(request, quteproc): + if not request.config.webengine: + line = quteproc.wait_for(message='Error while loading *: SSL ' + 'handshake failed') + line.expected = True + quteproc.wait_for(message="Changing title for idx * to 'Error " + "loading page: *'") + content = quteproc.get_content().strip() + assert "Unable to load page" in content From c6f83d3148c151692ac9a751ef49f4ab2a2e74d9 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 13:29:45 +0100 Subject: [PATCH 04/27] Don't show SSL error page for subresources --- qutebrowser/browser/webengine/webview.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index 9b9741317..a486d40db 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -137,16 +137,23 @@ class WebEnginePage(QWebEnginePage): 'error.html', title="Error loading page: {}".format(url_string), url=url_string, error=str(error), icon='') - if not error.is_overridable(): + if error.is_overridable(): + ignore = shared.ignore_certificate_errors( + url, [error], abort_on=[self.loadStarted, self.shutting_down]) + else: log.webview.error("Non-overridable certificate error: " "{}".format(error)) - self.setHtml(error_page) - return False + ignore = False - ignore = shared.ignore_certificate_errors( - url, [error], abort_on=[self.loadStarted, self.shutting_down]) - - if not ignore: + # We can't really know when to show an error page, as the error might + # have happened when loading some resource. + # However, self.url() is not available yet and self.requestedUrl() might + # not match the URL we get from the error - so we just apply a heuristic + # here. + # See https://bugreports.qt.io/browse/QTBUG-56207 + log.webview.debug("ignore {}, URL {}, requested {}".format( + ignore, url, self.requestedUrl())) + if not ignore and url.matches(self.requestedUrl(), QUrl.RemoveScheme): self.setHtml(error_page) return ignore From 8a4ca25b8d20c614f667cf181527b9c2c0fd6345 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 13:53:13 +0100 Subject: [PATCH 05/27] Avoid circular import --- qutebrowser/browser/browsertab.py | 17 ------ .../browser/webengine/certificateerror.py | 45 +++++++++++++++ qutebrowser/browser/webengine/webenginetab.py | 23 +------- qutebrowser/browser/webengine/webview.py | 4 +- .../browser/webkit/certificateerror.py | 55 +++++++++++++++++++ .../browser/webkit/network/networkmanager.py | 4 +- qutebrowser/browser/webkit/webkittab.py | 31 ----------- qutebrowser/utils/usertypes.py | 17 ++++++ tests/end2end/features/test_prompts_bdd.py | 17 ++++++ tests/unit/utils/usertypes/test_misc.py | 27 +++++++++ 10 files changed, 166 insertions(+), 74 deletions(-) create mode 100644 qutebrowser/browser/webengine/certificateerror.py create mode 100644 qutebrowser/browser/webkit/certificateerror.py create mode 100644 tests/unit/utils/usertypes/test_misc.py diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 2589154a5..22f1c7e51 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -75,23 +75,6 @@ class UnsupportedOperationError(WebTabError): """Raised when an operation is not supported with the given backend.""" -class AbstractCertificateErrorWrapper: - - """A wrapper over an SSL/certificate error.""" - - def __init__(self, error): - self._error = error - - def __str__(self): - raise NotImplementedError - - def __repr__(self): - raise NotImplementedError - - def is_overridable(self): - raise NotImplementedError - - class TabData: """A simple namespace with a fixed set of attributes. diff --git a/qutebrowser/browser/webengine/certificateerror.py b/qutebrowser/browser/webengine/certificateerror.py new file mode 100644 index 000000000..dcc791a56 --- /dev/null +++ b/qutebrowser/browser/webengine/certificateerror.py @@ -0,0 +1,45 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2016 Florian Bruhin (The Compiler) +# +# This file is part of qutebrowser. +# +# qutebrowser is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# qutebrowser is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with qutebrowser. If not, see . + +"""Wrapper over a QWebEngineCertificateError.""" + + +from PyQt5.QtWebEngineWidgets import QWebEngineCertificateError + +from qutebrowser.utils import usertypes, utils, debug + + +class CertificateErrorWrapper(usertypes.AbstractCertificateErrorWrapper): + + """A wrapper over a QWebEngineCertificateError.""" + + def __init__(self, error): + self._error = error + + def __str__(self): + return self._error.errorDescription() + + def __repr__(self): + return utils.get_repr( + self, error=debug.qenum_key(QWebEngineCertificateError, + self._error.error()), + string=str(self)) + + def is_overridable(self): + return self._error.isOverridable() diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 9ec89836a..8a2846999 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -30,8 +30,7 @@ from PyQt5.QtGui import QKeyEvent, QIcon # pylint: disable=no-name-in-module,import-error,useless-suppression from PyQt5.QtWidgets import QOpenGLWidget, QApplication from PyQt5.QtWebEngineWidgets import (QWebEnginePage, QWebEngineScript, - QWebEngineProfile, - QWebEngineCertificateError) + QWebEngineProfile) # pylint: enable=no-name-in-module,import-error,useless-suppression from qutebrowser.browser import browsertab, mouse, shared @@ -79,26 +78,6 @@ _JS_WORLD_MAP = { } -class CertificateErrorWrapper(browsertab.AbstractCertificateErrorWrapper): - - """A wrapper over a QWebEngineCertificateError.""" - - def __init__(self, error): - self._error = error - - def __str__(self): - return self._error.errorDescription() - - def __repr__(self): - return utils.get_repr( - self, error=debug.qenum_key(QWebEngineCertificateError, - self._error.error()), - string=str(self)) - - def is_overridable(self): - return self._error.isOverridable() - - class WebEnginePrinting(browsertab.AbstractPrinting): """QtWebEngine implementations related to printing.""" diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index a486d40db..020eddf2a 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -27,7 +27,7 @@ from PyQt5.QtWebEngineWidgets import QWebEngineView, QWebEnginePage # pylint: enable=no-name-in-module,import-error,useless-suppression from qutebrowser.browser import shared -from qutebrowser.browser.webengine import webenginetab +from qutebrowser.browser.webengine import webenginetab, certificateerror from qutebrowser.config import config from qutebrowser.utils import log, debug, usertypes, objreg, qtutils, jinja @@ -129,7 +129,7 @@ class WebEnginePage(QWebEnginePage): def certificateError(self, error): self.certificate_error.emit() url = error.url() - error = webenginetab.CertificateErrorWrapper(error) + error = certificateerror.CertificateErrorWrapper(error) log.webview.debug("Certificate error: {}".format(error)) url_string = url.toDisplayString() diff --git a/qutebrowser/browser/webkit/certificateerror.py b/qutebrowser/browser/webkit/certificateerror.py new file mode 100644 index 000000000..14d8ccddd --- /dev/null +++ b/qutebrowser/browser/webkit/certificateerror.py @@ -0,0 +1,55 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2016 Florian Bruhin (The Compiler) +# +# This file is part of qutebrowser. +# +# qutebrowser is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# qutebrowser is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with qutebrowser. If not, see . + +"""Wrapper over a QSslCertificateError.""" + + +from PyQt5.QtNetwork import QSslError + +from qutebrowser.utils import usertypes, utils, debug + + +class CertificateErrorWrapper(usertypes.AbstractCertificateErrorWrapper): + + """A wrapper over a QSslError.""" + + def __init__(self, error): + self._error = error + + def __str__(self): + return self._error.errorString() + + def __repr__(self): + return utils.get_repr( + self, error=debug.qenum_key(QSslError, self._error.error()), + string=str(self)) + + def __hash__(self): + try: + # Qt >= 5.4 + return hash(self._error) + except TypeError: + return hash((self._error.certificate().toDer(), + self._error.error())) + + def __eq__(self, other): + return self._error == other._error + + def is_overridable(self): + return True diff --git a/qutebrowser/browser/webkit/network/networkmanager.py b/qutebrowser/browser/webkit/network/networkmanager.py index 6294efc1e..06c33fe7f 100644 --- a/qutebrowser/browser/webkit/network/networkmanager.py +++ b/qutebrowser/browser/webkit/network/networkmanager.py @@ -33,7 +33,7 @@ from qutebrowser.config import config from qutebrowser.utils import (message, log, usertypes, utils, objreg, qtutils, urlutils, debug) from qutebrowser.browser import shared -from qutebrowser.browser.webkit import webkittab +from qutebrowser.browser.webkit import webkittab, certificateerror from qutebrowser.browser.webkit.network import (webkitqutescheme, networkreply, filescheme) @@ -233,7 +233,7 @@ class NetworkManager(QNetworkAccessManager): reply: The QNetworkReply that is encountering the errors. errors: A list of errors. """ - errors = [webkittab.CertificateErrorWrapper(e) for e in errors] + errors = [certificateerror.CertificateErrorWrapper(e) for e in errors] log.webview.debug("Certificate errors: {!r}".format( ' / '.join(str(err) for err in errors))) try: diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index a2bbb2b9a..ded0d6c66 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -30,7 +30,6 @@ from PyQt5.QtWidgets import QApplication from PyQt5.QtWebKitWidgets import QWebPage, QWebFrame from PyQt5.QtWebKit import QWebSettings from PyQt5.QtPrintSupport import QPrinter -from PyQt5.QtNetwork import QSslError from qutebrowser.browser import browsertab from qutebrowser.browser.webkit import webview, tabhistory, webkitelem @@ -50,36 +49,6 @@ def init(): objreg.register('js-bridge', js_bridge) -class CertificateErrorWrapper(browsertab.AbstractCertificateErrorWrapper): - - """A wrapper over a QSslError.""" - - def __init__(self, error): - self._error = error - - def __str__(self): - return self._error.errorString() - - def __repr__(self): - return utils.get_repr( - self, error=debug.qenum_key(QSslError, self._error.error()), - string=str(self)) - - def __hash__(self): - try: - # Qt >= 5.4 - return hash(self._error) - except TypeError: - return hash((self._error.certificate().toDer(), - self._error.error())) - - def __eq__(self, other): - return self._error == other._error - - def is_overridable(self): - return True - - class WebKitPrinting(browsertab.AbstractPrinting): """QtWebKit implementations related to printing.""" diff --git a/qutebrowser/utils/usertypes.py b/qutebrowser/utils/usertypes.py index 1d2788ba1..7ad89b3a2 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -402,3 +402,20 @@ class Timer(QTimer): super().start(msec) else: super().start() + + +class AbstractCertificateErrorWrapper: + + """A wrapper over an SSL/certificate error.""" + + def __init__(self, error): + self._error = error + + def __str__(self): + raise NotImplementedError + + def __repr__(self): + raise NotImplementedError + + def is_overridable(self): + raise NotImplementedError diff --git a/tests/end2end/features/test_prompts_bdd.py b/tests/end2end/features/test_prompts_bdd.py index 2711d3888..6fe1c3e69 100644 --- a/tests/end2end/features/test_prompts_bdd.py +++ b/tests/end2end/features/test_prompts_bdd.py @@ -64,3 +64,20 @@ def ssl_error_page(request, quteproc): "loading page: *'") content = quteproc.get_content().strip() assert "Unable to load page" in content + + +class AbstractCertificateErrorWrapper: + + """A wrapper over an SSL/certificate error.""" + + def __init__(self, error): + self._error = error + + def __str__(self): + raise NotImplementedError + + def __repr__(self): + raise NotImplementedError + + def is_overridable(self): + raise NotImplementedError diff --git a/tests/unit/utils/usertypes/test_misc.py b/tests/unit/utils/usertypes/test_misc.py new file mode 100644 index 000000000..c2c0738e5 --- /dev/null +++ b/tests/unit/utils/usertypes/test_misc.py @@ -0,0 +1,27 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2016 Florian Bruhin (The Compiler) +# +# This file is part of qutebrowser. +# +# qutebrowser is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# qutebrowser is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with qutebrowser. If not, see . + + +from qutebrowser.utils import usertypes + + +def test_abstract_certificate_error_wrapper(): + err = object() + wrapper = usertypes.AbstractCertificateErrorWrapper(err) + assert wrapper._error is err From 8f55725555b9d97cbf189d35eb18a93f4b615597 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 13:59:06 +0100 Subject: [PATCH 06/27] Fix lint --- qutebrowser/browser/webengine/certificateerror.py | 3 --- qutebrowser/browser/webengine/webenginetab.py | 3 +-- qutebrowser/browser/webengine/webview.py | 12 +++++++----- qutebrowser/browser/webkit/certificateerror.py | 5 +---- qutebrowser/browser/webkit/network/networkmanager.py | 7 +++---- qutebrowser/browser/webkit/webkittab.py | 2 +- qutebrowser/browser/webkit/webpage.py | 4 ++-- tests/end2end/features/test_prompts_bdd.py | 1 - 8 files changed, 15 insertions(+), 22 deletions(-) diff --git a/qutebrowser/browser/webengine/certificateerror.py b/qutebrowser/browser/webengine/certificateerror.py index dcc791a56..b9d254c7f 100644 --- a/qutebrowser/browser/webengine/certificateerror.py +++ b/qutebrowser/browser/webengine/certificateerror.py @@ -29,9 +29,6 @@ class CertificateErrorWrapper(usertypes.AbstractCertificateErrorWrapper): """A wrapper over a QWebEngineCertificateError.""" - def __init__(self, error): - self._error = error - def __str__(self): return self._error.errorDescription() diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 8a2846999..46f6cdccc 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -22,7 +22,6 @@ """Wrapper over a QWebEngineView.""" -import html import functools from PyQt5.QtCore import pyqtSlot, Qt, QEvent, QPoint, QUrl, QTimer @@ -38,7 +37,7 @@ from qutebrowser.browser.webengine import (webview, webengineelem, tabhistory, interceptor, webenginequtescheme, webenginedownloads) from qutebrowser.utils import (usertypes, qtutils, log, javascript, utils, - objreg, message, debug) + objreg) _qute_scheme_handler = None diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index 020eddf2a..9569278bb 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -27,7 +27,7 @@ from PyQt5.QtWebEngineWidgets import QWebEngineView, QWebEnginePage # pylint: enable=no-name-in-module,import-error,useless-suppression from qutebrowser.browser import shared -from qutebrowser.browser.webengine import webenginetab, certificateerror +from qutebrowser.browser.webengine import certificateerror from qutebrowser.config import config from qutebrowser.utils import log, debug, usertypes, objreg, qtutils, jinja @@ -127,6 +127,7 @@ class WebEnginePage(QWebEnginePage): self.shutting_down.emit() def certificateError(self, error): + """Handle certificate errors coming from Qt.""" self.certificate_error.emit() url = error.url() error = certificateerror.CertificateErrorWrapper(error) @@ -147,9 +148,9 @@ class WebEnginePage(QWebEnginePage): # We can't really know when to show an error page, as the error might # have happened when loading some resource. - # However, self.url() is not available yet and self.requestedUrl() might - # not match the URL we get from the error - so we just apply a heuristic - # here. + # However, self.url() is not available yet and self.requestedUrl() + # might not match the URL we get from the error - so we just apply a + # heuristic here. # See https://bugreports.qt.io/browse/QTBUG-56207 log.webview.debug("ignore {}, URL {}, requested {}".format( ignore, url, self.requestedUrl())) @@ -159,6 +160,7 @@ class WebEnginePage(QWebEnginePage): return ignore def javaScriptConfirm(self, url, js_msg): + """Override javaScriptConfirm to use qutebrowser prompts.""" if self._is_shutting_down: return False try: @@ -181,7 +183,7 @@ class WebEnginePage(QWebEnginePage): # return super().javaScriptPrompt(url, js_msg, default) def javaScriptAlert(self, url, js_msg): - """Override javaScriptAlert to use the statusbar.""" + """Override javaScriptAlert to use qutebrowser prompts.""" if self._is_shutting_down: return try: diff --git a/qutebrowser/browser/webkit/certificateerror.py b/qutebrowser/browser/webkit/certificateerror.py index 14d8ccddd..f50a58f79 100644 --- a/qutebrowser/browser/webkit/certificateerror.py +++ b/qutebrowser/browser/webkit/certificateerror.py @@ -29,9 +29,6 @@ class CertificateErrorWrapper(usertypes.AbstractCertificateErrorWrapper): """A wrapper over a QSslError.""" - def __init__(self, error): - self._error = error - def __str__(self): return self._error.errorString() @@ -49,7 +46,7 @@ class CertificateErrorWrapper(usertypes.AbstractCertificateErrorWrapper): self._error.error())) def __eq__(self, other): - return self._error == other._error + return self._error == other._error # pylint: disable=protected-access def is_overridable(self): return True diff --git a/qutebrowser/browser/webkit/network/networkmanager.py b/qutebrowser/browser/webkit/network/networkmanager.py index 06c33fe7f..5da5250b4 100644 --- a/qutebrowser/browser/webkit/network/networkmanager.py +++ b/qutebrowser/browser/webkit/network/networkmanager.py @@ -26,14 +26,13 @@ import html from PyQt5.QtCore import (pyqtSlot, pyqtSignal, PYQT_VERSION, QCoreApplication, QUrl, QByteArray) -from PyQt5.QtNetwork import (QNetworkAccessManager, QNetworkReply, QSslError, - QSslSocket) +from PyQt5.QtNetwork import QNetworkAccessManager, QNetworkReply, QSslSocket from qutebrowser.config import config from qutebrowser.utils import (message, log, usertypes, utils, objreg, qtutils, - urlutils, debug) + urlutils) from qutebrowser.browser import shared -from qutebrowser.browser.webkit import webkittab, certificateerror +from qutebrowser.browser.webkit import certificateerror from qutebrowser.browser.webkit.network import (webkitqutescheme, networkreply, filescheme) diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index ded0d6c66..61de1bd54 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -34,7 +34,7 @@ from PyQt5.QtPrintSupport import QPrinter from qutebrowser.browser import browsertab from qutebrowser.browser.webkit import webview, tabhistory, webkitelem from qutebrowser.browser.webkit.network import proxy, webkitqutescheme -from qutebrowser.utils import qtutils, objreg, usertypes, utils, log, debug +from qutebrowser.utils import qtutils, objreg, usertypes, utils, log def init(): diff --git a/qutebrowser/browser/webkit/webpage.py b/qutebrowser/browser/webkit/webpage.py index c72e01faa..04d9de2e4 100644 --- a/qutebrowser/browser/webkit/webpage.py +++ b/qutebrowser/browser/webkit/webpage.py @@ -95,7 +95,7 @@ class BrowserPage(QWebPage): # See http://www.riverbankcomputing.com/pipermail/pyqt/2014-June/034385.html def javaScriptPrompt(self, frame, js_msg, default): - """Override javaScriptPrompt to use the statusbar.""" + """Override javaScriptPrompt to use qutebrowser prompts.""" if self._is_shutting_down: return (False, "") try: @@ -434,7 +434,7 @@ class BrowserPage(QWebPage): return handler(opt, out) def javaScriptAlert(self, frame, js_msg): - """Override javaScriptAlert to use the statusbar.""" + """Override javaScriptAlert to use qutebrowser prompts.""" if self._is_shutting_down: return try: diff --git a/tests/end2end/features/test_prompts_bdd.py b/tests/end2end/features/test_prompts_bdd.py index 6fe1c3e69..83e03f721 100644 --- a/tests/end2end/features/test_prompts_bdd.py +++ b/tests/end2end/features/test_prompts_bdd.py @@ -17,7 +17,6 @@ # You should have received a copy of the GNU General Public License # along with qutebrowser. If not, see . -import pytest import pytest_bdd as bdd bdd.scenarios('prompts.feature') From bbcbb24cb5d12d5614c5d9205280f712d790e118 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 18:37:36 +0100 Subject: [PATCH 07/27] Handle feature permissions with QtWebEngine --- doc/help/settings.asciidoc | 30 ++++++++++++ qutebrowser/browser/shared.py | 34 ++++++++++++++ qutebrowser/browser/webengine/webview.py | 52 ++++++++++++++++++++- qutebrowser/browser/webkit/webpage.py | 58 +++++++++--------------- qutebrowser/config/configdata.py | 10 ++++ tests/end2end/features/prompts.feature | 23 ++++------ 6 files changed, 156 insertions(+), 51 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 159d173cb..e9b3dbb1b 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -160,6 +160,8 @@ |<>|Enable or disable hyperlink auditing (). |<>|Allow websites to request geolocations. |<>|Allow websites to show notifications. +|<>|Allow websites to record audio/video. +|<>|Allow websites to lock the mouse pointer. |<>|Whether JavaScript programs can open new windows without user interaction. |<>|Whether JavaScript programs can close windows. |<>|Whether JavaScript programs can read or write to the clipboard. @@ -1450,6 +1452,34 @@ Valid values: Default: +pass:[ask]+ +[[content-media-capture]] +=== media-capture +Allow websites to record audio/video. + +Valid values: + + * +true+ + * +false+ + * +ask+ + +Default: +pass:[ask]+ + +This setting is only available with the QtWebEngine backend. + +[[content-mouse-lock]] +=== mouse-lock +Allow websites to lock the mouse pointer. + +Valid values: + + * +true+ + * +false+ + * +ask+ + +Default: +pass:[ask]+ + +This setting is only available with the QtWebEngine backend. + [[content-javascript-can-open-windows-automatically]] === javascript-can-open-windows-automatically Whether JavaScript programs can open new windows without user interaction. diff --git a/qutebrowser/browser/shared.py b/qutebrowser/browser/shared.py index 417b741c6..fa086cf2c 100644 --- a/qutebrowser/browser/shared.py +++ b/qutebrowser/browser/shared.py @@ -157,3 +157,37 @@ def ignore_certificate_errors(url, errors, abort_on): else: raise ValueError("Invalid ssl_strict value {!r}".format(ssl_strict)) raise AssertionError("Not reached") + + +def feature_permission(url, option, msg, yes_action, no_action, abort_on): + """Handle a feature permission request. + + Args: + url: The URL the request was done for. + option: A (section, option) tuple for the option to check. + msg: A string like "show notifications" + yes_action: A callable to call if the request was approved + no_action: A callable to call if the request was denied + abort_on: A list of signals which interrupt the question. + + Return: + The Question object if a question was asked, None otherwise. + """ + config_val = config.get(*option) + if config_val == 'ask': + if url.isValid(): + text = "Allow the website at {} to {}?".format( + html.escape(url.toDisplayString()), msg) + else: + text = "Allow the website to {}?".format(msg) + + return message.confirm_async( + yes_action=yes_action, no_action=no_action, + cancel_action=no_action, abort_on=abort_on, + title='Permission request', text=text) + elif config_val: + yes_action() + return None + else: + no_action() + return None diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index 9569278bb..f3fc9bfbe 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -20,8 +20,9 @@ """The main browser widget for QtWebEngine.""" import os +import functools -from PyQt5.QtCore import pyqtSignal, QUrl +from PyQt5.QtCore import pyqtSignal, pyqtSlot, QUrl # pylint: disable=no-name-in-module,import-error,useless-suppression from PyQt5.QtWebEngineWidgets import QWebEngineView, QWebEnginePage # pylint: enable=no-name-in-module,import-error,useless-suppression @@ -121,6 +122,55 @@ class WebEnginePage(QWebEnginePage): super().__init__(parent) self._tabdata = tabdata self._is_shutting_down = False + self.featurePermissionRequested.connect( + self._on_feature_permission_requested) + + @pyqtSlot(QUrl, 'QWebEnginePage::Feature') + def _on_feature_permission_requested(self, url, feature): + """Ask the user for approval for geolocation/media/etc..""" + options = { + QWebEnginePage.Geolocation: ('content', 'geolocation'), + QWebEnginePage.MediaAudioCapture: ('content', 'media-capture'), + QWebEnginePage.MediaVideoCapture: ('content', 'media-capture'), + QWebEnginePage.MediaAudioVideoCapture: ('content', 'media-capture'), + QWebEnginePage.MouseLock: ('content', 'mouse-lock'), + } + messages = { + QWebEnginePage.Geolocation: 'access your location', + QWebEnginePage.MediaAudioCapture: 'record audio', + QWebEnginePage.MediaVideoCapture: 'record video', + QWebEnginePage.MediaAudioVideoCapture: 'record audio/video', + } + yes_action = functools.partial( + self.setFeaturePermission, url, feature, + QWebEnginePage.PermissionGrantedByUser) + no_action = functools.partial( + self.setFeaturePermission, url, feature, + QWebEnginePage.PermissionDeniedByUser) + + question = shared.feature_permission( + url=url, option=options[feature], msg=messages[feature], + yes_action=yes_action, no_action=no_action, + abort_on=[self.shutting_down, self.loadStarted]) + + if question is not None: + self.featurePermissionRequestCanceled.connect( + functools.partial(self._on_feature_permission_cancelled, + question, url, feature)) + + def _on_feature_permission_cancelled(self, question, url, feature, + cancelled_url, cancelled_feature): + """Slot invoked when a feature permission request was cancelled. + + To be used with functools.partial. + """ + if url == cancelled_url and feature == cancelled_feature: + try: + question.abort() + except RuntimeError: + # The question could already be deleted, e.g. because it was + # aborted after a loadStarted signal. + pass def shutdown(self): self._is_shutting_down = True diff --git a/qutebrowser/browser/webkit/webpage.py b/qutebrowser/browser/webkit/webpage.py index 04d9de2e4..bf41ec10a 100644 --- a/qutebrowser/browser/webkit/webpage.py +++ b/qutebrowser/browser/webkit/webpage.py @@ -82,7 +82,7 @@ class BrowserPage(QWebPage): self.unsupportedContent.connect(self.on_unsupported_content) self.loadStarted.connect(self.on_load_started) self.featurePermissionRequested.connect( - self.on_feature_permission_requested) + self._on_feature_permission_requested) self.saveFrameStateRequested.connect( self.on_save_frame_state_requested) self.restoreFrameStateRequested.connect( @@ -289,7 +289,7 @@ class BrowserPage(QWebPage): self.error_occurred = False @pyqtSlot('QWebFrame*', 'QWebPage::Feature') - def on_feature_permission_requested(self, frame, feature): + def _on_feature_permission_requested(self, frame, feature): """Ask the user for approval for geolocation/notifications.""" if not isinstance(frame, QWebFrame): # pragma: no cover # This makes no sense whatsoever, but someone reported this being @@ -302,46 +302,30 @@ class BrowserPage(QWebPage): QWebPage.Notifications: ('content', 'notifications'), QWebPage.Geolocation: ('content', 'geolocation'), } - config_val = config.get(*options[feature]) - if config_val == 'ask': - msgs = { - QWebPage.Notifications: 'show notifications', - QWebPage.Geolocation: 'access your location', - } + messages = { + QWebPage.Notifications: 'show notifications', + QWebPage.Geolocation: 'access your location', + } + yes_action = functools.partial( + self.setFeaturePermission, frame, feature, + QWebPage.PermissionGrantedByUser) + no_action = functools.partial( + self.setFeaturePermission, frame, feature, + QWebPage.PermissionDeniedByUser) - host = frame.url().host() - if host: - text = "Allow the website at {} to {}?".format( - html.escape(frame.url().toDisplayString()), msgs[feature]) - else: - text = "Allow the website to {}?".format(msgs[feature]) + question = shared.feature_permission( + url=frame.url(), + option=options[feature], msg=messages[feature], + yes_action=yes_action, no_action=no_action, + abort_on=[self.shutting_down, self.loadStarted]) - yes_action = functools.partial( - self.setFeaturePermission, frame, feature, - QWebPage.PermissionGrantedByUser) - no_action = functools.partial( - self.setFeaturePermission, frame, feature, - QWebPage.PermissionDeniedByUser) - - question = message.confirm_async(yes_action=yes_action, - no_action=no_action, - cancel_action=no_action, - abort_on=[self.shutting_down, - self.loadStarted], - title='Permission request', - text=text) + if question is not None: self.featurePermissionRequestCanceled.connect( - functools.partial(self.on_feature_permission_cancelled, + functools.partial(self._on_feature_permission_cancelled, question, frame, feature)) - elif config_val: - self.setFeaturePermission(frame, feature, - QWebPage.PermissionGrantedByUser) - else: - self.setFeaturePermission(frame, feature, - QWebPage.PermissionDeniedByUser) - def on_feature_permission_cancelled(self, question, frame, feature, - cancelled_frame, cancelled_feature): + def _on_feature_permission_cancelled(self, question, frame, feature, + cancelled_frame, cancelled_feature): """Slot invoked when a feature permission request was cancelled. To be used with functools.partial. diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index a841d0567..0037acbeb 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -821,6 +821,16 @@ def data(readonly=False): SettingValue(typ.BoolAsk(), 'ask'), "Allow websites to show notifications."), + ('media-capture', + SettingValue(typ.BoolAsk(), 'ask', + backends=[usertypes.Backend.QtWebEngine]), + "Allow websites to record audio/video."), + + ('mouse-lock', + SettingValue(typ.BoolAsk(), 'ask', + backends=[usertypes.Backend.QtWebEngine]), + "Allow websites to lock the mouse pointer."), + ('javascript-can-open-windows-automatically', SettingValue(typ.Bool(), 'false'), "Whether JavaScript programs can open new windows without user " diff --git a/tests/end2end/features/prompts.feature b/tests/end2end/features/prompts.feature index 1b75bfd70..3146c2d5b 100644 --- a/tests/end2end/features/prompts.feature +++ b/tests/end2end/features/prompts.feature @@ -101,7 +101,7 @@ Feature: Prompts Then the javascript message "Alert done" should be logged And the javascript message "notification permission granted" should be logged - @qtwebengine_todo: Permissions are not implemented yet + @qtwebengine_todo: Notifications are not implemented in QtWebEngine Scenario: Async question interrupted by async one When I set content -> notifications to ask And I open data/prompt/notifications.html in a new tab @@ -116,7 +116,7 @@ Feature: Prompts Then the javascript message "notification permission granted" should be logged And "Added quickmark test for *" should be logged - @qtwebengine_todo: Permissions are not implemented yet + @qtwebengine_todo: Notifications are not implemented in QtWebEngine Scenario: Async question interrupted by blocking one When I set content -> notifications to ask And I set content -> ignore-javascript-alert to false @@ -199,21 +199,20 @@ Feature: Prompts # Geolocation - @qtwebengine_todo: Permissions are not implemented yet Scenario: Always rejecting geolocation When I set content -> geolocation to false And I open data/prompt/geolocation.html in a new tab And I run :click-element id button Then the javascript message "geolocation permission denied" should be logged - @ci @not_osx @qtwebengine_todo: Permissions are not implemented yet + @ci @not_osx Scenario: Always accepting geolocation When I set content -> geolocation to true And I open data/prompt/geolocation.html in a new tab And I run :click-element id button Then the javascript message "geolocation permission denied" should not be logged - @ci @not_osx @qtwebengine_todo: Permissions are not implemented yet + @ci @not_osx Scenario: geolocation with ask -> true When I set content -> geolocation to ask And I open data/prompt/geolocation.html in a new tab @@ -222,7 +221,6 @@ Feature: Prompts And I run :prompt-accept yes Then the javascript message "geolocation permission denied" should not be logged - @qtwebengine_todo: Permissions are not implemented yet Scenario: geolocation with ask -> false When I set content -> geolocation to ask And I open data/prompt/geolocation.html in a new tab @@ -231,7 +229,6 @@ Feature: Prompts And I run :prompt-accept no Then the javascript message "geolocation permission denied" should be logged - @qtwebengine_todo: Permissions are not implemented yet Scenario: geolocation with ask -> abort When I set content -> geolocation to ask And I open data/prompt/geolocation.html in a new tab @@ -242,21 +239,21 @@ Feature: Prompts # Notifications - @qtwebengine_todo: Permissions are not implemented yet + @qtwebengine_todo: Notifications are not implemented in QtWebEngine Scenario: Always rejecting notifications When I set content -> notifications to false And I open data/prompt/notifications.html in a new tab And I run :click-element id button Then the javascript message "notification permission denied" should be logged - @qtwebengine_todo: Permissions are not implemented yet + @qtwebengine_todo: Notifications are not implemented in QtWebEngine Scenario: Always accepting notifications When I set content -> notifications to true And I open data/prompt/notifications.html in a new tab And I run :click-element id button Then the javascript message "notification permission granted" should be logged - @qtwebengine_todo: Permissions are not implemented yet + @qtwebengine_todo: Notifications are not implemented in QtWebEngine Scenario: notifications with ask -> false When I set content -> notifications to ask And I open data/prompt/notifications.html in a new tab @@ -265,7 +262,7 @@ Feature: Prompts And I run :prompt-accept no Then the javascript message "notification permission denied" should be logged - @qtwebengine_todo: Permissions are not implemented yet + @qtwebengine_todo: Notifications are not implemented in QtWebEngine Scenario: notifications with ask -> true When I set content -> notifications to ask And I open data/prompt/notifications.html in a new tab @@ -284,7 +281,7 @@ Feature: Prompts And I run :leave-mode Then the javascript message "notification permission aborted" should be logged - @qtwebengine_todo: Permissions are not implemented yet + @qtwebengine_todo: Notifications are not implemented in QtWebEngine Scenario: answering notification after closing tab When I set content -> notifications to ask And I open data/prompt/notifications.html in a new tab @@ -467,7 +464,7 @@ Feature: Prompts # https://github.com/The-Compiler/qutebrowser/issues/1249#issuecomment-175205531 # https://github.com/The-Compiler/qutebrowser/pull/2054#issuecomment-258285544 - @qtwebengine_todo: Permissions are not implemented yet + @qtwebengine_todo: Notifications are not implemented in QtWebEngine Scenario: Interrupting SSL prompt during a notification prompt When I set content -> notifications to ask And I set network -> ssl-strict to ask From adb2ce01601c2e7bc8de48d10baed7f45175069a Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 21:00:03 +0100 Subject: [PATCH 08/27] Remove MouseLock permission support For some reason it doesn't work... --- doc/help/settings.asciidoc | 15 --------------- qutebrowser/browser/webengine/webview.py | 10 +++++++++- qutebrowser/config/configdata.py | 5 ----- 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index e9b3dbb1b..c7da3e82e 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -161,7 +161,6 @@ |<>|Allow websites to request geolocations. |<>|Allow websites to show notifications. |<>|Allow websites to record audio/video. -|<>|Allow websites to lock the mouse pointer. |<>|Whether JavaScript programs can open new windows without user interaction. |<>|Whether JavaScript programs can close windows. |<>|Whether JavaScript programs can read or write to the clipboard. @@ -1466,20 +1465,6 @@ Default: +pass:[ask]+ This setting is only available with the QtWebEngine backend. -[[content-mouse-lock]] -=== mouse-lock -Allow websites to lock the mouse pointer. - -Valid values: - - * +true+ - * +false+ - * +ask+ - -Default: +pass:[ask]+ - -This setting is only available with the QtWebEngine backend. - [[content-javascript-can-open-windows-automatically]] === javascript-can-open-windows-automatically Whether JavaScript programs can open new windows without user interaction. diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index f3fc9bfbe..92d67b1db 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -133,7 +133,6 @@ class WebEnginePage(QWebEnginePage): QWebEnginePage.MediaAudioCapture: ('content', 'media-capture'), QWebEnginePage.MediaVideoCapture: ('content', 'media-capture'), QWebEnginePage.MediaAudioVideoCapture: ('content', 'media-capture'), - QWebEnginePage.MouseLock: ('content', 'mouse-lock'), } messages = { QWebEnginePage.Geolocation: 'access your location', @@ -141,6 +140,15 @@ class WebEnginePage(QWebEnginePage): QWebEnginePage.MediaVideoCapture: 'record video', QWebEnginePage.MediaAudioVideoCapture: 'record audio/video', } + assert options.keys() == messages.keys() + + if feature not in options: + log.webview.error("Unhandled feature permission {}".format( + debug.qenum_key(QWebEnginePage, feature))) + self.setFeaturePermission(url, feature, + QWebEnginePage.PermissionDeniedByUser) + return + yes_action = functools.partial( self.setFeaturePermission, url, feature, QWebEnginePage.PermissionGrantedByUser) diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 0037acbeb..e90ac15b3 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -826,11 +826,6 @@ def data(readonly=False): backends=[usertypes.Backend.QtWebEngine]), "Allow websites to record audio/video."), - ('mouse-lock', - SettingValue(typ.BoolAsk(), 'ask', - backends=[usertypes.Backend.QtWebEngine]), - "Allow websites to lock the mouse pointer."), - ('javascript-can-open-windows-automatically', SettingValue(typ.Bool(), 'false'), "Whether JavaScript programs can open new windows without user " From 62d258190f3a7253a277ab87eec712413519c354 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 21:11:59 +0100 Subject: [PATCH 09/27] Only support :debug-clear-ssl-errors with QtWebKit --- qutebrowser/browser/commands.py | 2 +- qutebrowser/browser/webengine/webenginetab.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index b4e3e5507..27dd38808 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1992,7 +1992,7 @@ class CommandDispatcher: tab.send_event(release_event) @cmdutils.register(instance='command-dispatcher', scope='window', - debug=True) + debug=True, backend=usertypes.Backend.QtWebKit) def debug_clear_ssl_errors(self): """Clear remembered SSL error answers.""" self._current_widget().clear_ssl_errors() diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 46f6cdccc..7150187a2 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -569,7 +569,7 @@ class WebEngineTab(browsertab.AbstractTab): self._widget.setHtml(html, base_url) def clear_ssl_errors(self): - log.stub() + raise browsertab.UnsupportedOperationError @pyqtSlot() def _on_history_trigger(self): From 013c2691d506fb18b375f3456a66c69fbfb08e7f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 21:15:55 +0100 Subject: [PATCH 10/27] Fix javaScriptPrompt override for newer PyQt versions --- qutebrowser/browser/webengine/webview.py | 26 +++++++++++++----------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index 92d67b1db..4e3617b8a 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -22,7 +22,7 @@ import os import functools -from PyQt5.QtCore import pyqtSignal, pyqtSlot, QUrl +from PyQt5.QtCore import pyqtSignal, pyqtSlot, QUrl, PYQT_VERSION # pylint: disable=no-name-in-module,import-error,useless-suppression from PyQt5.QtWebEngineWidgets import QWebEngineView, QWebEnginePage # pylint: enable=no-name-in-module,import-error,useless-suppression @@ -228,17 +228,19 @@ class WebEnginePage(QWebEnginePage): except shared.CallSuper: return super().javaScriptConfirm(url, js_msg) - # Can't override javaScriptPrompt currently - # https://www.riverbankcomputing.com/pipermail/pyqt/2016-November/038293.html - # def javaScriptPrompt(self, url, js_msg, default, result): - # if self._is_shutting_down: - # return (False, "") - # try: - # return shared.javascript_prompt(url, js_msg, default, - # abort_on=[self.loadStarted, - # self.shutting_down]) - # except shared.CallSuper: - # return super().javaScriptPrompt(url, js_msg, default) + if PYQT_VERSION > 0x050700: + # WORKAROUND + # Can't override javaScriptPrompt with older PyQt versions + # https://www.riverbankcomputing.com/pipermail/pyqt/2016-November/038293.html + def javaScriptPrompt(self, url, js_msg, default, result): + if self._is_shutting_down: + return (False, "") + try: + return shared.javascript_prompt(url, js_msg, default, + abort_on=[self.loadStarted, + self.shutting_down]) + except shared.CallSuper: + return super().javaScriptPrompt(url, js_msg, default) def javaScriptAlert(self, url, js_msg): """Override javaScriptAlert to use qutebrowser prompts.""" From 2d23ed52debc9be3770a3517fb5f2a8de2eca15c Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 21:20:22 +0100 Subject: [PATCH 11/27] Adjust check_coverage --- scripts/dev/check_coverage.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/dev/check_coverage.py b/scripts/dev/check_coverage.py index eeda04287..895993bae 100644 --- a/scripts/dev/check_coverage.py +++ b/scripts/dev/check_coverage.py @@ -74,8 +74,8 @@ PERFECT_FILES = [ ('tests/unit/browser/test_signalfilter.py', 'qutebrowser/browser/signalfilter.py'), - ('tests/unit/browser/test_shared.py', - 'qutebrowser/browser/shared.py'), + (None, + 'qutebrowser/browser/webkit/certificateerror.py'), # ('tests/unit/browser/test_tab.py', # 'qutebrowser/browser/tab.py'), From 6b14cda5d084845103ef5faebf1099f4e04f658f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 21:20:43 +0100 Subject: [PATCH 12/27] Re-enable SSL download test on QtWebEngine --- tests/end2end/features/conftest.py | 9 +++++++++ tests/end2end/features/downloads.feature | 3 +-- tests/end2end/features/test_prompts_bdd.py | 9 --------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/end2end/features/conftest.py b/tests/end2end/features/conftest.py index 9b7a9e2d5..dea0e9a83 100644 --- a/tests/end2end/features/conftest.py +++ b/tests/end2end/features/conftest.py @@ -352,6 +352,15 @@ def javascript_message_when(quteproc, message): quteproc.wait_for_js(message) +@bdd.when("I clear SSL errors") +def clear_ssl_errors(request, quteproc): + if request.config.webengine: + quteproc.terminate() + quteproc.start() + else: + quteproc.send_cmd(':debug-clear-ssl-errors') + + ## Then diff --git a/tests/end2end/features/downloads.feature b/tests/end2end/features/downloads.feature index f187a7336..1e1e59b34 100644 --- a/tests/end2end/features/downloads.feature +++ b/tests/end2end/features/downloads.feature @@ -75,9 +75,8 @@ Feature: Downloading things from a website. And I run :leave-mode Then no crash should happen - @qtwebengine_todo: ssl-strict is not implemented yet Scenario: Downloading with SSL errors (issue 1413) - When I run :debug-clear-ssl-errors + When I clear SSL errors And I set network -> ssl-strict to ask And I download an SSL page And I wait for "Entering mode KeyMode.* (reason: question asked)" in the log diff --git a/tests/end2end/features/test_prompts_bdd.py b/tests/end2end/features/test_prompts_bdd.py index 83e03f721..3ebd53e8c 100644 --- a/tests/end2end/features/test_prompts_bdd.py +++ b/tests/end2end/features/test_prompts_bdd.py @@ -21,15 +21,6 @@ import pytest_bdd as bdd bdd.scenarios('prompts.feature') -@bdd.when("I clear SSL errors") -def clear_ssl_errors(request, quteproc): - if request.config.webengine: - quteproc.terminate() - quteproc.start() - else: - quteproc.send_cmd(':debug-clear-ssl-errors') - - @bdd.when("I load an SSL page") def load_ssl_page(quteproc, ssl_server): # We don't wait here as we can get an SSL question. From 8d781c68c940b02d46ed9b80a4226e73b9ceaf29 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 21:23:09 +0100 Subject: [PATCH 13/27] Fix lint --- qutebrowser/browser/webengine/certificateerror.py | 3 ++- qutebrowser/browser/webengine/webview.py | 6 ++++-- scripts/dev/run_vulture.py | 1 + tests/end2end/fixtures/quteprocess.py | 3 ++- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/qutebrowser/browser/webengine/certificateerror.py b/qutebrowser/browser/webengine/certificateerror.py index b9d254c7f..19b59c522 100644 --- a/qutebrowser/browser/webengine/certificateerror.py +++ b/qutebrowser/browser/webengine/certificateerror.py @@ -19,8 +19,9 @@ """Wrapper over a QWebEngineCertificateError.""" - +# pylint: disable=no-name-in-module,import-error,useless-suppression from PyQt5.QtWebEngineWidgets import QWebEngineCertificateError +# pylint: enable=no-name-in-module,import-error,useless-suppression from qutebrowser.utils import usertypes, utils, debug diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index 4e3617b8a..a1e19f37d 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -132,7 +132,8 @@ class WebEnginePage(QWebEnginePage): QWebEnginePage.Geolocation: ('content', 'geolocation'), QWebEnginePage.MediaAudioCapture: ('content', 'media-capture'), QWebEnginePage.MediaVideoCapture: ('content', 'media-capture'), - QWebEnginePage.MediaAudioVideoCapture: ('content', 'media-capture'), + QWebEnginePage.MediaAudioVideoCapture: + ('content', 'media-capture'), } messages = { QWebEnginePage.Geolocation: 'access your location', @@ -232,7 +233,8 @@ class WebEnginePage(QWebEnginePage): # WORKAROUND # Can't override javaScriptPrompt with older PyQt versions # https://www.riverbankcomputing.com/pipermail/pyqt/2016-November/038293.html - def javaScriptPrompt(self, url, js_msg, default, result): + def javaScriptPrompt(self, url, js_msg, default): + """Override javaScriptPrompt to use qutebrowser prompts.""" if self._is_shutting_down: return (False, "") try: diff --git a/scripts/dev/run_vulture.py b/scripts/dev/run_vulture.py index 3d8fa7e17..ade05ac94 100755 --- a/scripts/dev/run_vulture.py +++ b/scripts/dev/run_vulture.py @@ -98,6 +98,7 @@ def whitelist_generator(): yield 'scripts.dev.pylint_checkers.config.' + attr yield 'scripts.dev.pylint_checkers.modeline.process_module' + yield 'scripts.dev.pylint_checkers.qute_pylint.config.msgs' for attr in ['_get_default_metavar_for_optional', '_get_default_metavar_for_positional', '_metavar_formatter']: diff --git a/tests/end2end/fixtures/quteprocess.py b/tests/end2end/fixtures/quteprocess.py index 4db75e55b..53f3bee88 100644 --- a/tests/end2end/fixtures/quteprocess.py +++ b/tests/end2end/fixtures/quteprocess.py @@ -235,7 +235,8 @@ class QuteProc(testprocess.Process): except testprocess.InvalidLine: if not line.strip(): return None - elif is_ignored_qt_message(line) or is_ignored_lowlevel_message(line): + elif (is_ignored_qt_message(line) or + is_ignored_lowlevel_message(line)): return None else: raise From 29cb9279e5da47bc3761ea10aac85a417333e2c6 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 22:03:12 +0100 Subject: [PATCH 14/27] Emit shutting_down signal when a WebEngineTab shuts down Fixes #2109 --- qutebrowser/browser/webengine/webenginetab.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 7150187a2..ab82d2f83 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -538,6 +538,7 @@ class WebEngineTab(browsertab.AbstractTab): self._widget.page().runJavaScript(code, callback) def shutdown(self): + self.shutting_down.emit() self._widget.shutdown() def reload(self, *, force=False): From b270c69ea70cd3283765af2277a3b5e63068bc0a Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 22:16:26 +0100 Subject: [PATCH 15/27] Improve authentication dialog with no realm --- qutebrowser/browser/shared.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/qutebrowser/browser/shared.py b/qutebrowser/browser/shared.py index fa086cf2c..b58ffade5 100644 --- a/qutebrowser/browser/shared.py +++ b/qutebrowser/browser/shared.py @@ -53,9 +53,13 @@ def custom_headers(): def authentication_required(url, authenticator, abort_on): """Ask a prompt for an authentication question.""" - msg = '{} says:
{}'.format( - html.escape(url.toDisplayString()), - html.escape(authenticator.realm())) + realm = authenticator.realm() + if realm: + msg = '{} says:
{}'.format( + html.escape(url.toDisplayString()), html.escape(realm)) + else: + msg = '{} needs authentication'.format( + html.escape(url.toDisplayString())) answer = message.ask(title="Authentication required", text=msg, mode=usertypes.PromptMode.user_pwd, abort_on=abort_on) From a9d48753efac02e9d1cead2ef51da2d1e3eb8a3d Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 22:23:56 +0100 Subject: [PATCH 16/27] Adjust docstring --- qutebrowser/browser/webkit/certificateerror.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/browser/webkit/certificateerror.py b/qutebrowser/browser/webkit/certificateerror.py index f50a58f79..41cf2866f 100644 --- a/qutebrowser/browser/webkit/certificateerror.py +++ b/qutebrowser/browser/webkit/certificateerror.py @@ -17,7 +17,7 @@ # You should have received a copy of the GNU General Public License # along with qutebrowser. If not, see . -"""Wrapper over a QSslCertificateError.""" +"""Wrapper over a QSslError.""" from PyQt5.QtNetwork import QSslError From a6215be864cc8d098edd4ec951862cff63ff820b Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 22:25:32 +0100 Subject: [PATCH 17/27] Get rid of NetworkManager._ask --- .../browser/webkit/network/networkmanager.py | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/qutebrowser/browser/webkit/network/networkmanager.py b/qutebrowser/browser/webkit/network/networkmanager.py index 5da5250b4..100db9ab0 100644 --- a/qutebrowser/browser/webkit/network/networkmanager.py +++ b/qutebrowser/browser/webkit/network/networkmanager.py @@ -197,23 +197,6 @@ class NetworkManager(QNetworkAccessManager): abort_on.append(tab.load_started) return abort_on - def _ask(self, title, text, mode, owner=None, default=None): - """Ask a blocking question in the statusbar. - - Args: - title: The title to display to the user. - text: The text to display to the user. - mode: A PromptMode. - owner: An object which will abort the question if destroyed, or - None. - - Return: - The answer the user gave or None if the prompt was cancelled. - """ - abort_on = self._get_abort_signals(owner) - return message.ask(title=title, text=text, mode=mode, - abort_on=abort_on, default=default) - def shutdown(self): """Abort all running requests.""" self.setNetworkAccessible(QNetworkAccessManager.NotAccessible) @@ -325,9 +308,10 @@ class NetworkManager(QNetworkAccessManager): msg = '{} says:
{}'.format( html.escape(proxy.hostName()), html.escape(authenticator.realm())) - answer = self._ask( - "Proxy authentication required", msg, - mode=usertypes.PromptMode.user_pwd) + abort_on = self._get_abort_signals() + answer = message.ask( + title="Proxy authentication required", text=msg, + mode=usertypes.PromptMode.user_pwd, abort_on=abort_on) if answer is not None: authenticator.setUser(answer.user) authenticator.setPassword(answer.password) From 40c5c75a6c4665c07f49e5f556e0c8d33c3e45dd Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 22:37:53 +0100 Subject: [PATCH 18/27] tests: Add a js_prompt marker --- pytest.ini | 1 + tests/conftest.py | 9 +++++++++ tests/end2end/features/misc.feature | 2 +- tests/end2end/features/prompts.feature | 14 +++++++------- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/pytest.ini b/pytest.ini index 0c2d00f21..3ab397c10 100644 --- a/pytest.ini +++ b/pytest.ini @@ -21,6 +21,7 @@ markers = qtwebengine_createWindow: Tests using createWindow with QtWebEngine (QTBUG-54419) qtwebengine_flaky: Tests which are flaky (and currently skipped) with QtWebEngine qtwebengine_osx_xfail: Tests which fail on OS X with QtWebEngine + js_prompt: Tests needing to display a javascript prompt this: Used to mark tests during development qt_log_level_fail = WARNING qt_log_ignore = diff --git a/tests/conftest.py b/tests/conftest.py index 907656b80..3b3fc3248 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,6 +27,7 @@ import warnings import pytest import hypothesis +from PyQt5.QtCore import PYQT_VERSION pytest.register_assert_rewrite('helpers') @@ -122,6 +123,14 @@ def pytest_collection_modifyitems(config, items): item.add_marker(pytest.mark.xfail(run=False)) if item.get_marker('flaky_once'): item.add_marker(pytest.mark.flaky()) + if item.get_marker('js_prompt'): + if config.webengine: + js_prompt_pyqt_version = 0x050700 + else: + js_prompt_pyqt_version = 0x050300 + item.add_marker(pytest.mark.skipif( + PYQT_VERSION <= js_prompt_pyqt_version, + reason='JS prompts are not supported with this PyQt version')) if deselected: deselected_items.append(item) diff --git a/tests/end2end/features/misc.feature b/tests/end2end/features/misc.feature index 46e4dc287..40f17d1a0 100644 --- a/tests/end2end/features/misc.feature +++ b/tests/end2end/features/misc.feature @@ -382,7 +382,7 @@ Feature: Various utility commands. And I press the key "" Then no crash should happen - @pyqt>=5.3.1 @qtwebengine_skip: JS prompt is not implemented yet + @js_prompt Scenario: Focusing download widget via Tab (original issue) When I open data/prompt/jsprompt.html And I run :click-element id button diff --git a/tests/end2end/features/prompts.feature b/tests/end2end/features/prompts.feature index 3146c2d5b..978e79406 100644 --- a/tests/end2end/features/prompts.feature +++ b/tests/end2end/features/prompts.feature @@ -40,7 +40,7 @@ Feature: Prompts And I run :leave-mode Then the javascript message "confirm reply: false" should be logged - @pyqt>=5.3.1 @qtwebengine_skip + @js_prompt Scenario: Javascript prompt When I open data/prompt/jsprompt.html And I run :click-element id button @@ -49,7 +49,7 @@ Feature: Prompts And I run :prompt-accept Then the javascript message "Prompt reply: prompt test" should be logged - @pyqt>=5.3.1 @qtwebengine_skip + @js_prompt Scenario: Javascript prompt with default When I open data/prompt/jsprompt.html And I run :click-element id button-default @@ -57,7 +57,7 @@ Feature: Prompts And I run :prompt-accept Then the javascript message "Prompt reply: default" should be logged - @pyqt>=5.3.1 @qtwebengine_skip + @js_prompt Scenario: Rejected javascript prompt When I open data/prompt/jsprompt.html And I run :click-element id button @@ -135,7 +135,7 @@ Feature: Prompts # Shift-Insert with prompt (issue 1299) - @pyqt>=5.3.1 @qtwebengine_skip + @js_prompt Scenario: Pasting via shift-insert in prompt mode When selection is supported And I put "insert test" into the primary selection @@ -146,7 +146,7 @@ Feature: Prompts And I run :prompt-accept Then the javascript message "Prompt reply: insert test" should be logged - @pyqt>=5.3.1 @qtwebengine_skip + @js_prompt Scenario: Pasting via shift-insert without it being supported When selection is not supported And I put "insert test" into the primary selection @@ -157,7 +157,7 @@ Feature: Prompts And I run :prompt-accept Then the javascript message "Prompt reply: " should be logged - @pyqt>=5.3.1 @qtwebengine_skip + @js_prompt Scenario: Using content -> ignore-javascript-prompt When I set content -> ignore-javascript-prompt to true And I open data/prompt/jsprompt.html @@ -357,7 +357,7 @@ Feature: Prompts Then the javascript message "Alert done" should be logged And the error "No value is permitted with alert prompts!" should be shown - @pyqt>=5.3.1 @qtwebengine_skip + @js_prompt Scenario: Javascript prompt with value When I set content -> ignore-javascript-prompt to false And I open data/prompt/jsprompt.html From 2ded5ef6dd72d3f91240c4e59fbcba7a98030bcf Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 10 Nov 2016 22:40:16 +0100 Subject: [PATCH 19/27] tests: Get rid of flaky_once mark --- pytest.ini | 1 - tests/conftest.py | 2 -- tests/end2end/features/history.feature | 2 +- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/pytest.ini b/pytest.ini index 3ab397c10..109e30fde 100644 --- a/pytest.ini +++ b/pytest.ini @@ -14,7 +14,6 @@ markers = end2end: End to end tests which run qutebrowser as subprocess xfail_norun: xfail the test with out running it ci: Tests which should only run on CI. - flaky_once: Try to rerun this test once if it fails qtwebengine_todo: Features still missing with QtWebEngine qtwebengine_skip: Tests not applicable with QtWebEngine qtwebkit_skip: Tests not applicable with QtWebKit diff --git a/tests/conftest.py b/tests/conftest.py index 3b3fc3248..7fa94ffc3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -121,8 +121,6 @@ def pytest_collection_modifyitems(config, items): _apply_platform_markers(item) if item.get_marker('xfail_norun'): item.add_marker(pytest.mark.xfail(run=False)) - if item.get_marker('flaky_once'): - item.add_marker(pytest.mark.flaky()) if item.get_marker('js_prompt'): if config.webengine: js_prompt_pyqt_version = 0x050700 diff --git a/tests/end2end/features/history.feature b/tests/end2end/features/history.feature index 165bdd792..529cced46 100644 --- a/tests/end2end/features/history.feature +++ b/tests/end2end/features/history.feature @@ -34,7 +34,7 @@ Feature: Page history Then the history file should contain: http://localhost:(port)/data/%C3%A4%C3%B6%C3%BC.html Chäschüechli - @flaky_once @qtwebengine_todo: Error page message is not implemented + @flaky @qtwebengine_todo: Error page message is not implemented Scenario: History with an error When I run :open file:///does/not/exist And I wait for "Error while loading file:///does/not/exist: Error opening /does/not/exist: *" in the log From 5de07246be9e0c34ebdad8a6da297b50ee6b2826 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 14 Nov 2016 06:45:52 +0100 Subject: [PATCH 20/27] Handle clicks via createWindow with QtWebEngine Before, we used the same logic for QtWebKit and QtWebEngine, where we simply set an attribute on the tab on a mousePressEvent and then handled opening links in acceptNavigationRequest. However, this caused random links to pop up in new tabs (probably to things being much more async?) on QtWebEngine, so we now handle those cases in createWindow and only use override_target from the tab there. Fixes #2102. --- qutebrowser/browser/browsertab.py | 48 +----------- qutebrowser/browser/mouse.py | 22 ------ qutebrowser/browser/shared.py | 29 ++++++- qutebrowser/browser/webengine/webenginetab.py | 1 - qutebrowser/browser/webengine/webview.py | 78 ++++++++++--------- qutebrowser/browser/webkit/webkittab.py | 3 +- qutebrowser/browser/webkit/webpage.py | 33 +++++--- qutebrowser/browser/webkit/webview.py | 22 ++++++ 8 files changed, 118 insertions(+), 118 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 22f1c7e51..e8d0a4099 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -30,7 +30,7 @@ from qutebrowser.config import config from qutebrowser.utils import (utils, objreg, usertypes, message, log, qtutils, urlutils) from qutebrowser.misc import miscwidgets -from qutebrowser.browser import mouse, hints +from qutebrowser.browser import mouse, hints, shared tab_id_gen = itertools.count(0) @@ -84,7 +84,6 @@ class TabData: load. inspector: The QWebInspector used for this webview. viewing_source: Set if we're currently showing a source view. - open_target: How the next clicked link should be opened. override_target: Override for open_target for fake clicks (like hints). """ @@ -92,15 +91,8 @@ class TabData: self.keep_icon = False self.viewing_source = False self.inspector = None - self.open_target = usertypes.ClickTarget.normal self.override_target = None - def combined_target(self): - if self.override_target is not None: - return self.override_target - else: - return self.open_target - class AbstractPrinting: @@ -610,44 +602,6 @@ class AbstractTab(QWidget): evt.posted = True QApplication.postEvent(recipient, evt) - @pyqtSlot(QUrl) - def _on_link_clicked(self, url): - log.webview.debug("link clicked: url {}, override target {}, " - "open_target {}".format( - url.toDisplayString(), - self.data.override_target, - self.data.open_target)) - - if not url.isValid(): - msg = urlutils.get_errstring(url, "Invalid link clicked") - message.error(msg) - self.data.open_target = usertypes.ClickTarget.normal - return False - - target = self.data.combined_target() - - if target == usertypes.ClickTarget.normal: - return - elif target == usertypes.ClickTarget.tab: - win_id = self.win_id - bg_tab = False - elif target == usertypes.ClickTarget.tab_bg: - win_id = self.win_id - bg_tab = True - elif target == usertypes.ClickTarget.window: - from qutebrowser.mainwindow import mainwindow - window = mainwindow.MainWindow() - window.show() - win_id = window.win_id - bg_tab = False - else: - raise ValueError("Invalid ClickTarget {}".format(target)) - - tabbed_browser = objreg.get('tabbed-browser', scope='window', - window=win_id) - tabbed_browser.tabopen(url, background=bg_tab) - self.data.open_target = usertypes.ClickTarget.normal - @pyqtSlot(QUrl) def _on_url_changed(self, url): """Update title when URL has changed and no title is available.""" diff --git a/qutebrowser/browser/mouse.py b/qutebrowser/browser/mouse.py index 63b5b8751..db5055d51 100644 --- a/qutebrowser/browser/mouse.py +++ b/qutebrowser/browser/mouse.py @@ -96,7 +96,6 @@ class MouseEventFilter(QObject): return True self._ignore_wheel_event = True - self._mousepress_opentarget(e) self._tab.elements.find_at_pos(e.pos(), self._mousepress_insertmode_cb) return False @@ -197,27 +196,6 @@ class MouseEventFilter(QObject): else: message.error("At end of history.") - def _mousepress_opentarget(self, e): - """Set the open target when something was clicked. - - Args: - e: The QMouseEvent. - """ - if e.button() == Qt.MidButton or e.modifiers() & Qt.ControlModifier: - background_tabs = config.get('tabs', 'background-tabs') - if e.modifiers() & Qt.ShiftModifier: - background_tabs = not background_tabs - if background_tabs: - target = usertypes.ClickTarget.tab_bg - else: - target = usertypes.ClickTarget.tab - self._tab.data.open_target = target - log.mouse.debug("Ctrl/Middle click, setting target: {}".format( - target)) - else: - self._tab.data.open_target = usertypes.ClickTarget.normal - log.mouse.debug("Normal click, setting normal target") - def eventFilter(self, obj, event): """Filter events going to a QWeb(Engine)View.""" evtype = event.type() diff --git a/qutebrowser/browser/shared.py b/qutebrowser/browser/shared.py index b58ffade5..e2acd96d5 100644 --- a/qutebrowser/browser/shared.py +++ b/qutebrowser/browser/shared.py @@ -24,7 +24,7 @@ import html import jinja2 from qutebrowser.config import config -from qutebrowser.utils import usertypes, message, log +from qutebrowser.utils import usertypes, message, log, objreg class CallSuper(Exception): @@ -195,3 +195,30 @@ def feature_permission(url, option, msg, yes_action, no_action, abort_on): else: no_action() return None + + +def get_tab(win_id, target): + """Get a tab widget for the given usertypes.ClickTarget. + + Args: + win_id: The window ID to open new tabs in + target: An usertypes.ClickTarget + """ + if target == usertypes.ClickTarget.tab: + win_id = win_id + bg_tab = False + elif target == usertypes.ClickTarget.tab_bg: + win_id = win_id + bg_tab = True + elif target == usertypes.ClickTarget.window: + from qutebrowser.mainwindow import mainwindow + window = mainwindow.MainWindow() + window.show() + win_id = window.win_id + bg_tab = False + else: + raise ValueError("Invalid ClickTarget {}".format(target)) + + tabbed_browser = objreg.get('tabbed-browser', scope='window', + window=win_id) + return tabbed_browser.tabopen(url=None, background=bg_tab) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index afb687c86..66aaf733d 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -616,7 +616,6 @@ class WebEngineTab(browsertab.AbstractTab): view.urlChanged.connect(self._on_url_changed) page.loadFinished.connect(self._on_load_finished) page.certificate_error.connect(self._on_ssl_errors) - page.link_clicked.connect(self._on_link_clicked) page.authenticationRequired.connect(self._on_authentication_required) try: view.iconChanged.connect(self.icon_changed) diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index fbb47b23e..d927c8c32 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -30,7 +30,8 @@ from PyQt5.QtWebEngineWidgets import QWebEngineView, QWebEnginePage from qutebrowser.browser import shared from qutebrowser.browser.webengine import certificateerror from qutebrowser.config import config -from qutebrowser.utils import log, debug, usertypes, objreg, qtutils, jinja +from qutebrowser.utils import (log, debug, usertypes, objreg, qtutils, jinja, + urlutils, message) class WebEngineView(QWebEngineView): @@ -40,7 +41,8 @@ class WebEngineView(QWebEngineView): def __init__(self, tabdata, win_id, parent=None): super().__init__(parent) self._win_id = win_id - self.setPage(WebEnginePage(tabdata, parent=self)) + self._tabdata = tabdata + self.setPage(WebEnginePage(parent=self)) def shutdown(self): self.page().shutdown() @@ -71,24 +73,40 @@ class WebEngineView(QWebEngineView): The new QWebEngineView object. """ debug_type = debug.qenum_key(QWebEnginePage, wintype) - log.webview.debug("createWindow with type {}".format(debug_type)) + background_tabs = config.get('tabs', 'background-tabs') + override_target = self._tabdata.override_target - background = False - if wintype in [QWebEnginePage.WebBrowserWindow, - QWebEnginePage.WebDialog]: + log.webview.debug("createWindow with type {}, background_tabs " + "{}, override_target {}".format( + debug_type, background_tabs, override_target)) + + if override_target is not None: + target = override_target + elif wintype == QWebEnginePage.WebBrowserWindow: + log.webview.debug("createWindow with WebBrowserWindow - when does " + "this happen?!") + target = usertypes.ClickTarget.window + elif wintype == QWebEnginePage.WebDialog: log.webview.warning("{} requested, but we don't support " "that!".format(debug_type)) + target = usertypes.ClickTarget.tab elif wintype == QWebEnginePage.WebBrowserTab: - pass + # Middle-click / Ctrl-Click with Shift + if background_tabs: + target = usertypes.ClickTarget.tab + else: + target = usertypes.ClickTarget.tab_bg elif (hasattr(QWebEnginePage, 'WebBrowserBackgroundTab') and wintype == QWebEnginePage.WebBrowserBackgroundTab): - background = True + # Middle-click / Ctrl-Click + if background_tabs: + target = usertypes.ClickTarget.tab_bg + else: + target = usertypes.ClickTarget.tab else: raise ValueError("Invalid wintype {}".format(debug_type)) - tabbed_browser = objreg.get('tabbed-browser', scope='window', - window=self._win_id) - tab = tabbed_browser.tabopen(background=background) + tab = shared.get_tab(self._win_id, target) # WORKAROUND for https://bugreports.qt.io/browse/QTBUG-54419 vercheck = qtutils.version_check @@ -110,17 +128,14 @@ class WebEnginePage(QWebEnginePage): Signals: certificate_error: Emitted on certificate errors. - link_clicked: Emitted when a link was clicked on a page. shutting_down: Emitted when the page is shutting down. """ certificate_error = pyqtSignal() - link_clicked = pyqtSignal(QUrl) shutting_down = pyqtSignal() - def __init__(self, tabdata, parent=None): + def __init__(self, parent=None): super().__init__(parent) - self._tabdata = tabdata self._is_shutting_down = False self.featurePermissionRequested.connect( self._on_feature_permission_requested) @@ -277,24 +292,17 @@ class WebEnginePage(QWebEnginePage): is_main_frame: bool): """Override acceptNavigationRequest to handle clicked links. - Setting linkDelegationPolicy to DelegateAllLinks and using a slot bound - to linkClicked won't work correctly, because when in a frameset, we - have no idea in which frame the link should be opened. - - Checks if it should open it in a tab (middle-click or control) or not, - and then conditionally opens the URL. Opening it in a new tab/window - is handled in the slot connected to link_clicked. + This only show an error on invalid links - everything else is handled + in createWindow. """ - target = self._tabdata.combined_target() - log.webview.debug("navigation request: url {}, type {}, " - "target {}, is_main_frame {}".format( - url.toDisplayString(), - debug.qenum_key(QWebEnginePage, typ), - target, is_main_frame)) - - if typ != QWebEnginePage.NavigationTypeLinkClicked: - return True - - self.link_clicked.emit(url) - - return url.isValid() and target == usertypes.ClickTarget.normal + log.webview.debug("navigation request: url {}, type {}, is_main_frame " + "{}".format(url.toDisplayString(), + debug.qenum_key(QWebEnginePage, typ), + is_main_frame)) + if (typ == QWebEnginePage.NavigationTypeLinkClicked and + not url.isValid()): + msg = urlutils.get_errstring(url, "Invalid link clicked") + message.error(msg) + self.page().open_target = usertypes.ClickTarget.normal + return False + return True diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 61de1bd54..9362e5d75 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -329,7 +329,7 @@ class WebKitCaret(browsertab.AbstractCaret): if QWebSettings.globalSettings().testAttribute( QWebSettings.JavascriptEnabled): if tab: - self._tab.data.open_target = usertypes.ClickTarget.tab + self._tab.data.override_target = usertypes.ClickTarget.tab self._tab.run_js_async( 'window.getSelection().anchorNode.parentNode.click()') else: @@ -714,7 +714,6 @@ class WebKitTab(browsertab.AbstractTab): page.frameCreated.connect(self._on_frame_created) frame.contentsSizeChanged.connect(self._on_contents_size_changed) frame.initialLayoutCompleted.connect(self._on_history_trigger) - page.link_clicked.connect(self._on_link_clicked) def _event_target(self): return self._widget diff --git a/qutebrowser/browser/webkit/webpage.py b/qutebrowser/browser/webkit/webpage.py index bf41ec10a..d7cf7cdf1 100644 --- a/qutebrowser/browser/webkit/webpage.py +++ b/qutebrowser/browser/webkit/webpage.py @@ -34,7 +34,7 @@ from qutebrowser.browser import pdfjs, shared from qutebrowser.browser.webkit import http from qutebrowser.browser.webkit.network import networkmanager from qutebrowser.utils import (message, usertypes, log, jinja, qtutils, utils, - objreg, debug) + objreg, debug, urlutils) class BrowserPage(QWebPage): @@ -54,12 +54,10 @@ class BrowserPage(QWebPage): shutting_down: Emitted when the page is currently shutting down. reloading: Emitted before a web page reloads. arg: The URL which gets reloaded. - link_clicked: Emitted when a link was clicked on a page. """ shutting_down = pyqtSignal() reloading = pyqtSignal(QUrl) - link_clicked = pyqtSignal(QUrl) def __init__(self, win_id, tab_id, tabdata, parent=None): super().__init__(parent) @@ -72,6 +70,7 @@ class BrowserPage(QWebPage): } self._ignore_load_started = False self.error_occurred = False + self.open_target = usertypes.ClickTarget.normal self._networkmanager = networkmanager.NetworkManager( win_id, tab_id, self) self.setNetworkAccessManager(self._networkmanager) @@ -462,16 +461,20 @@ class BrowserPage(QWebPage): have no idea in which frame the link should be opened. Checks if it should open it in a tab (middle-click or control) or not, - and then conditionally opens the URL. Opening it in a new tab/window - is handled in the slot connected to link_clicked. + and then conditionally opens the URL here or in another tab/window. """ url = request.url() - target = self._tabdata.combined_target() log.webview.debug("navigation request: url {}, type {}, " - "target {}".format( + "target {} override {}".format( url.toDisplayString(), debug.qenum_key(QWebPage, typ), - target)) + self.open_target, + self._tabdata.override_target)) + + if self._tabdata.override_target is not None: + target = self._tabdata.override_target + else: + target = self.open_target if typ == QWebPage.NavigationTypeReload: self.reloading.emit(url) @@ -479,6 +482,16 @@ class BrowserPage(QWebPage): elif typ != QWebPage.NavigationTypeLinkClicked: return True - self.link_clicked.emit(url) + if not url.isValid(): + msg = urlutils.get_errstring(url, "Invalid link clicked") + message.error(msg) + self.open_target = usertypes.ClickTarget.normal + return False - return url.isValid() and target == usertypes.ClickTarget.normal + if target == usertypes.ClickTarget.normal: + return True + + tab = shared.get_tab(self._win_id, target) + tab.openurl(url) + self.open_target = usertypes.ClickTarget.normal + return False diff --git a/qutebrowser/browser/webkit/webview.py b/qutebrowser/browser/webkit/webview.py index 94519fa86..7795f3962 100644 --- a/qutebrowser/browser/webkit/webview.py +++ b/qutebrowser/browser/webkit/webview.py @@ -64,6 +64,7 @@ class WebView(QWebView): # FIXME:qtwebengine this is only used to set the zoom factor from # the QWebPage - we should get rid of it somehow (signals?) self.tab = tab + self._tabdata = tab.data self.win_id = win_id self.scroll_pos = (-1, -1) self._old_scroll_pos = (-1, -1) @@ -280,3 +281,24 @@ class WebView(QWebView): pass super().hideEvent(e) + + def mousePressEvent(self, e): + """Set the tabdata ClickTarget on a mousepress. + + This is implemented here as we don't need it for QtWebEngine. + """ + if e.button() == Qt.MidButton or e.modifiers() & Qt.ControlModifier: + background_tabs = config.get('tabs', 'background-tabs') + if e.modifiers() & Qt.ShiftModifier: + background_tabs = not background_tabs + if background_tabs: + target = usertypes.ClickTarget.tab_bg + else: + target = usertypes.ClickTarget.tab + self.page().open_target = target + log.mouse.debug("Ctrl/Middle click, setting target: {}".format( + target)) + else: + self.page().open_target = usertypes.ClickTarget.normal + log.mouse.debug("Normal click, setting normal target") + super().mousePressEvent(e) From 30827c12390cabc8c8dc9f697ca90fb8ca0b6785 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 14 Nov 2016 07:11:52 +0100 Subject: [PATCH 21/27] Fix rapid hinting with QtWebEngine There were actually two issues here: - The override_target was reset too early - now acceptNavigationRequest/createWindow is responsible for resetting it. - The QTBUG-54419 workaround opened any tabs in the background instead of preserving their position/focus. Fixes #2086. --- qutebrowser/browser/webelem.py | 3 +-- qutebrowser/browser/webengine/webview.py | 1 + qutebrowser/browser/webkit/webpage.py | 1 + qutebrowser/mainwindow/tabbedbrowser.py | 9 +++++++-- tests/end2end/features/hints.feature | 12 ++++++++++++ 5 files changed, 22 insertions(+), 4 deletions(-) diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py index 2903aec72..cd56a9127 100644 --- a/qutebrowser/browser/webelem.py +++ b/qutebrowser/browser/webelem.py @@ -365,10 +365,9 @@ class AbstractWebElement(collections.abc.MutableMapping): self._tab.send_event(evt) def after_click(): - """Move cursor to end and reset override_target after clicking.""" + """Move cursor to end after clicking.""" if self.is_text_input() and self.is_editable(): self._tab.caret.move_to_end_of_document() - self._tab.data.override_target = None QTimer.singleShot(0, after_click) def hover(self): diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index d927c8c32..98bf75b99 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -82,6 +82,7 @@ class WebEngineView(QWebEngineView): if override_target is not None: target = override_target + self._tabdata.override_target = None elif wintype == QWebEnginePage.WebBrowserWindow: log.webview.debug("createWindow with WebBrowserWindow - when does " "this happen?!") diff --git a/qutebrowser/browser/webkit/webpage.py b/qutebrowser/browser/webkit/webpage.py index d7cf7cdf1..464fc3785 100644 --- a/qutebrowser/browser/webkit/webpage.py +++ b/qutebrowser/browser/webkit/webpage.py @@ -473,6 +473,7 @@ class BrowserPage(QWebPage): if self._tabdata.override_target is not None: target = self._tabdata.override_target + self._tabdata.override_target = None else: target = self.open_target diff --git a/qutebrowser/mainwindow/tabbedbrowser.py b/qutebrowser/mainwindow/tabbedbrowser.py index e38610ad2..4198c35c6 100644 --- a/qutebrowser/mainwindow/tabbedbrowser.py +++ b/qutebrowser/mainwindow/tabbedbrowser.py @@ -369,7 +369,10 @@ class TabbedBrowser(tabwidget.TabWidget): """ if url is not None: qtutils.ensure_valid(url) - log.webview.debug("Creating new tab with URL {}".format(url)) + log.webview.debug("Creating new tab with URL {}, background {}, " + "explicit {}, idx {}".format( + url, background, explicit, idx)) + if config.get('tabs', 'tabs-are-windows') and self.count() > 0: from qutebrowser.mainwindow import mainwindow window = mainwindow.MainWindow() @@ -521,13 +524,15 @@ class TabbedBrowser(tabwidget.TabWidget): # If needed, re-open the tab as a workaround for QTBUG-54419. # See https://bugreports.qt.io/browse/QTBUG-54419 + background = self.currentIndex() != idx + if (tab.backend == usertypes.Backend.QtWebEngine and tab.needs_qtbug54419_workaround): log.misc.debug("Doing QTBUG-54419 workaround for {}, " "url {}".format(tab, url)) self.setUpdatesEnabled(False) try: - self.tabopen(url) + self.tabopen(url, background=background, idx=idx) self.close_tab(tab, add_undo=False) finally: self.setUpdatesEnabled(True) diff --git a/tests/end2end/features/hints.feature b/tests/end2end/features/hints.feature index 0f44adff4..5369cb56f 100644 --- a/tests/end2end/features/hints.feature +++ b/tests/end2end/features/hints.feature @@ -112,6 +112,18 @@ Feature: Using hints And I hint with args "links yank-primary" and follow a Then the clipboard should contain "http://localhost:(port)/data/hello.txt" + Scenario: Rapid hinting + When I open data/hints/rapid.html + And I run :tab-only + And I hint with args "all tab-bg --rapid" + And I run :follow-hint a + And I run :follow-hint s + And I run :leave-mode + Then the following tabs should be open: + - data/hints/rapid.html (active) + - data/hello.txt + - data/hello2.txt + Scenario: Using hint --rapid to hit multiple buttons When I open data/hints/buttons.html And I hint with args "--rapid" From 004d0b7ae59d9a69a62c9fa26003ae2f4d300da7 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 14 Nov 2016 08:31:57 +0100 Subject: [PATCH 22/27] Add missing rapid.html --- tests/end2end/data/hints/rapid.html | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 tests/end2end/data/hints/rapid.html diff --git a/tests/end2end/data/hints/rapid.html b/tests/end2end/data/hints/rapid.html new file mode 100644 index 000000000..b8184b383 --- /dev/null +++ b/tests/end2end/data/hints/rapid.html @@ -0,0 +1,12 @@ + + + + + + Two links + + + Hello + Hello 2 + + From 781a326648ec54a31ad3cca1810d6374398357eb Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 14 Nov 2016 08:33:50 +0100 Subject: [PATCH 23/27] Fix lint --- qutebrowser/browser/browsertab.py | 5 ++--- qutebrowser/browser/shared.py | 2 +- qutebrowser/browser/webengine/webview.py | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index e8d0a4099..cbf566847 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -27,10 +27,9 @@ from PyQt5.QtWidgets import QWidget, QApplication from qutebrowser.keyinput import modeman from qutebrowser.config import config -from qutebrowser.utils import (utils, objreg, usertypes, message, log, qtutils, - urlutils) +from qutebrowser.utils import utils, objreg, usertypes, log, qtutils from qutebrowser.misc import miscwidgets -from qutebrowser.browser import mouse, hints, shared +from qutebrowser.browser import mouse, hints tab_id_gen = itertools.count(0) diff --git a/qutebrowser/browser/shared.py b/qutebrowser/browser/shared.py index e2acd96d5..ea1597ee5 100644 --- a/qutebrowser/browser/shared.py +++ b/qutebrowser/browser/shared.py @@ -202,7 +202,7 @@ def get_tab(win_id, target): Args: win_id: The window ID to open new tabs in - target: An usertypes.ClickTarget + target: A usertypes.ClickTarget """ if target == usertypes.ClickTarget.tab: win_id = win_id diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index 98bf75b99..1451d2f5a 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -30,8 +30,8 @@ from PyQt5.QtWebEngineWidgets import QWebEngineView, QWebEnginePage from qutebrowser.browser import shared from qutebrowser.browser.webengine import certificateerror from qutebrowser.config import config -from qutebrowser.utils import (log, debug, usertypes, objreg, qtutils, jinja, - urlutils, message) +from qutebrowser.utils import (log, debug, usertypes, qtutils, jinja, urlutils, + message) class WebEngineView(QWebEngineView): From 69452a9813bd850a6f37735427058b177bbf7bdb Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 14 Nov 2016 09:37:01 +0100 Subject: [PATCH 24/27] Fix hint tests --- tests/end2end/features/hints.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/end2end/features/hints.feature b/tests/end2end/features/hints.feature index 5369cb56f..a8d87b892 100644 --- a/tests/end2end/features/hints.feature +++ b/tests/end2end/features/hints.feature @@ -186,6 +186,7 @@ Feature: Using hints @qtwebengine_createWindow Scenario: Opening a link with specific target frame in a new tab When I open data/hints/iframe_target.html + And I run :tab-only And I hint with args "links tab" and follow a And I wait until data/hello.txt is loaded Then the following tabs should be open: From ba1bcc658e92ba365d6cc6c3c2a78bc8bdec85e4 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 14 Nov 2016 09:37:23 +0100 Subject: [PATCH 25/27] Fix WebEnginePage acceptNavigationRequest --- qutebrowser/browser/webengine/webview.py | 1 - 1 file changed, 1 deletion(-) diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index 1451d2f5a..882e2379f 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -304,6 +304,5 @@ class WebEnginePage(QWebEnginePage): not url.isValid()): msg = urlutils.get_errstring(url, "Invalid link clicked") message.error(msg) - self.page().open_target = usertypes.ClickTarget.normal return False return True From cd3305b4dda2da68ccd83a7715d982c77fb699a8 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 14 Nov 2016 09:42:08 +0100 Subject: [PATCH 26/27] Stabilize :hint --rapid test --- tests/end2end/features/hints.feature | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/end2end/features/hints.feature b/tests/end2end/features/hints.feature index a8d87b892..1abc33131 100644 --- a/tests/end2end/features/hints.feature +++ b/tests/end2end/features/hints.feature @@ -119,6 +119,8 @@ Feature: Using hints And I run :follow-hint a And I run :follow-hint s And I run :leave-mode + And I wait until data/hello.txt is loaded + And I wait until data/hello2.txt is loaded Then the following tabs should be open: - data/hints/rapid.html (active) - data/hello.txt From 81e8421f62f59c3da5ffd0c928325166801ec885 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 14 Nov 2016 10:52:30 +0100 Subject: [PATCH 27/27] Stabilize rapid hinting test more --- tests/end2end/features/hints.feature | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/end2end/features/hints.feature b/tests/end2end/features/hints.feature index 1abc33131..2a8f6082e 100644 --- a/tests/end2end/features/hints.feature +++ b/tests/end2end/features/hints.feature @@ -113,7 +113,7 @@ Feature: Using hints Then the clipboard should contain "http://localhost:(port)/data/hello.txt" Scenario: Rapid hinting - When I open data/hints/rapid.html + When I open data/hints/rapid.html in a new tab And I run :tab-only And I hint with args "all tab-bg --rapid" And I run :follow-hint a @@ -121,10 +121,17 @@ Feature: Using hints And I run :leave-mode And I wait until data/hello.txt is loaded And I wait until data/hello2.txt is loaded - Then the following tabs should be open: - - data/hints/rapid.html (active) - - data/hello.txt - - data/hello2.txt + # We should check what the active tab is, but for some reason that makes + # the test flaky + Then the session should look like: + windows: + - tabs: + - history: + - url: http://localhost:*/data/hints/rapid.html + - history: + - url: http://localhost:*/data/hello.txt + - history: + - url: http://localhost:*/data/hello2.txt Scenario: Using hint --rapid to hit multiple buttons When I open data/hints/buttons.html