Bug 1426216 - Allow users to choose whether search suggestions or history suggestions come first in the address bar. r?mak draft
authorDrew Willcoxon <adw@mozilla.com>
Wed, 10 Jan 2018 08:41:16 -0800
changeset 718591 387d2d0b8611be758dcbf0eab2912b3c7ee7bddf
parent 717183 ca379fcca95b1f4a3744242ea8647004b99b3507
child 745547 32c953be7eb83a494b9c0fe630914121f6e8d38f
push id94993
push userbmo:adw@mozilla.com
push dateWed, 10 Jan 2018 16:41:47 +0000
reviewersmak
bugs1426216
milestone59.0a1
Bug 1426216 - Allow users to choose whether search suggestions or history suggestions come first in the address bar. r?mak MozReview-Commit-ID: HVWRyEu19Dv
browser/components/nsBrowserGlue.js
browser/components/preferences/in-content/search.js
browser/components/preferences/in-content/search.xul
browser/components/preferences/in-content/tests/browser.ini
browser/components/preferences/in-content/tests/browser_searchShowSuggestionsFirst.js
browser/components/tests/browser/browser.ini
browser/components/tests/browser/browser_urlbar_matchBuckets_migration59.js
browser/themes/shared/incontentprefs/search.css
toolkit/components/places/UnifiedComplete.js
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -27,16 +27,17 @@ XPCOMUtils.defineLazyModuleGetters(this,
   AsyncShutdown: "resource://gre/modules/AsyncShutdown.jsm",
   AutoCompletePopup: "resource://gre/modules/AutoCompletePopup.jsm",
   BookmarkHTMLUtils: "resource://gre/modules/BookmarkHTMLUtils.jsm",
   BookmarkJSONUtils: "resource://gre/modules/BookmarkJSONUtils.jsm",
   BrowserUITelemetry: "resource:///modules/BrowserUITelemetry.jsm",
   BrowserUsageTelemetry: "resource:///modules/BrowserUsageTelemetry.jsm",
   ContentClick: "resource:///modules/ContentClick.jsm",
   ContextualIdentityService: "resource://gre/modules/ContextualIdentityService.jsm",
+  CustomizableUI: "resource:///modules/CustomizableUI.jsm",
   DateTimePickerHelper: "resource://gre/modules/DateTimePickerHelper.jsm",
   DirectoryLinksProvider: "resource:///modules/DirectoryLinksProvider.jsm",
   ExtensionsUI: "resource:///modules/ExtensionsUI.jsm",
   Feeds: "resource:///modules/Feeds.jsm",
   FileUtils: "resource://gre/modules/FileUtils.jsm",
   FileSource: "resource://gre/modules/L10nRegistry.jsm",
   FormValidationHandler: "resource:///modules/FormValidationHandler.jsm",
   HybridContentTelemetry: "resource://gre/modules/HybridContentTelemetry.jsm",
@@ -433,16 +434,18 @@ BrowserGlue.prototype = {
         } else if (data == "mock-alerts-service") {
           Object.defineProperty(this, "AlertsService", {
             value: subject.wrappedJSObject
           });
         } else if (data == "places-browser-init-complete") {
           if (this._placesBrowserInitComplete) {
             Services.obs.notifyObservers(null, "places-browser-init-complete");
           }
+        } else if (data == "migrateMatchBucketsPrefForUIVersion60") {
+          this._migrateMatchBucketsPrefForUIVersion60();
         }
         break;
       case "initial-migration-will-import-default-bookmarks":
         this._migrationImportsDefaultBookmarks = true;
         break;
       case "initial-migration-did-import-default-bookmarks":
         this._initPlaces(true);
         break;
@@ -1771,17 +1774,17 @@ BrowserGlue.prototype = {
       if (toolbarIsCustomized || getToolbarFolderCount() > NUM_TOOLBAR_BOOKMARKS_TO_UNHIDE) {
         xulStore.setValue(BROWSER_DOCURL, "PersonalToolbar", "collapsed", "false");
       }
     }
   },
 
   // eslint-disable-next-line complexity
   _migrateUI: function BG__migrateUI() {
-    const UI_VERSION = 59;
+    const UI_VERSION = 60;
     const BROWSER_DOCURL = "chrome://browser/content/browser.xul";
 
     let currentUIVersion;
     if (Services.prefs.prefHasUserValue("browser.migration.version")) {
       currentUIVersion = Services.prefs.getIntPref("browser.migration.version");
     } else {
       // This is a new profile, nothing to migrate.
       Services.prefs.setIntPref("browser.migration.version", UI_VERSION);
@@ -2256,16 +2259,22 @@ BrowserGlue.prototype = {
             } catch (e) { /* Don't panic if the value is not a valid locale code. */ }
           }
         }
         Services.prefs.clearUserPref(SELECTED_LOCALE_PREF);
         Services.prefs.clearUserPref(MATCHOS_LOCALE_PREF);
       }
     }
 
