Bug 1187753 - Autocomplete popup displays wrong message when typing only a search keyword. r=adw
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 05 Aug 2015 22:04:58 +0200
changeset 288063 c9adece82ecc6d8cbbc955c731b21fdb340e7523
parent 288062 a65727f3f5c47f33069142bdaced3c2f67ce51ad
child 288064 816eb5a838ebcf0262f6c0a9a69770fea9d22635
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1187753
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1187753 - Autocomplete popup displays wrong message when typing only a search keyword. r=adw
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
toolkit/components/places/tests/unifiedcomplete/test_searchEngine_alias.js
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -73,16 +73,19 @@ const FRECENCY_DEFAULT = 1000;
 // always try to have at least MINIMUM_LOCAL_MATCHES local matches.
 const MINIMUM_LOCAL_MATCHES = 5;
 
 // A regex that matches "single word" hostnames for whitelisting purposes.
 // The hostname will already have been checked for general validity, so we
 // don't need to be exhaustive here, so allow dashes anywhere.
 const REGEXP_SINGLEWORD_HOST = new RegExp("^[a-z0-9-]+$", "i");
 
+// Regex used to match one or more whitespace.
+const REGEXP_SPACES = /\s+/;
+
 // Sqlite result row index constants.
 const QUERYINDEX_QUERYTYPE     = 0;
 const QUERYINDEX_URL           = 1;
 const QUERYINDEX_TITLE         = 2;
 const QUERYINDEX_ICONURL       = 3;
 const QUERYINDEX_BOOKMARKED    = 4;
 const QUERYINDEX_BOOKMARKTITLE = 5;
 const QUERYINDEX_TAGS          = 6;
@@ -512,17 +515,17 @@ function fixupSearchText(spec)
  * @param searchString
  *        The string to generate tokens from.
  * @return an array of tokens.
  * @note Calling split on an empty string will return an array containing one
  *       empty string.  We don't want that, as it'll break our logic, so return
  *       an empty array then.
  */
 function getUnfilteredSearchTokens(searchString)
-  searchString.length ? searchString.split(" ") : [];
+  searchString.length ? searchString.split(REGEXP_SPACES) : [];
 
 /**
  * Strip prefixes from the URI that we don't care about for searching.
  *
  * @param spec
  *        The text to modify.
  * @return the modified spec.
  */
@@ -972,17 +975,17 @@ Search.prototype = {
   _prohibitSearchSuggestionsFor(searchString) {
     if (this._prohibitSearchSuggestions)
       return true;
 
     // Suggestions for a single letter are unlikely to be useful.
     if (searchString.length < 2)
       return true;
 
-    let tokens = searchString.split(/\s/);
+    let tokens = searchString.split(REGEXP_SPACES);
 
     // The first token may be a whitelisted host.
     if (REGEXP_SINGLEWORD_HOST.test(tokens[0]) &&
         Services.uriFixup.isDomainWhitelisted(tokens[0], -1))
       return true;
 
     // Disallow fetching search suggestions for strings looking like URLs, to
     // avoid disclosing information about networks or passwords.
@@ -1100,17 +1103,17 @@ Search.prototype = {
       style: "priority-search",
       finalCompleteValue: match.url,
       frecency: FRECENCY_DEFAULT
     });
     return true;
   },
 
   _matchSearchEngineAlias: function* () {
-    if (this._searchTokens.length < 2)
+    if (this._searchTokens.length < 1)
       return false;
 
     let alias = this._searchTokens[0];
     let match = yield PlacesSearchAutocompleteProvider.findMatchByAlias(alias);
     if (!match)
       return false;
 
     match.engineAlias = alias;
@@ -1584,17 +1587,17 @@ Search.prototype = {
     // autoFill doesn't search titles or tags.
     if (this.hasBehavior("title") || this.hasBehavior("tag"))
       return false;
 
     // Don't try to autofill if the search term includes any whitespace.
     // This may confuse completeDefaultIndex cause the AUTOCOMPLETE_MATCH
     // tokenizer ends up trimming the search string and returning a value
     // that doesn't match it, or is even shorter.
-    if (/\s/.test(this._originalSearchString))
+    if (REGEXP_SPACES.test(this._originalSearchString))
       return false;
 
     if (this._searchString.length == 0)
       return false;
 
     if (this._prohibitAutoFill)
       return false;
 
--- a/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
+++ b/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
@@ -354,17 +354,17 @@ function makeActionURI(action, params) {
 // an entry to check_autocomplete.
 function makeSearchMatch(input, extra = {}) {
   // Note that counter-intuitively, the order the object properties are defined
   // in the object passed to makeActionURI is important for check_autocomplete
   // to match them :(
   let params = {
     engineName: extra.engineName || "MozSearch",
     input,
-    searchQuery: extra.searchQuery || input,
+    searchQuery: "searchQuery" in extra ? extra.searchQuery : input,
     alias: extra.alias, // may be undefined which is expected.
   }
   return {
     uri: makeActionURI("searchengine", params),
     title: params.engineName,
     style: [ "action", "searchengine" ],
   }
 }
--- a/toolkit/components/places/tests/unifiedcomplete/test_searchEngine_alias.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_searchEngine_alias.js
@@ -7,17 +7,23 @@ add_task(function*() {
   // Here we add another engine with a search alias.
   Services.search.addEngineWithDetails("AliasedMozSearch", "", "doit", "",
                                        "GET", "http://s.example.com/search");
 
 
   yield check_autocomplete({
     search: "doit",
     searchParam: "enable-actions",
-    matches: [ makeSearchMatch("doit") ]
+    matches: [ makeSearchMatch("doit", { engineName: "AliasedMozSearch", searchQuery: "", alias: "doit" }) ]
+  });
+
+  yield check_autocomplete({
+    search: "doit ",
+    searchParam: "enable-actions",
+    matches: [ makeSearchMatch("doit ", { engineName: "AliasedMozSearch", searchQuery: "", alias: "doit" }) ]
   });
 
   yield check_autocomplete({
     search: "doit mozilla",
     searchParam: "enable-actions",
     matches: [ makeSearchMatch("doit mozilla", { engineName: "AliasedMozSearch", searchQuery: "mozilla", alias: "doit" }) ]
   });