Bug 1060675 - Only cap local form history results for the search bar if there are remote suggestions. r=MattN, a=lmandel
authorGavin Sharp <gavin@gavinsharp.com>
Fri, 17 Oct 2014 09:29:40 -0700
changeset 225777 a963eab53a09
parent 225776 1af716db5215
child 225778 5c014e511661
push id4013
push userryanvm@gmail.com
push date2014-10-22 23:14 +0000
treeherdermozilla-beta@a963eab53a09 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN, lmandel
bugs1060675
milestone34.0
Bug 1060675 - Only cap local form history results for the search bar if there are remote suggestions. r=MattN, a=lmandel
toolkit/components/search/SearchSuggestionController.jsm
toolkit/components/search/nsSearchSuggestions.js
toolkit/components/search/tests/xpcshell/test_searchSuggest.js
--- a/toolkit/components/search/SearchSuggestionController.jsm
+++ b/toolkit/components/search/SearchSuggestionController.jsm
@@ -42,17 +42,18 @@ Services.prefs.addObserver(BROWSER_SUGGE
  * @constructor
  */
 this.SearchSuggestionController = function SearchSuggestionController(callback = null) {
   this._callback = callback;
 };
 
 this.SearchSuggestionController.prototype = {
   /**
-   * The maximum number of local form history results to return.
+   * The maximum number of local form history results to return. This limit is
+   * only enforced if remote results are also returned.
    */
   maxLocalResults: 7,
 
   /**
    * The maximum number of remote search engine results to return.
    */
   maxRemoteResults: 10,
 
@@ -195,18 +196,17 @@ this.SearchSuggestionController.prototyp
         switch (result.searchResult) {
           case Ci.nsIAutoCompleteResult.RESULT_SUCCESS:
           case Ci.nsIAutoCompleteResult.RESULT_NOMATCH:
             if (result.searchString !== this._searchString) {
               deferredFormHistory.resolve("Unexpected response, this._searchString does not match form history response");
               return;
             }
             let fhEntries = [];
-            let maxHistoryItems = Math.min(result.matchCount, this.maxLocalResults);
-            for (let i = 0; i < maxHistoryItems; ++i) {
+            for (let i = 0; i < result.matchCount; ++i) {
               fhEntries.push(result.getValueAt(i));
             }
             deferredFormHistory.resolve({
               result: fhEntries,
               formHistoryResult: result,
             });
             break;
           case Ci.nsIAutoCompleteResult.RESULT_FAILURE:
@@ -330,18 +330,23 @@ this.SearchSuggestionController.prototyp
       } else if (result.formHistoryResult) { // Local results have a formHistoryResult property.
         results.formHistoryResult = result.formHistoryResult;
         results.local = result.result || [];
       } else { // Remote result
         results.remote = result.result || [];
       }
     }
 
+    // If we have remote results, cap the number of local results
+    if (results.remote.length) {
+      results.local = results.local.slice(0, this.maxLocalResults);
+    }
+
     // We don't want things to appear in both history and suggestions so remove entries from
-    // remote results that are alrady in local.
+    // remote results that are already in local.
     if (results.remote.length && results.local.length) {
       for (let i = 0; i < results.local.length; ++i) {
         let term = results.local[i];
         let dupIndex = results.remote.indexOf(term);
         if (dupIndex != -1) {
           results.remote.splice(dupIndex, 1);
         }
       }
--- a/toolkit/components/search/nsSearchSuggestions.js
+++ b/toolkit/components/search/nsSearchSuggestions.js
@@ -20,16 +20,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
  */
 function SuggestAutoComplete() {
   this._init();
 }
 SuggestAutoComplete.prototype = {
 
   _init: function() {
     this._suggestionController = new SearchSuggestionController(obj => this.onResultsReturned(obj));
+    this._suggestionController.maxLocalResults = this._historyLimit;
   },
 
   get _suggestionLabel() {
     delete this._suggestionLabel;
     let bundle = Services.strings.createBundle("chrome://global/locale/search/search.properties");
     return this._suggestionLabel = bundle.GetStringFromName("suggestion_label");
   },
 
@@ -52,18 +53,17 @@ SuggestAutoComplete.prototype = {
    * Callback for handling results from SearchSuggestionController.jsm
    * @private
    */
   onResultsReturned: function(results) {
     let finalResults = [];
     let finalComments = [];
 
     // If form history has results, add them to the list.
-    let maxHistoryItems = Math.min(results.local.length, this._historyLimit);
-    for (let i = 0; i < maxHistoryItems; ++i) {
+    for (let i = 0; i < results.local.length; ++i) {
       finalResults.push(results.local[i]);
       finalComments.push("");
     }
 
     // If there are remote matches, add them.
     if (results.remote.length) {
       // "comments" column values for suggestions starts as empty strings
       let comments = new Array(results.remote.length).fill("", 1);
--- a/toolkit/components/search/tests/xpcshell/test_searchSuggest.js
+++ b/toolkit/components/search/tests/xpcshell/test_searchSuggest.js
@@ -144,17 +144,16 @@ add_task(function* simple_non_ascii() {
   let result = yield controller.fetch("I ❤️", false, getEngine);
   do_check_eq(result.term, "I ❤️");
   do_check_eq(result.local.length, 1);
   do_check_eq(result.local[0], "I ❤️ XUL");
   do_check_eq(result.remote.length, 1);
   do_check_eq(result.remote[0], "I ❤️ Mozilla");
 });
 
-
 add_task(function* both_local_remote_result_dedupe() {
   yield updateSearchHistory("bump", "Mozilla");
 
   let controller = new SearchSuggestionController();
   let result = yield controller.fetch("mo", false, getEngine);
   do_check_eq(result.term, "mo");
   do_check_eq(result.local.length, 1);
   do_check_eq(result.local[0], "Mozilla");
@@ -264,16 +263,46 @@ add_task(function* both_identical_with_m
   }
   do_check_eq(result.remote.length, 10);
   for (let i = 0; i < controller.maxRemoteResults; i++) {
     do_check_eq(result.remote[i],
                 "letter " + String.fromCharCode("A".charCodeAt() + controller.maxLocalResults + i));
   }
 });
 
+add_task(function* noremote_maxLocal() {
+  let controller = new SearchSuggestionController();
+  controller.maxLocalResults = 2; // (should be ignored because no remote results)
+  controller.maxRemoteResults = 0;
+  let result = yield controller.fetch("letter ", false, getEngine);
+  do_check_eq(result.term, "letter ");
+  do_check_eq(result.local.length, 26);
+  for (let i = 0; i < result.local.length; i++) {
+    do_check_eq(result.local[i], "letter " + String.fromCharCode("A".charCodeAt() + i));
+  }
+  do_check_eq(result.remote.length, 0);
+});
+
+add_task(function* someremote_maxLocal() {
+  let controller = new SearchSuggestionController();
+  controller.maxLocalResults = 2;
+  controller.maxRemoteResults = 2;
+  let result = yield controller.fetch("letter ", false, getEngine);
+  do_check_eq(result.term, "letter ");
+  do_check_eq(result.local.length, 2);
+  for (let i = 0; i < result.local.length; i++) {
+    do_check_eq(result.local[i], "letter " + String.fromCharCode("A".charCodeAt() + i));
+  }
+  do_check_eq(result.remote.length, 2);
+  // "A" and "B" will have been de-duped, start at C for remote results
+  for (let i = 0; i < result.remote.length; i++) {
+    do_check_eq(result.remote[i], "letter " + String.fromCharCode("C".charCodeAt() + i));
+  }
+});
+
 add_task(function* one_of_each() {
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 1;
   controller.maxRemoteResults = 1;
   let result = yield controller.fetch("letter ", false, getEngine);
   do_check_eq(result.term, "letter ");
   do_check_eq(result.local.length, 1);
   do_check_eq(result.local[0], "letter A");
@@ -283,31 +312,35 @@ add_task(function* one_of_each() {
 
 add_task(function* local_result_returned_remote_result_disabled() {
   Services.prefs.setBoolPref("browser.search.suggest.enabled", false);
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 1;
   controller.maxRemoteResults = 1;
   let result = yield controller.fetch("letter ", false, getEngine);
   do_check_eq(result.term, "letter ");
-  do_check_eq(result.local.length, 1);
-  do_check_eq(result.local[0], "letter A");
+  do_check_eq(result.local.length, 26);
+  for (let i = 0; i < 26; i++) {
+    do_check_eq(result.local[i], "letter " + String.fromCharCode("A".charCodeAt() + i));
+  }
   do_check_eq(result.remote.length, 0);
   Services.prefs.setBoolPref("browser.search.suggest.enabled", true);
 });
 
 add_task(function* local_result_returned_remote_result_disabled_after_creation_of_controller() {
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 1;
   controller.maxRemoteResults = 1;
   Services.prefs.setBoolPref("browser.search.suggest.enabled", false);
   let result = yield controller.fetch("letter ", false, getEngine);
   do_check_eq(result.term, "letter ");
-  do_check_eq(result.local.length, 1);
-  do_check_eq(result.local[0], "letter A");
+  do_check_eq(result.local.length, 26);
+  for (let i = 0; i < 26; i++) {
+    do_check_eq(result.local[i], "letter " + String.fromCharCode("A".charCodeAt() + i));
+  }
   do_check_eq(result.remote.length, 0);
   Services.prefs.setBoolPref("browser.search.suggest.enabled", true);
 });
 
 add_task(function* one_of_each_disabled_before_creation_enabled_after_creation_of_controller() {
   Services.prefs.setBoolPref("browser.search.suggest.enabled", false);
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 1;
@@ -326,18 +359,20 @@ add_task(function* reset_suggestions_pre
 });
 
 add_task(function* one_local_zero_remote() {
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 1;
   controller.maxRemoteResults = 0;
   let result = yield controller.fetch("letter ", false, getEngine);
   do_check_eq(result.term, "letter ");
-  do_check_eq(result.local.length, 1);
-  do_check_eq(result.local[0], "letter A");
+  do_check_eq(result.local.length, 26);
+  for (let i = 0; i < 26; i++) {
+    do_check_eq(result.local[i], "letter " + String.fromCharCode("A".charCodeAt() + i));
+  }
   do_check_eq(result.remote.length, 0);
 });
 
 add_task(function* zero_local_one_remote() {
   let controller = new SearchSuggestionController();
   controller.maxLocalResults = 0;
   controller.maxRemoteResults = 1;
   let result = yield controller.fetch("letter ", false, getEngine);