From 61754b1086cf43e50ff108a6e9d96bc761de6bce Mon Sep 17 00:00:00 2001
From: Drew Willcoxon <adw@mozilla.com>
Date: Wed, 24 Jul 2024 00:18:11 +0000
Subject: [PATCH] Bug 1909565 - Move Fakespot suggestions to the bottom of the
 Firefox Suggest section, r=daisuke

This also adds a Nimbus variable so we can easily change the index for
experiments.

Depends on D217510

Differential Revision: https://phabricator.services.mozilla.com/D217524
---
 browser/app/profile/firefox.js                |  4 +++
 browser/components/urlbar/UrlbarPrefs.sys.mjs |  4 +++
 .../private/FakespotSuggestions.sys.mjs       | 22 +++++++++-------
 .../unit/test_quicksuggest_fakespot.js        | 25 ++++++++++++++++++-
 .../components/nimbus/FeatureManifest.yaml    |  6 +++++
 5 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js
index 19377e2c0ad9..cb072e1ae50d 100644
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -657,6 +657,10 @@ pref("browser.urlbar.fakespot.featureGate", false);
 // the suggestion. 0 means the min length should be taken from Nimbus.
 pref("browser.urlbar.fakespot.minKeywordLength", 4);
 
+// The index of Fakespot results within the Firefox Suggest section. A negative
+// index is relative to the end of the section.
+pref("browser.urlbar.fakespot.suggestedIndex", -1);
+
 // If `browser.urlbar.fakespot.featureGate` is true, this controls whether
 // Fakespot suggestions are turned on.
 pref("browser.urlbar.suggest.fakespot", true);
diff --git a/browser/components/urlbar/UrlbarPrefs.sys.mjs b/browser/components/urlbar/UrlbarPrefs.sys.mjs
index c527854453c6..86f6ad403ecd 100644
--- a/browser/components/urlbar/UrlbarPrefs.sys.mjs
+++ b/browser/components/urlbar/UrlbarPrefs.sys.mjs
@@ -139,6 +139,10 @@ const PREF_URLBAR_DEFAULTS = new Map([
   // for Fakespot suggestions.
   ["fakespot.showLessFrequentlyCount", 0],
 
+  // The index of Fakespot results within the Firefox Suggest section. A
+  // negative index is relative to the end of the section.
+  ["fakespot.suggestedIndex", -1],
+
   // When true, `javascript:` URLs are not included in search results.
   ["filter.javascript", true],
 
diff --git a/browser/components/urlbar/private/FakespotSuggestions.sys.mjs b/browser/components/urlbar/private/FakespotSuggestions.sys.mjs
index 6723d24405ee..97fb13c3b8a3 100644
--- a/browser/components/urlbar/private/FakespotSuggestions.sys.mjs
+++ b/browser/components/urlbar/private/FakespotSuggestions.sys.mjs
@@ -134,16 +134,20 @@ export class FakespotSuggestions extends BaseFeature {
       dynamicType: "fakespot",
     };
 
-    const result = new lazy.UrlbarResult(
-      lazy.UrlbarUtils.RESULT_TYPE.DYNAMIC,
-      lazy.UrlbarUtils.RESULT_SOURCE.SEARCH,
-      ...lazy.UrlbarResult.payloadAndSimpleHighlights(
-        queryContext.tokens,
-        payload
-      )
+    return Object.assign(
+      new lazy.UrlbarResult(
+        lazy.UrlbarUtils.RESULT_TYPE.DYNAMIC,
+        lazy.UrlbarUtils.RESULT_SOURCE.SEARCH,
+        ...lazy.UrlbarResult.payloadAndSimpleHighlights(
+          queryContext.tokens,
+          payload
+        )
+      ),
+      {
+        isSuggestedIndexRelativeToGroup: true,
+        suggestedIndex: lazy.UrlbarPrefs.get("fakespotSuggestedIndex"),
+      }
     );
-
-    return result;
   }
 
   getViewUpdate(result) {
diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_fakespot.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_fakespot.js
index 49642177abed..6a6db8f84ba7 100644
--- a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_fakespot.js
+++ b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_fakespot.js
@@ -792,10 +792,33 @@ async function doMinKeywordLengthTest({
   }
 }
 
+// Tests the `fakespotSuggestedIndex` Nimbus variable.
+add_task(async function suggestedIndex() {
+  // If this fails, please update this task. Otherwise it's not actually testing
+  // a non-default suggested index.
+  Assert.notEqual(
+    UrlbarPrefs.get("fakespotSuggestedIndex"),
+    0,
+    "Sanity check: Default value of fakespotSuggestedIndex is not 0"
+  );
+
+  let cleanUpNimbusEnable = await UrlbarTestUtils.initNimbusFeature({
+    fakespotSuggestedIndex: 0,
+  });
+  await check_results({
+    context: createContext(PRIMARY_SEARCH_STRING, {
+      providers: [UrlbarProviderQuickSuggest.name],
+      isPrivate: false,
+    }),
+    matches: [makeExpectedResult({ suggestedIndex: 0 })],
+  });
+  await cleanUpNimbusEnable();
+});
+
 function makeExpectedResult({
   url = PRIMARY_URL,
   title = PRIMARY_TITLE,
-  suggestedIndex = 0,
+  suggestedIndex = -1,
   isSuggestedIndexRelativeToGroup = true,
   originalUrl = undefined,
   displayUrl = undefined,
diff --git a/toolkit/components/nimbus/FeatureManifest.yaml b/toolkit/components/nimbus/FeatureManifest.yaml
index 36fd2df74f42..095aad8bc292 100644
--- a/toolkit/components/nimbus/FeatureManifest.yaml
+++ b/toolkit/components/nimbus/FeatureManifest.yaml
@@ -243,6 +243,12 @@ urlbar:
         will be able to click the "Show less frequently" command for Fakespot
         suggestions. If undefined or zero, the user will be able to click the
         command without any limit.
+    fakespotSuggestedIndex:
+      type: int
+      fallbackPref: browser.urlbar.fakespot.suggestedIndex
+      description: >-
+        The index of Fakespot results within the Firefox Suggest section. A
+        negative index is relative to the end of the section.
     mdnFeatureGate:
       type: boolean
       setPref: