Bug 1916873 - Allow Suggest to be safely enabled in non-Suggest locales. r=daisuke,settings-reviewers,mossop

In addition to addressing the bug, this also adds some exposure tests to make
sure that they can be triggered in non-Suggest locales and that non-exposure
suggestions are not triggered.

Depends on D220501

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

browser/app/profile/firefox.js

@@ -505,7 +505,7 @@ pref("browser.urlbar.quicksuggest.enabled", false);
 pref("browser.urlbar.quicksuggest.rustEnabled", true);
 
 // Whether to show the QuickSuggest onboarding dialog.
-pref("browser.urlbar.quicksuggest.shouldShowOnboardingDialog", true);
+pref("browser.urlbar.quicksuggest.shouldShowOnboardingDialog", false);
 
 // Show QuickSuggest onboarding dialog on the nth browser restarts.
 pref("browser.urlbar.quicksuggest.showOnboardingDialogAfterNRestarts", 0);

browser/components/preferences/search.js

@@ -380,9 +380,10 @@ var gSearchPane = {
       let elementIds = ["locationBarGroupHeader", "locationBarSuggestionLabel"];
       for (let id of elementIds) {
         let element = document.getElementById(id);
-        element.dataset.l10nId = element.dataset.l10nIdOriginal;
+        if (element.dataset.l10nIdOriginal) {
+          document.l10n.setAttributes(element, element.dataset.l10nIdOriginal);
           delete element.dataset.l10nIdOriginal;
-        document.l10n.translateElements([element]);
+        }
       }
     }
   },

browser/components/urlbar/UrlbarPrefs.sys.mjs

