Bug 1828334 r=Gijs,dveditz,extension-reviewers,fluent-reviewers,flod,robwu,perftest-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D185671

testing/profiles/perf/user.js

@@ -78,6 +78,7 @@ user_pref("privacy.trackingprotection.enabled", false);
 user_pref("privacy.trackingprotection.introURL", "http://127.0.0.1/trackingprotection/tour");
 user_pref("privacy.trackingprotection.pbmode.enabled", false);
 user_pref("security.enable_java", false);
+user_pref("security.external_protocol_requires_permission", false);
 user_pref("security.fileuri.strict_origin_policy", false);
 user_pref("toolkit.telemetry.server", "https://127.0.0.1/telemetry-dummy/");
 user_pref("telemetry.fog.test.localhost_port", -1);

toolkit/components/extensions/test/mochitest/test_ext_protocolHandlers.html

@@ -198,6 +198,11 @@ add_task(async function test_protocolHandler() {
   // Test that handling a URL from the commandline works.
   chromeScript = SpecialPowers.loadChromeScript(() => {
     /* eslint-env mozilla/chrome-script */
+    const CONTENT_HANDLING_URL =
+      "chrome://mozapps/content/handling/permissionDialog.xhtml";
+    const { BrowserTestUtils } = ChromeUtils.importESModule(
+      "resource://testing-common/BrowserTestUtils.sys.mjs"
+    );
     let cmdLineHandler = Cc["@mozilla.org/browser/final-clh;1"].getService(
       Ci.nsICommandLineHandler
     );
@@ -207,6 +212,33 @@ add_task(async function test_protocolHandler() {
       Ci.nsICommandLine.STATE_REMOTE_EXPLICIT
     );
     cmdLineHandler.handle(fakeCmdLine);
+
+    // We aren't awaiting for this promise to resolve since it returns undefined
+    // (because it returns the reference to the dialog window that we close, below)
+    // once the callback promise below finishes, and because its not needed for anything
+    // outside of this loadChromeScript block.
+    BrowserTestUtils.promiseAlertDialogOpen(
+      null,
+      CONTENT_HANDLING_URL,
+      {
+        isSubDialog: true,
+        async callback(dialogWin) {
+          is(dialogWin.document.documentURI, CONTENT_HANDLING_URL, "Open dialog is the permission dialog")
+
+          let closePromise = BrowserTestUtils.waitForEvent(
+            dialogWin.browsingContext.topChromeWindow,
+            "dialogclose",
+            true,
+          );
+          let dialog = dialogWin.document.querySelector("dialog");
+          let btn = dialog.getButton("accept");
+          // The security delay disables this button, just bypass it.
+          btn.disabled = false;
+          btn.click();
+          return closePromise;
+        }
+      }
+    );
   });
   query = await extension.awaitMessage("test-query");
   is(query, "?val=ext%2Bfoo%3Acmdline", "cmdline query ok");
@@ -220,7 +252,7 @@ add_task(async function test_protocolHandler() {
   consoleMonitor.start([{ message: /NS_ERROR_FILE_NOT_FOUND/ }]);
 
   // Expect the chooser window to be open, close it.
-  chromeScript = SpecialPowers.loadChromeScript(async () => {
+  chromeScript = SpecialPowers.loadChromeScript(() => {
     /* eslint-env mozilla/chrome-script */
     const CONTENT_HANDLING_URL =
       "chrome://mozapps/content/handling/appChooser.xhtml";
@@ -228,39 +260,39 @@ add_task(async function test_protocolHandler() {
       "resource://testing-common/BrowserTestUtils.sys.mjs"
     );
 
-    let windowOpen = BrowserTestUtils.domWindowOpenedAndLoaded();
-
     sendAsyncMessage("listenWindow");
 
-    let window = await windowOpen;
+    // We aren't awaiting for this promise to resolve since it returns undefined
-    let gBrowser = window.gBrowser;
+    // (because it returns the reference to the dialog window that we close, below)
-    let tabDialogBox = gBrowser.getTabDialogBox(gBrowser.selectedBrowser);
+    // once the callback promise below finishes, and because its not needed for anything
-    let dialogStack = tabDialogBox.getTabDialogManager()._dialogStack;
+    // outside of this loadChromeScript block.
+    BrowserTestUtils.promiseAlertDialogOpen(
+      null,
+      CONTENT_HANDLING_URL,
+      {
+        isSubDialog: true,
+        async callback(dialogWin) {
+          is(dialogWin.document.documentURI, CONTENT_HANDLING_URL, "Open dialog is the app chooser dialog")
 
-    let checkFn = dialogEvent =>
+          let entry = dialogWin.document.getElementById("items")
-      dialogEvent.detail.dialog?._openedURL == CONTENT_HANDLING_URL;
-
-    let eventPromise = BrowserTestUtils.waitForEvent(
-      dialogStack,
-      "dialogopen",
-      true,
-      checkFn
-    );
-
-    sendAsyncMessage("listenDialog");
-
-    let event = await eventPromise;
-
-    let { dialog } = event.detail;
-
-    let entry = dialog._frame.contentDocument.getElementById("items")
             .firstChild;
           sendAsyncMessage("handling", {
             name: entry.getAttribute("name"),
             disabled: entry.disabled,
           });
 
-    dialog.close();
+          let closePromise = BrowserTestUtils.waitForEvent(
+            dialogWin.browsingContext.topChromeWindow,
+            "dialogclose",
+            true,
+          );
+          dialogWin.close();
+          return closePromise;
+        }
+      }
+    );
+
+    sendAsyncMessage("listenDialog");
   });
 
   // Wait for the chrome script to attach window listener

toolkit/locales/en-US/toolkit/global/handlerDialog.ftl

@@ -33,6 +33,12 @@ permission-dialog-description-file-app =
 permission-dialog-description-extension-app =
   Allow the extension { $extension } to open the { $scheme } link with { $appName }?
 
+permission-dialog-description-system-app =
+  Open the { $scheme } link with { $appName }?
+
+permission-dialog-description-system-noapp =
+  Open the { $scheme } link?
+
 ## Please keep the emphasis around the hostname and scheme (ie the
 ## `<strong>` HTML tags). Please also keep the hostname as close to the start
 ## of the sentence as your language's grammar allows.

toolkit/mozapps/handling/ContentDispatchChooser.sys.mjs

@@ -47,7 +47,8 @@ export class nsContentDispatchChooser {
   ) {
     let callerHasPermission = this._hasProtocolHandlerPermission(
       aHandler.type,
-      aPrincipal
+      aPrincipal,
+      aTriggeredExternally
     );
 
     // Force showing the dialog for links passed from outside the application.
@@ -269,7 +270,7 @@ export class nsContentDispatchChooser {
    * @param {nsIPrincipal} aPrincipal - Principal to test for permission.
    * @returns {boolean} - true if permission is set, false otherwise.
    */
-  _hasProtocolHandlerPermission(scheme, aPrincipal) {
+  _hasProtocolHandlerPermission(scheme, aPrincipal, aTriggeredExternally) {
     // Permission disabled by pref
     if (!nsContentDispatchChooser.isPermissionEnabled) {
       return true;
@@ -285,7 +286,10 @@ export class nsContentDispatchChooser {
       return true;
     }
 
-    if (!aPrincipal) {
+    if (
+      !aPrincipal ||
+      (aPrincipal.isSystemPrincipal && !aTriggeredExternally)
+    ) {
       return false;
     }
 

toolkit/mozapps/handling/content/permissionDialog.js

@@ -38,12 +38,15 @@ let dialog = {
     }
 
     let changeAppLink = document.getElementById("change-app");
-    if (this._preferredHandlerName) {
+
+    // allow the user to choose another application if they wish,
+    // but don't offer this if the protocol was opened via
+    // system principal (URLbar) and there's a preferred handler
+    if (this._preferredHandlerName && !this._principal?.isSystemPrincipal) {
       changeAppLink.hidden = false;
 
       changeAppLink.addEventListener("click", () => this.onChangeApp());
     }
-
     document.addEventListener("dialogaccept", () => this.onAccept());
     this.initL10n();
 
@@ -96,6 +99,14 @@ let dialog = {
       return "permission-dialog-description-file";
     }
 
+    if (this._principal?.isSystemPrincipal && this._preferredHandlerName) {
+      return "permission-dialog-description-system-app";
+    }
+
+    if (this._principal?.isSystemPrincipal && !this._preferredHandlerName) {
+      return "permission-dialog-description-system-noapp";
+    }
+
     // We only show the website address if the request didn't come from the top
     // level frame. If we can't get a host to display, fall back to the copy
     // without host.
@@ -158,6 +169,8 @@ let dialog = {
 
     // Fluent id for dialog accept button
     let idAcceptButton;
+    let acceptButton = this._dialog.getButton("accept");
+
     if (this._preferredHandlerName) {
       idAcceptButton = "permission-dialog-btn-open-link";
     } else {
@@ -165,8 +178,8 @@ let dialog = {
 
       let descriptionExtra = document.getElementById("description-extra");
       descriptionExtra.hidden = false;
+      acceptButton.addEventListener("click", () => this.onChangeApp());
     }
-    let acceptButton = this._dialog.getButton("accept");
     document.l10n.setAttributes(acceptButton, idAcceptButton);
 
     let description = document.getElementById("description");

uriloader/exthandler/tests/mochitest/browser_protocol_ask_dialog_permission.js

@@ -60,6 +60,18 @@ function getSkipProtoDialogPermissionKey(aProtocolScheme) {
   );
 }
 
+function getSystemProtocol() {
+  // TODO add a scheme for Windows 10 or greater once support is added (see bug 1764599).
+  if (AppConstants.platform == "macosx") {
+    return "itunes";
+  }
+
+  info(
+    "Skipping this test since there isn't a suitable default protocol on this platform"
+  );
+  return null;
+}
+
 /**
  * Creates dummy web protocol handlers used for testing.
  */
@@ -605,13 +617,43 @@ add_task(async function test_permission_application_set() {
 
 /**
  * Tests that we correctly handle system principals. They should always
- * skip the permission dialog.
+ * show the permission dialog, but give the option to choose another
+ * app if there isn't a default handler.
  */
 add_task(async function test_permission_system_principal() {
   let scheme = TEST_PROTOS[0];
   await BrowserTestUtils.withNewTab(ORIGIN1, async browser => {
     await testOpenProto(browser, scheme, {
-      chooserDialogOptions: { hasCheckbox: true, actionConfirm: false },
+      permDialogOptions: {
+        hasCheckbox: false,
+        hasChangeApp: false,
+        chooserIsNext: true,
+        actionChangeApp: false,
+      },
+      triggerLoad: useTriggeringPrincipal(
+        Services.scriptSecurityManager.getSystemPrincipal()
+      ),
+    });
+  });
+});
+
+/**
+ * Tests that we correctly handle system principals and show
+ * a simplified permission dialog if there is a default handler.
+ */
+add_task(async function test_permission_system_principal() {
+  let scheme = getSystemProtocol();
+  if (!scheme) {
+    return;
+  }
+  await BrowserTestUtils.withNewTab(ORIGIN1, async browser => {
+    await testOpenProto(browser, scheme, {
+      permDialogOptions: {
+        hasCheckbox: false,
+        hasChangeApp: false,
+        chooserIsNext: false,
+        actionChangeApp: false,
+      },
       triggerLoad: useTriggeringPrincipal(
         Services.scriptSecurityManager.getSystemPrincipal()
       ),
@@ -762,17 +804,10 @@ add_task(async function test_no_principal() {
  * and the user hasn't selected an alternative only the permission dialog is shown.
  */
 add_task(async function test_non_standard_protocol() {
-  let scheme = null;
+  let scheme = getSystemProtocol();
-  // TODO add a scheme for Windows 10 or greater once support is added (see bug 1764599).
+  if (!scheme) {
-  if (AppConstants.platform == "macosx") {
-    scheme = "itunes";
-  } else {
-    info(
-      "Skipping this test since there isn't a suitable default protocol on this platform"
-    );
     return;
   }
-
   await BrowserTestUtils.withNewTab(ORIGIN1, async browser => {
     await testOpenProto(browser, scheme, {
       permDialogOptions: {

uriloader/exthandler/tests/mochitest/browser_protocolhandler_loop.js

@@ -59,7 +59,10 @@ add_task(async function test_helperapp() {
 
     let askedUserPromise = waitForProtocolAppChooserDialog(browser, true);
 
-    BrowserTestUtils.startLoadingURIString(browser, kProt + ":test");
+    gBrowser.fixupAndLoadURIString(kProt + ":test", {
+      triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(),
+      loadFlags: Ci.nsIWebNavigation.LOAD_FLAGS_FROM_EXTERNAL,
+    });
     let dialog = await Promise.race([
       wrongThingHappenedPromise,
       askedUserPromise,