Bug 1426216 - Allow users to choose whether search suggestions or history suggestions come first in the address bar. r=mak,past
authorDrew Willcoxon <adw@mozilla.com>
Wed, 10 Jan 2018 08:41:16 -0800
changeset 452854 440dbdb51f9405b61db8647799a00f4f6335c09a
parent 452853 be8ebea8beb1e445a3382b6b071401f78de6e622
child 452855 49462334e363e8b248396b954be72bffe40c5bd0
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, past
bugs1426216
milestone59.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 1426216 - Allow users to choose whether search suggestions or history suggestions come first in the address bar. r=mak,past 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;
@@ -1766,17 +1769,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);
@@ -2251,16 +2254,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;
@@ -2332,16 +2341,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.