Backed out changeset 9ed80152b595 (bug 1881073) on request by adw. CLOSED TREE

browser/components/urlbar/private/YelpSuggestions.sys.mjs

@@ -19,7 +19,7 @@ const RESULT_MENU_COMMAND = {
   INACCURATE_LOCATION: "inaccurate_location",
   NOT_INTERESTED: "not_interested",
   NOT_RELEVANT: "not_relevant",
-  SHOW_LESS_FREQUENTLY: "show_less_frequently",
+  SHOW_LESS_FREQUENTLY: "show_less_frequentry",
 };
 
 /**
@@ -42,19 +42,6 @@ export class YelpSuggestions extends BaseFeature {
     return ["Yelp"];
   }
 
-  get showLessFrequentlyCount() {
-    const count = lazy.UrlbarPrefs.get("yelp.showLessFrequentlyCount") || 0;
-    return Math.max(count, 0);
-  }
-
-  get canShowLessFrequently() {
-    const cap =
-      lazy.UrlbarPrefs.get("yelpShowLessFrequentlyCap") ||
-      lazy.QuickSuggest.backend.config?.showLessFrequentlyCap ||
-      0;
-    return !cap || this.showLessFrequentlyCount < cap;
-  }
-
   getSuggestionTelemetryType(suggestion) {
     return "yelp";
   }
@@ -70,9 +57,9 @@ export class YelpSuggestions extends BaseFeature {
     // subject wasn't typed in full, then apply the min length threshold and
     // return null if the entire search string is too short.
     if (
-      (this.showLessFrequentlyCount || !suggestion.subjectExactMatch) &&
+      (this.#showLessFrequentlyCount || !suggestion.subjectExactMatch) &&
       searchString.length <
-        this.showLessFrequentlyCount + this.#minKeywordLength
+        this.#showLessFrequentlyCount + this.#minKeywordLength
     ) {
       return null;
     }
@@ -124,7 +111,7 @@ export class YelpSuggestions extends BaseFeature {
       },
     ];
 
-    if (this.canShowLessFrequently) {
+    if (this.#canShowLessFrequently) {
       commands.push({
         name: RESULT_MENU_COMMAND.SHOW_LESS_FREQUENTLY,
         l10n: {
@@ -195,19 +182,19 @@ export class YelpSuggestions extends BaseFeature {
         break;
       case RESULT_MENU_COMMAND.SHOW_LESS_FREQUENTLY:
         view.acknowledgeFeedback(result);
-        this.incrementShowLessFrequentlyCount();
-        if (!this.canShowLessFrequently) {
+        this.#incrementShowLessFrequentlyCount();
+        if (!this.#canShowLessFrequently) {
           view.invalidateResultMenuCommands();
         }
         break;
     }
   }
 
-  incrementShowLessFrequentlyCount() {
-    if (this.canShowLessFrequently) {
+  #incrementShowLessFrequentlyCount() {
+    if (this.#canShowLessFrequently) {
       lazy.UrlbarPrefs.set(
         "yelp.showLessFrequentlyCount",
-        this.showLessFrequentlyCount + 1
+        this.#showLessFrequentlyCount + 1
       );
     }
   }
@@ -217,6 +204,16 @@ export class YelpSuggestions extends BaseFeature {
     return Math.max(len, 0);
   }
 
+  get #showLessFrequentlyCount() {
+    const count = lazy.UrlbarPrefs.get("yelp.showLessFrequentlyCount") || 0;
+    return Math.max(count, 0);
+  }
+
+  get #canShowLessFrequently() {
+    const cap = lazy.UrlbarPrefs.get("yelpShowLessFrequentlyCap") || 0;
+    return !cap || this.#showLessFrequentlyCount < cap;
+  }
+
   async #fetchCity() {
     if (!this.#merino) {
       this.#merino = new lazy.MerinoClient(this.constructor.name);

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

@@ -169,14 +169,6 @@ class _MerinoTestUtils {
     return SEARCH_PARAMS;
   }
 
-  /**
-   * @returns {object}
-   *   Mock geolocation data.
-   */
-  get GEOLOCATION() {
-    return { ...GEOLOCATION_DATA.custom_details.geolocation };
-  }
-
   /**
    * @returns {string}
    *   The weather keyword in `WEATHER_RS_DATA`. Can be used as a search string

browser/components/urlbar/tests/quicksuggest/browser/browser_quicksuggest_yelp.js

@@ -202,7 +202,7 @@ async function doShowLessFrequently({
         );
         await UrlbarTestUtils.openResultMenuAndClickItem(
           window,
-          "show_less_frequently",
+          "show_less_frequentry",
           { resultIndex, openByMouse: true }
         );
 

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

@@ -622,13 +622,8 @@ async function doMigrateTest({
  *   The name of the Nimbus variable that stores the "show less frequently" cap
  *   being tested.
  * @param {object} options.keyword
- *   The primary keyword to use during the test.
- * @param {number} options.keywordBaseIndex
- *   The index in `keyword` to base substring checks around. This function will
- *   test substrings starting at the beginning of keyword and ending at the
- *   following indexes: one index before `keywordBaseIndex`,
- *   `keywordBaseIndex`, `keywordBaseIndex` + 1, `keywordBaseIndex` + 2, and
- *   `keywordBaseIndex` + 3.
+ *   The primary keyword to use during the test. It must contain more than one
+ *   word, and it must have at least two chars after the first space.
  */
 async function doShowLessFrequentlyTests({
   feature,
@@ -636,19 +631,18 @@ async function doShowLessFrequentlyTests({
   showLessFrequentlyCountPref,
   nimbusCapVariable,
   keyword,
-  keywordBaseIndex = keyword.indexOf(" "),
 }) {
   // Do some sanity checks on the keyword. Any checks that fail are errors in
   // the test.
-  if (keywordBaseIndex <= 0) {
-    throw new Error(
-      "keywordBaseIndex must be > 0, but it's " + keywordBaseIndex
-    );
+  let spaceIndex = keyword.indexOf(" ");
+  if (spaceIndex < 0) {
+    throw new Error("keyword must contain a space");
   }
-  if (keyword.length < keywordBaseIndex + 3) {
-    throw new Error(
-      "keyword must have at least two chars after keywordBaseIndex"
-    );
+  if (spaceIndex == 0) {
+    throw new Error("keyword must not start with a space");
+  }
+  if (keyword.length < spaceIndex + 3) {
+    throw new Error("keyword must have at least two chars after the space");
   }
 
   let tests = [
@@ -656,32 +650,32 @@ async function doShowLessFrequentlyTests({
       showLessFrequentlyCount: 0,
       canShowLessFrequently: true,
       newSearches: {
-        [keyword.substring(0, keywordBaseIndex - 1)]: false,
-        [keyword.substring(0, keywordBaseIndex)]: true,
-        [keyword.substring(0, keywordBaseIndex + 1)]: true,
-        [keyword.substring(0, keywordBaseIndex + 2)]: true,
-        [keyword.substring(0, keywordBaseIndex + 3)]: true,
+        [keyword.substring(0, spaceIndex - 1)]: false,
+        [keyword.substring(0, spaceIndex)]: true,
+        [keyword.substring(0, spaceIndex + 1)]: true,
+        [keyword.substring(0, spaceIndex + 2)]: true,
+        [keyword.substring(0, spaceIndex + 3)]: true,
       },
     },
     {
       showLessFrequentlyCount: 1,
       canShowLessFrequently: true,
       newSearches: {
-        [keyword.substring(0, keywordBaseIndex)]: false,
+        [keyword.substring(0, spaceIndex)]: false,
       },
     },
     {
       showLessFrequentlyCount: 2,
       canShowLessFrequently: true,
       newSearches: {
-        [keyword.substring(0, keywordBaseIndex + 1)]: false,
+        [keyword.substring(0, spaceIndex + 1)]: false,
       },
     },
     {
       showLessFrequentlyCount: 3,
       canShowLessFrequently: false,
       newSearches: {
-        [keyword.substring(0, keywordBaseIndex + 2)]: false,
+        [keyword.substring(0, spaceIndex + 2)]: false,
       },
     },
     {
@@ -691,16 +685,19 @@ async function doShowLessFrequentlyTests({
     },
   ];
 
-  info("Testing 'show less frequently' with cap in remote settings");
-  await doOneShowLessFrequentlyTest({
-    tests,
-    feature,
-    expectedResult,
-    showLessFrequentlyCountPref,
-    rs: {
-      show_less_frequently_cap: 3,
-    },
-  });
+  // The Rust implementation doesn't support the remote settings config.
+  if (!UrlbarPrefs.get("quicksuggest.rustEnabled")) {
+    info("Testing 'show less frequently' with cap in remote settings");
+    await doOneShowLessFrequentlyTest({
+      tests,
+      feature,
+      expectedResult,
+      showLessFrequentlyCountPref,
+      rs: {
+        show_less_frequently_cap: 3,
+      },
+    });
+  }
 
   // Nimbus should override remote settings.
   info("Testing 'show less frequently' with cap in Nimbus and remote settings");

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

@@ -6,8 +6,6 @@
 
 "use strict";
 
-const { GEOLOCATION } = MerinoTestUtils;
-
 const REMOTE_SETTINGS_RECORDS = [
   {
     type: "yelp-suggestions",
@@ -510,9 +508,9 @@ add_task(async function notRelevant() {
   });
 });
 
-// Tests the "Not interested" command: all Yelp suggestions should be disabled
+// Tests the "Not interested" command: all Pocket suggestions should be disabled
 // and not added anymore.
-add_task(async function notInterested() {
+add_tasks_with_rust(async function notInterested() {
   let result = makeExpectedResult({
     url: "https://www.yelp.com/search?find_desc=ramen&find_loc=tokyo",
     title: "ramen in tokyo",
@@ -541,7 +539,7 @@ add_task(async function notInterested() {
     matches: [],
   });
 
-  info("Doing search for another Yelp suggestion");
+  info("Doing search for another Pocket suggestion");
   await check_results({
     context: createContext("alongerkeyword in tokyo", {
       providers: [UrlbarProviderQuickSuggest.name],
@@ -554,31 +552,6 @@ add_task(async function notInterested() {
   await QuickSuggestTestUtils.forceSync();
 });
 
-// Tests the "show less frequently" behavior.
-add_task(async function showLessFrequently() {
-  let location = `${GEOLOCATION.city}, ${GEOLOCATION.region}`;
-
-  let originalUrl = new URL("https://www.yelp.com/search");
-  originalUrl.searchParams.set("find_desc", "alongerkeyword");
-
-  let url = new URL(originalUrl);
-  url.searchParams.set("find_loc", location);
-
-  await doShowLessFrequentlyTests({
-    feature: QuickSuggest.getFeature("YelpSuggestions"),
-    showLessFrequentlyCountPref: "yelp.showLessFrequentlyCount",
-    nimbusCapVariable: "yelpShowLessFrequentlyCap",
-    expectedResult: () =>
-      makeExpectedResult({
-        url: url.toString(),
-        originalUrl: originalUrl.toString(),
-        title: `alongerkeyword in ${location}`,
-      }),
-    keyword: "alongerkeyword",
-    keywordBaseIndex: 5,
-  });
-});
-
 // The `Yelp` Rust provider should be passed to the Rust component when
 // querying depending on whether Yelp suggestions are enabled.
 add_task(async function rustProviders() {
@@ -613,10 +586,8 @@ function makeExpectedResult(expected) {
   let originalUrl = expected.originalUrl ?? expected.url;
   let displayUrl =
     (expected.displayUrl ??
-      expected.url
-        .replace(/^https:\/\/www[.]/, "")
-        .replace("%20", " ")
-        .replace("%2C", ",")) + utmParameters;
+      expected.url.replace(/^https:\/\/www[.]/, "").replace("%20", " ")) +
+    utmParameters;
 
   return {
     type: UrlbarUtils.RESULT_TYPE.URL,