Bug 1595155, support click handler which allows modifier+click in out of process iframes, r=Gijs
authorNeil Deakin <neil@mozilla.com>
Thu, 14 Nov 2019 00:47:48 +0000
changeset 501862 c4ff245706aba8e0dcbcabfadaf4cb752500b9c6
parent 501861 d4c8a1b56b1ad44c9222fb78a470a5f4bc3b6423
child 501863 1f37c31422e2737db9e6bcc9826facd3fdb15bf2
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1595155
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1595155, support click handler which allows modifier+click in out of process iframes, r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D52642
browser/actors/ClickHandlerChild.jsm
browser/actors/ClickHandlerParent.jsm
browser/actors/moz.build
browser/base/content/test/general/browser_contentAltClick.js
browser/components/BrowserGlue.jsm
browser/modules/ContentClick.jsm
browser/modules/moz.build
devtools/client/responsive/browser/tunnel.js
toolkit/components/extensions/WebNavigation.jsm
--- a/browser/actors/ClickHandlerChild.jsm
+++ b/browser/actors/ClickHandlerChild.jsm
@@ -1,18 +1,15 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 var EXPORTED_SYMBOLS = ["ClickHandlerChild"];
 
-const { ActorChild } = ChromeUtils.import(
-  "resource://gre/modules/ActorChild.jsm"
-);
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 ChromeUtils.defineModuleGetter(
   this,
   "BrowserUtils",
   "resource://gre/modules/BrowserUtils.jsm"
 );
 ChromeUtils.defineModuleGetter(
@@ -26,17 +23,17 @@ ChromeUtils.defineModuleGetter(
   "resource://gre/modules/WebNavigationFrames.jsm"
 );
 ChromeUtils.defineModuleGetter(
   this,
   "E10SUtils",
   "resource://gre/modules/E10SUtils.jsm"
 );
 
-class ClickHandlerChild extends ActorChild {
+class ClickHandlerChild extends JSWindowActorChild {
   handleEvent(event) {
     if (
       !event.isTrusted ||
       event.defaultPrevented ||
       event.button == 2 ||
       (event.type == "click" && event.button == 1)
     ) {
       return;
@@ -117,17 +114,17 @@ class ClickHandlerChild extends ActorChi
         json.title = node.getAttribute("title");
       }
 
       // Check if the link needs to be opened with mixed content allowed.
       // Only when the owner doc has |mixedContentChannel| and the same origin
       // 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);
           let isPrivateWin =
             ownerDoc.nodePrincipal.originAttributes.privateBrowsingId > 0;
           sm.checkSameOriginURI(
             docshell.mixedContentChannel.URI,
             targetURI,
@@ -146,39 +143,39 @@ class ClickHandlerChild extends ActorChi
       // when it's clicked with middle button, we should prevent multiple
       // actions here to avoid leaking clipboard content unexpectedly.
       // Note that whether the link will work actually or not does not matter
       // because in this case, user does not intent to paste clipboard content.
       if (event.button === 1) {
         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);
     }
   }
 
   /**
    * Extracts linkNode and href for the current click target.
    *
    * @param event
    *        The click event.
    * @return [href, linkNode, linkPrincipal].
    *
    * @note linkNode will be null if the click wasn't on an anchor
    *       element. This includes SVG links, because callers expect |node|
    *       to behave like an <a> 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 (
         (aNode instanceof content.HTMLAnchorElement && aNode.href) ||
         (aNode instanceof content.HTMLAreaElement && aNode.href) ||
         aNode instanceof content.HTMLLinkElement
       );
     }
rename from browser/modules/ContentClick.jsm
rename to browser/actors/ClickHandlerParent.jsm
--- a/browser/modules/ContentClick.jsm
+++ b/browser/actors/ClickHandlerParent.jsm
@@ -1,16 +1,16 @@
 /* -*- mode: js; indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-var EXPORTED_SYMBOLS = ["ContentClick"];
+var EXPORTED_SYMBOLS = ["ClickHandlerParent"];
 
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 ChromeUtils.defineModuleGetter(
   this,
   "PlacesUIUtils",
   "resource:///modules/PlacesUIUtils.jsm"
 );
@@ -20,44 +20,58 @@ ChromeUtils.defineModuleGetter(
   "resource://gre/modules/PrivateBrowsingUtils.jsm"
 );
 ChromeUtils.defineModuleGetter(
   this,
   "E10SUtils",
   "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<browser>}
    */
-  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;
     }
 
     // If the browser is not in a place where we can open links, bail out.
     // This can happen in osx sheets, dialogs, etc. that are not browser
     // windows.  Specifically the payments UI is in an osx sheet.
     if (window.openLinkIn === undefined) {
@@ -65,44 +79,59 @@ var ContentClick = {
     }
 
     // Mark the page as a user followed link.  This is done so that history can
     // distinguish automatic embed visits from user activated ones.  For example
     // pages loaded in frames are embed visits and lost with the session, while
     // 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;
     }
 
     // Todo(903022): code for where == save
 
     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);
+      }
+    }
+  }
+}
--- a/browser/actors/moz.build
+++ b/browser/actors/moz.build
@@ -23,16 +23,17 @@ with Files("WebRTCChild.jsm"):
     BUG_COMPONENT = ("Firefox", "Site Permissions")
 
 FINAL_TARGET_FILES.actors += [
     'AboutReaderChild.jsm',
     'BlockedSiteChild.jsm',
     'BrowserTabChild.jsm',
     'BrowserTabParent.jsm',
     'ClickHandlerChild.jsm',
+    'ClickHandlerParent.jsm',
     'ContentMetaChild.jsm',
     'ContentMetaParent.jsm',
     'ContentSearchChild.jsm',
     'ContextMenuChild.jsm',
     'ContextMenuParent.jsm',
     'DOMFullscreenChild.jsm',
     'DOMFullscreenParent.jsm',
     'FormValidationChild.jsm',
--- a/browser/base/content/test/general/browser_contentAltClick.js
+++ b/browser/base/content/test/general/browser_contentAltClick.js
@@ -3,17 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /**
  * Test for Bug 1109146.
  * The tests opens a new tab and alt + clicks to download files
  * 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";
 
 ChromeUtils.defineModuleGetter(
   this,
   "Downloads",
   "resource://gre/modules/Downloads.jsm"
 );
@@ -21,17 +21,18 @@ ChromeUtils.defineModuleGetter(
 function setup() {
   Services.prefs.setBoolPref("browser.altClickSave", true);
 
   let testPage =
     "data:text/html," +
     '<p><a id="commonlink" href="http://mochi.test/moz/">Common link</a></p>' +
     '<p><math id="mathlink" xmlns="http://www.w3.org/1998/Math/MathML" href="http://mochi.test/moz/"><mtext>MathML XLink</mtext></math></p>' +
     '<p><svg id="svgxlink" xmlns="http://www.w3.org/2000/svg" width="100px" height="50px" version="1.1"><a xlink:type="simple" xlink:href="http://mochi.test/moz/"><text transform="translate(10, 25)">SVG XLink</text></a></svg></p><br>' +
-    '<span id="host"></span><script>document.getElementById("host").attachShadow({mode: "closed"}).appendChild(document.getElementById("commonlink").cloneNode(true));</script>';
+    '<span id="host"></span><script>document.getElementById("host").attachShadow({mode: "closed"}).appendChild(document.getElementById("commonlink").cloneNode(true));</script>' +
+    '<iframe id="frame" src="https://test2.example.com:443/browser/browser/base/content/test/general/file_with_link_to_http.html"></iframe>';
 
   return BrowserTestUtils.openNewForegroundTab(gBrowser, testPage);
 }
 
 async function clean_up() {
   // Remove downloads.
   let downloadList = await Downloads.getList(Downloads.ALL);
   let downloads = await downloadList.getAll();
@@ -160,8 +161,46 @@ add_task(async function test_alt_click_o
   is(
     downloads[1].source.url,
     "http://mochi.test/moz/",
     "Downloaded #svgxlink element"
   );
 
   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();
+});
--- a/browser/components/BrowserGlue.jsm
+++ b/browser/components/BrowserGlue.jsm
@@ -52,16 +52,31 @@ let ACTORS = {
         DOMWindowCreated: {},
         MozAfterPaint: {},
         "MozDOMPointerLock:Entered": {},
         "MozDOMPointerLock:Exited": {},
       },
     },
   },
 
+  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: {
       moduleURI: "resource:///actors/ContentMetaParent.jsm",
     },
 
     child: {
       moduleURI: "resource:///actors/ContentMetaChild.jsm",
@@ -308,26 +323,16 @@ let LEGACY_ACTORS = {
         click: {},
       },
       matches: ["about:blocked?*"],
       allFrames: true,
       messages: ["DeceptiveBlockedDetails"],
     },
   },
 
-  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",
       group: "browsers",
       matches: [
         "about:home",
         "about:newtab",
         "about:welcome",
@@ -562,17 +567,16 @@ XPCOMUtils.defineLazyModuleGetters(this,
   WebChannel: "resource://gre/modules/WebChannel.jsm",
   WindowsRegistry: "resource://gre/modules/WindowsRegistry.jsm",
 });
 
 // eslint-disable-next-line no-unused-vars
 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",
 });
 
 /**
  * IF YOU ADD OR REMOVE FROM THIS LIST, PLEASE UPDATE THE LIST ABOVE AS WELL.
  * XXX Bug 1325373 is for making eslint detect these automatically.
  */
@@ -661,17 +665,16 @@ const listeners = {
     "AboutLogins:OpenMobileAndroid": ["AboutLoginsParent"],
     "AboutLogins:OpenMobileIos": ["AboutLoginsParent"],
     "AboutLogins:OpenSite": ["AboutLoginsParent"],
     "AboutLogins:SortChanged": ["AboutLoginsParent"],
     "AboutLogins:Subscribe": ["AboutLoginsParent"],
     "AboutLogins:SyncEnable": ["AboutLoginsParent"],
     "AboutLogins:SyncOptions": ["AboutLoginsParent"],
     "AboutLogins:UpdateLogin": ["AboutLoginsParent"],
-    "Content:Click": ["ContentClick"],
     ContentSearch: ["ContentSearch"],
     "Reader:FaviconRequest": ["ReaderParent"],
     "Reader:UpdateReaderButton": ["ReaderParent"],
     "rtcpeer:CancelRequest": ["webrtcUI"],
     "rtcpeer:Request": ["webrtcUI"],
     "webrtc:CancelRequest": ["webrtcUI"],
     "webrtc:Request": ["webrtcUI"],
     "webrtc:StopRecording": ["webrtcUI"],
--- a/browser/modules/moz.build
+++ b/browser/modules/moz.build
@@ -131,17 +131,16 @@ BROWSER_CHROME_MANIFESTS += [
 ]
 XPCSHELL_TESTS_MANIFESTS += ['test/unit/xpcshell.ini']
 
 EXTRA_JS_MODULES += [
     'AboutNewTab.jsm',
     'AsyncTabSwitcher.jsm',
     'BrowserUsageTelemetry.jsm',
     'BrowserWindowTracker.jsm',
-    'ContentClick.jsm',
     'ContentCrashHandlers.jsm',
     'ContentObservers.js',
     'ContentSearch.jsm',
     'Discovery.jsm',
     'EveryWindow.jsm',
     'ExtensionsUI.jsm',
     'FaviconLoader.jsm',
     'HomePage.jsm',
--- a/devtools/client/responsive/browser/tunnel.js
+++ b/devtools/client/responsive/browser/tunnel.js
@@ -42,20 +42,16 @@ const SWAPPED_BROWSER_STATE = [
  * When RDM is enabled, these bits of code instead reach the RDM tool's window instead of
  * the browser window, which won't have the properties they are looking for. At the
  * moment, we address this by exposing them from the browser window on RDM's window as
  * needed.
  */
 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",
 ];
 
 /**
  * This module takes an "outer" <xul:browser> from a browser tab as described by
  * Firefox's tabbrowser.xml and wires it up to an "inner" <iframe mozbrowser>
--- a/toolkit/components/extensions/WebNavigation.jsm
+++ b/toolkit/components/extensions/WebNavigation.jsm
@@ -18,16 +18,21 @@ ChromeUtils.defineModuleGetter(
   "PrivateBrowsingUtils",
   "resource://gre/modules/PrivateBrowsingUtils.jsm"
 );
 ChromeUtils.defineModuleGetter(
   this,
   "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,
 // e.g. nsNavHistory::CheckIsRecentEvent, but with a lower threshold value).
 const RECENT_DATA_THRESHOLD = 5 * 1000000;
 
 var Manager = {
   // Map[string -> Map[listener -> URLFilter]]
@@ -42,17 +47,18 @@ var Manager = {
     // pair the message received from the source tab to the one received from
     // the new tab.
     this.createdNavigationTargetByOuterWindowId = new Map();
 
     Services.obs.addObserver(this, "urlbar-user-start-navigation", true);
 
     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);
     Services.mm.addMessageListener("Extension:HistoryChange", this);
     Services.mm.addMessageListener("Extension:CreatedNavigationTarget", this);
 
     Services.mm.loadFrameScript(
       "resource://gre/modules/WebNavigationContent.js",
@@ -60,17 +66,18 @@ var Manager = {
     );
   },
 
   uninit() {
     // Stop collecting recent tab transition data and reset the WeakMap.
     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);
     Services.mm.removeMessageListener("Extension:DOMContentLoaded", this);
     Services.mm.removeMessageListener(
       "Extension:CreatedNavigationTarget",
       this
     );
@@ -300,20 +307,16 @@ var Manager = {
       case "Extension:HistoryChange":
         this.onHistoryChange(target, data);
         break;
 
       case "Extension:DOMContentLoaded":
         this.onLoad(target, data);
         break;
 
-      case "Content:Click":
-        this.onContentClick(target, data);
-        break;
-
       case "Extension:CreatedNavigationTarget":
         this.onCreatedNavigationTarget(target, data);
         break;
     }
   },
 
   onContentClick(target, data) {
     // We are interested only on clicks to links which are not "add to bookmark" commands