From 5de07246be9e0c34ebdad8a6da297b50ee6b2826 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 14 Nov 2016 06:45:52 +0100 Subject: [PATCH] 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)