Bug 1672507 - History results are not shown in search mode when they …

…are set to come before search suggestions. r=harry

When the user unchecks the "Show search suggestions before history results" pref,
we change browser.urlbar.matchBuckets to general:5;suggestions:Infinity.
When the code inverted the buckets, it put Infinity suggestions before general
results, pushing them away.
That pref could also be modified by the users or an experiment, so we can't just
set a suggestions limit from the default value (4 at this time).
The safest solution seems to be to get the results, then transplant suggestions
at the top, that allows to keep the matchBuckets defined number of general
results and fill remaining space above them with suggestions.

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

browser/components/urlbar/UrlbarMuxerUnifiedComplete.jsm

@@ -133,18 +133,6 @@ class MuxerUnifiedComplete extends UrlbarMuxer {
       context.heuristicResult?.type == UrlbarUtils.RESULT_TYPE.SEARCH
         ? UrlbarPrefs.get("matchBucketsSearch")
         : UrlbarPrefs.get("matchBuckets");
-    // In search mode for an engine, we always want to show search suggestions
-    // before general results.
-    if (context.searchMode?.engineName) {
-      let suggestionsIndex = buckets.findIndex(b => b[0] == "suggestion");
-      let generalIndex = buckets.findIndex(b => b[0] == "general");
-      if (generalIndex < suggestionsIndex) {
-        // Copy the array, otherwise we'd end up modifying the cached pref.
-        buckets = buckets.slice();
-        buckets[generalIndex] = "suggestion";
-        buckets[suggestionsIndex] = "general";
-      }
-    }
     logger.debug(`Buckets: ${buckets}`);
 
     // Do the second pass to fill each bucket.  We'll build a list where each
@@ -155,6 +143,24 @@ class MuxerUnifiedComplete extends UrlbarMuxer {
       resultsByBucketIndex.push(results);
     }
 
+    // In search mode for an engine, search suggestions should always appear
+    // before general results. Transplanting them allows us to keep history
+    // results up to the limit set in matchBuckets, while filling the space
+    // above them with suggestions.
+    if (context.searchMode?.engineName) {
+      let suggestionsIndex = resultsByBucketIndex.findIndex(
+        results =>
+          results[0] &&
+          !results[0].heuristic &&
+          results[0].type == UrlbarUtils.RESULT_TYPE.SEARCH
+      );
+      if (suggestionsIndex > 1) {
+        logger.debug(`Transplanting suggestions before general results.`);
+        let removed = resultsByBucketIndex.splice(suggestionsIndex, 1);
+        resultsByBucketIndex.splice(1, 0, ...removed);
+      }
+    }
+
     // Build the sorted results list by concatenating each bucket's results.
     let sortedResults = [];
     let remainingCount = context.maxResults;

browser/components/urlbar/tests/browser/browser.ini

@@ -177,6 +177,7 @@ skip-if = os == 'mac' && debug && verify # bug 1671045
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
+  searchSuggestionEngineMany.xml
 [browser_searchMode_switchTabs.js]
 [browser_searchSettings.js]
 [browser_searchSingleWordNotification.js]

browser/components/urlbar/tests/browser/browser_searchMode_suggestions.js

@@ -9,6 +9,7 @@
 
 const DEFAULT_ENGINE_NAME = "Test";
 const SUGGESTIONS_ENGINE_NAME = "searchSuggestionEngine.xml";
+const MANY_SUGGESTIONS_ENGINE_NAME = "searchSuggestionEngineMany.xml";
 const MAX_HISTORICAL_SEARCH_SUGGESTIONS = UrlbarPrefs.get(
   "maxHistoricalSearchSuggestions"
 );
@@ -399,6 +400,9 @@ add_task(async function nonEmptySearch_nonMatching() {
 });
 
 add_task(async function nonEmptySearch_withHistory() {
+  let manySuggestionsEngine = await SearchTestUtils.promiseNewSearchEngine(
+    getRootDirectory(gTestPath) + MANY_SUGGESTIONS_ENGINE_NAME
+  );
   // URLs with the same host as the search engine.
   let query = "ciao";
   await PlacesTestUtils.addVisits([
@@ -409,44 +413,94 @@ add_task(async function nonEmptySearch_withHistory() {
     `http://example.com/mochi.test/${query}`,
   ]);
 
+  function makeSuggestionResult(suffix) {
+    return {
+      heuristic: false,
+      type: UrlbarUtils.RESULT_TYPE.SEARCH,
+      source: UrlbarUtils.RESULT_SOURCE.SEARCH,
+      searchParams: {
+        query,
+        suggestion: `${query}${suffix}`,
+        engine: manySuggestionsEngine.name,
+      },
+    };
+  }
+
   await BrowserTestUtils.withNewTab("about:robots", async function(browser) {
     await UrlbarTestUtils.promiseAutocompleteResultPopup({
       window,
       value: query,
     });
-
-    await UrlbarTestUtils.enterSearchMode(window);
+    await UrlbarTestUtils.enterSearchMode(window, {
+      engineName: manySuggestionsEngine.name,
+    });
     Assert.equal(gURLBar.value, query, "Urlbar value should be set.");
+
     await checkResults([
       {
         heuristic: true,
         type: UrlbarUtils.RESULT_TYPE.SEARCH,
         source: UrlbarUtils.RESULT_SOURCE.SEARCH,
         searchParams: {
           query,
-          engine: suggestionsEngine.name,
+          engine: manySuggestionsEngine.name,
         },
       },
+      makeSuggestionResult("foo"),
+      makeSuggestionResult("bar"),
+      makeSuggestionResult("1"),
+      makeSuggestionResult("2"),
       {
         heuristic: false,
-        type: UrlbarUtils.RESULT_TYPE.SEARCH,
-        source: UrlbarUtils.RESULT_SOURCE.SEARCH,
-        searchParams: {
-          query,
-          suggestion: `${query}foo`,
-          engine: suggestionsEngine.name,
-        },
+        type: UrlbarUtils.RESULT_TYPE.URL,
+        source: UrlbarUtils.RESULT_SOURCE.HISTORY,
+        url: `http://mochi.test/${query}1`,
       },
       {
         heuristic: false,
+        type: UrlbarUtils.RESULT_TYPE.URL,
+        source: UrlbarUtils.RESULT_SOURCE.HISTORY,
+        url: `http://mochi.test/${query}`,
+      },
+      makeSuggestionResult("3"),
+      makeSuggestionResult("4"),
+      makeSuggestionResult("5"),
+    ]);
+
+    await UrlbarTestUtils.exitSearchMode(window, { clickClose: true });
+    await UrlbarTestUtils.promisePopupClose(window);
+
+    info("Test again with history before suggestions");
+    await SpecialPowers.pushPrefEnv({
+      set: [["browser.urlbar.matchBuckets", "general:5,suggestion:Infinity"]],
+    });
+
+    await UrlbarTestUtils.promiseAutocompleteResultPopup({
+      window,
+      value: query,
+    });
+    await UrlbarTestUtils.enterSearchMode(window, {
+      engineName: manySuggestionsEngine.name,
+    });
+    Assert.equal(gURLBar.value, query, "Urlbar value should be set.");
+
+    await checkResults([
+      {
+        heuristic: true,
         type: UrlbarUtils.RESULT_TYPE.SEARCH,
         source: UrlbarUtils.RESULT_SOURCE.SEARCH,
         searchParams: {
           query,
-          suggestion: `${query}bar`,
-          engine: suggestionsEngine.name,
+          engine: manySuggestionsEngine.name,
         },
       },
+      makeSuggestionResult("foo"),
+      makeSuggestionResult("bar"),
+      makeSuggestionResult("1"),
+      makeSuggestionResult("2"),
+      makeSuggestionResult("3"),
+      makeSuggestionResult("4"),
+      makeSuggestionResult("5"),
       {
         heuristic: false,
         type: UrlbarUtils.RESULT_TYPE.URL,
@@ -463,6 +517,7 @@ add_task(async function nonEmptySearch_withHistory() {
 
     await UrlbarTestUtils.exitSearchMode(window, { clickClose: true });
     await UrlbarTestUtils.promisePopupClose(window);
+    await SpecialPowers.popPrefEnv();
   });
 
   await PlacesUtils.history.clear();

browser/components/urlbar/tests/browser/searchSuggestionEngine.sjs

@@ -36,6 +36,13 @@ function handleRequest(req, resp) {
 function writeResponse(params, resp) {
   // Echo back the search string with "foo" and "bar" appended.
   let suffixes = ["foo", "bar"];
+  if (params["count"]) {
+    // Add more suffixes.
+    let serial = 0;
+    while (suffixes.length < params["count"]) {
+      suffixes.push(++serial);
+    }
+  }
   let data = [params["query"], suffixes.map(s => params["query"] + s)];
   resp.setHeader("Content-Type", "application/json", false);
   resp.write(JSON.stringify(data));

browser/components/urlbar/tests/browser/searchSuggestionEngineMany.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<SearchPlugin xmlns="http://www.mozilla.org/2006/browser/search/">
+<ShortName>browser_searchSuggestionEngineMany searchSuggestionEngineMany.xml</ShortName>
+<Url type="application/x-suggestions+json" method="GET" template="http://mochi.test:8888/browser/browser/components/urlbar/tests/browser/searchSuggestionEngine.sjs?{searchTerms}&amp;count=10"/>
+<Url type="text/html" method="GET" template="http://mochi.test:8888/" rel="searchform">
+  <Param name="terms" value="{searchTerms}"/>
+</Url>
+</SearchPlugin>