Bug 1499581 - Autofill search engine aliases in the address bar when '@' is typed as the first character r=mak
authorDrew Willcoxon <adw@mozilla.com>
Fri, 19 Oct 2018 16:08:26 +0000
changeset 490532 418fa1fa4f746f82b0a1a070ef9dfb12849b5281
parent 490531 eab3a0cf64298bbe275a6598bea9ae73811652db
child 490533 887b52d6d707a0f3030c5a4ec10fc6ab3724c2be
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersmak
bugs1499581
milestone64.0a1
Bug 1499581 - Autofill search engine aliases in the address bar when '@' is typed as the first character r=mak One important unrelated change this makes is that previously (in my other patches), the only "@" aliases we recognized were the internal "@" aliases in nsSearchService. While I was working on the new test here I realized that engines with user (or test) aliases that start with "@" aren't recognized as having "@" aliases, but why not IMO. Differential Revision: https://phabricator.services.mozilla.com/D9074
browser/base/content/test/urlbar/browser_urlbarHighlightSearchAlias.js
toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/test_autofill_search_engine_aliases.js
toolkit/components/places/tests/unifiedcomplete/test_search_engine_alias.js
toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
toolkit/components/satchel/AutoCompletePopup.jsm
toolkit/content/widgets/autocomplete.xml
--- a/browser/base/content/test/urlbar/browser_urlbarHighlightSearchAlias.js
+++ b/browser/base/content/test/urlbar/browser_urlbarHighlightSearchAlias.js
@@ -133,17 +133,22 @@ add_task(async function inputDoesntMatch
 
 // Selecting a non-heuristic (non-first) search engine result with an alias and
 // empty query should put the alias in the urlbar and highlight it.
 add_task(async function nonHeuristicAliases() {
   // Get the list of token alias engines (those with aliases that start with
   // "@").
   let tokenEngines = [];
   for (let engine of Services.search.getEngines()) {
-    let tokenAliases = engine.wrappedJSObject._internalAliases;
+    let aliases = [];
+    if (engine.alias) {
+      aliases.push(engine.alias);
+    }
+    aliases.push(...engine.wrappedJSObject._internalAliases);
+    let tokenAliases = aliases.filter(a => a.startsWith("@"));
     if (tokenAliases.length) {
       tokenEngines.push({ engine, tokenAliases });
     }
   }
   if (!tokenEngines.length) {
     Assert.ok(true, "No token alias engines, skipping task.");
     return;
   }
@@ -166,16 +171,23 @@ add_task(async function nonHeuristicAlia
 
   // Hide the popup.
   EventUtils.synthesizeKey("KEY_Escape");
   await promisePopupHidden(gURLBar.popup);
 });
 
 
 async function doSimpleTest(revertBetweenSteps) {
+  // When autofill is enabled, searching for "@tes" will autofill to "@test",
+  // which gets in the way of this test task, so temporarily disable it.
+  Services.prefs.setBoolPref("browser.urlbar.autoFill", false);
+  registerCleanupFunction(() => {
+    Services.prefs.clearUserPref("browser.urlbar.autoFill");
+  });
+
   // "@tes" -- not an alias, no highlight
   gURLBar.search(ALIAS.substr(0, ALIAS.length - 1));
   await promiseSearchComplete();
   await waitForAutocompleteResultAt(0);
   assertAlias(false);
 
   if (revertBetweenSteps) {
     gURLBar.handleRevert();
@@ -218,16 +230,18 @@ async function doSimpleTest(revertBetwee
   // "@tes" -- not an alias, no highlight
   gURLBar.search(ALIAS.substr(0, ALIAS.length - 1));
   await promiseSearchComplete();
   await waitForAutocompleteResultAt(0);
   assertAlias(false);
 
   EventUtils.synthesizeKey("KEY_Escape");
   await promisePopupHidden(gURLBar.popup);
+
+  Services.prefs.clearUserPref("browser.urlbar.autoFill");
 }
 
 function assertAlias(aliasPresent, expectedAlias = ALIAS) {
   assertFirstResultIsAlias(aliasPresent, expectedAlias);
   assertHighlighted(aliasPresent, expectedAlias);
 }
 
 function assertFirstResultIsAlias(isAlias, expectedAlias) {
--- a/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
+++ b/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm
@@ -87,22 +87,22 @@ const SearchAutocompleteProviderInternal
     if (domain && !engine.hidden) {
       this.enginesByDomain.set(domain, engine);
     }
 
     let aliases = [];
     if (engine.alias) {
       aliases.push(engine.alias);
     }
-    let tokenAliases = engine.wrappedJSObject._internalAliases;
-    aliases.push(...tokenAliases);
+    aliases.push(...engine.wrappedJSObject._internalAliases);
     for (let alias of aliases) {
       this.enginesByAlias.set(alias.toLocaleLowerCase(), engine);
     }
 
+    let tokenAliases = aliases.filter(a => a.startsWith("@"));
     if (tokenAliases.length) {
       this.tokenAliasEngines.push({ engine, tokenAliases });
     }
   },
 
   QueryInterface: ChromeUtils.generateQI([Ci.nsIObserver,
                                           Ci.nsISupportsWeakReference]),
 };
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -1102,16 +1102,62 @@ Search.prototype = {
         engine,
         alias,
         input: alias + " ",
       });
     }
     return true;
   },
 
