Bug 1648385 - Allow search suggestions in the Urlbar when they are disabled by a pref but a restriction token or token alias is used. r=adw, a=RyanVM
authorHarry Twyford <htwyford@mozilla.com>
Fri, 26 Jun 2020 13:55:00 +0000
changeset 601890 6b7ea9dbf4056d25dc407850108b641f2874a90a
parent 601889 30db13a0b50193ca71c6186b8c27d6b2a7ef85ad
child 601891 a0161364227e85a58f75f2cec1e7a9b6149cc9ff
push id13341
push userryanvm@gmail.com
push dateThu, 02 Jul 2020 15:55:10 +0000
treeherdermozilla-beta@a0161364227e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw, RyanVM
bugs1648385
milestone79.0
Bug 1648385 - Allow search suggestions in the Urlbar when they are disabled by a pref but a restriction token or token alias is used. r=adw, a=RyanVM Differential Revision: https://phabricator.services.mozilla.com/D81166
browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm
browser/components/urlbar/tests/unit/test_search_suggestions.js
browser/components/urlbar/tests/unit/test_search_suggestions_aliases.js
--- a/browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm
+++ b/browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm
@@ -100,30 +100,52 @@ class ProviderSearchSuggestions extends 
 
     return (
       this._getFormHistoryCount(queryContext) ||
       this._allowRemoteSuggestions(queryContext)
     );
   }
 
   /**
+   * Returns whether the user typed a token alias or a restriction token. We use
+   * this value to override the pref to disable search suggestions in the
+   * Urlbar.
+   * @param {UrlbarQueryContext} queryContext  The query context object.
+   * @returns {boolean} True if the user typed a token alias or search
+   *   restriction token.
+   */
+  _isTokenOrRestrictionPresent(queryContext) {
+    return (
+      queryContext.searchString.startsWith("@") ||
+      (queryContext.restrictSource &&
+        queryContext.restrictSource == UrlbarUtils.RESULT_SOURCE.SEARCH) ||
+      queryContext.tokens.some(
+        t => t.type == UrlbarTokenizer.TYPE.RESTRICT_SEARCH
+      )
+    );
+  }
+
+  /**
    * Returns whether suggestions in general are allowed for a given query
    * context.  If this returns false, then we shouldn't fetch either form
    * history or remote suggestions.  Otherwise further checks are necessary to
    * determine whether to allow either form history or remote suggestions; see
    * _getFormHistoryCount and _allowRemoteSuggestions.
    *
    * @param {object} queryContext The query context object
    * @returns {boolean} True if suggestions in general are allowed and false if
    *   not.
    */
   _allowSuggestions(queryContext) {
     if (
       !queryContext.allowSearchSuggestions ||
-      !UrlbarPrefs.get("suggest.searches") ||
+      // If the user typed a restriction token or token alias, we ignore the
+      // pref to disable suggestions in the Urlbar.
+      (!UrlbarPrefs.get("suggest.searches") &&
+        !this._isTokenOrRestrictionPresent(queryContext)) ||
       !UrlbarPrefs.get("browser.search.suggest.enabled") ||
       (queryContext.isPrivate &&
         !UrlbarPrefs.get("browser.search.suggest.enabled.private"))
     ) {
       return false;
     }
     return true;
   }
@@ -136,20 +158,20 @@ class ProviderSearchSuggestions extends 
    */
   _allowRemoteSuggestions(queryContext) {
     // Check whether suggestions in general are allowed.
     if (!this._allowSuggestions(queryContext)) {
       return false;
     }
 
     // Skip all remaining checks and allow remote suggestions at this point if
-    // the user used a search engine token alias.  We want "@engine query" to
-    // return suggestions from the engine.  We'll return early from startQuery
+    // the user used a token alias or restriction token. We want "@engine query"
+    // to return suggestions from the engine. We'll return early from startQuery
     // if the query doesn't match an alias.
-    if (queryContext.searchString.startsWith("@")) {
+    if (this._isTokenOrRestrictionPresent(queryContext)) {
       return true;
     }
 
     // If the user is just adding on to a query that previously didn't return
     // many remote suggestions, we are unlikely to get any more results.
     if (
       !!this._lastLowResultsSearchSuggestion &&
       queryContext.searchString.length >
--- a/browser/components/urlbar/tests/unit/test_search_suggestions.js
+++ b/browser/components/urlbar/tests/unit/test_search_suggestions.js
@@ -168,16 +168,88 @@ add_task(async function disabled_private
     context,
     matches: [
       makeSearchResult(context, { engineName: ENGINE_NAME, heuristic: true }),
     ],
   });
   await cleanUpSuggestions();
 });
 