+    if (currentUIVersion < 60) {
+      // Set whether search suggestions or history results come first in the
+      // urlbar results.
+      this._migrateMatchBucketsPrefForUIVersion60();
+    }
+
     // Update the migration version.
     Services.prefs.setIntPref("browser.migration.version", UI_VERSION);
   },
 
   _checkForDefaultBrowser() {
     // Perform default browser checking.
     if (!ShellService) {
       return;
@@ -2337,16 +2346,36 @@ BrowserGlue.prototype = {
                         .add(promptCount);
     } catch (ex) { /* Don't break the default prompt if telemetry is broken. */ }
 
     if (willPrompt) {
       DefaultBrowserCheck.prompt(RecentWindow.getMostRecentBrowserWindow());
     }
   },
 
+  _migrateMatchBucketsPrefForUIVersion60() {
+    let prefName = "browser.urlbar.matchBuckets";
+    let pref = Services.prefs.getCharPref(prefName, "");
+    if (!pref) {
+      // Set the pref based on the search bar's current placement.  If it's
+      // placed (the urlbar and search bar are not unified), then set the pref
+      // (so that history results will come before search suggestions).  If it's
+      // not placed (the urlbar and search bar are unified), then leave the pref
+      // cleared so that UnifiedComplete.js uses the default value (so that
+      // search suggestions will come before history results).
+      if (CustomizableUI.getPlacementOfWidget("search-container")) {
+        Services.prefs.setCharPref(prefName, "general:5,suggestion:Infinity");
+      }
+    }
+    // Else, the pref has already been set.  Normally this pref does not exist.
+    // Either the user customized it, or they were enrolled in the Shield study
+    // in Firefox 57 that effectively already migrated the pref.  Either way,
+    // leave it at its current value.
+  },
+
   // ------------------------------
   // public nsIBrowserGlue members
   // ------------------------------
 
   sanitize: function BG_sanitize(aParentWindow) {
     this._sanitizer.sanitize(aParentWindow);
   },
 
--- a/browser/components/preferences/in-content/search.js
+++ b/browser/components/preferences/in-content/search.js
@@ -10,16 +10,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionSettingsStore",
                                   "resource://gre/modules/ExtensionSettingsStore.jsm");
 
 Preferences.addAll([
   { id: "browser.search.suggest.enabled", type: "bool" },
   { id: "browser.urlbar.suggest.searches", type: "bool" },
   { id: "browser.search.hiddenOneOffs", type: "unichar" },
   { id: "browser.search.widget.inNavBar", type: "bool" },
+  { id: "browser.urlbar.matchBuckets", type: "string" },
 ]);
 
 const ENGINE_FLAVOR = "text/x-moz-search-engine";
 const SEARCH_TYPE = "default_search";
 const SEARCH_KEY = "defaultSearch";
 
 var gEngineView = null;
 
@@ -55,16 +56,57 @@ var gSearchPane = {
       Services.obs.removeObserver(this, "browser-search-engine-modified");
     });
 
     this._initAutocomplete();
 
     let suggestsPref = Preferences.get("browser.search.suggest.enabled");
     suggestsPref.on("change", this.updateSuggestsCheckbox.bind(this));
     this.updateSuggestsCheckbox();
