Bug 1034381 - Enhance previous searches in awesomebar dropdown by removing URL. r=mak
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Fri, 01 Aug 2014 16:41:32 +0100
changeset 197329 55f293b3d8163f18982aa007d46b75de4ddc78fb
parent 197328 2bbd5e4afe2d94306d6a4a60694989a7efadd7fd
child 197330 e3e739ad7b5503f973dfea8fbe891ef1a995f575
push id27240
push userryanvm@gmail.com
push dateFri, 01 Aug 2014 19:41:39 +0000
treeherdermozilla-central@48609b922d5e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1034381
milestone34.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 1034381 - Enhance previous searches in awesomebar dropdown by removing URL. r=mak
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
toolkit/components/places/tests/unifiedcomplete/test_searchEngine.js
toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
toolkit/content/widgets/autocomplete.xml
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -17,35 +17,35 @@ const Cu = Components.utils;
 const TOPIC_SHUTDOWN = "places-shutdown";
 const TOPIC_PREFCHANGED = "nsPref:changed";
 
 const DEFAULT_BEHAVIOR = 0;
 
 const PREF_BRANCH = "browser.urlbar.";
 
 // Prefs are defined as [pref name, default value].
-const PREF_ENABLED =            [ "autocomplete.enabled", true ];
-const PREF_AUTOFILL =           [ "autoFill",             true ];
-const PREF_AUTOFILL_TYPED =     [ "autoFill.typed",       true ];
-const PREF_AUTOFILL_PRIORITY =  [ "autoFill.priority",    true ];
-const PREF_DELAY =              [ "delay",                  50 ];
-const PREF_BEHAVIOR =           [ "matchBehavior", MATCH_BOUNDARY_ANYWHERE ];
-const PREF_DEFAULT_BEHAVIOR =   [ "default.behavior", DEFAULT_BEHAVIOR ];
-const PREF_EMPTY_BEHAVIOR =     [ "default.behavior.emptyRestriction",
-                                  Ci.mozIPlacesAutoComplete.BEHAVIOR_HISTORY |
-                                  Ci.mozIPlacesAutoComplete.BEHAVIOR_TYPED ];
-const PREF_FILTER_JS =          [ "filter.javascript",    true ];
-const PREF_MAXRESULTS =         [ "maxRichResults",         25 ];
-const PREF_RESTRICT_HISTORY =   [ "restrict.history",      "^" ];
-const PREF_RESTRICT_BOOKMARKS = [ "restrict.bookmark",     "*" ];
-const PREF_RESTRICT_TYPED =     [ "restrict.typed",        "~" ];
-const PREF_RESTRICT_TAG =       [ "restrict.tag",          "+" ];
-const PREF_RESTRICT_SWITCHTAB = [ "restrict.openpage",     "%" ];
-const PREF_MATCH_TITLE =        [ "match.title",           "#" ];
-const PREF_MATCH_URL =          [ "match.url",             "@" ];
+const PREF_ENABLED =                [ "autocomplete.enabled",   true ];
+const PREF_AUTOFILL =               [ "autoFill",               true ];
+const PREF_AUTOFILL_TYPED =         [ "autoFill.typed",         true ];
+const PREF_AUTOFILL_SEARCHENGINES = [ "autoFill.searchEngines", true ];
+const PREF_DELAY =                  [ "delay",                  50 ];
+const PREF_BEHAVIOR =               [ "matchBehavior", MATCH_BOUNDARY_ANYWHERE ];
+const PREF_DEFAULT_BEHAVIOR =       [ "default.behavior", DEFAULT_BEHAVIOR ];
+const PREF_EMPTY_BEHAVIOR =         [ "default.behavior.emptyRestriction",
+                                      Ci.mozIPlacesAutoComplete.BEHAVIOR_HISTORY |
+                                      Ci.mozIPlacesAutoComplete.BEHAVIOR_TYPED ];
+const PREF_FILTER_JS =              [ "filter.javascript",      true ];
+const PREF_MAXRESULTS =             [ "maxRichResults",         25 ];
+const PREF_RESTRICT_HISTORY =       [ "restrict.history",       "^" ];
+const PREF_RESTRICT_BOOKMARKS =     [ "restrict.bookmark",      "*" ];
+const PREF_RESTRICT_TYPED =         [ "restrict.typed",         "~" ];
+const PREF_RESTRICT_TAG =           [ "restrict.tag",           "+" ];
+const PREF_RESTRICT_SWITCHTAB =     [ "restrict.openpage",      "%" ];
+const PREF_MATCH_TITLE =            [ "match.title",            "#" ];
+const PREF_MATCH_URL =              [ "match.url",              "@" ];
 
 // Match type constants.
 // These indicate what type of search function we should be using.
 const MATCH_ANYWHERE = Ci.mozIPlacesAutoComplete.MATCH_ANYWHERE;
 const MATCH_BOUNDARY_ANYWHERE = Ci.mozIPlacesAutoComplete.MATCH_BOUNDARY_ANYWHERE;
 const MATCH_BOUNDARY = Ci.mozIPlacesAutoComplete.MATCH_BOUNDARY;
 const MATCH_BEGINNING = Ci.mozIPlacesAutoComplete.MATCH_BEGINNING;
 const MATCH_BEGINNING_CASE_SENSITIVE = Ci.mozIPlacesAutoComplete.MATCH_BEGINNING_CASE_SENSITIVE;