+add_task(async function disabled_urlbarSuggestions_withRestrictionToken() {
+  Services.prefs.setBoolPref(SUGGEST_PREF, false);
+  Services.prefs.setBoolPref(SUGGEST_ENABLED_PREF, true);
+  let context = createContext(
+    `${UrlbarTokenizer.RESTRICT.SEARCH} ${SEARCH_STRING}`,
+    { isPrivate: false }
+  );
+  await check_results({
+    context,
+    matches: [
+      makeSearchResult(context, {
+        query: SEARCH_STRING,
+        engineName: ENGINE_NAME,
+        heuristic: true,
+      }),
+      ...makeExpectedSuggestionResults(context, {
+        query: SEARCH_STRING,
+      }),
+    ],
+  });
+  await cleanUpSuggestions();
+});
+
+add_task(
+  async function disabled_urlbarSuggestions_withRestrictionToken_private() {
+    Services.prefs.setBoolPref(SUGGEST_PREF, false);
+    Services.prefs.setBoolPref(SUGGEST_ENABLED_PREF, true);
+    Services.prefs.setBoolPref(PRIVATE_ENABLED_PREF, false);
+    let context = createContext(
+      `${UrlbarTokenizer.RESTRICT.SEARCH} ${SEARCH_STRING}`,
+      { isPrivate: true }
+    );
+    await check_results({
+      context,
+      matches: [
+        makeSearchResult(context, {
+          query: SEARCH_STRING,
+          engineName: ENGINE_NAME,
+          heuristic: true,
+        }),
+      ],
+    });
+    await cleanUpSuggestions();
+  }
+);
+
+add_task(
+  async function disabled_urlbarSuggestions_withRestrictionToken_private_enabled() {
+    Services.prefs.setBoolPref(SUGGEST_PREF, false);
+    Services.prefs.setBoolPref(SUGGEST_ENABLED_PREF, true);
+    Services.prefs.setBoolPref(PRIVATE_ENABLED_PREF, true);
+    let context = createContext(
+      `${UrlbarTokenizer.RESTRICT.SEARCH} ${SEARCH_STRING}`,
+      { isPrivate: true }
+    );
+    await check_results({
+      context,
+      matches: [
+        makeSearchResult(context, {
+          query: SEARCH_STRING,
+          engineName: ENGINE_NAME,
+          heuristic: true,
+        }),
+        ...makeExpectedSuggestionResults(context, {
+          query: SEARCH_STRING,
+        }),
+      ],
+    });
+    await cleanUpSuggestions();
+  }
+);
+
 add_task(async function enabled_by_pref_privateWindow() {
   Services.prefs.setBoolPref(SUGGEST_PREF, true);
   Services.prefs.setBoolPref(SUGGEST_ENABLED_PREF, true);
   Services.prefs.setBoolPref(PRIVATE_ENABLED_PREF, true);
   let context = createContext(SEARCH_STRING, { isPrivate: true });
   await check_results({
     context,
     matches: [
--- a/browser/components/urlbar/tests/unit/test_search_suggestions_aliases.js
+++ b/browser/components/urlbar/tests/unit/test_search_suggestions_aliases.js
@@ -2,20 +2,26 @@
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
  * Tests that an engine with suggestions works with our alias autocomplete
  * behavior.
  */
 
 const SUGGESTIONS_ENGINE_NAME = "engine-suggestions.xml";
+const SUGGEST_PREF = "browser.urlbar.suggest.searches";
+const SUGGEST_ENABLED_PREF = "browser.search.suggest.enabled";
+
+let engine;
+
+add_task(async function setup() {
+  engine = await addTestSuggestionsEngine();
+});
 
 add_task(async function engineWithSuggestions() {
-  let engine = await addTestSuggestionsEngine();
-
   // History matches should not appear with @ aliases, so this visit/match
   // should not appear when searching with the @ alias below.
   let historyTitle = "fire";
   await PlacesTestUtils.addVisits({
     uri: engine.searchForm,
     title: historyTitle,
   });
 
@@ -121,8 +127,55 @@ add_task(async function engineWithSugges
       });
     }
   }
 
   engine.alias = "";
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesUtils.history.clear();
 });
+
+add_task(async function disabled_urlbarSuggestions() {
+  Services.prefs.setBoolPref(SUGGEST_PREF, false);
+  Services.prefs.setBoolPref(SUGGEST_ENABLED_PREF, true);
+  let alias = "@moz";
+  engine.alias = alias;
+  Assert.equal(engine.alias, alias);
+
+  for (let private of [false, true]) {
+    let context = createContext(`${alias} term`, { isPrivate: private });
+    let expectedMatches = [
+      makeSearchResult(context, {
+        engineName: SUGGESTIONS_ENGINE_NAME,
+        alias,
+        query: "term",
+        heuristic: true,
+      }),
+    ];
+
+    if (!private) {
+      expectedMatches.push(
+        makeSearchResult(context, {
+          engineName: SUGGESTIONS_ENGINE_NAME,
+          alias,
+          query: "term",
+          suggestion: "term foo",
+        })
+      );
+      expectedMatches.push(
+        makeSearchResult(context, {
+          engineName: SUGGESTIONS_ENGINE_NAME,
+          alias,
+          query: "term",
+          suggestion: "term bar",
+        })
+      );
+    }
+    await check_results({
+      context,
+      matches: expectedMatches,
+    });
+  }
+
+  engine.alias = "";
+  Services.prefs.clearUserPref(SUGGEST_PREF);
+  Services.prefs.clearUserPref(SUGGEST_ENABLED_PREF);
+});