+
+    this._initShowSearchSuggestionsFirst();
+  },
+
+  _initShowSearchSuggestionsFirst() {
+    let pref = Preferences.get("browser.urlbar.matchBuckets");
+    let checkbox =
+      document.getElementById("showSearchSuggestionsFirstCheckbox");
+
+    pref.on("change", () => {
+      this.syncFromShowSearchSuggestionsFirstPref(checkbox, pref);
+    });
+    this._syncFromShowSearchSuggestionsFirstPref(checkbox, pref);
+
+    checkbox.addEventListener("command", () => {
+      this._syncToShowSearchSuggestionsFirstPref(checkbox.checked, pref);
+    });
+  },
+
+  _syncFromShowSearchSuggestionsFirstPref(checkbox, pref) {
+    if (!pref.value) {
+      // The pref is cleared, meaning search suggestions are shown first.
+      checkbox.checked = true;
+      return;
+    }
+    // The pref has a value.  If the first bucket in the pref is search
+    // suggestions, then check the checkbox.
+    let bucketPair = pref.value.split(",")[0];
+    let bucketName = bucketPair.split(":")[0];
+    checkbox.checked = bucketName == "suggestion";
+  },
+
+  _syncToShowSearchSuggestionsFirstPref(checked, pref) {
+    if (checked) {
+      // Show search suggestions first, so clear the pref since that's the
+      // default.
+      pref.reset();
+      return;
+    }
+    // Show history first.
+    pref.value = "general:5,suggestion:Infinity";
   },
 
   updateSuggestsCheckbox() {
     let suggestsPref = Preferences.get("browser.search.suggest.enabled");
     let permanentPB =
       Services.prefs.getBoolPref("browser.privatebrowsing.autostart");
     let urlbarSuggests = document.getElementById("urlBarSuggestion");
     urlbarSuggests.disabled = !suggestsPref.value || permanentPB;
--- a/browser/components/preferences/in-content/search.xul
+++ b/browser/components/preferences/in-content/search.xul
@@ -13,16 +13,18 @@
     <groupbox id="searchbarGroup" data-category="paneSearch">
       <caption><label id="searchbarLabel">&searchBar.label;</label></caption>
       <radiogroup id="searchBarVisibleGroup" aria-labelledby="searchbarLabel" preference="browser.search.widget.inNavBar">
         <radio id="searchBarHiddenRadio" value="false" label="&searchBar.hidden.label;"/>
         <image class="searchBarImage searchBarHiddenImage" role="presentation"/>
         <radio id="searchBarShownRadio" value="true" label="&searchBar.shown.label;"/>
         <image class="searchBarImage searchBarShownImage" role="presentation"/>
       </radiogroup>
+      <checkbox id="showSearchSuggestionsFirstCheckbox"
+                label="&showSearchSuggestionsAboveHistory.label;"/>
     </groupbox>
 
     <!-- Default Search Engine -->
     <groupbox id="defaultEngineGroup" data-category="paneSearch">
       <caption><label>&defaultSearchEngine.label;</label></caption>
       <description>&chooseYourDefaultSearchEngine2.label;</description>
 
       <hbox id="browserDefaultSearchExtensionContent" align="center" hidden="true">
--- a/browser/components/preferences/in-content/tests/browser.ini
+++ b/browser/components/preferences/in-content/tests/browser.ini
@@ -62,16 +62,17 @@ skip-if = e10s
 [browser_permissions_urlFieldHidden.js]
 [browser_proxy_backup.js]
 [browser_privacypane_1.js]
 [browser_privacypane_3.js]
 [browser_privacypane_4.js]
 [browser_privacypane_5.js]
 [browser_privacypane_8.js]
 [browser_sanitizeOnShutdown_prefLocked.js]
+[browser_searchShowSuggestionsFirst.js]
 [browser_searchsuggestions.js]
 [browser_security-1.js]
 [browser_security-2.js]
 [browser_siteData.js]
 [browser_siteData2.js]
 [browser_siteData3.js]
 [browser_spotlight.js]
 [browser_site_login_exceptions.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/preferences/in-content/tests/browser_searchShowSuggestionsFirst.js
@@ -0,0 +1,53 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const PREF_NAME = "browser.urlbar.matchBuckets";
+const HISTORY_FIRST_PREF_VALUE = "general:5,suggestion:Infinity";
+const CHECKBOX_ID = "showSearchSuggestionsFirstCheckbox";
+
+// Open preferences with search suggestions shown first (the default).
+add_task(async function openWithSearchSuggestionsShownFirst() {
+  // The pref should be cleared initially so that search suggestions are shown
+  // first (the default).
+  Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), "",
+               "Pref should be cleared initially");
+
+  // Open preferences.  The checkbox should be checked.
+  await openPreferencesViaOpenPreferencesAPI("search", { leaveOpen: true });
+  let doc = gBrowser.selectedBrowser.contentDocument;
+  let checkbox = doc.getElementById(CHECKBOX_ID);
+  Assert.equal(checkbox.checked, true, "Checkbox should be checked");
+
+  // Uncheck the checkbox.
+  checkbox.checked = false;
+  checkbox.doCommand();
+
+  // The pref should now be set so that history is shown first.
+  Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""),
+               HISTORY_FIRST_PREF_VALUE,
+               "Pref should now be set to show history first");
+
+  gBrowser.removeCurrentTab();
+});
+
+// Open preferences with history shown first.
+add_task(async function openWithHistoryShownFirst() {
+  // Set the pref to show history first.
+  Services.prefs.setCharPref(PREF_NAME, HISTORY_FIRST_PREF_VALUE);
+
+  // Open preferences.  The checkbox should be unchecked.
+  await openPreferencesViaOpenPreferencesAPI("search", { leaveOpen: true });
+  let doc = gBrowser.selectedBrowser.contentDocument;
+  let checkbox = doc.getElementById(CHECKBOX_ID);
+  Assert.equal(checkbox.checked, false, "Checkbox should be unchecked");
+
+  // Check the checkbox.
+  checkbox.checked = true;
+  checkbox.doCommand();
+
+  // The pref should now be cleared so that search suggestions are shown first.
+  Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), "",
+               "Pref should now be cleared to show search suggestions first");
+
+  gBrowser.removeCurrentTab();
+});
--- a/browser/components/tests/browser/browser.ini
+++ b/browser/components/tests/browser/browser.ini
@@ -1,7 +1,8 @@
 [DEFAULT]
 
 [browser_bug538331.js]
 skip-if = !updater
 reason = test depends on update channel
 [browser_contentpermissionprompt.js]
 [browser_default_bookmark_toolbar_visibility.js]
