diff --git a/browser/components/extensions/parent/ext-chrome-settings-overrides.js b/browser/components/extensions/parent/ext-chrome-settings-overrides.js index c832aa7d3b4a..c2aaf30581e6 100644 --- a/browser/components/extensions/parent/ext-chrome-settings-overrides.js +++ b/browser/components/extensions/parent/ext-chrome-settings-overrides.js @@ -185,31 +185,23 @@ this.chrome_settings_overrides = class extends ExtensionAPI { await ExtensionSettingsStore.initialize(); let item = ExtensionSettingsStore.getSetting( DEFAULT_SEARCH_STORE_TYPE, - DEFAULT_SEARCH_SETTING_NAME + DEFAULT_SEARCH_SETTING_NAME, + id ); if (!item) { return; } - if ( - Services.search.defaultEngine.name != item.value && - Services.search.defaultEngine.name != item.initialValue - ) { - // The current engine is not the same as the value that the ExtensionSettingsStore has. - // This means that the user changed the engine, so we shouldn't control it anymore. - // Do nothing and remove our entry from the ExtensionSettingsStore. - ExtensionSettingsStore.removeSetting( - id, - DEFAULT_SEARCH_STORE_TYPE, - DEFAULT_SEARCH_SETTING_NAME - ); - return; - } + let control = await ExtensionSettingsStore.getLevelOfControl( + id, + DEFAULT_SEARCH_STORE_TYPE, + DEFAULT_SEARCH_SETTING_NAME + ); item = ExtensionSettingsStore[action]( id, DEFAULT_SEARCH_STORE_TYPE, DEFAULT_SEARCH_SETTING_NAME ); - if (item) { + if (item && control == "controlled_by_this_extension") { try { let engine = Services.search.getEngineByName( item.value || item.initialValue @@ -261,7 +253,19 @@ this.chrome_settings_overrides = class extends ExtensionAPI { } static async onEnabling(id) { - chrome_settings_overrides.processDefaultSearchSetting("enable", id); + await ExtensionSettingsStore.initialize(); + let item = await ExtensionSettingsStore.getSetting( + DEFAULT_SEARCH_STORE_TYPE, + DEFAULT_SEARCH_SETTING_NAME, + id + ); + if (item) { + ExtensionSettingsStore.enable( + id, + DEFAULT_SEARCH_STORE_TYPE, + DEFAULT_SEARCH_SETTING_NAME + ); + } } static async onUninstall(id) { @@ -311,11 +315,11 @@ this.chrome_settings_overrides = class extends ExtensionAPI { } } - static onDisable(id) { + static async onDisable(id) { homepagePopup.clearConfirmation(id); - chrome_settings_overrides.processDefaultSearchSetting("disable", id); - chrome_settings_overrides.removeEngine(id); + await chrome_settings_overrides.processDefaultSearchSetting("disable", id); + await chrome_settings_overrides.removeEngine(id); } async onManifestEntry(entryName) { @@ -391,6 +395,8 @@ this.chrome_settings_overrides = class extends ExtensionAPI { await this.setDefault(engineName); } if (!result.canInstallEngine) { + // This extension is overriding an app-provided one, so we don't + // add its engine as well. return; } await this.addSearchEngine(); @@ -443,6 +449,9 @@ this.chrome_settings_overrides = class extends ExtensionAPI { if (extension.startupReason === "ADDON_INSTALL") { let defaultEngine = await Services.search.getDefault(); await ExtensionSettingsStore.initialize(); + // We should only get here if an extension is setting an app-provided + // engine to default and we are ignoring the addons other engine settings. + // In this case we do not show the prompt to the user. let item = await ExtensionSettingsStore.addSetting( extension.id, DEFAULT_SEARCH_STORE_TYPE, @@ -454,10 +463,18 @@ this.chrome_settings_overrides = class extends ExtensionAPI { Services.search.getEngineByName(item.value) ); } else if (extension.startupReason === "ADDON_ENABLE") { - chrome_settings_overrides.processDefaultSearchSetting( - "enable", - extension.id + // We would be called for every extension being enabled, we should verify + // that it has control and only then set it as default + let control = await ExtensionSettingsStore.getLevelOfControl( + extension.id, + DEFAULT_SEARCH_STORE_TYPE, + DEFAULT_SEARCH_SETTING_NAME ); + if (control === "controlled_by_this_extension") { + await Services.search.setDefault( + Services.search.getEngineByName(engineName) + ); + } } } diff --git a/browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js b/browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js index 29257cd169de..aa83e85cecec 100644 --- a/browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js +++ b/browser/components/extensions/test/browser/browser_ext_settings_overrides_default_search.js @@ -12,9 +12,14 @@ ChromeUtils.defineModuleGetter( const { AddonTestUtils } = ChromeUtils.import( "resource://testing-common/AddonTestUtils.jsm" ); +const { SearchTestUtils } = ChromeUtils.import( + "resource://testing-common/SearchTestUtils.jsm" +); const EXTENSION1_ID = "extension1@mozilla.com"; const EXTENSION2_ID = "extension2@mozilla.com"; +const DEFAULT_SEARCH_STORE_TYPE = "default_search"; +const DEFAULT_SEARCH_SETTING_NAME = "defaultSearch"; AddonTestUtils.initMochitest(this); @@ -115,6 +120,11 @@ add_task(async function test_extension_setting_default_engine_external() { async function startExtension(win = window) { let extension = ExtensionTestUtils.loadExtension({ manifest: { + applications: { + gecko: { + id: EXTENSION1_ID, + }, + }, chrome_settings_overrides: { search_provider: { name: NAME, @@ -170,6 +180,29 @@ add_task(async function test_extension_setting_default_engine_external() { "Default engine was changed after accepting prompt" ); + // Do this twice to make sure we're definitely handling disable/enable + // correctly. + let disabledPromise = awaitEvent("shutdown", EXTENSION1_ID); + let addon = await AddonManager.getAddonByID(EXTENSION1_ID); + await addon.disable(); + await disabledPromise; + + is( + (await Services.search.getDefault()).name, + "Google", + "Default engine is Google after disabling" + ); + + let processedPromise = awaitEvent("searchEngineProcessed", EXTENSION1_ID); + await addon.enable(); + await processedPromise; + + is( + (await Services.search.getDefault()).name, + NAME, + `Default engine is ${NAME} after enabling` + ); + await extension.unload(); is( @@ -353,6 +386,12 @@ add_task(async function test_user_changing_default_engine() { let engine = Services.search.getEngineByName("Bing"); await Services.search.setDefault(engine); + // This simulates the preferences UI when the setting is changed. + ExtensionSettingsStore.select( + ExtensionSettingsStore.SETTING_USER_SET, + DEFAULT_SEARCH_STORE_TYPE, + DEFAULT_SEARCH_SETTING_NAME + ); await ext1.unload(); @@ -396,6 +435,12 @@ add_task(async function test_user_change_with_disabling() { let engine = Services.search.getEngineByName("Bing"); await Services.search.setDefault(engine); + // This simulates the preferences UI when the setting is changed. + ExtensionSettingsStore.select( + ExtensionSettingsStore.SETTING_USER_SET, + DEFAULT_SEARCH_STORE_TYPE, + DEFAULT_SEARCH_SETTING_NAME + ); is( (await Services.search.getDefault()).name, @@ -684,9 +729,14 @@ add_task(async function test_two_addons_with_second_disabled() { "Default engine is DuckDuckGo" ); + let defaultPromise = SearchTestUtils.promiseSearchNotification( + "engine-default", + "browser-search-engine-modified" + ); let enabledPromise = awaitEvent("ready", EXTENSION2_ID); await addon2.enable(); await enabledPromise; + await defaultPromise; is( (await Services.search.getDefault()).name, diff --git a/toolkit/components/extensions/ExtensionSettingsStore.jsm b/toolkit/components/extensions/ExtensionSettingsStore.jsm index 28e05c514e5c..cedcfb114191 100644 --- a/toolkit/components/extensions/ExtensionSettingsStore.jsm +++ b/toolkit/components/extensions/ExtensionSettingsStore.jsm @@ -588,7 +588,7 @@ var ExtensionSettingsStore = { // When user set, the setting is never "controllable" unless the installDate // is later than the user date. let addon = await AddonManager.getAddonByID(id); - return keyInfo.selectedDate > addon.installDate.valueOf() + return !addon || keyInfo.selectedDate > addon.installDate.valueOf() ? "not_controllable" : "controllable_by_this_extension"; } @@ -604,7 +604,7 @@ var ExtensionSettingsStore = { } let addon = await AddonManager.getAddonByID(id); - return topItem.installDate > addon.installDate.valueOf() + return !addon || topItem.installDate > addon.installDate.valueOf() ? "controlled_by_other_extensions" : "controllable_by_this_extension"; },