From 2b59a70e77bc913557da0cf26b3edde3ac4753e0 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Sat, 11 Dec 2021 06:25:57 +0000 Subject: [PATCH] Bug 1743914 - Part 2: Allow to experiment with setting Firefox as default PDF handler on Windows 10+. r=mhowell,Mardak This adds a `setDefaultPDFHandler` that extends the existing `setDefaultBrowserUserChoice` to also set Firefox as the default PDF handler when setting Firefox as the default browser. (Since this uses User Choice, it's Windows 10+ only.) Differential Revision: https://phabricator.services.mozilla.com/D132660 --- browser/app/profile/firefox.js | 3 + browser/components/shell/ShellService.jsm | 52 +++++-- browser/components/shell/test/browser.ini | 2 + .../test/browser_setDefaultPDFHandler.js | 146 ++++++++++++++++++ .../components/nimbus/FeatureManifest.yaml | 4 + 5 files changed, 197 insertions(+), 10 deletions(-) create mode 100644 browser/components/shell/test/browser_setDefaultPDFHandler.js diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 9b35427a275f..2e523e45fc06 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -282,6 +282,9 @@ pref("browser.shell.defaultBrowserCheckCount", 0); // Attempt to set the default browser on Windows 10 using the UserChoice registry keys, // before falling back to launching the modern Settings dialog. pref("browser.shell.setDefaultBrowserUserChoice", true); +// When setting the default browser on Windows 10 using the UserChoice +// registry keys, also try to set Firefox as the default PDF handler. +pref("browser.shell.setDefaultPDFHandler", false); #endif diff --git a/browser/components/shell/ShellService.jsm b/browser/components/shell/ShellService.jsm index b7d45bcc2308..af24c65f931f 100644 --- a/browser/components/shell/ShellService.jsm +++ b/browser/components/shell/ShellService.jsm @@ -120,6 +120,38 @@ let ShellServiceInternal = { return false; }, + /* + * Invoke the Windows Default Browser agent with the given options. + * + * Separated for easy stubbing in tests. + */ + _callExternalDefaultBrowserAgent(options = {}) { + const wdba = Services.dirsvc.get("XREExeF", Ci.nsIFile); + wdba.leafName = "default-browser-agent.exe"; + return Subprocess.call({ + ...options, + command: options.command || wdba.path, + }); + }, + + /* + * Check if UserChoice is impossible. + * + * Separated for easy stubbing in tests. + * + * @return string telemetry result like "Err*", or null if UserChoice + * is possible. + */ + _userChoiceImpossibleTelemetryResult() { + if (!ShellService.checkAllProgIDsExist()) { + return "ErrProgID"; + } + if (!ShellService.checkBrowserUserChoiceHashes()) { + return "ErrHash"; + } + return null; + }, + /* * Set the default browser through the UserChoice registry keys on Windows. * @@ -144,24 +176,24 @@ let ShellServiceInternal = { let telemetryResult = "ErrOther"; try { - if (!ShellService.checkAllProgIDsExist()) { - telemetryResult = "ErrProgID"; + telemetryResult = + this._userChoiceImpossibleTelemetryResult() ?? "ErrOther"; + if (telemetryResult == "ErrProgID") { throw new Error("checkAllProgIDsExist() failed"); } - - if (!ShellService.checkBrowserUserChoiceHashes()) { - telemetryResult = "ErrHash"; + if (telemetryResult == "ErrHash") { throw new Error("checkBrowserUserChoiceHashes() failed"); } - const wdba = Services.dirsvc.get("XREExeF", Ci.nsIFile); - wdba.leafName = "default-browser-agent.exe"; const aumi = XreDirProvider.getInstallHash(); telemetryResult = "ErrLaunchExe"; - const exeProcess = await Subprocess.call({ - command: wdba.path, - arguments: ["set-default-browser-user-choice", aumi], + const exeArgs = ["set-default-browser-user-choice", aumi]; + if (NimbusFeatures.shellService.getVariable("setDefaultPDFHandler")) { + exeArgs.push(".pdf"); + } + const exeProcess = await this._callExternalDefaultBrowserAgent({ + arguments: exeArgs, }); telemetryResult = "ErrOther"; diff --git a/browser/components/shell/test/browser.ini b/browser/components/shell/test/browser.ini index 12f0b724bba9..778c0fd1c9f0 100644 --- a/browser/components/shell/test/browser.ini +++ b/browser/components/shell/test/browser.ini @@ -10,6 +10,8 @@ support-files = skip-if = os != "mac" || verify [browser_doesAppNeedPin.js] [browser_setDefaultBrowser.js] +[browser_setDefaultPDFHandler.js] +run-if = os == "win" [browser_setDesktopBackgroundPreview.js] [browser_headless_screenshot_1.js] support-files = diff --git a/browser/components/shell/test/browser_setDefaultPDFHandler.js b/browser/components/shell/test/browser_setDefaultPDFHandler.js new file mode 100644 index 000000000000..244113b03aaa --- /dev/null +++ b/browser/components/shell/test/browser_setDefaultPDFHandler.js @@ -0,0 +1,146 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +XPCOMUtils.defineLazyModuleGetters(this, { + ExperimentAPI: "resource://nimbus/ExperimentAPI.jsm", + ExperimentFakes: "resource://testing-common/NimbusTestUtils.jsm", + NimbusFeatures: "resource://nimbus/ExperimentAPI.jsm", + sinon: "resource://testing-common/Sinon.jsm", +}); + +XPCOMUtils.defineLazyServiceGetter( + this, + "XreDirProvider", + "@mozilla.org/xre/directory-provider;1", + "nsIXREDirProvider" +); + +const _callExternalDefaultBrowserAgentStub = sinon + .stub(ShellService, "_callExternalDefaultBrowserAgent") + .callsFake(async () => ({ + async wait() { + return { exitCode: 0 }; + }, + })); + +const _userChoiceImpossibleTelemetryResultStub = sinon + .stub(ShellService, "_userChoiceImpossibleTelemetryResult") + .callsFake(() => null); + +// Ensure we don't fall back to a real implementation. +const setDefaultStub = sinon.stub(); +const shellStub = sinon + .stub(ShellService, "shellService") + .value({ setDefaultBrowser: setDefaultStub }); + +registerCleanupFunction(() => { + _callExternalDefaultBrowserAgentStub.restore(); + _userChoiceImpossibleTelemetryResultStub.restore(); + shellStub.restore(); + + ExperimentAPI._store._deleteForTests("shellService"); +}); + +add_task(async function ready() { + await ExperimentAPI.ready(); +}); + +if (AppConstants.isPlatformAndVersionAtLeast("win", "10")) { + // Everything here is Windows 10+. + + add_task(async function remoteEnableWithPDF() { + await ExperimentFakes.remoteDefaultsHelper({ + feature: NimbusFeatures.shellService, + configuration: { + slug: "shellService_remoteEnableWithPDF", + variables: { + setDefaultBrowserUserChoice: true, + setDefaultPDFHandler: true, + enabled: true, + }, + targeting: "true", + }, + }); + + Assert.equal( + NimbusFeatures.shellService.getVariable("setDefaultBrowserUserChoice"), + true + ); + Assert.equal( + NimbusFeatures.shellService.getVariable("setDefaultPDFHandler"), + true + ); + + _callExternalDefaultBrowserAgentStub.resetHistory(); + ShellService.setDefaultBrowser(); + + const aumi = XreDirProvider.getInstallHash(); + Assert.ok(_callExternalDefaultBrowserAgentStub.called); + Assert.deepEqual(_callExternalDefaultBrowserAgentStub.firstCall.args, [ + { arguments: ["set-default-browser-user-choice", aumi, ".pdf"] }, + ]); + }); + + add_task(async function remoteEnableWithoutPDF() { + await ExperimentFakes.remoteDefaultsHelper({ + feature: NimbusFeatures.shellService, + configuration: { + slug: "shellService_remoteEnableWithoutPDF", + variables: { + setDefaultBrowserUserChoice: true, + setDefaultPDFHandler: false, + enabled: true, + }, + targeting: "true", + }, + }); + + Assert.equal( + NimbusFeatures.shellService.getVariable("setDefaultBrowserUserChoice"), + true + ); + Assert.equal( + NimbusFeatures.shellService.getVariable("setDefaultPDFHandler"), + false + ); + + _callExternalDefaultBrowserAgentStub.resetHistory(); + ShellService.setDefaultBrowser(); + + const aumi = XreDirProvider.getInstallHash(); + Assert.ok(_callExternalDefaultBrowserAgentStub.called); + Assert.deepEqual(_callExternalDefaultBrowserAgentStub.firstCall.args, [ + { arguments: ["set-default-browser-user-choice", aumi] }, + ]); + }); + + add_task(async function remoteDisable() { + await ExperimentFakes.remoteDefaultsHelper({ + feature: NimbusFeatures.shellService, + configuration: { + slug: "shellService_remoteDisable", + variables: { + setDefaultBrowserUserChoice: false, + setDefaultPDFHandler: true, + enabled: false, + }, + targeting: "true", + }, + }); + + Assert.equal( + NimbusFeatures.shellService.getVariable("setDefaultBrowserUserChoice"), + false + ); + Assert.equal( + NimbusFeatures.shellService.getVariable("setDefaultPDFHandler"), + true + ); + + _callExternalDefaultBrowserAgentStub.resetHistory(); + ShellService.setDefaultBrowser(); + + Assert.ok(_callExternalDefaultBrowserAgentStub.notCalled); + Assert.ok(setDefaultStub.called); + }); +} diff --git a/toolkit/components/nimbus/FeatureManifest.yaml b/toolkit/components/nimbus/FeatureManifest.yaml index ff7ebe5a1a35..9792fa100af9 100644 --- a/toolkit/components/nimbus/FeatureManifest.yaml +++ b/toolkit/components/nimbus/FeatureManifest.yaml @@ -225,6 +225,10 @@ shellService: type: boolean fallbackPref: browser.shell.setDefaultBrowserUserChoice description: Should it set as default browser + setDefaultPDFHandler: + type: boolean + fallbackPref: browser.shell.setDefaultPDFHandler + description: Should setting it as the default browser set it as the default PDF handler. upgradeDialog: description: The dialog shown for major upgrades hasExposure: false