+[browser_urlbar_matchBuckets_migration59.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/tests/browser/browser_urlbar_matchBuckets_migration59.js
@@ -0,0 +1,100 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Makes sure the browser.urlbar.matchBuckets pref is set correctly starting in
+// Firefox 59 (nsBrowserGlue UI version 60).
+
+const SEARCHBAR_WIDGET_ID = "search-container";
+const PREF_NAME = "browser.urlbar.matchBuckets";
+const SEARCHBAR_PRESENT_PREF_VALUE = "general:5,suggestion:Infinity";
+
+add_task(async function test() {
+  // Initial checks.
+  Assert.equal(CustomizableUI.getPlacementOfWidget(SEARCHBAR_WIDGET_ID), null,
+               "Searchbar should not be placed initially");
+  Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), "",
+               "Pref should be cleared initially");
+
+  // Add the searchbar.
+  let widgetPromise = promiseWidget("onWidgetAdded");
+  CustomizableUI.addWidgetToArea(SEARCHBAR_WIDGET_ID,
+                                 CustomizableUI.AREA_NAVBAR);
+  info("Waiting for searchbar to be added");
+  await widgetPromise;
+
+  // Force nsBrowserGlue to attempt update the pref again via UI version
+  // migration.  It shouldn't actually though since the UI version has already
+  // been migrated.  If it erroneously does, then the matchBuckets pref will be
+  // set since the searchbar is now placed.
+  messageBrowserGlue("force-ui-migration");
+
+  // The pref should remain cleared since the migration already happened even
+  // though the searchbar is now present.
+  Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), "",
+               "Pref should remain cleared even though searchbar present");
+
+  // Force nsBrowserGlue to update the pref.
+  forceBrowserGlueUpdatePref();
+
+  // The pref should be set since the searchbar is present.
+  Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""),
+               SEARCHBAR_PRESENT_PREF_VALUE,
+               "Pref should be set to show history first");
+
+  // Set the pref to something custom.
+  let customValue = "test:Infinity";
+  Services.prefs.setCharPref(PREF_NAME, customValue);
+
+  // Force nsBrowserGlue to update the pref again.
+  forceBrowserGlueUpdatePref();
+
+  // The pref should remain the custom value.
+  Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), customValue,
+               "Pref should remain the custom value");
+
+  // Remove the searchbar.
+  widgetPromise = promiseWidget("onWidgetRemoved");
+  CustomizableUI.removeWidgetFromArea(SEARCHBAR_WIDGET_ID);
+  info("Waiting for searchbar to be removed");
+  await widgetPromise;
+
+  // Force nsBrowserGlue to update the pref again.
+  forceBrowserGlueUpdatePref();
+
+  // The pref should remain the custom value.
+  Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), customValue,
+               "Pref should remain the custom value");
+
+  // Clear the pref.
+  Services.prefs.clearUserPref(PREF_NAME);
+
+  // Force nsBrowserGlue to update the pref again.
+  forceBrowserGlueUpdatePref();
+
+  // The pref should remain cleared since the searchbar isn't placed.
+  Assert.equal(Services.prefs.getCharPref(PREF_NAME, ""), "",
+               "Pref should remain cleared");
+});
+
+function promiseWidget(observerName) {
+  return new Promise(resolve => {
+    let listener = {};
+    listener[observerName] = widgetID => {
+      if (widgetID == SEARCHBAR_WIDGET_ID) {
+        CustomizableUI.removeListener(listener);
+        executeSoon(resolve);
+      }
+    };
+    CustomizableUI.addListener(listener);
+  });
+}
+
+function messageBrowserGlue(msgName) {
+  Cc["@mozilla.org/browser/browserglue;1"]
+    .getService(Ci.nsIObserver)
+    .observe(null, "browser-glue-test", msgName);
+}
+
+function forceBrowserGlueUpdatePref() {
+  messageBrowserGlue("migrateMatchBucketsPrefForUIVersion60");
+}
--- a/browser/themes/shared/incontentprefs/search.css
+++ b/browser/themes/shared/incontentprefs/search.css
@@ -7,18 +7,19 @@
   width: 631px;
   margin-left: 33px;
 }
 
 .searchBarHiddenImage {
   list-style-image: url("chrome://browser/skin/preferences/in-content/no-search-bar.svg");
 }
 
