This is an automated email from the git hooks/post-receive script.
richard pushed a commit to branch tor-browser-91.8.0esr-11.0-1 in repository tor-browser.
commit 423c2d6b03593c30969141143140433dc59c0a77 Author: Shane Caraveo scaraveo@mozilla.com AuthorDate: Fri Mar 18 19:43:43 2022 +0000
Bug 1757760 retain correct search default setting after addon update r=rpl a=RyanVM
Differential Revision: https://phabricator.services.mozilla.com/D141294 --- .../parent/ext-chrome-settings-overrides.js | 11 +- .../test_ext_chrome_settings_overrides_update.js | 185 ++++++++++++++++++++- 2 files changed, 190 insertions(+), 6 deletions(-)
diff --git a/browser/components/extensions/parent/ext-chrome-settings-overrides.js b/browser/components/extensions/parent/ext-chrome-settings-overrides.js index 259e443c86a6f..ef2b5f040cc03 100644 --- a/browser/components/extensions/parent/ext-chrome-settings-overrides.js +++ b/browser/components/extensions/parent/ext-chrome-settings-overrides.js @@ -339,7 +339,9 @@ this.chrome_settings_overrides = class extends ExtensionAPI { extension.startupReason ); } - if (disable && item?.enabled) { + // Ensure the item is disabled. If addSetting was called above, + // Item may be null, and enabled may be undefined. + if (disable && item?.enabled !== false) { item = await ExtensionSettingsStore.disable( extension.id, DEFAULT_SEARCH_STORE_TYPE, @@ -367,7 +369,10 @@ this.chrome_settings_overrides = class extends ExtensionAPI { // This is a hack because we don't have the browser of // the actual install. This means the popup might show // in a different window. Will be addressed in a followup bug. - browser: windowTracker.topWindow.gBrowser.selectedBrowser, + // As well, we still notify if no topWindow exists to support + // testing from xpcshell. + browser: windowTracker.topWindow?.gBrowser.selectedBrowser, + id: extension.id, name: extension.name, icon: extension.iconURL, currentEngine: defaultEngine.name, @@ -476,7 +481,7 @@ this.chrome_settings_overrides = class extends ExtensionAPI { Services.search.defaultEngine = Services.search.getEngineByName( engineName ); - } else { + } else if (extension.startupReason == "ADDON_ENABLE") { // This extension has precedence, but is not in control. Ask the user. await this.promptDefaultSearch(engineName); } diff --git a/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js b/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js index 56af1f2642593..bbbcf0ea098e1 100644 --- a/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js +++ b/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js @@ -6,7 +6,9 @@ const { AddonTestUtils } = ChromeUtils.import( "resource://testing-common/AddonTestUtils.jsm" ); XPCOMUtils.defineLazyModuleGetters(this, { + AddonManager: "resource://gre/modules/AddonManager.jsm", HomePage: "resource:///modules/HomePage.jsm", + PromiseUtils: "resource://gre/modules/PromiseUtils.jsm", RemoteSettings: "resource://services-settings/remote-settings.js", sinon: "resource://testing-common/Sinon.jsm", }); @@ -24,6 +26,29 @@ AddonTestUtils.createAppInfo( // search service needs it. Services.prefs.clearUserPref("services.settings.default_bucket");
+// Similar to TestUtils.topicObserved, but returns a deferred promise that +// can be resolved +function topicObservable(topic, checkFn) { + let deferred = PromiseUtils.defer(); + function observer(subject, topic, data) { + try { + if (checkFn && !checkFn(subject, data)) { + return; + } + deferred.resolve([subject, data]); + } catch (ex) { + deferred.reject(ex); + } + } + deferred.promise.finally(() => { + Services.obs.removeObserver(observer, topic); + checkFn = null; + }); + Services.obs.addObserver(observer, topic); + + return deferred; +} + async function setupRemoteSettings() { const settings = await RemoteSettings("hijack-blocklists"); sinon.stub(settings, "get").returns([ @@ -50,7 +75,7 @@ add_task(async function test_overrides_update_removal() { /* This tests the scenario where the manifest key for homepage and/or * search_provider are removed between updates and therefore the * settings are expected to revert. It also tests that an extension - * can make a builtin extension the default extension without user + * can make a builtin extension the default search without user * interaction. */
const EXTENSION_ID = "test_overrides_update@tests.mozilla.org"; @@ -82,7 +107,20 @@ add_task(async function test_overrides_update_removal() { ok(defaultEngineName !== "DuckDuckGo", "Default engine is not DuckDuckGo.");
let prefPromise = promisePrefChanged(HOMEPAGE_URI); - await extension.startup(); + + // When an addon is installed that overrides an app-provided engine (builtin) + // that is the default, we do not prompt for default. + let deferredPrompt = topicObservable( + "webextension-defaultsearch-prompt", + (subject, message) => { + if (subject.wrappedJSObject.id == extension.id) { + ok(false, "default override should not prompt"); + } + } + ); + + await Promise.race([extension.startup(), deferredPrompt.promise]); + deferredPrompt.resolve(); await AddonTestUtils.waitForSearchProviderStartup(extension); await prefPromise;
@@ -197,7 +235,21 @@ add_task(async function test_overrides_update_adding() { };
let prefPromise = promisePrefChanged(HOMEPAGE_URI); - await extension.upgrade(extensionInfo); + + let deferredUpgradePrompt = topicObservable( + "webextension-defaultsearch-prompt", + (subject, message) => { + if (subject.wrappedJSObject.id == extension.id) { + ok(false, "should not prompt on update"); + } + } + ); + + await Promise.race([ + extension.upgrade(extensionInfo), + deferredUpgradePrompt.promise, + ]); + deferredUpgradePrompt.resolve(); await AddonTestUtils.waitForSearchProviderStartup(extension); await prefPromise;
@@ -287,3 +339,130 @@ add_task(async function test_overrides_update_homepage_change() {
await extension.unload(); }); + +add_task(async function test_default_search_prompts() { + /* This tests the scenario where an addon did not gain + * default search during install, and later upgrades. + * The addon should not gain default in updates. + * If the addon is disabled, it should prompt again when + * enabled. + */ + + const EXTENSION_ID = "test_default_update@tests.mozilla.org"; + + let extensionInfo = { + useAddonManager: "permanent", + manifest: { + version: "1.0", + applications: { + gecko: { + id: EXTENSION_ID, + }, + }, + chrome_settings_overrides: { + search_provider: { + name: "Example", + search_url: "https://example.com/?q=%7BsearchTerms%7D", + is_default: true, + }, + }, + }, + }; + + let extension = ExtensionTestUtils.loadExtension(extensionInfo); + + // Mock a response from the default search prompt where we + // say no to setting this as the default when installing. + let prompted = TestUtils.topicObserved( + "webextension-defaultsearch-prompt", + (subject, message) => { + if (subject.wrappedJSObject.id == extension.id) { + return subject.wrappedJSObject.respond(false); + } + } + ); + + let defaultEngineName = (await Services.search.getDefault()).name; + ok(defaultEngineName !== "Example", "Search is not Example."); + + await extension.startup(); + await prompted; + + equal( + extension.version, + "1.0", + "The installed addon has the expected version." + ); + equal( + (await Services.search.getDefault()).name, + defaultEngineName, + "Default engine is the default after startup." + ); + + extensionInfo.manifest = { + version: "2.0", + applications: { + gecko: { + id: EXTENSION_ID, + }, + }, + chrome_settings_overrides: { + search_provider: { + name: "Example", + search_url: "https://example.com/?q=%7BsearchTerms%7D", + is_default: true, + }, + }, + }; + + let deferredUpgradePrompt = topicObservable( + "webextension-defaultsearch-prompt", + (subject, message) => { + if (subject.wrappedJSObject.id == extension.id) { + ok(false, "should not prompt on update"); + } + } + ); + + await Promise.race([ + extension.upgrade(extensionInfo), + deferredUpgradePrompt.promise, + ]); + deferredUpgradePrompt.resolve(); + + await AddonTestUtils.waitForSearchProviderStartup(extension); + + equal( + extension.version, + "2.0", + "The updated addon has the expected version." + ); + // An upgraded extension does not become the default engine. + equal( + (await Services.search.getDefault()).name, + defaultEngineName, + "Default engine is still the default after startup." + ); + + let addon = await AddonManager.getAddonByID(EXTENSION_ID); + await addon.disable(); + + prompted = TestUtils.topicObserved( + "webextension-defaultsearch-prompt", + (subject, message) => { + if (subject.wrappedJSObject.id == extension.id) { + return subject.wrappedJSObject.respond(false); + } + } + ); + await Promise.all([addon.enable(), prompted]); + + // we still said no. + equal( + (await Services.search.getDefault()).name, + defaultEngineName, + "Default engine is the default after startup." + ); + + await extension.unload(); +});