Bug 1504854 - Autofilled search shortcuts should show a single "Search with Engine" heuristic result in the popup. r=mak, a=jcristau
authorDrew Willcoxon <adw@mozilla.com>
Mon, 12 Nov 2018 15:45:39 +0000
changeset 498511 d719e13dbb8b9f9729d5c8bbd00157a43452721b
parent 498510 e14b94e8cc97a78bf833f809d2f668db6d615c43
child 498512 da77ae6bcf6403ceb94574399fe23a13aa8decbc
push id10195
push userryanvm@gmail.com
push dateWed, 14 Nov 2018 19:22:22 +0000
treeherdermozilla-beta@d719e13dbb8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, jcristau
bugs1504854
milestone64.0
Bug 1504854 - Autofilled search shortcuts should show a single "Search with Engine" heuristic result in the popup. r=mak, a=jcristau All we need to do is set `_searchEngineAliasMatch` with an empty query so that we don't try to add any more results. That hits the existing case where the user types out a full @ alias and we show only the "search with engine" heuristic result. Differential Revision: https://phabricator.services.mozilla.com/D11547
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/test_autofill_search_engine_aliases.js
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -1131,17 +1131,17 @@ Search.prototype = {
       this._addSearchEngineMatch({
         engine,
         alias: tokenAliases[0],
       });
     }
     return true;
   },
 
-  async _matchSearchEngineTokenAlias() {
+  async _matchSearchEngineTokenAliasForAutofill() {
     // 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;
     }
@@ -1169,16 +1169,27 @@ Search.prototype = {
               searchQuery: "",
             }),
             comment: engine.name,
             frecency: FRECENCY_DEFAULT,
             style: "autofill action searchengine",
             icon: engine.iconURI ? engine.iconURI.spec : null,
           });
           this._result.setDefaultIndex(0);
+
+          // Set _searchEngineAliasMatch with an empty query so that we don't
+          // attempt to add any more matches.  When a token alias is autofilled,
+          // the only match should be the one we just added.
+          this._searchEngineAliasMatch = {
+            engine,
+            alias: aliasPreservingUserCase,
+            query: "",
+            isTokenAlias: true,
+          };
+
           return true;
         }
       }
     }
 
     return false;
   },
 
@@ -1241,17 +1252,17 @@ Search.prototype = {
     if (this.pending && shouldAutofill) {
       let matched = this._matchPreloadedSiteForAutofill();
       if (matched) {
         return true;
       }
     }
 
     if (this.pending && shouldAutofill) {
-      let matched = await this._matchSearchEngineTokenAlias();
+      let matched = await this._matchSearchEngineTokenAliasForAutofill();
       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.
--- a/toolkit/components/places/tests/unifiedcomplete/test_autofill_search_engine_aliases.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_autofill_search_engine_aliases.js
@@ -19,16 +19,24 @@ add_task(async function init() {
     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() {
+  // Add a history visit that should normally match but for the fact that the
+  // search uses an @ alias.  When an @ alias is autofilled, there should be no
+  // other matches except the autofill heuristic match.
+  await PlacesTestUtils.addVisits({
+    uri: "http://example.com/",
+    title: TEST_ENGINE_ALIAS,
+  });
+
   let autofilledValue = TEST_ENGINE_ALIAS + " ";
   let completedURL = PlacesUtils.mozActionURI("searchengine", {
     engineName: TEST_ENGINE_NAME,
     alias: TEST_ENGINE_ALIAS,
     input: autofilledValue,
     searchQuery: "",
   });
   await check_autocomplete({
@@ -44,16 +52,24 @@ add_task(async function basic() {
     }],
   });
   await cleanup();
 });
 
 // Searching for @AUTOFI should autofill to @AUTOFIlltest, preserving the case
 // in the search string.
 add_task(async function preserveCase() {
+  // Add a history visit that should normally match but for the fact that the
+  // search uses an @ alias.  When an @ alias is autofilled, there should be no
+  // other matches except the autofill heuristic match.
+  await PlacesTestUtils.addVisits({
+    uri: "http://example.com/",
+    title: TEST_ENGINE_ALIAS,
+  });
+
   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,