@@ -308,7 +308,7 @@ const PREF_URLBAR_DEFAULTS = new Map([
   ["quicksuggest.seenRestarts", 0],
 
   // Whether to show the quick suggest onboarding dialog.
-  ["quicksuggest.shouldShowOnboardingDialog", true],
+  ["quicksuggest.shouldShowOnboardingDialog", false],
 
   // Whether the user has seen the onboarding dialog.
   ["quicksuggest.showedOnboardingDialog", false],
@@ -1108,6 +1108,10 @@ class Preferences {
     return {
       history: {
         "quicksuggest.enabled": false,
+        "quicksuggest.dataCollection.enabled": false,
+        "quicksuggest.shouldShowOnboardingDialog": false,
+        "suggest.quicksuggest.nonsponsored": false,
+        "suggest.quicksuggest.sponsored": false,
       },
       offline: {
         "quicksuggest.enabled": true,

browser/components/urlbar/docs/preferences.rst

@@ -173,7 +173,7 @@ browser.urlbar.quicksuggest.enabled (boolean, default: false)
 browser.urlbar.quicksuggest.dataCollection.enabled (boolean, default: false)
   Whether data collection is enabled for quick suggest results.
 
-browser.urlbar.quicksuggest.shouldShowOnboardingDialog (boolean, default: true)
+browser.urlbar.quicksuggest.shouldShowOnboardingDialog (boolean, default: false)
   Whether to show the quick suggest onboarding dialog.
 
 browser.urlbar.richSuggestions.tail (boolean, default: true)

browser/components/urlbar/tests/UrlbarTestUtils.sys.mjs

@@ -17,10 +17,8 @@ ChromeUtils.defineESModuleGetters(lazy, {
   ExperimentAPI: "resource://nimbus/ExperimentAPI.sys.mjs",
   ExperimentFakes: "resource://testing-common/NimbusTestUtils.sys.mjs",
   ExperimentManager: "resource://nimbus/lib/ExperimentManager.sys.mjs",
-
   FormHistoryTestUtils:
     "resource://testing-common/FormHistoryTestUtils.sys.mjs",
-
   NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
   PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.sys.mjs",
   TestUtils: "resource://testing-common/TestUtils.sys.mjs",

browser/components/urlbar/tests/quicksuggest/QuickSuggestTestUtils.sys.mjs

@@ -11,6 +11,7 @@ ChromeUtils.defineESModuleGetters(lazy, {
   ExperimentFakes: "resource://testing-common/NimbusTestUtils.sys.mjs",
   NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
   QuickSuggest: "resource:///modules/QuickSuggest.sys.mjs",
+  Region: "resource://gre/modules/Region.sys.mjs",
   RemoteSettings: "resource://services-settings/remote-settings.sys.mjs",
   RemoteSettingsConfig: "resource://gre/modules/RustRemoteSettings.sys.mjs",
   RemoteSettingsServer:
@@ -1166,17 +1167,23 @@ class _QuickSuggestTestUtils {
   }
 
   /**
-   * Sets the app's locales, calls your callback, and resets locales.
+   * Sets the app's home region and locales, calls your callback, and resets
+   * the region and locales.
    *
-   * @param {Array} 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} callback
+   * @param {Function} options.callback
    *  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 withLocales(locales, callback) {
+  async withLocales({ locales, callback, homeRegion = undefined }) {
     let promiseChanges = async desiredLocales => {
       this.#log(
         "withLocales",
@@ -1188,6 +1195,7 @@ class _QuickSuggestTestUtils {
 
       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;
       }
 
@@ -1225,6 +1233,11 @@ class _QuickSuggestTestUtils {
       this.#log("withLocales", "Done waiting for locale changes");
     };
 
+    let originalHome = lazy.Region.home;
+    if (homeRegion) {
+      lazy.Region._setHomeRegion(homeRegion, false);
+    }
+
     let available = Services.locale.availableLocales;
     let requested = Services.locale.requestedLocales;
 
@@ -1242,6 +1255,10 @@ class _QuickSuggestTestUtils {
 
     await callback();
 
+    if (homeRegion) {
+      lazy.Region._setHomeRegion(originalHome, false);
+    }
+
     promise = promiseChanges(requested);
     Services.locale.availableLocales = available;
     Services.locale.requestedLocales = requested;

browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_exposures_locales.js

@@ -0,0 +1,259 @@
+/* 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/. */
+
+// Checks that exposure suggestions can be enabled in Nimbus experiments
+// regardless of region and locale, even for regions and locales where Suggest
+// is normally disabled.
+
+"use strict";
+
+ChromeUtils.defineESModuleGetters(this, {
+  ExperimentManager: "resource://nimbus/lib/ExperimentManager.sys.mjs",
+});
+
+const REMOTE_SETTINGS_RECORDS = [
+  {
+    type: "exposure-suggestions",
+    suggestion_type: "aaa",
+    attachment: {
+      keywords: ["aaa keyword", "aaa bbb keyword", "amp", "wikipedia"],
+    },
+  },
+  {
+    type: "exposure-suggestions",
+    suggestion_type: "bbb",
+    attachment: {
+      keywords: ["bbb keyword", "aaa bbb keyword", "amp", "wikipedia"],
+    },
+  },
+  {
+    type: "data",
+    attachment: [
+      QuickSuggestTestUtils.ampRemoteSettings(),
+      QuickSuggestTestUtils.wikipediaRemoteSettings(),
+    ],
+  },
+];
+
+add_setup(async function () {
+  // This test calls `UrlbarPrefs.updateFirefoxSuggestScenario()`, which relies
+  // on `ExperimentManager` startup, which doesn't happen by default in xpcshell
+  // tests, so trigger that now.
+  info("Awaiting ExperimentManager.onStartup");
+  await ExperimentManager.onStartup();
+  info("Done awaiting ExperimentManager.onStartup");
+
+  await QuickSuggestTestUtils.ensureQuickSuggestInit({
+    remoteSettingsRecords: REMOTE_SETTINGS_RECORDS,
+  });
+
+  // `ensureQuickSuggestInit()` enabled Suggest, but we want to start with it
+  // disabled so that when we change locales, we can verify Suggest is properly
+  // disabled or enabled depending on the locale.
+  UrlbarPrefs.clear("quicksuggest.enabled");
+});
+
+add_task(async function suggestEnabledLocales() {
+  let tests = [
+    {
+      homeRegion: "US",
+      locales: ["en-US", "en-CA", "en-GB"],
+      expectedQuickSuggestEnabled: true,
+      queries: [
+        {
+          query: "amp",
+          expectedResults: [
+            QuickSuggestTestUtils.ampResult(),
+            makeExpectedExposureResult("bbb"),
+            makeExpectedExposureResult("aaa"),
+          ],
+        },
+        {
+          query: "wikipedia",
+          expectedResults: [
+            QuickSuggestTestUtils.wikipediaResult(),
+            makeExpectedExposureResult("bbb"),
+            makeExpectedExposureResult("aaa"),
+          ],
+        },
+        {
+          query: "aaa keyword",
+          expectedResults: [makeExpectedExposureResult("aaa")],
+        },
+        {
+          query: "aaa bbb keyword",
+          expectedResults: [
+            makeExpectedExposureResult("bbb"),
+            makeExpectedExposureResult("aaa"),
+          ],
+        },
+      ],
+    },
+  ];
+
+  for (let test of tests) {
+    await doLocaleTest(test);
+  }
+});
+
+add_task(async function suggestDisabledLocales() {
+  let queries = [
+    {
+      query: "amp",
+      expectedResults: [
+        // No AMP result!
+        makeExpectedExposureResult("bbb"),
+        makeExpectedExposureResult("aaa"),
+      ],
+    },
+    {
+      query: "wikipedia",
+      expectedResults: [
+        // No Wikipedia result!
+        makeExpectedExposureResult("bbb"),
+        makeExpectedExposureResult("aaa"),
+      ],
+    },
+    {
+      query: "aaa keyword",
+      expectedResults: [makeExpectedExposureResult("aaa")],
+    },
+    {
+      query: "aaa bbb keyword",
+      expectedResults: [
+        makeExpectedExposureResult("bbb"),
+        makeExpectedExposureResult("aaa"),
+      ],
+    },
+  ];
+
+  let tests = [
+    {
+      homeRegion: "US",
+      locales: ["de", "fr", "ja"],
+      expectedQuickSuggestEnabled: false,
+      queries,
+    },
+    {
+      homeRegion: "CA",
+      locales: ["en-US", "en-CA", "en-GB", "fr"],
+      expectedQuickSuggestEnabled: false,
+      queries,
+    },
+    {
+      homeRegion: "DE",
+      locales: ["de", "en-US", "fr"],
+      expectedQuickSuggestEnabled: false,
+      queries,
+    },
+  ];
+
+  for (let test of tests) {
+    await doLocaleTest(test);
+  }
+});
+
+async function doLocaleTest({
+  homeRegion,
+  locales,
+  expectedQuickSuggestEnabled,
+  queries,
+}) {
+  for (let locale of locales) {
+    info("Doing locale test: " + JSON.stringify({ homeRegion, locale }));
+
+    // Set the region and locale.
+    await QuickSuggestTestUtils.withLocales({
+      homeRegion,
+      locales: [locale],
+      callback: async () => {
+        // Update the Suggest scenario, which will set default-branch values for
+        // Suggest prefs appropriate to the locale.
+        info("Updating Suggest scenario");
+        await UrlbarPrefs.updateFirefoxSuggestScenario();
+        info("Done updating Suggest scenario");
+
+        // Sanity-check prefs. At this point, the value of `quickSuggestEnabled`
+        // will be the value of its fallback pref, `quicksuggest.enabled`.
+        assertSuggestPrefs(expectedQuickSuggestEnabled);
+        Assert.equal(
+          UrlbarPrefs.get("quickSuggestEnabled"),
+          expectedQuickSuggestEnabled,
+          "quickSuggestEnabled Nimbus variable should be correct after setting locale"
+        );
+
+        // Install an experiment that enables Suggest and exposures.
+        let nimbusCleanup = await UrlbarTestUtils.initNimbusFeature({
+          quickSuggestEnabled: true,
+          quickSuggestExposureSuggestionTypes: "aaa,bbb",
+        });
+        await QuickSuggestTestUtils.forceSync();
+
+        // All default- and user-branch Suggest prefs should remain the same.
+        assertSuggestPrefs(expectedQuickSuggestEnabled);
+
+        // But `quickSuggestEnabled` should be true, since we just installed
+        // an experiment with it set to true.
+        Assert.ok(
+          UrlbarPrefs.get("quickSuggestEnabled"),
+          "quickSuggestEnabled Nimbus variable should be enabled after installing experiment"
+        );
+
+        // Do a search and check the results.
+        for (let { query, expectedResults } of queries) {
+          await check_results({
+            context: createContext(query, {
+              providers: [UrlbarProviderQuickSuggest.name],
+              isPrivate: false,
+            }),
+            matches: expectedResults,
+          });
+        }
+
+        await nimbusCleanup();
+        await QuickSuggestTestUtils.forceSync();
+      },
+    });
+  }
+
+  // Reset Suggest prefs to their defaults by updating the scenario now that the
+  // app is back to its default locale.
+  await UrlbarPrefs.updateFirefoxSuggestScenario();
+}
+
+function assertSuggestPrefs(expectedEnabled) {
+  let prefs = [
+    "browser.urlbar.quicksuggest.enabled",
+    "browser.urlbar.suggest.quicksuggest.sponsored",
+    "browser.urlbar.suggest.quicksuggest.nonsponsored",
+  ];
+  for (let p of prefs) {
+    Assert.equal(
+      Services.prefs.getDefaultBranch("").getBoolPref(p),
+      expectedEnabled,
+      "Default-branch value should be correct: " + p
+    );
+    Assert.equal(
+      Services.prefs.getBranch("").getBoolPref(p),
+      expectedEnabled,
+      "User-branch value should be correct: " + p
+    );
+  }
+}
+
+function makeExpectedExposureResult(exposureSuggestionType) {
+  return {
+    type: UrlbarUtils.RESULT_TYPE.DYNAMIC,
+    source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+    heuristic: false,
+    exposureTelemetry: UrlbarUtils.EXPOSURE_TELEMETRY.HIDDEN,
+    payload: {
+      exposureSuggestionType,
+      source: "rust",
+      dynamicType: "exposure",
+      provider: "Exposure",
+      telemetryType: "exposure",
+    },
+  };
+}

browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_offlineDefault.js

@@ -7,10 +7,6 @@
 
 "use strict";
 
-ChromeUtils.defineESModuleGetters(this, {
-  Region: "resource://gre/modules/Region.sys.mjs",
-});
-
 // All the prefs that `updateFirefoxSuggestScenario` sets along with the
 // expected default-branch values when offline is enabled and when it's not
 // enabled.
@@ -27,7 +23,7 @@ const PREFS = [
     get: "getBoolPref",
     set: "setBoolPref",
     expectedOfflineValue: false,
-    expectedOtherValue: true,
+    expectedOtherValue: false,
   },
   {
     name: "browser.urlbar.suggest.quicksuggest.nonsponsored",
@@ -90,10 +86,17 @@ async function doTest({ locale, home, expectedOfflineDefault }) {
   }
 
   // Set the region and locale, call the function, check the pref values.
-  Region._setHomeRegion(home, false);
+  await QuickSuggestTestUtils.withLocales({
-  await QuickSuggestTestUtils.withLocales([locale], async () => {
+    homeRegion: home,
+    locales: [locale],
+    callback: async () => {
       await UrlbarPrefs.updateFirefoxSuggestScenario();
-    for (let { name, get, expectedOfflineValue, expectedOtherValue } of PREFS) {
+      for (let {
+        name,
+        get,
+        expectedOfflineValue,
+        expectedOtherValue,
+      } of PREFS) {
         let expectedValue = expectedOfflineDefault
           ? expectedOfflineValue
           : expectedOtherValue;
@@ -114,6 +117,7 @@ async function doTest({ locale, home, expectedOfflineDefault }) {
           `UrlbarPrefs.get() value for ${name}, locale ${locale}, home ${home}`
         );
       }
+    },
   });
 
   // Teardown: Restore original default-branch values for the next task.

browser/components/urlbar/tests/quicksuggest/unit/test_weather.js

@@ -656,7 +656,9 @@ async function doLocaleTest({ shouldRunTask, osUnit, unitsByLocale }) {
 
   // Check locales.
   for (let [locale, temperatureUnit] of Object.entries(unitsByLocale)) {
-    await QuickSuggestTestUtils.withLocales([locale], async () => {
+    await QuickSuggestTestUtils.withLocales({
+      locales: [locale],
+      callback: async () => {
         info("Checking locale: " + locale);
         await check_results({
           context: createContext(MerinoTestUtils.WEATHER_KEYWORD, {
@@ -686,6 +688,7 @@ async function doLocaleTest({ shouldRunTask, osUnit, unitsByLocale }) {
           ],
         });
         Services.prefs.clearUserPref("intl.regional_prefs.use_os_locales");
+      },
     });
   }
 }

browser/components/urlbar/tests/quicksuggest/unit/xpcshell.toml

@@ -15,6 +15,8 @@ firefox-appdir = "browser"
 
 ["test_quicksuggest_exposures.js"]
 
+["test_quicksuggest_exposures_locales.js"]
+
 ["test_quicksuggest_fakespot.js"]
 
 ["test_quicksuggest_impressionCaps.js"]