+  async _matchSearchEngineTokenAlias() {
+    // We need a single "@engine" search token.
+    if (this._searchTokens.length != 1) {
+      return false;
+    }
+    let token = this._searchTokens[0];
+    if (token[0] != "@" || token.length == 1) {
+      return false;
+    }
+
+    // See if any engine token alias starts with the search token.
+    let engines = await PlacesSearchAutocompleteProvider.tokenAliasEngines();
+    for (let { engine, tokenAliases } of engines) {
+      for (let alias of tokenAliases) {
+        if (alias.startsWith(token.toLocaleLowerCase())) {
+          // We found one.  The match we add here is a little special compared
+          // to others.  It needs to be an autofill match and its `value` must
+          // be the string that will be autofilled so that the controller will
+          // autofill it.  But it also must be a searchengine action so that the
+          // front end will style it as a search engine result.  The front end
+          // uses `finalCompleteValue` as the URL for autofill results, so set
+          // that to the moz-action URL.
+          let aliasPreservingUserCase = token + alias.substr(token.length);
+          let value = aliasPreservingUserCase + " ";
+          this._addMatch({
+            value,
+            finalCompleteValue: PlacesUtils.mozActionURI("searchengine", {
+              engineName: engine.name,
+              alias: aliasPreservingUserCase,
+              input: value,
+              searchQuery: "",
+            }),
+            comment: engine.name,
+            frecency: FRECENCY_DEFAULT,
+            style: "autofill action searchengine",
+            icon: engine.iconURI ? engine.iconURI.spec : null,
+          });
+          this._result.setDefaultIndex(0);
+          return true;
+        }
+      }
+    }
+
+    return false;
+  },
+
   async _matchFirstHeuristicResult(conn) {
     // We always try to make the first result a special "heuristic" result.  The
     // heuristics below determine what type of result it will be, if any.
 
     let hasSearchTerms = this._searchTokens.length > 0;
 
     if (hasSearchTerms) {
       // It may be a keyword registered by an extension.
@@ -1165,16 +1211,23 @@ Search.prototype = {
 
     if (this.pending && shouldAutofill) {
       let matched = this._matchPreloadedSiteForAutofill();
       if (matched) {
         return true;
       }
     }
 
+    if (this.pending && shouldAutofill) {
+      let matched = await this._matchSearchEngineTokenAlias();
+      if (matched) {
+        return true;
+      }
+    }
+
     if (this.pending && hasSearchTerms && this._enableActions) {
       // If we don't have a result that matches what we know about, then
       // we use a fallback for things we don't know about.
 
       // We may not have auto-filled, but this may still look like a URL.
       // However, even if the input is a valid URL, we may not want to use
       // it as such. This can happen if the host would require whitelisting,
       // but isn't in the whitelist.
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/unifiedcomplete/test_autofill_search_engine_aliases.js
@@ -0,0 +1,75 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// Tests autofilling search engine token ("@") aliases.
+
+"use strict";
+
+const TEST_ENGINE_NAME = "test autofill aliases";
+const TEST_ENGINE_ALIAS = "@autofilltest";
+
+add_task(async function init() {
+  // Add an engine with an "@" alias.
+  Services.search.addEngineWithDetails(TEST_ENGINE_NAME, {
+    alias: TEST_ENGINE_ALIAS,
+    template: "http://example.com/?search={searchTerms}",
+  });
+  registerCleanupFunction(() => {
+    let engine = Services.search.getEngineByName(TEST_ENGINE_NAME);
+    Assert.ok(engine);
+    Services.search.removeEngine(engine);
+  });
+});
+
+// Searching for @autofi should autofill to @autofilltest.
+add_task(async function basic() {
+  let autofilledValue = TEST_ENGINE_ALIAS + " ";
+  let completedURL = PlacesUtils.mozActionURI("searchengine", {
+    engineName: TEST_ENGINE_NAME,
+    alias: TEST_ENGINE_ALIAS,
+    input: autofilledValue,
+    searchQuery: "",
+  });
+  await check_autocomplete({
+    search:
+      TEST_ENGINE_ALIAS.substr(0, Math.round(TEST_ENGINE_ALIAS.length / 2)),
+    autofilled: autofilledValue,
+    completed: completedURL,
+    matches: [{
+      value: autofilledValue,
+      comment: TEST_ENGINE_NAME,
+      uri: completedURL,
+      style: ["autofill", "action", "searchengine", "heuristic"],
+    }],
+  });
+  await cleanup();
+});
+
+// Searching for @AUTOFI should autofill to @AUTOFIlltest, preserving the case
+// in the search string.
+add_task(async function preserveCase() {
+  let search =
+    TEST_ENGINE_ALIAS.toUpperCase()
+    .substr(0, Math.round(TEST_ENGINE_ALIAS.length / 2));
+  let alias = search + TEST_ENGINE_ALIAS.substr(search.length);
+  let autofilledValue = alias + " ";
+  let completedURL = PlacesUtils.mozActionURI("searchengine", {
+    engineName: TEST_ENGINE_NAME,
+    alias,
+    input: autofilledValue,
+    searchQuery: "",
+  });
+  await check_autocomplete({
+    search,
+    autofilled: autofilledValue,
+    completed: completedURL,
+    matches: [{
+      value: autofilledValue,
+      comment: TEST_ENGINE_NAME,
+      uri: completedURL,
+      style: ["autofill", "action", "searchengine", "heuristic"],
+    }],
+  });
+  await cleanup();
+});
--- a/toolkit/components/places/tests/unifiedcomplete/test_search_engine_alias.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_search_engine_alias.js
@@ -159,36 +159,40 @@ add_task(async function engineWithSugges
           engineName: SUGGESTIONS_ENGINE_NAME,
           searchQuery: "fire",
           searchSuggestion: "fire bar",
         }),
       ],
     });
   }
 