-#searchBarShownRadio {
-  /* Allow a little visual space to separate the radio from the image above it. */
+#searchBarShownRadio,
+#showSearchSuggestionsFirstCheckbox {
+  /* A little space to separate these from the elements above them. */
   margin-top: 10px;
 }
 
 .searchBarShownImage  {
   list-style-image: url("chrome://browser/skin/preferences/in-content/search-bar.svg");
 }
 
 .searchBarImage:-moz-locale-dir(rtl) {
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -45,17 +45,17 @@ const PREF_URLBAR_DEFAULTS = new Map([
   ["suggest.bookmark", true],
   ["suggest.openpage", true],
   ["suggest.history.onlyTyped", false],
   ["suggest.searches", false],
   ["maxCharsForSearchSuggestions", 20],
   ["maxHistoricalSearchSuggestions", 0],
   ["usepreloadedtopurls.enabled", true],
   ["usepreloadedtopurls.expire_days", 14],
-  ["matchBuckets", "general:5,suggestion:Infinity"],
+  ["matchBuckets", "suggestion:4,general:Infinity"],
   ["matchBucketsSearch", ""],
   ["insertMethod", INSERTMETHOD.MERGE_RELATED],
 ]);
 const PREF_OTHER_DEFAULTS = new Map([
   ["keyword.enabled", true],
 ]);
 
 // AutoComplete query type constants.