Bug 1618769 - Increase max chars for search suggestions, and don't fetch suggestions at all when max is reached due to paste. r=mak

* Add a new `allowSearchSuggestions` property to the query context. It defaults to true.
* `UrlbarInput` sets this property when it starts a query. If the event that started the query is a paste event and the pasted string's length is larger than maxChars, then don't allow suggestions. Otherwise do.
* `UrlbarProviderSearchSuggestions.isActive` returns false when `!context.allowSearchSuggestions`.
* `UrlbarProviderSearchSuggestions` doesn't truncate the query anymore.
* Keep the `browser.urlbar.maxCharsForSearchSuggestions` pref but use it only for pastes, and increase it from 20 to 100. I considered making a new pref but this way if a user changed it, then it still applies to pastes at least. I'm not sure it's important though.

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

browser/app/profile/firefox.js

@@ -281,9 +281,9 @@ pref("browser.urlbar.suggest.bookmark",             true);
 pref("browser.urlbar.suggest.openpage",             true);
 pref("browser.urlbar.suggest.searches",             true);
 
-// Limit the number of characters sent to the current search engine to fetch
-// suggestions.
-pref("browser.urlbar.maxCharsForSearchSuggestions", 20);
+// As a user privacy measure, don't fetch search suggestions if a pasted string
+// is longer than this.
+pref("browser.urlbar.maxCharsForSearchSuggestions", 100);
 
 pref("browser.urlbar.formatting.enabled", true);
 pref("browser.urlbar.trimURLs", true);

browser/components/urlbar/UrlbarInput.jsm

@@ -935,6 +935,11 @@ class UrlbarInput {
           "usercontextid"
         ),
         currentPage: this.window.gBrowser.currentURI.spec,
+        allowSearchSuggestions:
+          !event ||
+          !UrlbarUtils.isPasteEvent(event) ||
+          !event.data ||
+          event.data.length <= UrlbarPrefs.get("maxCharsForSearchSuggestions"),
       })
     );
   }

browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm

@@ -120,6 +120,10 @@ class ProviderSearchSuggestions extends UrlbarProvider {
       return false;
     }
 