+  engine.alias = "";
   await cleanup();
 });
 
 
 // When the search is simply "@", the results should be a list of all the "@"
 // alias engines.
 add_task(async function tokenAliasEngines() {
   let tokenEngines = [];
   for (let engine of Services.search.getEngines()) {
-    let tokenAliases = engine.wrappedJSObject._internalAliases;
+    let aliases = [];
+    if (engine.alias) {
+      aliases.push(engine.alias);
+    }
+    aliases.push(...engine.wrappedJSObject._internalAliases);
+    let tokenAliases = aliases.filter(a => a.startsWith("@"));
     if (tokenAliases.length) {
       tokenEngines.push({ engine, tokenAliases });
     }
   }
-
   if (!tokenEngines.length) {
     Assert.ok(true, "No token alias engines, skipping task.");
     return;
   }
-
   info("Got token alias engines: " +
        tokenEngines.map(({ engine }) => engine.name));
 
   await check_autocomplete({
     search: "@",
     searchParam: "enable-actions",
     matches: tokenEngines.map(({ engine, tokenAliases }) => {
       let alias = tokenAliases[0];
--- a/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
+++ b/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
@@ -16,16 +16,17 @@ support-files =
 [test_422277.js]
 [test_adaptive.js]
 [test_adaptive_behaviors.js]
 [test_adaptive_limited.js]
 [test_autocomplete_functional.js]
 [test_autocomplete_stopSearch_no_throw.js]
 [test_autofill_about_urls.js]
 [test_autofill_origins.js]
+[test_autofill_search_engine_aliases.js]
 [test_autofill_search_engines.js]
 [test_autofill_urls.js]
 [test_avoid_middle_complete.js]
 [test_avoid_stripping_to_empty_tokens.js]
 [test_casing.js]
 [test_do_not_trim.js]
 [test_download_embed_bookmarks.js]
 [test_dupe_urls.js]
--- a/toolkit/components/satchel/AutoCompletePopup.jsm
+++ b/toolkit/components/satchel/AutoCompletePopup.jsm
@@ -26,16 +26,20 @@ var AutoCompleteResultView = {
   get matchCount() {
     return this.results.length;
   },
 
   getValueAt(index) {
     return this.results[index].value;
   },
 
+  getFinalCompleteValueAt(index) {
+    return this.results[index].value;
+  },
+
   getLabelAt(index) {
     // Unused by richlist autocomplete - see getCommentAt.
     return "";
   },
 
   getCommentAt(index) {
     // The richlist autocomplete popup uses comment for its main
     // display of an item, which is why we're returning the label
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -959,19 +959,17 @@
             }
             let item;
             let reusable = false;
             let itemExists = this._currentIndex < existingItemsCount;
 
             let originalValue, originalText, originalType;
             let style = controller.getStyleAt(this._currentIndex);
             let value =
-              style &&
-                style.includes("autofill") &&
-                style.includes("heuristic") ?
+              style && style.includes("autofill") ?
               controller.getFinalCompleteValueAt(this._currentIndex) :
               controller.getValueAt(this._currentIndex);
             let label = controller.getLabelAt(this._currentIndex);
             let comment = controller.getCommentAt(this._currentIndex);
             let image = controller.getImageAt(this._currentIndex);
             // trim the leading/trailing whitespace
             let trimmedSearchString = controller.searchString.replace(/^\s+/, "").replace(/\s+$/, "");
 
@@ -1798,18 +1796,19 @@
           // Remove types that should ultimately not be in the `type` string.
           types.delete("action");
           types.delete("autofill");
           types.delete("heuristic");
           type = [...types][0] || "";
 
           let action;
 
-          if (initialTypes.has("autofill")) {
-            // Treat autofills as visiturl actions.
+          if (initialTypes.has("autofill") && !initialTypes.has("action")) {
+            // Treat autofills as visiturl actions, unless they are already also
+            // actions.
             action = {
               type: "visiturl",
               params: { url: title },
             };
           }
 
           this.removeAttribute("actiontype");
           this.classList.remove(