@@ -57,21 +57,24 @@ const QUERYTYPE_FILTERED      = 1;
 const QUERYTYPE_AUTOFILL_HOST = 2;
 const QUERYTYPE_AUTOFILL_URL  = 3;
 
 // This separator is used as an RTL-friendly way to split the title and tags.
 // It can also be used by an nsIAutoCompleteResult consumer to re-split the
 // "comment" back into the title and the tag.
 const TITLE_TAGS_SEPARATOR = " \u2013 ";
 
+// This separator identifies the search engine name in the title.
+const TITLE_SEARCH_ENGINE_SEPARATOR = " \u00B7\u2013\u00B7 ";
+
 // Telemetry probes.
 const TELEMETRY_1ST_RESULT = "PLACES_AUTOCOMPLETE_1ST_RESULT_TIME_MS";
 
-// The default frecency value used when inserting priority results.
-const FRECENCY_PRIORITY_DEFAULT = 1000;
+// The default frecency value used when inserting search engine results.
+const FRECENCY_SEARCHENGINES_DEFAULT = 1000;
 
 // 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;
@@ -354,17 +357,17 @@ XPCOMUtils.defineLazyGetter(this, "Switc
  */
 XPCOMUtils.defineLazyGetter(this, "Prefs", () => {
   let prefs = new Preferences(PREF_BRANCH);
 
   function loadPrefs() {
     store.enabled = prefs.get(...PREF_ENABLED);
     store.autofill = prefs.get(...PREF_AUTOFILL);
     store.autofillTyped = prefs.get(...PREF_AUTOFILL_TYPED);
-    store.autofillPriority = prefs.get(...PREF_AUTOFILL_PRIORITY);
+    store.autofillSearchEngines = prefs.get(...PREF_AUTOFILL_SEARCHENGINES);
     store.delay = prefs.get(...PREF_DELAY);
     store.matchBehavior = prefs.get(...PREF_BEHAVIOR);
     store.filterJavaScript = prefs.get(...PREF_FILTER_JS);
     store.maxRichResults = prefs.get(...PREF_MAXRESULTS);
     store.restrictHistoryToken = prefs.get(...PREF_RESTRICT_HISTORY);
     store.restrictBookmarkToken = prefs.get(...PREF_RESTRICT_BOOKMARKS);
     store.restrictTypedToken = prefs.get(...PREF_RESTRICT_TYPED);
     store.restrictTagToken = prefs.get(...PREF_RESTRICT_TAG);
@@ -624,18 +627,22 @@ Search.prototype = {
    * Execute the search and populate results.
    * @param conn
    *        The Sqlite connection.
    */
   execute: Task.async(function* (conn) {
     this._pendingQuery = true;
     TelemetryStopwatch.start(TELEMETRY_1ST_RESULT);
 
+    // Since we call the synchronous parseSubmissionURL function later, we must
+    // wait for the initialization of PlacesSearchAutocompleteProvider first.
+    yield PlacesSearchAutocompleteProvider.ensureInitialized();
+
     // For any given search, we run many queries:
-    // 1) priority domains
+    // 1) search engine domains
     // 2) inline completion
     // 3) keywords (this._keywordQuery)
     // 4) adaptive learning (this._adaptiveQuery)
     // 5) open pages not supported by history (this._switchToTabQuery)
     // 6) query based on match behavior
     //
     // (3) only gets ran if we get any filtered tokens, since if there are no
     // tokens, there is nothing to match.
@@ -644,17 +651,17 @@ Search.prototype = {
     let queries = [ this._adaptiveQuery,
                     this._switchToTabQuery,
                     this._searchQuery ];
 
     if (this._searchTokens.length > 0 &&
         PlacesUtils.bookmarks.getURIForKeyword(this._searchTokens[0])) {
       queries.unshift(this._keywordQuery);
     } else if (this._searchTokens.length == 1) {
-      yield this._matchPriorityUrl();
+      yield this._matchSearchEngineUrl();
     }
 
     if (this._shouldAutofill) {
       // Hosts have no "/" in them.
       let lastSlashIndex = this._searchString.lastIndexOf("/");
       // Search only URLs if there's a slash in the search string...
       if (lastSlashIndex != -1) {
         // ...but not if it's exactly at the end of the search string.
@@ -695,32 +702,31 @@ Search.prototype = {
 
     // If we didn't find enough matches and we have some frecency-driven
     // matches, add them.
     if (this._frecencyMatches) {
       this._frecencyMatches.forEach(this._addMatch, this);
     }
   }),
 
-  _matchPriorityUrl: function* () {
-    if (!Prefs.autofillPriority)
+  _matchSearchEngineUrl: function* () {
+    if (!Prefs.autofillSearchEngines)
       return;
 
-    // Handle priority matches for search engine domains.
-    let priorityMatch =
-        yield PlacesSearchAutocompleteProvider.findMatchByToken(this._searchString);
-    if (priorityMatch) {
+    let match = yield PlacesSearchAutocompleteProvider.findMatchByToken(
+                                                           this._searchString);
+    if (match) {
       this._result.setDefaultIndex(0);
       this._addFrecencyMatch({
-        value: priorityMatch.token,
-        comment: priorityMatch.engineName,
-        icon: priorityMatch.iconUrl,
+        value: match.token,
+        comment: match.engineName,
+        icon: match.iconUrl,
         style: "priority-search",
-        finalCompleteValue: priorityMatch.url,
-        frecency: FRECENCY_PRIORITY_DEFAULT
+        finalCompleteValue: match.url,
+        frecency: FRECENCY_SEARCHENGINES_DEFAULT
       });
     }
   },
 
   _onResultRow: function (row) {
     TelemetryStopwatch.finish(TELEMETRY_1ST_RESULT);
     let queryType = row.getResultByIndex(QUERYINDEX_QUERYTYPE);
     let match;
@@ -749,16 +755,40 @@ Search.prototype = {
       this._frecencyMatches = [];
     this._frecencyMatches.push(match);
     // We keep this array in reverse order, so we can walk it and remove stuff
     // from it in one pass.  Notice that for frecency reverse order means from
     // lower to higher.
     this._frecencyMatches.sort((a, b) => a.frecency - b.frecency);
   },
 
+  _maybeRestyleSearchMatch: function (match) {
+    // Return if the URL does not represent a search result.
+    let parseResult =
+      PlacesSearchAutocompleteProvider.parseSubmissionURL(match.value);
+    if (!parseResult) {
+      return;
+    }
+
+    // Do not apply the special style if the user is doing a search from the
+    // location bar but the entered terms match an irrelevant portion of the
+    // URL. For example, "https://www.google.com/search?q=terms&client=firefox"
+    // when searching for "Firefox".
+    let terms = parseResult.terms.toLowerCase();
+    if (this._searchTokens.length > 0 &&
+        this._searchTokens.every(token => terms.indexOf(token) == -1)) {
+      return;
+    }
+
+    // Use the special separator that the binding will use to style the item.
+    match.style = "search " + match.style;
+    match.comment = parseResult.terms + TITLE_SEARCH_ENGINE_SEPARATOR +
+                    parseResult.engineName;
+  },
+
   _addMatch: function (match) {
     let notifyResults = false;
 
     if (this._frecencyMatches) {
       for (let i = this._frecencyMatches.length - 1;  i >= 0 ; i--) {
         if (this._frecencyMatches[i].frecency > match.frecency) {
           this._addMatch(this._frecencyMatches.splice(i, 1)[0]);
         }
@@ -774,20 +804,29 @@ Search.prototype = {
       // Not all entries have a place id, thus we fallback to the url for them.
       // We cannot use only the url since keywords entries are modified to
       // include the search string, and would be returned multiple times.  Ids
       // are faster too.
       if (match.placeId)
         this._usedPlaceIds.add(match.placeId);
       this._usedURLs.add(urlMapKey);
 
+      if (!match.style) {
+        match.style = "favicon";
+      }
+
+      // Restyle past searches, unless they are bookmarks or special results.
+      if (match.style == "favicon") {
+        this._maybeRestyleSearchMatch(match);
+      }
+
       this._result.appendMatch(match.value,
                                match.comment,
                                match.icon || PlacesUtils.favicons.defaultFavicon.spec,
-                               match.style || "favicon",
+                               match.style,
                                match.finalCompleteValue);
       notifyResults = true;
     }
 
     if (this._result.matchCount == Prefs.maxRichResults || !this.pending) {
       // We have enough results, so stop running our search.
       this.cancel();
       // This tells Sqlite.jsm to stop providing us results and cancel the
--- a/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
+++ b/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
@@ -12,16 +12,18 @@ Cu.import("resource://gre/modules/Servic
 // Import common head.
 let (commonFile = do_get_file("../head_common.js", false)) {
   let uri = Services.io.newFileURI(commonFile);
   Services.scriptloader.loadSubScript(uri.spec, this);
 }
 
 // Put any other stuff relative to this test folder below.
 
+const TITLE_SEARCH_ENGINE_SEPARATOR = " \u00B7\u2013\u00B7 ";
+
 function run_test() {
   run_next_test();
 }
 
 function* cleanup() {
   Services.prefs.clearUserPref("browser.urlbar.autocomplete.enabled");
   Services.prefs.clearUserPref("browser.urlbar.autoFill");
   Services.prefs.clearUserPref("browser.urlbar.autoFill.typed");
@@ -142,19 +144,21 @@ function* check_autocomplete(test) {
       let comment = controller.getCommentAt(i);
       do_log_info("Looking for '" + value + "', '" + comment + "' in expected results...");
       let j;
       for (j = 0; j < matches.length; j++) {
         // Skip processed expected results
         if (matches[j] == undefined)
           continue;
 
-        let { uri, title, tags } = matches[j];
+        let { uri, title, tags, searchEngine } = matches[j];
         if (tags)
           title += " \u2013 " + tags.sort().join(", ");
+        if (searchEngine)
+          title += TITLE_SEARCH_ENGINE_SEPARATOR + searchEngine;
 
         do_log_info("Checking against expected '" + uri.spec + "', '" + title + "'...");
         // Got a match on both uri and title?
         if (stripPrefix(uri.spec) == stripPrefix(value) && title == comment) {
           do_log_info("Got a match at index " + j + "!");
           // Make it undefined so we don't process it again
           matches[j] = undefined;
           if (uri.spec.startsWith("moz-action:")) {
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/unifiedcomplete/test_searchEngine.js
@@ -0,0 +1,25 @@
+/* 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/. */
+
+add_task(function* test_searchEngine() {
+  Services.search.addEngineWithDetails("SearchEngine", "", "", "",
+                                       "GET", "http://s.example.com/search");
+  let engine = Services.search.getEngineByName("SearchEngine");
+  engine.addParam("q", "{searchTerms}", null);
+  do_register_cleanup(() => Services.search.removeEngine(engine));
+
+  let uri1 = NetUtil.newURI("http://s.example.com/search?q=Terms&client=1");
+  let uri2 = NetUtil.newURI("http://s.example.com/search?q=Terms&client=2");
+  yield promiseAddVisits({ uri: uri1, title: "Terms - SearchEngine Search" });
+  addBookmark({ uri: uri2, title: "Terms - SearchEngine Search" });
+
+  do_log_info("Past search terms should be styled, unless bookmarked");
+  yield check_autocomplete({
+    search: "term",
+    matches: [ { uri: uri1, title: "Terms", searchEngine: "SearchEngine" },
+               { uri: uri2, title: "Terms - SearchEngine Search" } ]
+  });
+
+  yield cleanup();
+});
--- a/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
+++ b/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini
@@ -18,15 +18,16 @@ tail =
 [test_enabled.js]
 [test_escape_self.js]
 [test_ignore_protocol.js]
 [test_keyword_search.js]
 [test_keywords.js]
 [test_match_beginning.js]
 [test_multi_word_search.js]
 [test_queryurl.js]
+[test_searchEngine.js]
 [test_special_search.js]
 [test_swap_protocol.js]
 [test_tabmatches.js]
 [test_trimming.js]
 [test_typed.js]
 [test_word_boundary_search.js]
 [test_zero_frecency.js]
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1431,16 +1431,30 @@ extends="chrome://global/content/binding
             this._setUpDescription(this._action, desc, true);
 
             // Remove the "action" substring so that the correct style, if any,
             // is applied below.
             types.splice(actionIndex, 1);
             type = types.join(" ");
           }
 
+          // Check if we have a search engine name
+          let searchEngine = "";
+          let searchIndex = types.indexOf("search");
+          if (searchIndex >= 0) {
+            const TITLE_SEARCH_ENGINE_SEPARATOR = " \u00B7\u2013\u00B7 ";
+
+            [title, searchEngine] = title.split(TITLE_SEARCH_ENGINE_SEPARATOR);
+
+            // Remove the "search" substring so that the correct style, if any,
+            // is applied below.
+            types.splice(searchIndex, 1);
+            type = types.join(" ");
+          }
+
           // If we have a tag match, show the tags and icon
           if (type == "tag") {
             // Configure the extra box for tags display
             this._extraBox.hidden = false;
             this._extraBox.childNodes[0].hidden = false;
             this._extraBox.childNodes[1].hidden = true;
             this._extraBox.pack = "end";
             this._titleBox.flex = 1;
@@ -1491,17 +1505,22 @@ extends="chrome://global/content/binding
             (type ? " ac-result-type-" + type : "");
 
           // Show the url as the title if we don't have a title
           if (title == "")
             title = url;
 
           // Emphasize the matching search terms for the description
           this._setUpDescription(this._title, title);
-          this._setUpDescription(this._url, url);
+          if (!searchEngine) {
+            this._setUpDescription(this._url, url);
+          } else {
+            // The search engine name, when present, is not emphasized.
+            this._setUpDescription(this._url, searchEngine, true);
+          }
 
           // Set up overflow on a timeout because the contents of the box
           // might not have a width yet even though we just changed them
           setTimeout(this._setUpOverflow, 0, this._titleBox, this._titleOverflowEllipsis);
           setTimeout(this._setUpOverflow, 0, this._urlBox, this._urlOverflowEllipsis);
           ]]>
         </body>
       </method>