+    if (!queryContext.allowSearchSuggestions) {
+      return false;
+    }
+
     if (
       queryContext.isPrivate &&
       !UrlbarPrefs.get("browser.search.suggest.enabled.private")
@@ -252,9 +256,6 @@ class ProviderSearchSuggestions extends UrlbarProvider {
       query = substringAfter(query, leadingRestrictionToken).trim();
     }
 
-    // Limit the string sent for search suggestions to a maximum length.
-    query = query.substr(0, UrlbarPrefs.get("maxCharsForSearchSuggestions"));
-
     // Find our search engine. It may have already been set with an alias.
     let engine;
     if (aliasEngine) {

browser/components/urlbar/UrlbarUtils.jsm

@@ -810,6 +810,11 @@ class UrlbarQueryContext {
    *   If sources is restricting to just SEARCH, this property can be used to
    *   pick a specific search engine, by setting it to the name under which the
    *   engine is registered with the search service.
+   * @param {boolean} [options.allowSearchSuggestions]
+   *   Whether to allow search suggestions.  This is a veto, meaning that when
+   *   false, suggestions will not be fetched, but when true, some other
+   *   condition may still prohibit suggestions, like private browsing mode.
+   *   Defaults to true.
    */
   constructor(options = {}) {
     this._checkRequiredOptions(options, [
@@ -826,20 +831,26 @@ class UrlbarQueryContext {
     }
 
     // Manage optional properties of options.
-    for (let [prop, checkFn] of [
+    for (let [prop, checkFn, defaultValue] of [
       ["providers", v => Array.isArray(v) && v.length],
       ["sources", v => Array.isArray(v) && v.length],
       ["engineName", v => typeof v == "string" && !!v.length],
       ["currentPage", v => typeof v == "string" && !!v.length],
+      ["allowSearchSuggestions", v => true, true],
     ]) {
-      if (options[prop]) {
+      if (prop in options) {
         if (!checkFn(options[prop])) {
           throw new Error(`Invalid value for option "${prop}"`);
         }
         this[prop] = options[prop];
+      } else if (defaultValue !== undefined) {
+        this[prop] = defaultValue;
       }
     }
 
+    this.allowSearchSuggestions =
+      "allowSearchSuggestions" in this ? !!this.allowSearchSuggestions : true;
+
     this.lastResultCount = 0;
     this.userContextId =
       options.userContextId ||

browser/components/urlbar/docs/overview.rst

@@ -52,6 +52,12 @@ It is augmented as it progresses through the system, with various information:
                 // with the search service.
     currentPage: // {string} url of the page that was loaded when the search
                  // began.
+    allowSearchSuggestions: // {boolean} Whether to allow search suggestions.
+                            // This is a veto, meaning that when false,
+                            // suggestions will not be fetched, but when true,
+                            // some other condition may still prohibit
+                            // suggestions, like private browsing mode. Defaults
+                            // to true.
 
     // Properties added by the Model.
     results; // {array} list of UrlbarResult objects.

browser/components/urlbar/tests/browser/browser_searchSuggestions.js

@@ -11,6 +11,8 @@
 const SUGGEST_URLBAR_PREF = "browser.urlbar.suggest.searches";
 const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
 
+const MAX_CHARS_PREF = "browser.urlbar.maxCharsForSearchSuggestions";
+
 // Must run first.
 add_task(async function prepare() {
   let suggestionsEnabled = Services.prefs.getBoolPref(SUGGEST_URLBAR_PREF);
@@ -38,7 +40,7 @@ add_task(async function clickSuggestion() {
     waitForFocus: SimpleTest.waitForFocus,
     value: "foo",
   });
-  let [idx, suggestion, engineName] = await promiseFirstSuggestion();
+  let [idx, suggestion, engineName] = await getFirstSuggestion();
   Assert.equal(
     engineName,
     "browser_searchSuggestionEngine searchSuggestionEngine.xml",
@@ -68,7 +70,7 @@ async function testPressEnterOnSuggestion(
     waitForFocus: SimpleTest.waitForFocus,
     value: "foo",
   });
-  let [idx, suggestion, engineName] = await promiseFirstSuggestion();
+  let [idx, suggestion, engineName] = await getFirstSuggestion();
   Assert.equal(
     engineName,
     "browser_searchSuggestionEngine searchSuggestionEngine.xml",
@@ -109,7 +111,7 @@ add_task(async function copySuggestionText() {
     waitForFocus: SimpleTest.waitForFocus,
     value: "foo",
   });
-  let [idx, suggestion] = await promiseFirstSuggestion();
+  let [idx, suggestion] = await getFirstSuggestion();
   for (let i = 0; i < idx; ++i) {
     EventUtils.synthesizeKey("KEY_ArrowDown");
   }
@@ -119,7 +121,125 @@ add_task(async function copySuggestionText() {
   });
 });
 
+add_task(async function typeMaxChars() {
+  gURLBar.focus();
+
+  let maxChars = 10;
+  await SpecialPowers.pushPrefEnv({
+    set: [[MAX_CHARS_PREF, maxChars]],
+  });
+
+  // Make a string as long as maxChars and type it.
+  let value = "";
+  for (let i = 0; i < maxChars; i++) {
+    value += String.fromCharCode("a".charCodeAt(0) + i);
+  }
+  await UrlbarTestUtils.promiseAutocompleteResultPopup({
+    window,
+    value,
+    waitForFocus: SimpleTest.waitForFocus,
+  });
+
+  // Suggestions should be fetched since we allow them when typing, and the
+  // value so far isn't longer than maxChars anyway.
+  await assertSuggestions([value + "foo", value + "bar"]);
+
+  // Now type some additional chars.  Suggestions should still be fetched since
+  // we allow them when typing.
+  for (let i = 0; i < 3; i++) {
+    let char = String.fromCharCode("z".charCodeAt(0) - i);
+    value += char;
+    EventUtils.synthesizeKey(char);
+    await assertSuggestions([value + "foo", value + "bar"]);
+  }
+
+  await SpecialPowers.popPrefEnv();
+});
+
+add_task(async function pasteMaxChars() {
+  gURLBar.focus();
+
+  let maxChars = 10;
+  await SpecialPowers.pushPrefEnv({
+    set: [[MAX_CHARS_PREF, maxChars]],
+  });
+
+  // Make a string as long as maxChars and paste it.
+  let value = "";
+  for (let i = 0; i < maxChars; i++) {
+    value += String.fromCharCode("a".charCodeAt(0) + i);
+  }
+  await selectAndPaste(value);
+
+  // Suggestions should be fetched since the pasted string is not longer than
+  // maxChars.
+  await assertSuggestions([value + "foo", value + "bar"]);
+
+  // Now type some additional chars.  Suggestions should still be fetched since
+  // we allow them when typing.
+  for (let i = 0; i < 3; i++) {
+    let char = String.fromCharCode("z".charCodeAt(0) - i);
+    value += char;
+    EventUtils.synthesizeKey(char);
+    await assertSuggestions([value + "foo", value + "bar"]);
+  }
+
+  await SpecialPowers.popPrefEnv();
+});
+
+add_task(async function pasteMoreThanMaxChars() {
+  gURLBar.focus();
+
+  let maxChars = 10;
+  await SpecialPowers.pushPrefEnv({
+    set: [[MAX_CHARS_PREF, maxChars]],
+  });
+
+  // Make a string longer than maxChars and paste it.
+  let value = "";
+  for (let i = 0; i < 2 * maxChars; i++) {
+    value += String.fromCharCode("a".charCodeAt(0) + i);
+  }
+  await selectAndPaste(value);
+
+  // Suggestions should not be fetched since the value was pasted and it was
+  // longer than maxChars.
+  await assertSuggestions([]);
+
+  // Now type some additional chars.  Suggestions should now be fetched since we
+  // allow them when typing.
+  for (let i = 0; i < 3; i++) {
+    let char = String.fromCharCode("z".charCodeAt(0) - i);
+    value += char;
+    EventUtils.synthesizeKey(char);
+    await assertSuggestions([value + "foo", value + "bar"]);
+  }
+
+  // Paste again.  The string is longer than maxChars, so suggestions should not
+  // be fetched.
+  await selectAndPaste(value);
+  await assertSuggestions([]);
+
+  await SpecialPowers.popPrefEnv();
+});
+
 async function getFirstSuggestion() {
+  let results = await getSuggestionResults();
+  if (!results.length) {
+    return [-1, null, null];
+  }
+  let result = results[0];
+  return [
+    result.index,
+    result.searchParams.suggestion,
+    result.searchParams.engine,
+  ];
+}
+
+async function getSuggestionResults() {
+  await UrlbarTestUtils.promiseSearchComplete(window);
+
+  let results = [];
   let matchCount = UrlbarTestUtils.getResultCount(window);
   for (let i = 0; i < matchCount; i++) {
     let result = await UrlbarTestUtils.getDetailsOfResultAt(window, i);
@@ -127,13 +247,19 @@ async function getFirstSuggestion() {
       result.type == UrlbarUtils.RESULT_TYPE.SEARCH &&
       result.searchParams.suggestion
     ) {
-      return [i, result.searchParams.suggestion, result.searchParams.engine];
+      result.index = i;
+      results.push(result);
     }
   }
-  return [-1, null, null];
+  return results;
 }
 
-async function promiseFirstSuggestion() {
-  await UrlbarTestUtils.promiseSuggestionsPresent(window);
-  return getFirstSuggestion();
+async function assertSuggestions(expectedSuggestions) {
+  let results = await getSuggestionResults();
+  let actualSuggestions = results.map(r => r.searchParams.suggestion);
+  Assert.deepEqual(
+    actualSuggestions,
+    expectedSuggestions,
+    "Expected suggestions"
+  );
 }

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

@@ -128,13 +128,16 @@ add_task(async function test_restrictions() {
 
 async function get_results(test) {
   let controller = UrlbarTestUtils.newMockController();
-  let queryContext = createContext(test.searchString, {
+  let options = {
     allowAutofill: false,
     isPrivate: false,
     maxResults: 10,
     sources: test.sources,
-    engineName: test.engineName,
-  });
+  };
+  if (test.engineName) {
+    options.engineName = test.engineName;
+  }
+  let queryContext = createContext(test.searchString, options);
   await controller.startQuery(queryContext);
   return queryContext.results;
 }