Bug 1637060 - Don't show tail suggestions if there are other non-heuristic results. r=mak

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

browser/components/extensions/test/xpcshell/test_ext_urlbar.js

@@ -275,8 +275,7 @@ add_task(async function test_onProviderResultsRequested() {
 
   // Check the results.
   let expectedResults = [
-    // The first result should be a search result returned by
-    // UrlbarProviderSearchSuggestions.
+    // The first result should be a search result returned by UnifiedComplete.
     {
       type: UrlbarUtils.RESULT_TYPE.SEARCH,
       source: UrlbarUtils.RESULT_SOURCE.SEARCH,

browser/components/urlbar/UrlbarMuxerUnifiedComplete.jsm

@@ -100,6 +100,11 @@ class MuxerUnifiedComplete extends UrlbarMuxer {
     let canShowPrivateSearch = context.results.length > 1;
     let resultsWithSuggestedIndex = [];
 
+    // If we find results other than the heuristic, "Search in Private Window,"
+    // or tail suggestions on the first pass, we should hide tail suggestions on
+    // the second, since tail suggestions are a "last resort".
+    let canShowTailSuggestions = true;
+
     // Do the first pass through the results.  We only collect info for the
     // second pass here.
     for (let result of context.results) {
@@ -107,12 +112,21 @@ class MuxerUnifiedComplete extends UrlbarMuxer {
       // are other results and all of them are searches.  It should not be shown
       // if the user typed an alias because that's an explicit engine choice.
       if (
-        result.type != UrlbarUtils.RESULT_TYPE.SEARCH ||
-        result.payload.keywordOffer ||
-        (result.heuristic && result.payload.keyword)
+        canShowPrivateSearch &&
+        (result.type != UrlbarUtils.RESULT_TYPE.SEARCH ||
+          result.payload.keywordOffer ||
+          (result.heuristic && result.payload.keyword))
       ) {
         canShowPrivateSearch = false;
-        break;
+      }
+
+      if (
+        canShowTailSuggestions &&
+        !result.heuristic &&
+        (result.type != UrlbarUtils.RESULT_TYPE.SEARCH ||
+          (!result.payload.inPrivateWindow && !result.payload.tail))
+      ) {
+        canShowTailSuggestions = false;
       }
     }
 
@@ -143,6 +157,15 @@ class MuxerUnifiedComplete extends UrlbarMuxer {
         continue;
       }
 
+      // Exclude tail suggestions if we have non-tail suggestions.
+      if (
+        !canShowTailSuggestions &&
+        groupFromResult(result) == UrlbarUtils.RESULT_GROUP.SUGGESTION &&
+        result.payload.tail
+      ) {
+        continue;
+      }
+
       // Include this result.
       unsortedResults.push(result);
     }

browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm

@@ -20,6 +20,7 @@ XPCOMUtils.defineLazyModuleGetters(this, {
   SearchSuggestionController:
     "resource://gre/modules/SearchSuggestionController.jsm",
   Services: "resource://gre/modules/Services.jsm",
+  SkippableTimer: "resource:///modules/UrlbarUtils.jsm",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm",
   UrlbarProvider: "resource:///modules/UrlbarUtils.jsm",
   UrlbarResult: "resource:///modules/UrlbarResult.jsm",
@@ -359,6 +360,16 @@ class ProviderSearchSuggestions extends UrlbarProvider {
       this._lastLowResultsSearchSuggestion = searchString;
     }
 
+    // If we have only tail suggestions, we only show them if we have no other
+    // results. We need to wait for other results to arrive to avoid flickering.
+    // We will wait for this timer unless we have suggestions that don't have a
+    // tail.
+    let tailTimer = new SkippableTimer({
+      name: "ProviderSearchSuggestions",
+      time: 100,
+      logger,
+    });
+
     let results = [];
     for (let suggestion of suggestions) {
       if (
@@ -382,6 +393,10 @@ class ProviderSearchSuggestions extends UrlbarProvider {
         tailPrefix = suggestion.entry.matchPrefix;
       }
 
+      if (!tail) {
+        await tailTimer.fire().catch(Cu.reportError);
+      }
+
       try {
         results.push(
           new UrlbarResult(
@@ -414,6 +429,7 @@ class ProviderSearchSuggestions extends UrlbarProvider {
       }
     }
 
+    await tailTimer.promise;
     return results;
   }
 

browser/components/urlbar/tests/unit/test_search_suggestions_tail.js

@@ -150,8 +150,9 @@ add_task(async function basic_tail() {
 
 /**
  * Tests a suggestions provider that returns both normal and tail suggestions.
+ * Only normal results should be shown.
  */
-add_task(async function mixed_results() {
+add_task(async function mixed_suggestions() {
   // When normal suggestions are mixed with tail suggestions, they appear at the
   // correct position in the google:suggestdetail array as empty objects.
   setSuggestionsFn(searchStr => {
@@ -185,24 +186,75 @@ add_task(async function mixed_results() {
         suggestion: "what is the time today texas",
         tail: undefined,
       }),
-      makeSearchResult(context, {
-        engineName: ENGINE_NAME,
-        suggestion: query + "oronto",
-        tail: "toronto",
-      }),
-      makeSearchResult(context, {
-        engineName: ENGINE_NAME,
-        suggestion: query + "unisia",
-        tail: "tunisia",
-      }),
     ],
   });
   await cleanUpSuggestions();
 });
 
+/**
+ * Tests a search that returns history results, bookmark results and tail
+ * suggestions. Only the history and bookmark results should be shown.
+ */
+add_task(async function mixed_results() {
+  await PlacesTestUtils.addVisits([
+    {
+      uri: Services.io.newURI("http://example.com/1"),
+      title: "what time is",
+    },
+  ]);
+
+  await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    url: "http://example.com/2",
+    title: "what time is",
+  });
+
+  // Tail suggestions should not be shown.
+  const query = "what time is";
+  let context = createContext(query, { isPrivate: false });
+  await check_results({
+    context,
+    matches: [
+      makeSearchResult(context, { engineName: ENGINE_NAME, heuristic: true }),
+      makeBookmarkResult(context, {
+        uri: "http://example.com/2",
+        title: "what time is",
+      }),
+      makeVisitResult(context, {
+        uri: "http://example.com/1",
+        title: "what time is",
+      }),
+    ],
+  });
+
+  // Once we make the query specific enough to exclude the history and bookmark
+  // results, we should show tail suggestions.
+  const tQuery = "what time is it in t";
+  context = createContext(tQuery, { isPrivate: false });
+  await check_results({
+    context,
+    matches: [
+      makeSearchResult(context, { engineName: ENGINE_NAME, heuristic: true }),
+      makeSearchResult(context, {
+        engineName: ENGINE_NAME,
+        suggestion: tQuery + "oronto",
+        tail: "toronto",
+      }),
+      makeSearchResult(context, {
+        engineName: ENGINE_NAME,
+        suggestion: tQuery + "unisia",
+        tail: "tunisia",
+      }),
+    ],
+  });
+
+  await cleanUpSuggestions();
+});
+
 /**
  * Tests that tail suggestions are deduped if their full-text form is a dupe of
- * a local search suggestion.
+ * a local search suggestion. Remaining tail suggestions should also not be
+ * shown since we do not mix tail and non-tail suggestions.
  */
 add_task(async function dedupe_local() {
   Services.prefs.setIntPref("browser.urlbar.maxHistoricalSearchSuggestions", 1);
@@ -219,11 +271,6 @@ add_task(async function dedupe_local() {
         suggestion: query + "oronto",
         tail: undefined,
       }),
-      makeSearchResult(context, {
-        engineName: ENGINE_NAME,
-        suggestion: query + "unisia",
-        tail: "tunisia",
-      }),
     ],
   });
 

toolkit/components/places/tests/PlacesTestUtils.jsm

@@ -168,7 +168,8 @@ var PlacesTestUtils = Object.freeze({
   },
 
   /**
-   * Adds a bookmark to the database.
+   * Adds a bookmark to the database. This should only be used when you need to
+   * add keywords. Otherwise, use `PlacesUtils.bookmarks.insert()`.
    * @param {string} aBookmarkObj.uri
    * @param {string} [aBookmarkObj.title]
    * @param {string} [aBookmarkObj.keyword]