From 785a88d2976a4f4361ba18aac2b48f15ae736c93 Mon Sep 17 00:00:00 2001 From: Neil Deakin Date: Thu, 14 Nov 2019 00:47:48 +0000 Subject: [PATCH] Bug 1595155, support click handler which allows modifier+click in out of process iframes, r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D52642 --HG-- rename : browser/modules/ContentClick.jsm => browser/actors/ClickHandlerParent.jsm extra : moz-landing-system : lando --- browser/actors/ClickHandlerChild.jsm | 13 ++- .../ClickHandlerParent.jsm} | 79 +++++++++++++------ browser/actors/moz.build | 1 + .../test/general/browser_contentAltClick.js | 43 +++++++++- browser/components/BrowserGlue.jsm | 27 ++++--- browser/modules/moz.build | 1 - devtools/client/responsive/browser/tunnel.js | 4 - .../components/extensions/WebNavigation.jsm | 15 ++-- 8 files changed, 125 insertions(+), 58 deletions(-) rename browser/{modules/ContentClick.jsm => actors/ClickHandlerParent.jsm} (54%) diff --git a/browser/actors/ClickHandlerChild.jsm b/browser/actors/ClickHandlerChild.jsm index 06740dfdc02f..312e9ebdc5dc 100644 --- a/browser/actors/ClickHandlerChild.jsm +++ b/browser/actors/ClickHandlerChild.jsm @@ -5,9 +5,6 @@ var EXPORTED_SYMBOLS = ["ClickHandlerChild"]; -const { ActorChild } = ChromeUtils.import( - "resource://gre/modules/ActorChild.jsm" -); const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); ChromeUtils.defineModuleGetter( @@ -31,7 +28,7 @@ ChromeUtils.defineModuleGetter( "resource://gre/modules/E10SUtils.jsm" ); -class ClickHandlerChild extends ActorChild { +class ClickHandlerChild extends JSWindowActorChild { handleEvent(event) { if ( !event.isTrusted || @@ -122,7 +119,7 @@ class ClickHandlerChild extends ActorChild { // should we allow mixed content. json.allowMixedContent = false; let docshell = ownerDoc.defaultView.docShell; - if (this.mm.docShell.mixedContentChannel) { + if (this.docShell.mixedContentChannel) { const sm = Services.scriptSecurityManager; try { let targetURI = Services.io.newURI(href); @@ -151,13 +148,13 @@ class ClickHandlerChild extends ActorChild { event.preventMultipleActions(); } - this.mm.sendAsyncMessage("Content:Click", json); + this.sendAsyncMessage("Content:Click", json); return; } // This might be middle mouse navigation. if (event.button == 1) { - this.mm.sendAsyncMessage("Content:Click", json); + this.sendAsyncMessage("Content:Click", json); } } @@ -173,7 +170,7 @@ class ClickHandlerChild extends ActorChild { * to behave like an element, which SVG links (XLink) don't. */ _hrefAndLinkNodeForClickEvent(event) { - let { content } = this.mm; + let content = this.contentWindow; function isHTMLLink(aNode) { // Be consistent with what nsContextMenu.js does. return ( diff --git a/browser/modules/ContentClick.jsm b/browser/actors/ClickHandlerParent.jsm similarity index 54% rename from browser/modules/ContentClick.jsm rename to browser/actors/ClickHandlerParent.jsm index c3ef59d1a7c4..454c0fe69b27 100644 --- a/browser/modules/ContentClick.jsm +++ b/browser/actors/ClickHandlerParent.jsm @@ -5,7 +5,7 @@ "use strict"; -var EXPORTED_SYMBOLS = ["ContentClick"]; +var EXPORTED_SYMBOLS = ["ClickHandlerParent"]; const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); @@ -25,34 +25,48 @@ ChromeUtils.defineModuleGetter( "resource://gre/modules/E10SUtils.jsm" ); -var ContentClick = { - // Listeners are added in BrowserGlue.jsm +let gContentClickListeners = new Set(); + +class ClickHandlerParent extends JSWindowActorParent { + static addContentClickListener(listener) { + gContentClickListeners.add(listener); + } + + static removeContentClickListener(listener) { + gContentClickListeners.delete(listener); + } + receiveMessage(message) { switch (message.name) { case "Content:Click": - this.contentAreaClick(message.json, message.target); + this.contentAreaClick(message.data); + this.notifyClickListeners(message.data); break; } - }, + } /** * Handles clicks in the content area. * - * @param json {Object} JSON object that looks like an Event + * @param data {Object} object that looks like an Event * @param browser {Element} */ - contentAreaClick(json, browser) { + contentAreaClick(data) { // This is heavily based on contentAreaClick from browser.js (Bug 903016) - // The json is set up in a way to look like an Event. + // The data is set up in a way to look like an Event. + let browser = this.manager.browsingContext.top.embedderElement; + if (browser.outerBrowser) { + browser = browser.outerBrowser; // handle RDM mode + } let window = browser.ownerGlobal; - if (!json.href) { + if (!data.href) { // Might be middle mouse navigation. if ( Services.prefs.getBoolPref("middlemouse.contentLoadURL") && !Services.prefs.getBoolPref("general.autoScroll") ) { - window.middleMousePaste(json); + window.middleMousePaste(data); } return; } @@ -70,14 +84,14 @@ var ContentClick = { // visits across frames should be preserved. try { if (!PrivateBrowsingUtils.isWindowPrivate(window)) { - PlacesUIUtils.markPageAsFollowedLink(json.href); + PlacesUIUtils.markPageAsFollowedLink(data.href); } } catch (ex) { /* Skip invalid URIs. */ } // This part is based on handleLinkClick. - var where = window.whereToOpenLink(json); + var where = window.whereToOpenLink(data); if (where == "current") { return; } @@ -86,23 +100,38 @@ var ContentClick = { let params = { charset: browser.characterSet, - referrerInfo: E10SUtils.deserializeReferrerInfo(json.referrerInfo), - allowMixedContent: json.allowMixedContent, - isContentWindowPrivate: json.isContentWindowPrivate, - originPrincipal: json.originPrincipal, - originStoragePrincipal: json.originStoragePrincipal, - triggeringPrincipal: json.triggeringPrincipal, - csp: json.csp ? E10SUtils.deserializeCSP(json.csp) : null, - frameOuterWindowID: json.frameOuterWindowID, + referrerInfo: E10SUtils.deserializeReferrerInfo(data.referrerInfo), + allowMixedContent: data.allowMixedContent, + isContentWindowPrivate: data.isContentWindowPrivate, + originPrincipal: data.originPrincipal, + originStoragePrincipal: data.originStoragePrincipal, + triggeringPrincipal: data.triggeringPrincipal, + csp: data.csp ? E10SUtils.deserializeCSP(data.csp) : null, + frameOuterWindowID: data.frameOuterWindowID, }; // The new tab/window must use the same userContextId. - if (json.originAttributes.userContextId) { - params.userContextId = json.originAttributes.userContextId; + if (data.originAttributes.userContextId) { + params.userContextId = data.originAttributes.userContextId; } params.allowInheritPrincipal = true; - window.openLinkIn(json.href, where, params); - }, -}; + window.openLinkIn(data.href, where, params); + } + + notifyClickListeners(data) { + for (let listener of gContentClickListeners) { + try { + let browser = this.browsingContext.top.embedderElement; + if (browser.outerBrowser) { + browser = browser.outerBrowser; // Special-case responsive design mode + } + + listener.onContentClick(browser, data); + } catch (ex) { + Cu.reportError(ex); + } + } + } +} diff --git a/browser/actors/moz.build b/browser/actors/moz.build index cc6a233a0e0b..804d88a9c7b0 100644 --- a/browser/actors/moz.build +++ b/browser/actors/moz.build @@ -28,6 +28,7 @@ FINAL_TARGET_FILES.actors += [ 'BrowserTabChild.jsm', 'BrowserTabParent.jsm', 'ClickHandlerChild.jsm', + 'ClickHandlerParent.jsm', 'ContentMetaChild.jsm', 'ContentMetaParent.jsm', 'ContentSearchChild.jsm', diff --git a/browser/base/content/test/general/browser_contentAltClick.js b/browser/base/content/test/general/browser_contentAltClick.js index d61d17a702b2..4b6bd455ad12 100644 --- a/browser/base/content/test/general/browser_contentAltClick.js +++ b/browser/base/content/test/general/browser_contentAltClick.js @@ -8,7 +8,7 @@ * and confirms those files are on the download list. * * The difference between this and the test "browser_contentAreaClick.js" is that - * the code path in e10s uses ContentClick.jsm instead of browser.js::contentAreaClick() util. + * the code path in e10s uses the ClickHandler actor instead of browser.js::contentAreaClick() util. */ "use strict"; @@ -26,7 +26,8 @@ function setup() { '

Common link

' + '

MathML XLink

' + '

SVG XLink


' + - ''; + '' + + ''; return BrowserTestUtils.openNewForegroundTab(gBrowser, testPage); } @@ -165,3 +166,41 @@ add_task(async function test_alt_click_on_xlinks() { await clean_up(); }); + +// Alt+Click a link in a frame from another domain as the outer document. +add_task(async function test_alt_click_in_frame() { + await setup(); + + let downloadList = await Downloads.getList(Downloads.ALL); + let downloads = []; + let downloadView; + // When the download has been attempted, resolve the promise. + let finishedAllDownloads = new Promise(resolve => { + downloadView = { + onDownloadAdded(aDownload) { + downloads.push(aDownload); + resolve(); + }, + }; + }); + + await downloadList.addView(downloadView); + await BrowserTestUtils.synthesizeMouseAtCenter( + "#linkToExample", + { altKey: true }, + gBrowser.selectedBrowser.browsingContext.getChildren()[0] + ); + + // Wait for all downloads to be added to the download list. + await finishedAllDownloads; + await downloadList.removeView(downloadView); + + is(downloads.length, 1, "1 downloads"); + is( + downloads[0].source.url, + "http://example.org/", + "Downloaded link in iframe." + ); + + await clean_up(); +}); diff --git a/browser/components/BrowserGlue.jsm b/browser/components/BrowserGlue.jsm index 81e1eae6ecde..aeac73d2e412 100644 --- a/browser/components/BrowserGlue.jsm +++ b/browser/components/BrowserGlue.jsm @@ -57,6 +57,21 @@ let ACTORS = { }, }, + ClickHandler: { + parent: { + moduleURI: "resource:///actors/ClickHandlerParent.jsm", + }, + child: { + moduleURI: "resource:///actors/ClickHandlerChild.jsm", + events: { + click: { capture: true, mozSystemGroup: true }, + auxclick: { capture: true, mozSystemGroup: true }, + }, + }, + + allFrames: true, + }, + // Collects description and icon information from meta tags. ContentMeta: { parent: { @@ -313,16 +328,6 @@ let LEGACY_ACTORS = { }, }, - ClickHandler: { - child: { - module: "resource:///actors/ClickHandlerChild.jsm", - events: { - click: { capture: true, mozSystemGroup: true }, - auxclick: { capture: true, mozSystemGroup: true }, - }, - }, - }, - ContentSearch: { child: { module: "resource:///actors/ContentSearchChild.jsm", @@ -567,7 +572,6 @@ XPCOMUtils.defineLazyModuleGetters(this, { XPCOMUtils.defineLazyModuleGetters(this, { AboutLoginsParent: "resource:///modules/AboutLoginsParent.jsm", AsyncPrefs: "resource://gre/modules/AsyncPrefs.jsm", - ContentClick: "resource:///modules/ContentClick.jsm", PluginManager: "resource:///actors/PluginParent.jsm", ReaderParent: "resource:///modules/ReaderParent.jsm", }); @@ -666,7 +670,6 @@ const listeners = { "AboutLogins:SyncEnable": ["AboutLoginsParent"], "AboutLogins:SyncOptions": ["AboutLoginsParent"], "AboutLogins:UpdateLogin": ["AboutLoginsParent"], - "Content:Click": ["ContentClick"], ContentSearch: ["ContentSearch"], "Reader:FaviconRequest": ["ReaderParent"], "Reader:UpdateReaderButton": ["ReaderParent"], diff --git a/browser/modules/moz.build b/browser/modules/moz.build index a52af9c8bb74..39ad9f856282 100644 --- a/browser/modules/moz.build +++ b/browser/modules/moz.build @@ -136,7 +136,6 @@ EXTRA_JS_MODULES += [ 'AsyncTabSwitcher.jsm', 'BrowserUsageTelemetry.jsm', 'BrowserWindowTracker.jsm', - 'ContentClick.jsm', 'ContentCrashHandlers.jsm', 'ContentObservers.js', 'ContentSearch.jsm', diff --git a/devtools/client/responsive/browser/tunnel.js b/devtools/client/responsive/browser/tunnel.js index 431679e4d03b..d7dc3f748a6e 100644 --- a/devtools/client/responsive/browser/tunnel.js +++ b/devtools/client/responsive/browser/tunnel.js @@ -47,10 +47,6 @@ const SWAPPED_BROWSER_STATE = [ const PROPERTIES_FROM_BROWSER_WINDOW = [ // This is used by PermissionUI.jsm for permission doorhangers. "PopupNotifications", - // These are used by ContentClick.jsm when opening links in ways other than just - // navigating the viewport, such as a new tab by pressing Cmd-Click. - "whereToOpenLink", - "openLinkIn", // This is used by various event handlers, typically to call `getTabForBrowser` to map // a browser back to a tab. "gBrowser", diff --git a/toolkit/components/extensions/WebNavigation.jsm b/toolkit/components/extensions/WebNavigation.jsm index 5b6ec0967e53..628da5a6dcec 100644 --- a/toolkit/components/extensions/WebNavigation.jsm +++ b/toolkit/components/extensions/WebNavigation.jsm @@ -23,6 +23,11 @@ ChromeUtils.defineModuleGetter( "UrlbarUtils", "resource:///modules/UrlbarUtils.jsm" ); +ChromeUtils.defineModuleGetter( + this, + "ClickHandlerParent", + "resource:///actors/ClickHandlerParent.jsm" +); // Maximum amount of time that can be passed and still consider // the data recent (similar to how is done in nsNavHistory, @@ -47,7 +52,8 @@ var Manager = { Services.obs.addObserver(this, "webNavigation-createdNavigationTarget"); - Services.mm.addMessageListener("Content:Click", this); + ClickHandlerParent.addContentClickListener(this); + Services.mm.addMessageListener("Extension:DOMContentLoaded", this); Services.mm.addMessageListener("Extension:StateChange", this); Services.mm.addMessageListener("Extension:DocumentChange", this); @@ -65,7 +71,8 @@ var Manager = { Services.obs.removeObserver(this, "urlbar-user-start-navigation"); Services.obs.removeObserver(this, "webNavigation-createdNavigationTarget"); - Services.mm.removeMessageListener("Content:Click", this); + ClickHandlerParent.removeContentClickListener(this); + Services.mm.removeMessageListener("Extension:StateChange", this); Services.mm.removeMessageListener("Extension:DocumentChange", this); Services.mm.removeMessageListener("Extension:HistoryChange", this); @@ -305,10 +312,6 @@ var Manager = { this.onLoad(target, data); break; - case "Content:Click": - this.onContentClick(target, data); - break; - case "Extension:CreatedNavigationTarget": this.onCreatedNavigationTarget(target, data); break;