commit f5625a706d48c7bf9958847b4dc61e29d9f01ce3 parent 3c166bee68d7ea4c970086ba13f32e57f0e3ffc7 Author: Atila Butkovits <abutkovits@mozilla.com> Date: Mon, 13 Oct 2025 23:17:23 +0300 Revert "Bug 1992811, Bug 1993371, Bug 1966166 - Enable important-dates suggestions for en locales in DE, FR, and IT. r=daisuke" for causing failures at test_RustSharedRemoteSettingsService.js. This reverts commit c56b4740e9eb9d676874cec2551c67681b5a0b56. Revert "Bug 1993371 - Update region- and locale-handling in Suggest tests. r=daisuke" This reverts commit 4efbc26de19e03e6907b9d96c9bc7e9a212f4f8f. Revert "Bug 1966166 - Update the Rust SharedRemoteSettingsService on locale changes. r=bdk" This reverts commit 04eed55db9c3ed339758c5f03873af3836b73b8d. Diffstat:
11 files changed, 216 insertions(+), 762 deletions(-)
diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js @@ -577,9 +577,9 @@ pref("browser.urlbar.quicksuggest.mlEnabled", false); // backend. pref("browser.urlbar.quicksuggest.mlInitDelaySeconds", 0); -// Which Suggest settings to show in the settings UI, when Suggest is enabled. -// See `QuickSuggest.SETTINGS_UI` for values. -pref("browser.urlbar.quicksuggest.settingsUi", 1); +// Which Suggest settings to show in the settings UI. See +// `QuickSuggest.SETTINGS_UI` for values. +pref("browser.urlbar.quicksuggest.settingsUi", 0); // Whether unit conversion is enabled. pref("browser.urlbar.unitConversion.enabled", true); diff --git a/browser/components/urlbar/QuickSuggest.sys.mjs b/browser/components/urlbar/QuickSuggest.sys.mjs @@ -68,10 +68,10 @@ const SUGGEST_PREFS = Object.freeze({ }, "quicksuggest.enabled": { defaultValues: { - DE: [["de", ...EN_LOCALES], true], - FR: [["fr", ...EN_LOCALES], true], + DE: [["de"], true], + FR: [["fr"], true], GB: [EN_LOCALES, true], - IT: [["it", ...EN_LOCALES], true], + IT: [["it"], true], US: [EN_LOCALES, true], }, }, @@ -121,10 +121,10 @@ const SUGGEST_PREFS = Object.freeze({ }, "importantDates.featureGate": { defaultValues: { - DE: [["de", ...EN_LOCALES], true], - FR: [["fr", ...EN_LOCALES], true], + DE: [["de"], true], + FR: [["fr"], true], GB: [EN_LOCALES, true], - IT: [["it", ...EN_LOCALES], true], + IT: [["it"], true], US: [EN_LOCALES, true], }, }, diff --git a/browser/components/urlbar/tests/quicksuggest/QuickSuggestTestUtils.sys.mjs b/browser/components/urlbar/tests/quicksuggest/QuickSuggestTestUtils.sys.mjs @@ -1426,195 +1426,102 @@ class _QuickSuggestTestUtils { } /** - * Sets the app's home region and locale, calls your callback, and resets the - * region and locale. + * Sets the app's home region and locales, calls your callback, and resets + * the region and locales. * * @param {object} options * Options object. + * @param {Array} options.locales + * An array of locale strings. The entire array will be set as the available + * locales, and the first locale in the array will be set as the requested + * locale. * @param {Function} options.callback - * The callback to call. - * @param {string} [options.region] - * The region to set. See `setRegionAndLocale`. - * @param {string} [options.locale] - * The locale to set. See `setRegionAndLocale`. - * @param {boolean} [options.skipSuggestReset] - * Whether Suggest reset should be skipped after setting the new region and - * locale. See `setRegionAndLocale`. + * The callback to be called with the {@link locales} set. This function can + * be async. + * @param {string} options.homeRegion + * The home region to set, an all-caps country code, e.g., "US", "CA", "DE". + * Leave undefined to skip setting a region. */ - async withRegionAndLocale({ - callback, - region = undefined, - locale = undefined, - skipSuggestReset = false, - }) { - this.#log("withRegionAndLocale", "Calling setRegionAndLocale"); - let cleanup = await this.setRegionAndLocale({ - region, - locale, - skipSuggestReset, - }); + async withLocales({ locales, callback, homeRegion = undefined }) { + let promiseChanges = async desiredLocales => { + this.#log( + "withLocales", + "Changing locales from " + + JSON.stringify(Services.locale.requestedLocales) + + " to " + + JSON.stringify(desiredLocales) + ); - this.#log("withRegionAndLocale", "Calling callback"); - await callback(); + if (desiredLocales[0] == Services.locale.requestedLocales[0]) { + // Nothing happens when the locale doesn't actually change. + this.#log("withLocales", "Locale is already " + desiredLocales[0]); + return; + } - this.#log("withRegionAndLocale", "Calling cleanup"); - await cleanup(); + this.#log("withLocales", "Waiting for intl:requested-locales-changed"); + await lazy.TestUtils.topicObserved("intl:requested-locales-changed"); + this.#log("withLocales", "Observed intl:requested-locales-changed"); - this.#log("withRegionAndLocale", "Done"); - } + // Wait for the search service to reload engines. Otherwise tests can fail + // in strange ways due to internal search service state during shutdown. + // It won't always reload engines but it's hard to tell in advance when it + // won't, so also set a timeout. + this.#log("withLocales", "Waiting for TOPIC_SEARCH_SERVICE"); + await Promise.race([ + lazy.TestUtils.topicObserved( + lazy.SearchUtils.TOPIC_SEARCH_SERVICE, + (subject, data) => { + this.#log( + "withLocales", + "Observed TOPIC_SEARCH_SERVICE with data: " + data + ); + return data == "engines-reloaded"; + } + ), + new Promise(resolve => { + lazy.setTimeout(() => { + this.#log( + "withLocales", + "Timed out waiting for TOPIC_SEARCH_SERVICE" + ); + resolve(); + }, 2000); + }), + ]); - /** - * Sets the app's home region and locale and waits for all relevant - * notifications. Returns an async cleanup function that should be called to - * restore the previous region and locale. - * - * See also `withRegionAndLocale`. - * - * @param {object} options - * Options object. - * @param {string} [options.region] - * The home region to set. If falsey, the current region will remain - * unchanged. - * @param {string} [options.locale] - * The locale to set. If falsey, the current locale will remain unchanged. - * @param {Array} [options.availableLocales] - * Normally this should be left undefined. If defined, - * `Services.locale.availableLocales` will be set to this array. Otherwise - * it will be set to `[locale]`. - * @param {boolean} [options.skipSuggestReset] - * Normally this function resets `QuickSuggest` after the new region and - * locale are set, which will cause all Suggest prefs to be set according to - * the new region and locale. That's undesirable in some cases, for example - * when you're testing region/locale combinations where Suggest or one of - * its features isn't enabled by default. Pass in true to skip reset. - * @returns {Promise<Function>} - * An async cleanup function. - */ - async setRegionAndLocale({ - region = undefined, - locale = undefined, - availableLocales = undefined, - skipSuggestReset = false, - }) { - let oldRegion = lazy.Region.home; - let newRegion = region ?? oldRegion; - let regionPromise = this.#waitForAllRegionChanges(newRegion); - if (region) { - this.#log("setRegionAndLocale", "Setting region: " + region); - lazy.Region._setHomeRegion(region, true); - } + this.#log("withLocales", "Done waiting for locale changes"); + }; - let { - availableLocales: oldAvailableLocales, - requestedLocales: oldRequestedLocales, - } = Services.locale; - let newLocale = locale ?? oldRequestedLocales[0]; - let localePromise = this.#waitForAllLocaleChanges(newLocale); - if (locale) { - this.#log("setRegionAndLocale", "Setting locale: " + locale); - Services.locale.availableLocales = availableLocales ?? [locale]; - Services.locale.requestedLocales = [locale]; + let originalHome = lazy.Region.home; + if (homeRegion) { + lazy.Region._setHomeRegion(homeRegion, true); } - this.#log("setRegionAndLocale", "Waiting for region and locale changes"); - await Promise.all([regionPromise, localePromise]); + let available = Services.locale.availableLocales; + let requested = Services.locale.requestedLocales; + + let newRequested = locales.slice(0, 1); + let promise = promiseChanges(newRequested); + Services.locale.availableLocales = locales; + Services.locale.requestedLocales = newRequested; + await promise; - this.Assert.equal( - lazy.Region.home, - newRegion, - "Region is now " + newRegion - ); this.Assert.equal( Services.locale.appLocaleAsBCP47, - newLocale, - "App locale is now " + newLocale - ); - - if (!skipSuggestReset) { - this.#log("setRegionAndLocale", "Waiting for _test_reset"); - await lazy.QuickSuggest._test_reset(); - } - - if (this.#remoteSettingsServer) { - this.#log("setRegionAndLocale", "Waiting for forceSync"); - await this.forceSync(); - } - - this.#log("setRegionAndLocale", "Done"); - - return async () => { - this.#log( - "setRegionAndLocale", - "Cleanup started, calling setRegionAndLocale with old region and locale" - ); - await this.setRegionAndLocale({ - skipSuggestReset, - region: oldRegion, - locale: oldRequestedLocales[0], - availableLocales: oldAvailableLocales, - }); - this.#log("setRegionAndLocale", "Cleanup done"); - }; - } - - async #waitForAllRegionChanges(region) { - await lazy.TestUtils.waitForCondition( - () => lazy.SharedRemoteSettingsService.country == region, - "Waiting for SharedRemoteSettingsService.country to be " + region + locales[0], + "App locale is now " + locales[0] ); - } - - async #waitForAllLocaleChanges(locale) { - let promises = [ - lazy.TestUtils.waitForCondition( - () => lazy.SharedRemoteSettingsService.locale == locale, - "#waitForAllLocaleChanges: Waiting for SharedRemoteSettingsService.locale to be " + - locale - ), - ]; - if (locale == Services.locale.requestedLocales[0]) { - // "intl:app-locales-changed" isn't sent when the locale doesn't change. - this.#log("#waitForAllLocaleChanges", "Locale is already " + locale); - } else { - this.#log( - "#waitForAllLocaleChanges", - "Waiting for intl:app-locales-changed" - ); - promises.push(lazy.TestUtils.topicObserved("intl:app-locales-changed")); + await callback(); - // Wait for the search service to reload engines. Otherwise tests can fail - // in strange ways due to internal search service state during shutdown. - // It won't always reload engines but it's hard to tell in advance when it - // won't, so also set a timeout. - this.#log("#waitForAllLocaleChanges", "Waiting for TOPIC_SEARCH_SERVICE"); - promises.push( - Promise.race([ - lazy.TestUtils.topicObserved( - lazy.SearchUtils.TOPIC_SEARCH_SERVICE, - (subject, data) => { - this.#log( - "setLocales", - "Observed TOPIC_SEARCH_SERVICE with data: " + data - ); - return data == "engines-reloaded"; - } - ), - new Promise(resolve => { - lazy.setTimeout(() => { - this.#log( - "setLocales", - "Timed out waiting for TOPIC_SEARCH_SERVICE (not an error)" - ); - resolve(); - }, 2000); - }), - ]) - ); + if (homeRegion) { + lazy.Region._setHomeRegion(originalHome, true); } - await Promise.all(promises); - this.#log("#waitForAllLocaleChanges", "Done"); + promise = promiseChanges(requested); + Services.locale.availableLocales = available; + Services.locale.requestedLocales = requested; + await promise; } #log(fnName, msg) { diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_exposures_locales.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_exposures_locales.js @@ -175,10 +175,16 @@ async function doLocaleTest({ info("Doing locale test: " + JSON.stringify({ homeRegion, locale })); // Set the region and locale. - await QuickSuggestTestUtils.withRegionAndLocale({ - locale, - region: homeRegion, + await QuickSuggestTestUtils.withLocales({ + homeRegion, + locales: [locale], callback: async () => { + // Reinitialize Suggest, which will set default-branch values for + // Suggest prefs appropriate to the locale. + info("Reinitializing Suggest"); + await QuickSuggest._test_reset(); + info("Done reinitializing Suggest"); + // Sanity-check prefs. At this point, the value of `quickSuggestEnabled` // will be the value of its fallback pref, `quicksuggest.enabled`. assertSuggestPrefs(expectedQuickSuggestEnabled); @@ -222,6 +228,10 @@ async function doLocaleTest({ }, }); } + + // Reinitialize Suggest so prefs go back to their defaults now that the app is + // back to its default locale. + await QuickSuggest._test_reset(); } function assertSuggestPrefs(expectedEnabled) { diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_importantDatesSuggestions.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_importantDatesSuggestions.js @@ -4,11 +4,9 @@ https://creativecommons.org/publicdomain/zero/1.0/ */ "use strict"; const REMOTE_SETTINGS_RECORDS = [ - // US region, en-US locale { type: "dynamic-suggestions", suggestion_type: "important_dates", - filter_expression: "env.country == 'US' && env.locale == 'en-US'", attachment: [ { keywords: [["event", [" 1"]]], @@ -39,50 +37,6 @@ const REMOTE_SETTINGS_RECORDS = [ }, ], }, - - // DE region, de locale - { - type: "dynamic-suggestions", - suggestion_type: "important_dates", - filter_expression: "env.country == 'DE' && env.locale == 'de'", - attachment: [ - { - keywords: ["de de event"], - data: { - result: { - isImportantDate: true, - payload: { - dates: ["2026-01-01"], - name: "DE de Event", - }, - }, - }, - }, - ], - }, - - // DE region, en-US locale - { - type: "dynamic-suggestions", - suggestion_type: "important_dates", - filter_expression: "env.country == 'DE' && env.locale == 'en-US'", - attachment: [ - { - keywords: ["de en-us event"], - data: { - result: { - isImportantDate: true, - payload: { - dates: ["2026-01-02"], - name: "DE en-US Event", - }, - }, - }, - }, - ], - }, - - // other non-important-dates records { type: "dynamic-suggestions", suggestion_type: "other_suggestions", @@ -130,15 +84,11 @@ let SystemDate; add_setup(async function () { await QuickSuggestTestUtils.ensureQuickSuggestInit({ remoteSettingsRecords: REMOTE_SETTINGS_RECORDS, - prefs: [["quicksuggest.dynamicSuggestionTypes", "other_suggestions"]], - }); - - // All tasks will assume US region, en-US locale by default. - await QuickSuggestTestUtils.setRegionAndLocale({ - region: "US", - locale: "en-US", + prefs: [ + ["importantDates.featureGate", true], + ["quicksuggest.dynamicSuggestionTypes", "other_suggestions"], + ], }); - await Services.search.init(); SystemDate = Cu.getGlobalForObject(QuickSuggestTestUtils).Date; @@ -371,106 +321,6 @@ add_task(async function testTwoSuggestions() { await QuickSuggestTestUtils.forceSync(); }); -add_task(async function otherRegionsAndLocales() { - let tests = [ - // DE region, de locale (should match) - { - region: "DE", - locale: "de", - matchingQuery: "de de event", - expectedResultData: { - date: "Donnerstag, 1. Januar 2026", - description: "DE de Event", - }, - nonMatchingQueries: [ - "de en-US event", // DE region, en-US locale - "event 1", // US region, en-US locale - ], - }, - - // DE region, en-US locale (should match) - { - region: "DE", - locale: "en-US", - matchingQuery: "de en-us event", - expectedResultData: { - date: "Friday, January 2, 2026", - description: "DE en-US Event", - }, - nonMatchingQueries: [ - "de de event", // DE region, de locale - "event 1", // US region, en-US locale - ], - }, - - // XX region, en-US locale (should not match) - { - region: "XX", - locale: "en-US", - matchingQuery: null, - nonMatchingQueries: [ - "de de event", // DE region, de locale - "de en-us event", // DE region, en-US locale - "event 1", // US region, en-US locale - ], - }, - - // US region, de locale (should not match) - { - region: "US", - locale: "de", - matchingQuery: null, - nonMatchingQueries: [ - "de de event", // DE region, de locale - "de en-us event", // DE region, en-US locale - "event 1", // US region, en-US locale - ], - }, - ]; - - for (let { - region, - locale, - matchingQuery, - expectedResultData, - nonMatchingQueries, - } of tests) { - info("Doing subtest: " + JSON.stringify({ region, locale })); - - await QuickSuggestTestUtils.withRegionAndLocale({ - region, - locale, - callback: async () => { - Assert.equal( - UrlbarPrefs.get("quickSuggestEnabled"), - !!matchingQuery, - "quickSuggestEnabled should be enabled as expected" - ); - Assert.equal( - UrlbarPrefs.get("importantDates.featureGate"), - !!matchingQuery, - "importantDates.featureGate should be enabled as expected" - ); - - setTime("2025-03-01T00:00"); - - if (matchingQuery) { - info("Checking matching query: " + matchingQuery); - await checkDatesResults( - matchingQuery, - makeExpectedResult(expectedResultData) - ); - } - - for (let query of nonMatchingQueries) { - info("Checking non-matching query: " + query); - await checkDatesResults(query, null); - } - }, - }); - } -}); - /** * Stubs the Date object of the system global to use timeStr * when the constructor is called without arguments. diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_offlineDefault.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_offlineDefault.js @@ -12,26 +12,7 @@ ChromeUtils.defineESModuleGetters(this, { Preferences: "resource://gre/modules/Preferences.sys.mjs", }); -const EN_LOCALES = ["en-CA", "en-GB", "en-US", "en-ZA"]; - -// Expected prefs when Suggest is disabled. -const EXPECTED_PREFS_SUGGEST_DISABLED = { - "quicksuggest.enabled": false, - "quicksuggest.dataCollection.enabled": false, - "quicksuggest.settingsUi": QuickSuggest.SETTINGS_UI.NONE, - "suggest.quicksuggest.nonsponsored": false, - "suggest.quicksuggest.sponsored": false, - "addons.featureGate": false, - "amp.featureGate": false, - "importantDates.featureGate": false, - "mdn.featureGate": false, - "weather.featureGate": false, - "wikipedia.featureGate": false, - "yelp.featureGate": false, -}; - -// Expected prefs for native locales in EU countries (e.g., `de` locale in Germany). -const EXPECTED_PREFS_EU_NATIVE = { +const EXPECTED_EU_PREFS = { "quicksuggest.enabled": true, "quicksuggest.dataCollection.enabled": false, "quicksuggest.settingsUi": QuickSuggest.SETTINGS_UI.OFFLINE_ONLY, @@ -46,72 +27,55 @@ const EXPECTED_PREFS_EU_NATIVE = { "yelp.featureGate": false, }; -// Expected prefs for `en` locales in EU countries (e.g., `en-US` locale in -// Germany). -const EXPECTED_PREFS_EU_EN = { - ...EXPECTED_PREFS_SUGGEST_DISABLED, - "quicksuggest.enabled": true, - "importantDates.featureGate": true, -}; - -// Region -> locale -> expected prefs when Suggest is enabled -const EXPECTED_PREFS_BY_LOCALE_BY_REGION = { - DE: { - de: EXPECTED_PREFS_EU_NATIVE, - ...Object.fromEntries( - EN_LOCALES.map(locale => [locale, EXPECTED_PREFS_EU_EN]) - ), +// All the prefs that are initialized when Suggest is enabled, per region. +const EXPECTED_PREFS_BY_REGION = { + DE: EXPECTED_EU_PREFS, + FR: EXPECTED_EU_PREFS, + GB: { + "quicksuggest.enabled": true, + "quicksuggest.dataCollection.enabled": false, + "quicksuggest.settingsUi": QuickSuggest.SETTINGS_UI.OFFLINE_ONLY, + "suggest.quicksuggest.nonsponsored": true, + "suggest.quicksuggest.sponsored": true, + "addons.featureGate": false, + "amp.featureGate": true, + "importantDates.featureGate": true, + "mdn.featureGate": false, + "weather.featureGate": true, + "wikipedia.featureGate": true, + "yelp.featureGate": false, }, - FR: { - fr: EXPECTED_PREFS_EU_NATIVE, - ...Object.fromEntries( - EN_LOCALES.map(locale => [locale, EXPECTED_PREFS_EU_EN]) - ), + IT: EXPECTED_EU_PREFS, + US: { + "quicksuggest.enabled": true, + "quicksuggest.dataCollection.enabled": false, + "quicksuggest.settingsUi": QuickSuggest.SETTINGS_UI.OFFLINE_ONLY, + "suggest.quicksuggest.nonsponsored": true, + "suggest.quicksuggest.sponsored": true, + "addons.featureGate": true, + "amp.featureGate": true, + "importantDates.featureGate": true, + "mdn.featureGate": true, + "weather.featureGate": true, + "wikipedia.featureGate": true, + "yelp.featureGate": true, }, - GB: Object.fromEntries( - EN_LOCALES.map(locale => [ - locale, - { - "quicksuggest.enabled": true, - "quicksuggest.dataCollection.enabled": false, - "quicksuggest.settingsUi": QuickSuggest.SETTINGS_UI.OFFLINE_ONLY, - "suggest.quicksuggest.nonsponsored": true, - "suggest.quicksuggest.sponsored": true, - "addons.featureGate": false, - "amp.featureGate": true, - "importantDates.featureGate": true, - "mdn.featureGate": false, - "weather.featureGate": true, - "wikipedia.featureGate": true, - "yelp.featureGate": false, - }, - ]) - ), - IT: { - it: EXPECTED_PREFS_EU_NATIVE, - ...Object.fromEntries( - EN_LOCALES.map(locale => [locale, EXPECTED_PREFS_EU_EN]) - ), - }, - US: Object.fromEntries( - EN_LOCALES.map(locale => [ - locale, - { - "quicksuggest.enabled": true, - "quicksuggest.dataCollection.enabled": false, - "quicksuggest.settingsUi": QuickSuggest.SETTINGS_UI.OFFLINE_ONLY, - "suggest.quicksuggest.nonsponsored": true, - "suggest.quicksuggest.sponsored": true, - "addons.featureGate": true, - "amp.featureGate": true, - "importantDates.featureGate": true, - "mdn.featureGate": true, - "weather.featureGate": true, - "wikipedia.featureGate": true, - "yelp.featureGate": true, - }, - ]) - ), +}; + +// Expected prefs when Suggest is disabled. +const EXPECTED_PREFS_DISABLED = { + "quicksuggest.enabled": false, + "quicksuggest.dataCollection.enabled": false, + "quicksuggest.settingsUi": 0, + "suggest.quicksuggest.nonsponsored": false, + "suggest.quicksuggest.sponsored": false, + "addons.featureGate": false, + "amp.featureGate": false, + "importantDates.featureGate": false, + "mdn.featureGate": false, + "weather.featureGate": false, + "wikipedia.featureGate": false, + "yelp.featureGate": false, }; add_setup(async () => { @@ -120,36 +84,28 @@ add_setup(async () => { add_task(async function test() { let tests = [ - // Regions/locales where Suggest should be enabled to some extent - { region: "DE", locale: "de" }, - { region: "DE", locale: "en-GB" }, - { region: "DE", locale: "en-US" }, - - { region: "FR", locale: "fr" }, - { region: "FR", locale: "en-GB" }, - { region: "FR", locale: "en-US" }, - - { region: "GB", locale: "en-US" }, - { region: "GB", locale: "en-CA" }, - { region: "GB", locale: "en-GB" }, - - { region: "IT", locale: "it" }, - { region: "IT", locale: "en-GB" }, - { region: "IT", locale: "en-US" }, - - { region: "US", locale: "en-US" }, - { region: "US", locale: "en-CA" }, - { region: "US", locale: "en-GB" }, - - // Regions/locales where Suggest should be completely disabled - { region: "CA", locale: "en-US" }, - { region: "CA", locale: "en-CA" }, - { region: "GB", locale: "de" }, - { region: "US", locale: "de" }, + // Suggest should be enabled + { region: "DE", locale: "de", expectSuggestToBeEnabled: true }, + { region: "FR", locale: "fr", expectSuggestToBeEnabled: true }, + { region: "GB", locale: "en-US", expectSuggestToBeEnabled: true }, + { region: "GB", locale: "en-CA", expectSuggestToBeEnabled: true }, + { region: "GB", locale: "en-GB", expectSuggestToBeEnabled: true }, + { region: "IT", locale: "it", expectSuggestToBeEnabled: true }, + { region: "US", locale: "en-US", expectSuggestToBeEnabled: true }, + { region: "US", locale: "en-CA", expectSuggestToBeEnabled: true }, + { region: "US", locale: "en-GB", expectSuggestToBeEnabled: true }, + + // Suggest should be disabled + { region: "CA", locale: "en-US", expectSuggestToBeEnabled: false }, + { region: "CA", locale: "en-CA", expectSuggestToBeEnabled: false }, + { region: "DE", locale: "en-US", expectSuggestToBeEnabled: false }, + { region: "FR", locale: "en-US", expectSuggestToBeEnabled: false }, + { region: "GB", locale: "de", expectSuggestToBeEnabled: false }, + { region: "IT", locale: "en-US", expectSuggestToBeEnabled: false }, + { region: "US", locale: "de", expectSuggestToBeEnabled: false }, ]; - - for (let { locale, region } of tests) { - await doTest({ locale, region }); + for (let { locale, region, expectSuggestToBeEnabled } of tests) { + await doTest({ locale, region, expectSuggestToBeEnabled }); } }); @@ -163,11 +119,20 @@ add_task(async function test() { * The locale to simulate. * @param {string} options.region * The "home" region to simulate. + * @param {boolean} options.expectSuggestToBeEnabled + * Whether Suggest is expected to be enabled by default for the given locale + * and region. */ -async function doTest({ locale, region }) { - let expectedPrefs = - EXPECTED_PREFS_BY_LOCALE_BY_REGION[region]?.[locale] ?? - EXPECTED_PREFS_SUGGEST_DISABLED; +async function doTest({ locale, region, expectSuggestToBeEnabled }) { + let expectedPrefs = EXPECTED_PREFS_DISABLED; + if (expectSuggestToBeEnabled) { + expectedPrefs = EXPECTED_PREFS_BY_REGION[region]; + Assert.ok( + expectedPrefs, + "EXPECTED_PREFS_BY_REGION should have an entry for region since expectSuggestToBeEnabled is true, region=" + + region + ); + } let defaults = new Preferences({ branch: "browser.urlbar.", @@ -182,10 +147,12 @@ async function doTest({ locale, region }) { } // Set the region and locale, call the function, check the pref values. - await QuickSuggestTestUtils.withRegionAndLocale({ - region, - locale, + await QuickSuggestTestUtils.withLocales({ + homeRegion: region, + locales: [locale], callback: async () => { + await QuickSuggest._test_reset(); + for (let [name, value] of Object.entries(expectedPrefs)) { // Check the default-branch value. Assert.strictEqual( diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_remoteSettingsFilter.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_remoteSettingsFilter.js @@ -241,14 +241,19 @@ add_task(async function () { async function doTests({ locales, homeRegion, tests }) { for (let locale of locales) { - await QuickSuggestTestUtils.withRegionAndLocale({ - locale, - region: homeRegion, - // AMP and Wikipedia suggestions are not enabled by default for all - // regions/locales in this test, so don't reset Suggest so that they - // remain enabled rather than being set according to region/locale. - skipSuggestReset: true, + // Disable and reenable Suggest so its store is recreated with the + // appropriate remote settings app context for the locale and region. + info("Disabling Suggest: " + JSON.stringify({ locales, homeRegion })); + UrlbarPrefs.set("quicksuggest.enabled", false); + + await QuickSuggestTestUtils.withLocales({ + homeRegion, + locales: [locale], callback: async () => { + info("Reenabling Suggest: " + JSON.stringify({ locale, homeRegion })); + UrlbarPrefs.set("quicksuggest.enabled", true); + await QuickSuggestTestUtils.forceSync(); + for (let { expected, queries } of tests) { for (let query of queries) { info( diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js b/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js @@ -298,12 +298,8 @@ async function doLocaleTest({ shouldRunTask, osUnit, unitsByLocale }) { // Check locales. for (let [locale, temperatureUnit] of Object.entries(unitsByLocale)) { - await QuickSuggestTestUtils.withRegionAndLocale({ - locale, - // Weather suggestions are not enabled by default for all regions/locale - // combinations in this test, so don't reset Suggest so that they remain - // enabled rather than being set according to region/locale. - skipSuggestReset: true, + await QuickSuggestTestUtils.withLocales({ + locales: [locale], callback: async () => { info("Checking locale: " + locale); await check_results({ @@ -449,13 +445,9 @@ add_task(async function queryOutsideNorthAmerica_clientOutsideNorthAmerica() { }); async function doRegionTest({ homeRegion, locale, query, expectedTitleL10n }) { - await QuickSuggestTestUtils.withRegionAndLocale({ - locale, - region: homeRegion, - // Weather suggestions are not enabled by default for all regions/locale - // combinations in this test, so don't reset Suggest so that they remain - // enabled rather than being set according to region/locale. - skipSuggestReset: true, + await QuickSuggestTestUtils.withLocales({ + homeRegion, + locales: [locale], callback: async () => { info( "Doing region test: " + JSON.stringify({ homeRegion, locale, query }) diff --git a/toolkit/modules/RustSharedRemoteSettingsService.sys.mjs b/toolkit/modules/RustSharedRemoteSettingsService.sys.mjs @@ -36,7 +36,7 @@ class _SharedRemoteSettingsService { ); this.#config = new RemoteSettingsConfig2({ - server: this.#makeServer(Utils.SERVER_URL), + server: new RemoteSettingsServer.Custom({ url: Utils.SERVER_URL }), bucketName: Utils.actualBucketName("main"), appContext: new RemoteSettingsContext({ formFactor: "desktop", @@ -51,44 +51,21 @@ class _SharedRemoteSettingsService { }); Services.obs.addObserver(this, Region.REGION_TOPIC); - Services.obs.addObserver(this, "intl:app-locales-changed"); this.#rustService = RemoteSettingsService.init(storageDir, this.#config); } /** - * @returns {string} - * The country of the service's app context. - */ - get country() { - return this.#config.appContext.country; - } - - /** - * @returns {string} - * The locale of the service's app context. - */ - get locale() { - return this.#config.appContext.locale; - } - - /** - * @returns {RemoteSettingsServer} - * The service's server. - */ - get server() { - return this.#config.server; - } - - /** * Update the Remote Settings server * * @param {object} opts object with the following fields: * - `url`: server URL (defaults to the production URL) * - `bucketName`: bucket name (defaults to "main") */ - updateServer(opts = {}) { - this.#config.server = this.#makeServer(opts.url ?? Utils.SERVER_URL); + updateServer(opts) { + this.#config.server = opts.url + ? new RemoteSettingsServer.Custom({ url: opts.url }) + : undefined; this.#config.bucketName = opts.bucketName ?? Utils.actualBucketName("main"); this.#rustService.updateConfig(this.#config); } @@ -109,51 +86,14 @@ class _SharedRemoteSettingsService { } observe(subj, topic) { - switch (topic) { - case Region.REGION_TOPIC: { - const newCountry = subj.data; - if (newCountry != this.#config.appContext.country) { - this.#config.appContext.country = newCountry; - this.#rustService.updateConfig(this.#config); - } - break; - } - case "intl:app-locales-changed": { - const newLocale = Services.locale.appLocaleAsBCP47; - if (newLocale != this.#config.appContext.locale) { - this.#config.appContext.locale = newLocale; - this.#rustService.updateConfig(this.#config); - } - break; + if (topic == Region.REGION_TOPIC) { + const newCountry = subj.data; + if (newCountry != this.#config.appContext.country) { + this.#config.appContext.country = newCountry; + this.#rustService.updateConfig(this.#config); } } } - - #makeServer(url) { - // This is annoyingly complex but set `config.server` to a falsey value - // while tests are running and the URL is `Utils.SERVER_URL`. This will - // cause the Rust component to fall back to the production server, but it - // will avoid "cannot-be-a-base" errors. Since remote connections are not - // allowed during testing, consumers will need to avoid using the RS - // service. Ideally we would both handle the cannot-be-a-base errors and - // avoid pinging the production server for them somehow. - // - // Details: - // - // * During normal operation, `Utils.SERVER_URL` is the production URL, but - // during tests, it's a dummy data URI, `data:,#remote-settings-dummy/v1`, - // which is a "cannot-be-a-base" URL. - // - // * `RemoteSettingsService::new` falls back to the production URL when - // `config.server.url` is a cannot-be-a-base URL. So passing in the dummy - // data URI is actually fine for `new`. - // - // * In contrast, `RemoteSettingsService::update_config` returns the error - // when it parses a cannot-be-a-base `config.server.url`. - return !Utils.shouldSkipRemoteActivityDueToTests || url != Utils.SERVER_URL - ? new RemoteSettingsServer.Custom({ url }) - : null; - } } export const SharedRemoteSettingsService = new _SharedRemoteSettingsService(); diff --git a/toolkit/modules/tests/xpcshell/test_RustSharedRemoteSettingsService.js b/toolkit/modules/tests/xpcshell/test_RustSharedRemoteSettingsService.js @@ -1,215 +0,0 @@ -/* 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/. */ - -ChromeUtils.defineESModuleGetters(this, { - Region: "resource://gre/modules/Region.sys.mjs", - SharedRemoteSettingsService: - "resource://gre/modules/RustSharedRemoteSettingsService.sys.mjs", - Utils: "resource://services-settings/Utils.sys.mjs", -}); - -// When the app's region or locale changes, `SharedRemoteSettingsService` should -// update its country and locale. -add_task(async function regionOrLocaleChanged() { - let tests = [ - { country: "US", locale: "en-US" }, - { country: "US", locale: "es-MX" }, - { country: "DE", locale: "de" }, - { country: "DE", locale: "en-US" }, - ]; - - for (let { country, locale } of tests) { - await withRegionAndLocale({ - locale, - region: country, - callback: async () => { - await waitForServiceCountryAndLocale({ country, locale }); - Assert.equal( - SharedRemoteSettingsService.country, - country, - "SharedRemoteSettingsService.country should be the expected country" - ); - Assert.equal( - SharedRemoteSettingsService.locale, - locale, - "SharedRemoteSettingsService.locale should be the expected locale" - ); - }, - }); - } -}); - -// When the app's region or locale changes, the service's server URL should not -// change. This task makes sure we use the correct server URL when we update the -// config in response to region or locale changes. -add_task(async function serverUrl() { - let countriesAndLocales = [ - { newLocale: "de" }, - { newCountry: "DE" }, - { newCountry: "DE", newLocale: "de" }, - ]; - let serverUrls = [ - undefined, - Utils.SERVER_URL, - "https://example.com/test-shared-rs-service-1", - "https://example.com/test-shared-rs-service-2", - ]; - for (let { newCountry, newLocale } of countriesAndLocales) { - for (let url of serverUrls) { - await doServerUrlTest({ newCountry, newLocale, url }); - } - } -}); - -async function doServerUrlTest({ - url, - newCountry = "US", - newLocale = "en-US", -}) { - // First, set an initial app country and locale. - await withRegionAndLocale({ - region: "US", - locale: "en-US", - callback: async () => { - await waitForServiceCountryAndLocale({ country: "US", locale: "en-US" }); - - // Set the initial server URL. - SharedRemoteSettingsService.updateServer({ url }); - - // Now update the app country and locale. - await withRegionAndLocale({ - region: newCountry, - locale: newLocale, - callback: async () => { - await waitForServiceCountryAndLocale({ - country: newCountry, - locale: newLocale, - }); - - // The server URL set above should remain set. - if (!url || url == Utils.SERVER_URL) { - // A falsey URL should fall back to `Utils.SERVER_URL`, and during - // testing `Utils.SERVER_URL` is a cannot-be-a-base URL, so the - // server should be undefined. - Assert.ok( - !SharedRemoteSettingsService.server, - "SharedRemoteSettingsService.server should be undefined" - ); - } else { - Assert.ok( - SharedRemoteSettingsService.server, - "SharedRemoteSettingsService.server should be defined" - ); - Assert.equal( - SharedRemoteSettingsService.server.url, - url, - "SharedRemoteSettingsService.server.url should be as expected" - ); - } - }, - }); - }, - }); -} - -/** - * Sets the app's region (country) and locale, calls your callback, and restores - * the original region and locale. - * - * @param {string} region - * The app region to set. - * @param {string} locale - * The app locale to set. - * @param {function} callback - * Your callback. - */ -async function withRegionAndLocale({ region, locale, callback }) { - let originalRegion = Region.home; - - info("Setting region: " + region); - Region._setHomeRegion(region, true); - - Assert.equal(Region.home, region, "Region should now be the desired region"); - - let { availableLocales, requestedLocales } = Services.locale; - let localePromise = waitForLocaleChange(locale); - - info("Setting locale: " + locale); - Services.locale.availableLocales = [locale]; - Services.locale.requestedLocales = [locale]; - - info("Waiting for locale change"); - await localePromise; - info("Done waiting for locale change"); - - Assert.equal( - Services.locale.appLocaleAsBCP47, - locale, - "App locale should now be the desired locale" - ); - - info("Calling callback"); - await callback(); - info("Done calling callback"); - - // Restore the original region and locales. - info("Resetting region to orginal: " + originalRegion); - Region._setHomeRegion(originalRegion, true); - - Assert.equal( - Region.home, - originalRegion, - "Region should now be the original region" - ); - - let restoreLocalePromise = waitForLocaleChange(requestedLocales[0]); - Services.locale.availableLocales = availableLocales; - Services.locale.requestedLocales = requestedLocales; - - info("Waiting for original locale to be restored"); - await restoreLocalePromise; - info("Done waiting for original locale to be restored"); - - Assert.equal( - Services.locale.appLocaleAsBCP47, - requestedLocales[0], - "App locale should now be the original locale" - ); -} - -/** - * Waits for the app locale to be set to an expected value. - * - * @param {string} locale - * The expected locale. - */ -async function waitForLocaleChange(locale) { - // Nothing happens when the locale doesn't actually change. - if (locale == Services.locale.requestedLocales[0]) { - info("Locale is already set"); - } else { - info("Waiting for intl:app-locales-changed"); - await TestUtils.topicObserved("intl:app-locales-changed"); - info("Got intl:app-locales-changed"); - } -} - -/** - * Waits for the country and locale of the `SharedRemoteSettingsService` to be - * set to some expected values. - * - * @param {string} country - * The expected country. - * @param {string} locale - * The expected locale. - */ -async function waitForServiceCountryAndLocale({ country, locale }) { - await TestUtils.waitForCondition( - () => - SharedRemoteSettingsService.country == country && - SharedRemoteSettingsService.locale == locale, - "Waiting for SharedRemoteSettingsService country and locale to be set: " + - JSON.stringify({ country, locale }) - ); -} diff --git a/toolkit/modules/tests/xpcshell/xpcshell.toml b/toolkit/modules/tests/xpcshell/xpcshell.toml @@ -100,8 +100,6 @@ skip-if = ["os == 'android'"] ["test_Region_geocoding.js"] -["test_RustSharedRemoteSettingsService.js"] - ["test_Services.js"] ["test_UpdateUtils_updatechannel.js"]