Bug 1881071: Stop suggest if the query length is less than length of when clicking 'Show less frequently' r=adw

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

browser/app/profile/firefox.js

@@ -648,6 +648,10 @@ pref("browser.urlbar.suggest.mdn", true);
 // Feature gate pref for Yelp suggestions in the urlbar.
 pref("browser.urlbar.yelp.featureGate", false);
 
+// The minimum prefix length of yelp query the user must type to trigger
+// the suggestion.
+pref("browser.urlbar.yelp.minKeywordLength", 5);
+
 // If `browser.urlbar.yelp.featureGate` is true, this controls whether
 // Yelp suggestions are turned on.
 pref("browser.urlbar.suggest.yelp", true);

browser/components/urlbar/UrlbarProviderQuickSuggest.sys.mjs

@@ -255,7 +255,12 @@ class ProviderQuickSuggest extends UrlbarProvider {
     if (details.result?.providerName == this.name) {
       let feature = this.#getFeatureByResult(details.result);
       if (feature?.handleCommand) {
-        feature.handleCommand(controller.view, details.result, details.selType);
+        feature.handleCommand(
+          controller.view,
+          details.result,
+          details.selType,
+          this._trimmedSearchString
+        );
       } else if (details.selType == "dismiss") {
         // Handle dismissals.
         this.#dismissResult(controller, details.result);

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

@@ -71,8 +71,7 @@ export class YelpSuggestions extends BaseFeature {
     // return null if the entire search string is too short.
     if (
       (this.showLessFrequentlyCount || !suggestion.subjectExactMatch) &&
-      searchString.length <
-        this.showLessFrequentlyCount + this.#minKeywordLength
+      searchString.length < this.#minKeywordLength
     ) {
       return null;
     }
@@ -165,7 +164,7 @@ export class YelpSuggestions extends BaseFeature {
     return commands;
   }
 
-  handleCommand(view, result, selType) {
+  handleCommand(view, result, selType, searchString) {
     switch (selType) {
       case RESULT_MENU_COMMAND.HELP:
         // "help" is handled by UrlbarInput, no need to do anything here.
@@ -199,6 +198,7 @@ export class YelpSuggestions extends BaseFeature {
         if (!this.canShowLessFrequently) {
           view.invalidateResultMenuCommands();
         }
+        lazy.UrlbarPrefs.set("yelp.minKeywordLength", searchString.length + 1);
         break;
     }
   }
@@ -213,8 +213,19 @@ export class YelpSuggestions extends BaseFeature {
   }
 
   get #minKeywordLength() {
-    const len = lazy.UrlbarPrefs.get("yelpMinKeywordLength") || 0;
-    return Math.max(len, 0);
+    // It's unusual to get both a Nimbus variable and its fallback pref at the
+    // same time, but we have a good reason. To recap, if a variable doesn't
+    // have a value, then the value of its fallback will be returned; otherwise
+    // the variable value will be returned. That's usually what we want, but for
+    // Yelp, we set the pref each time the user clicks "show less frequently",
+    // and we want the variable to act only as an initial min length. In other
+    // words, if the pref has a user value (because we set it), use it;
+    // otherwise use the initial value defined by the variable.
+    return Math.max(
+      lazy.UrlbarPrefs.get("yelpMinKeywordLength") || 0,
+      lazy.UrlbarPrefs.get("yelp.minKeywordLength") || 0,
+      0
+    );
   }
 
   async #fetchCity() {

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

@@ -81,63 +81,87 @@ add_task(async function basic() {
 
 // Tests the "Show less frequently" result menu command.
 add_task(async function resultMenu_show_less_frequently() {
+  info("Test for no yelpMinKeywordLength and no yelpShowLessFrequentlyCap");
   await doShowLessFrequently({
     minKeywordLength: 0,
     frequentlyCap: 0,
-    initialFrequentlyCount: 2,
     testData: [
       {
-        input: "ramen",
+        input: "best ra",
         expected: {
           hasSuggestion: true,
           hasShowLessItem: true,
         },
       },
       {
-        input: "ramen",
+        input: "best ra",
+        expected: {
+          hasSuggestion: false,
+        },
+      },
+      {
+        input: "best ram",
         expected: {
           hasSuggestion: true,
           hasShowLessItem: true,
         },
       },
       {
-        input: "ramen",
+        input: "best ram",
+        expected: {
+          hasSuggestion: false,
+        },
+      },
+      {
+        input: "best rame",
         expected: {
           hasSuggestion: true,
           hasShowLessItem: true,
         },
       },
       {
-        input: "ramen",
+        input: "best rame",
         expected: {
-          hasSuggestion: true,
-          hasShowLessItem: true,
+          hasSuggestion: false,
         },
       },
     ],
   });
 
+  info("Test whether yelpShowLessFrequentlyCap can work");
   await doShowLessFrequently({
     minKeywordLength: 0,
-    frequentlyCap: 4,
-    initialFrequentlyCount: 2,
+    frequentlyCap: 2,
     testData: [
       {
-        input: "ramen",
+        input: "best ra",
         expected: {
           hasSuggestion: true,
           hasShowLessItem: true,
         },
       },
       {
-        input: "ramen",
+        input: "best ram",
         expected: {
           hasSuggestion: true,
           hasShowLessItem: true,
         },
       },
       {
-        input: "ramen",
+        input: "best ram",
+        expected: {
+          hasSuggestion: false,
+        },
+      },
+      {
+        input: "best rame",
+        expected: {
+          hasSuggestion: true,
+          hasShowLessItem: false,
+        },
+      },
+      {
+        input: "best ramen",
         expected: {
           hasSuggestion: true,
           hasShowLessItem: false,
@@ -146,30 +170,37 @@ add_task(async function resultMenu_show_less_frequently() {
     ],
   });
 
+  info(
+    "Test whether local yelp.minKeywordLength pref can override nimbus variable yelpMinKeywordLength"
+  );
   await doShowLessFrequently({
-    minKeywordLength: 1,
-    frequentlyCap: 4,
-    initialFrequentlyCount: 2,
+    minKeywordLength: 8,
+    frequentlyCap: 0,
     testData: [
       {
-        input: "ramen",
+        input: "best ra",
+        expected: {
+          hasSuggestion: false,
+        },
+      },
+      {
+        input: "best ram",
         expected: {
           hasSuggestion: true,
           hasShowLessItem: true,
         },
       },
       {
-        input: "ramen",
+        input: "best rame",
         expected: {
           hasSuggestion: true,
           hasShowLessItem: true,
         },
       },
       {
-        input: "ramen",
+        input: "best rame",
         expected: {
-          hasSuggestion: true,
-          hasShowLessItem: false,
+          hasSuggestion: false,
         },
       },
     ],
@@ -179,14 +210,15 @@ add_task(async function resultMenu_show_less_frequently() {
 async function doShowLessFrequently({
   minKeywordLength,
   frequentlyCap,
-  initialFrequentlyCount,
   testData,
 }) {
+  UrlbarPrefs.clear("yelp.showLessFrequentlyCount");
+  UrlbarPrefs.clear("yelp.minKeywordLength");
+
   let cleanUpNimbus = await UrlbarTestUtils.initNimbusFeature({
     yelpMinKeywordLength: minKeywordLength,
     yelpShowLessFrequentlyCap: frequentlyCap,
   });
-  UrlbarPrefs.set("yelp.showLessFrequentlyCount", initialFrequentlyCount);
 
   for (let { input, expected } of testData) {
     await UrlbarTestUtils.promiseAutocompleteResultPopup({
@@ -217,6 +249,10 @@ async function doShowLessFrequently({
           UrlbarPrefs.get("yelp.showLessFrequentlyCount"),
           previousShowLessFrequentlyCount + 1
         );
+        Assert.equal(
+          UrlbarPrefs.get("yelp.minKeywordLength"),
+          input.length + 1
+        );
       } else {
         let menuitem = await UrlbarTestUtils.openResultMenuAndGetItem({
           window,
@@ -238,7 +274,8 @@ async function doShowLessFrequently({
   }
 
   await cleanUpNimbus();
-  UrlbarPrefs.set("yelp.showLessFrequentlyCount", 0);
+  UrlbarPrefs.clear("yelp.showLessFrequentlyCount");
+  UrlbarPrefs.clear("yelp.minKeywordLength");
 }
 
 // Tests the "Not relevant" result menu dismissal command.

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

@@ -57,17 +57,6 @@ add_task(async function basic() {
         title: "BeSt RaMeN dElIvErY iN tOkYo",
       },
     },
-    {
-      description: "No specific location",
-      query: "ramen",
-      expected: {
-        url: "https://www.yelp.com/search?find_desc=ramen&find_loc=Yokohama%2C+Kanagawa",
-        originalUrl: "https://www.yelp.com/search?find_desc=ramen",
-        displayUrl:
-          "yelp.com/search?find_desc=ramen&find_loc=Yokohama,+Kanagawa",
-        title: "ramen in Yokohama, Kanagawa",
-      },
-    },
     {
       description: "Query too short, no subject exact match: ra",
       query: "ra",
@@ -143,7 +132,13 @@ add_task(async function basic() {
         "Subject exact match with length == minKeywordLength, showLessFrequentlyCount non-zero",
       query: "ramen",
       showLessFrequentlyCount: 1,
-      expected: null,
+      expected: {
+        url: "https://www.yelp.com/search?find_desc=ramen&find_loc=Yokohama%2C+Kanagawa",
+        originalUrl: "https://www.yelp.com/search?find_desc=ramen",
+        displayUrl:
+          "yelp.com/search?find_desc=ramen&find_loc=Yokohama,+Kanagawa",
+        title: "ramen in Yokohama, Kanagawa",
+      },
     },
     {
       description: "Query too short: alon",
@@ -166,7 +161,13 @@ add_task(async function basic() {
         "Query length == minKeywordLength, subject not exact match, showLessFrequentlyCount non-zero",
       query: "along",
       showLessFrequentlyCount: 1,
-      expected: null,
+      expected: {
+        url: "https://www.yelp.com/search?find_desc=alongerkeyword&find_loc=Yokohama%2C+Kanagawa",
+        originalUrl: "https://www.yelp.com/search?find_desc=alongerkeyword",
+        displayUrl:
+          "yelp.com/search?find_desc=alongerkeyword&find_loc=Yokohama,+Kanagawa",
+        title: "alongerkeyword in Yokohama, Kanagawa",
+      },
     },
     {
       description:
@@ -186,7 +187,13 @@ add_task(async function basic() {
         "Query length < minKeywordLength + showLessFrequentlyCount, subject not exact match",
       query: "alonge",
       showLessFrequentlyCount: 2,
-      expected: null,
+      expected: {
+        url: "https://www.yelp.com/search?find_desc=alongerkeyword&find_loc=Yokohama%2C+Kanagawa",
+        originalUrl: "https://www.yelp.com/search?find_desc=alongerkeyword",
+        displayUrl:
+          "yelp.com/search?find_desc=alongerkeyword&find_loc=Yokohama,+Kanagawa",
+        title: "alongerkeyword in Yokohama, Kanagawa",
+      },
     },
     {
       description:
@@ -556,27 +563,135 @@ add_task(async function notInterested() {
 
 // Tests the "show less frequently" behavior.
 add_task(async function showLessFrequently() {
+  UrlbarPrefs.set("yelp.showLessFrequentlyCount", 0);
+  UrlbarPrefs.set("yelp.minKeywordLength", 0);
+  let cleanUpNimbus = await UrlbarTestUtils.initNimbusFeature({
+    yelpMinKeywordLength: 0,
+    yelpShowLessFrequentlyCap: 3,
+  });
+
   let location = `${GEOLOCATION.city}, ${GEOLOCATION.region}`;
 
   let originalUrl = new URL("https://www.yelp.com/search");
-  originalUrl.searchParams.set("find_desc", "alongerkeyword");
+  originalUrl.searchParams.set("find_desc", "best ramen");
 
   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,
+  let result = makeExpectedResult({
+    url: url.toString(),
+    originalUrl: originalUrl.toString(),
+    title: `best ramen in ${location}`,
   });
+
+  const testData = [
+    {
+      input: "best ra",
+      before: {
+        canShowLessFrequently: true,
+        showLessFrequentlyCount: 0,
+        minKeywordLength: 0,
+      },
+      after: {
+        canShowLessFrequently: true,
+        showLessFrequentlyCount: 1,
+        minKeywordLength: 8,
+      },
+    },
+    {
+      input: "best ram",
+      before: {
+        canShowLessFrequently: true,
+        showLessFrequentlyCount: 1,
+        minKeywordLength: 8,
+      },
+      after: {
+        canShowLessFrequently: true,
+        showLessFrequentlyCount: 2,
+        minKeywordLength: 9,
+      },
+    },
+    {
+      input: "best rame",
+      before: {
+        canShowLessFrequently: true,
+        showLessFrequentlyCount: 2,
+        minKeywordLength: 9,
+      },
+      after: {
+        canShowLessFrequently: false,
+        showLessFrequentlyCount: 3,
+        minKeywordLength: 10,
+      },
+    },
+    {
+      input: "best ramen",
+      before: {
+        canShowLessFrequently: false,
+        showLessFrequentlyCount: 3,
+        minKeywordLength: 10,
+      },
+      after: {
+        canShowLessFrequently: false,
+        showLessFrequentlyCount: 3,
+        minKeywordLength: 11,
+      },
+    },
+  ];
+
+  for (let { input, before, after } of testData) {
+    let feature = QuickSuggest.getFeature("YelpSuggestions");
+
+    await check_results({
+      context: createContext(input, {
+        providers: [UrlbarProviderQuickSuggest.name],
+        isPrivate: false,
+      }),
+      matches: [result],
+    });
+
+    Assert.equal(
+      UrlbarPrefs.get("yelp.minKeywordLength"),
+      before.minKeywordLength
+    );
+    Assert.equal(feature.canShowLessFrequently, before.canShowLessFrequently);
+    Assert.equal(
+      feature.showLessFrequentlyCount,
+      before.showLessFrequentlyCount
+    );
+
+    feature.handleCommand(
+      {
+        acknowledgeFeedback: () => {},
+        invalidateResultMenuCommands: () => {},
+      },
+      result,
+      "show_less_frequently",
+      input
+    );
+
+    Assert.equal(
+      UrlbarPrefs.get("yelp.minKeywordLength"),
+      after.minKeywordLength
+    );
+    Assert.equal(feature.canShowLessFrequently, after.canShowLessFrequently);
+    Assert.equal(
+      feature.showLessFrequentlyCount,
+      after.showLessFrequentlyCount
+    );
+
+    await check_results({
+      context: createContext(input, {
+        providers: [UrlbarProviderQuickSuggest.name],
+        isPrivate: false,
+      }),
+      matches: [],
+    });
+  }
+
+  await cleanUpNimbus();
+  UrlbarPrefs.clear("yelp.showLessFrequentlyCount");
+  UrlbarPrefs.clear("yelp.minKeywordLength");
 });
 
 // The `Yelp` Rust provider should be passed to the Rust component when