Bug 1378035 - Allow the urlbar prefs to define different buckets for different contexts. r=Paolo
authorMarco Bonardo <mbonardo@mozilla.com>
Fri, 21 Jul 2017 15:01:56 +0200
changeset 419268 01e0c8b8e58176a94c6a66375c485caa5411d085
parent 419267 a5869e4529ac8d87845583c760215fa6568e9da9
child 419269 e4d183f4d9bab9b7073f420e4b4b1333569f22cd
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersPaolo
bugs1378035
milestone56.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 1378035 - Allow the urlbar prefs to define different buckets for different contexts. r=Paolo MozReview-Commit-ID: 2O58OkhFygl
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/test_search_suggestions.js
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -41,16 +41,18 @@ const PREF_SUGGEST_SEARCHES =       [ "s
 
 const PREF_MAX_CHARS_FOR_SUGGEST =  [ "maxCharsForSearchSuggestions", 20];
 const PREF_MAX_HISTORICAL_SUGGESTIONS =  [ "maxHistoricalSearchSuggestions", 0];
 
 const PREF_PRELOADED_SITES_ENABLED =  [ "usepreloadedtopurls.enabled",   true ];
 const PREF_PRELOADED_SITES_EXPIRE_DAYS = [ "usepreloadedtopurls.expire_days",  14 ];
 
 const PREF_MATCH_BUCKETS = [ "matchBuckets", "general:5,suggestion:Infinity" ];
+// Will default to matchBuckets if not defined.
+const PREF_MATCH_BUCKETS_SEARCH = [ "matchBucketsSearch", "" ];
 
 // AutoComplete query type constants.
 // Describes the various types of queries that we can process rows for.
 const QUERYTYPE_FILTERED            = 0;
 const QUERYTYPE_AUTOFILL_HOST       = 1;
 const QUERYTYPE_AUTOFILL_URL        = 2;
 
 // This separator is used as an RTL-friendly way to split the title and tags.
@@ -119,16 +121,18 @@ const MATCHTYPE = {
 };
 
 // Buckets for match insertion.
 // Every time a new match is returned, we go through each bucket in array order,
 // and look for the first one having available space for the given match type.
 // Each bucket is an array containing the following indices:
 //   0: The match type of the acceptable entries.
 //   1: available number of slots in this bucket.
+// There are different matchBuckets definition for different contexts, currently
+// a general one (_matchBuckets) and a search one (_matchBucketsSearch).
 //
 // First buckets. Anything with an Infinity frecency ends up here.
 const DEFAULT_BUCKETS_BEFORE = [
   [MATCHTYPE.HEURISTIC, 1],
   [MATCHTYPE.EXTENSION, MAXIMUM_ALLOWED_EXTENSION_MATCHES - 1],
 ];
 // => USER DEFINED BUCKETS WILL BE INSERTED HERE <=
 //
@@ -503,26 +507,40 @@ XPCOMUtils.defineLazyGetter(this, "Prefs
     store.suggestOpenpage = prefs.get(...PREF_SUGGEST_OPENPAGE);
     store.suggestTyped = prefs.get(...PREF_SUGGEST_HISTORY_ONLYTYPED);
     store.suggestSearches = prefs.get(...PREF_SUGGEST_SEARCHES);
     store.maxCharsForSearchSuggestions = prefs.get(...PREF_MAX_CHARS_FOR_SUGGEST);
     store.maxHistoricalSearchSuggestions = prefs.get(...PREF_MAX_HISTORICAL_SUGGESTIONS);
     store.preloadedSitesEnabled = prefs.get(...PREF_PRELOADED_SITES_ENABLED);
     store.preloadedSitesExpireDays = prefs.get(...PREF_PRELOADED_SITES_EXPIRE_DAYS);
     store.matchBuckets = prefs.get(...PREF_MATCH_BUCKETS);
-    // Convert from the string format to an array.
+    // Convert from pref char format to an array and add the default buckets.
     try {
-      store.matchBuckets = convertBucketsCharPrefToArray(store.matchBuckets)
+      store.matchBuckets = convertBucketsCharPrefToArray(store.matchBuckets);
     } catch (ex) {
       store.matchBuckets = convertBucketsCharPrefToArray(PREF_MATCH_BUCKETS[1]);
     }
-    // Add the default buckets.
     store.matchBuckets = [ ...DEFAULT_BUCKETS_BEFORE,
                            ...store.matchBuckets,
                            ...DEFAULT_BUCKETS_AFTER ];
+    store.matchBucketsSearch = prefs.get(...PREF_MATCH_BUCKETS_SEARCH);
+    // Default to matchBuckets if not defined.
+    if (!store.matchBucketsSearch) {
+      store.matchBucketsSearch = store.matchBuckets;
+    } else {
+      // Convert from pref char format to an array and add the default buckets.
+      try {
+        store.matchBucketsSearch = convertBucketsCharPrefToArray(store.matchBucketsSearch);
+        store.matchBucketsSearch = [ ...DEFAULT_BUCKETS_BEFORE,
+                                     ...store.matchBucketsSearch,
+                                     ...DEFAULT_BUCKETS_AFTER ];
+      } catch (ex) {
+        store.matchBucketsSearch = store.matchBuckets;
+      }
+    }
     store.keywordEnabled = Services.prefs.getBoolPref("keyword.enabled", true);
 
     // If history is not set, onlyTyped value should be ignored.
     if (!store.suggestHistory) {
       store.suggestTyped = false;
     }
     store.defaultBehavior = [...types, "Typed"].reduce((memo, type) => {
       let prefValue = store["suggest" + type];
@@ -810,24 +828,16 @@ function Search(searchString, searchPara
 
   // These are used to avoid adding duplicate entries to the results.
   this._usedURLs = new Set();
   this._usedPlaceIds = new Set();
 
   // Resolved when all the matches providers have been fetched.
   this._allMatchesPromises = [];
 
-  // Convert the buckets to readable objects with a count property.
-  this._buckets = Prefs.matchBuckets
-                       .map(([type, available]) => ({
-                         type,
-                         available,
-                         count: 0
-                       }));
-
   // Counters for the number of matches per MATCHTYPE.
   this._counts = Object.values(MATCHTYPE)
                        .reduce((o, p) => { o[p] = 0; return o; }, {});
 
   this._searchStringHasWWW = this._strippedPrefix.endsWith("www.");
   this._searchStringWWW = this._searchStringHasWWW ? "www." : "";
   this._searchStringFromWWW = this._searchStringWWW + this._searchString;
 
@@ -1819,16 +1829,27 @@ Search.prototype = {
     if (this._result.matchCount == 6)
       TelemetryStopwatch.finish(TELEMETRY_6_FIRST_RESULTS, this);
 
     this.notifyResults(true);
   },
 
   _getInsertIndexForMatch(match) {
     let index = 0;
+    // The buckets change depending on the context, that is currently decided by
+    // the first added match (the heuristic one).
+    if (!this._buckets) {
+      // Convert the buckets to readable objects with a count property.
+      let buckets = match.style.includes("searchengine") ? Prefs.matchBucketsSearch
+                                                         : Prefs.matchBuckets;
+      this._buckets = buckets.map(([type, available]) => ({ type,
+                                                            available,
+                                                            count: 0,
+                                                          }));
+    }
     for (let bucket of this._buckets) {
       // Move to the next bucket if the match type is incompatible, or if there
       // is no available space or if the frecency is below the threshold.
       if (match.type != bucket.type || !bucket.available) {
         index += bucket.count;
         continue;
       }
 
--- a/toolkit/components/places/tests/unifiedcomplete/test_search_suggestions.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_search_suggestions.js
@@ -438,17 +438,17 @@ add_task(async function mixup_frecency()
         title: "low frecency 2" },
       { uri: NetUtil.newURI("http://example.com/lo1"),
         title: "low frecency 1" },
       { uri: NetUtil.newURI("http://example.com/lo0"),
         title: "low frecency 0" },
     ],
   });
 
-  // Change the results mixup.
+  // Change the "general" context mixup.
   Services.prefs.setCharPref("browser.urlbar.matchBuckets",
                              "suggestion:1,general:5,suggestion:1");
 
   // Do an unrestricted search to make sure everything appears in it, including
   // the visit and bookmark.
   await check_autocomplete({
     checkSorting: true,
     search: "frecency",
@@ -497,17 +497,75 @@ add_task(async function mixup_frecency()
         title: "low frecency 2" },
       { uri: NetUtil.newURI("http://example.com/lo1"),
         title: "low frecency 1" },
       { uri: NetUtil.newURI("http://example.com/lo0"),
         title: "low frecency 0" },
     ],
   });
 
+  // Change the "search" context mixup.
+  Services.prefs.setCharPref("browser.urlbar.matchBucketsSearch",
+                             "suggestion:2,general:4");
+
+  await check_autocomplete({
+    checkSorting: true,
+    search: "frecency",
+    searchParam: "enable-actions",
+    matches: [
+      makeSearchMatch("frecency", { engineName: ENGINE_NAME, heuristic: true }),
+      {
+        uri: makeActionURI(("searchengine"), {
+          engineName: ENGINE_NAME,
+          input: "frecency foo",
+          searchQuery: "frecency",
+          searchSuggestion: "frecency foo",
+        }),
+        title: ENGINE_NAME,
+        style: ["action", "searchengine"],
+        icon: "",
+      },
+      {
+        uri: makeActionURI(("searchengine"), {
+          engineName: ENGINE_NAME,
+          input: "frecency bar",
+          searchQuery: "frecency",
+          searchSuggestion: "frecency bar",
+        }),
+        title: ENGINE_NAME,
+        style: ["action", "searchengine"],
+        icon: "",
+      },
+      { uri: NetUtil.newURI("http://example.com/hi3"),
+        title: "high frecency 3",
+        style: [ "bookmark" ] },
+      { uri: NetUtil.newURI("http://example.com/hi2"),
+        title: "high frecency 2",
+        style: [ "bookmark" ] },
+      { uri: NetUtil.newURI("http://example.com/hi1"),
+        title: "high frecency 1",
+        style: [ "bookmark" ] },
+      { uri: NetUtil.newURI("http://example.com/hi0"),
+        title: "high frecency 0",
+        style: [ "bookmark" ] },
+      { uri: NetUtil.newURI("http://example.com/lo4"),
+        title: "low frecency 4" },
+      { uri: NetUtil.newURI("http://example.com/lo3"),
+        title: "low frecency 3" },
+      { uri: NetUtil.newURI("http://example.com/lo2"),
+        title: "low frecency 2" },
+      { uri: NetUtil.newURI("http://example.com/lo1"),
+        title: "low frecency 1" },
+      { uri: NetUtil.newURI("http://example.com/lo0"),
+        title: "low frecency 0" },
+    ],
+  });
+
   Services.prefs.clearUserPref("browser.urlbar.matchBuckets");
+  Services.prefs.clearUserPref("browser.urlbar.matchBucketsSearch");
   await cleanUpSuggestions();
 });
 
 add_task(async function prohibit_suggestions() {
   Services.prefs.setBoolPref(SUGGEST_PREF, true);
 
   await check_autocomplete({
     search: "localhost",