Bug 1449317 - Update the default string in the address bar. r=florian
☠☠ backed out by bf7ed4125f1a ☠ ☠
authorMark Banner <standard8@mozilla.com>
Mon, 09 Apr 2018 15:32:19 +0100
changeset 467661 82d5734d2fea03684d72ca080330309c469d1f89
parent 467660 32badf66ccff025a7bfc0c86aea418118da126aa
child 467662 a89554c69f614a0ca5f8e9868e4cb4cd15fde1ce
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersflorian
bugs1449317
milestone61.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 1449317 - Update the default string in the address bar. r=florian MozReview-Commit-ID: C00zxCTJmHY
browser/base/content/browser.js
browser/base/content/browser.xul
browser/base/content/test/urlbar/browser.ini
browser/base/content/test/urlbar/browser_urlbarPlaceholder.js
browser/locales/en-US/chrome/browser/browser.dtd
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1262,16 +1262,18 @@ var gBrowserInit = {
       }
       initBrowser.removeAttribute("blank");
     }
 
     gBrowser.updateBrowserRemoteness(initBrowser, isRemote, {
       remoteType, sameProcessAsFrameLoader
     });
 
+    BrowserSearch.initPlaceHolder();
+
     // Hack to ensure that the about:home favicon is loaded
     // instantaneously, to avoid flickering and improve perceived performance.
     this._callWithURIToLoad(uriToLoad => {
       if (uriToLoad == "about:home") {
         gBrowser.setIcon(gBrowser.selectedTab, "chrome://branding/content/icon32.png");
       } else if (uriToLoad == "about:privatebrowsing") {
         gBrowser.setIcon(gBrowser.selectedTab, "chrome://browser/skin/privatebrowsing/favicon.svg");
       }
@@ -1457,16 +1459,17 @@ var gBrowserInit = {
     // We do this before the session restore service gets initialized so we can
     // apply full zoom settings to tabs restored by the session restore service.
     FullZoom.init();
     PanelUI.init();
 
     UpdateUrlbarSearchSplitterState();
 
     BookmarkingUI.init();
+    BrowserSearch.delayedStartupInit();
     AutoShowBookmarksToolbar.init();
 
     Services.prefs.addObserver(gHomeButton.prefDomain, gHomeButton);
 
     var homeButton = document.getElementById("home-button");
     gHomeButton.updateTooltip(homeButton);
 
     let safeMode = document.getElementById("helpSafeMode");
@@ -3762,20 +3765,35 @@ const DOMEventHandler = {
     if (!tab)
       return;
 
     BrowserSearch.addEngine(aBrowser, aEngine, makeURI(aURL));
   },
 };
 
 const BrowserSearch = {
+  _searchInitComplete: false,
+
   init() {
     Services.obs.addObserver(this, "browser-search-engine-modified");
   },
 
+  delayedStartupInit() {
+    // Asynchronously initialize the search service if necessary, to get the
+    // current engine for working out the placeholder.
+    Services.search.init(rv => {
+      if (Components.isSuccessCode(rv)) {
+        // Delay the update for this until so that we don't change it while
+        // the user is looking at it / isn't expecting it.
+        this._updateURLBarPlaceholder(Services.search.currentEngine, true);
+        this._searchInitComplete = true;
+      }
+    });
+  },
+
   uninit() {
     Services.obs.removeObserver(this, "browser-search-engine-modified");
   },
 
   observe(engine, topic, data) {
     // There are two kinds of search engine objects, nsISearchEngine objects and
     // plain { uri, title, icon } objects.  `engine` in this method is the
     // former.  The browser.engines and browser.hiddenEngines arrays are the
@@ -3792,16 +3810,21 @@ const BrowserSearch = {
       this._addMaybeOfferedEngine(engineName);
       break;
     case "engine-added":
       // An engine was added to the search service.  If a page is offering the
       // engine, then the engine needs to be removed from the corresponding
       // browser's offered engines.
       this._removeMaybeOfferedEngine(engineName);
       break;
+    case "engine-current":
+      if (this._searchInitComplete) {
+        this._updateURLBarPlaceholder(engine);
+      }
+      break;
     }
   },
 
   _addMaybeOfferedEngine(engineName) {
     let selectedBrowserOffersEngine = false;
     for (let browser of gBrowser.browsers) {
       for (let i = 0; i < (browser.hiddenEngines || []).length; i++) {
         if (browser.hiddenEngines[i].title == engineName) {
@@ -3839,16 +3862,98 @@ const BrowserSearch = {
         }
       }
     }
     if (selectedBrowserOffersEngine) {
       this.updateOpenSearchBadge();
     }
   },
 
+  /**
+   * Initializes the urlbar placeholder to the pre-saved engine name. We do this
+   * via a preference, to avoid needing to synchronously init the search service.
+   *
+   * This should be called around the time of DOMContentLoaded, so that it is
+   * initialized quickly before the user sees anything.
+   *
+   * Note: If the preference doesn't exist, we don't do anything as the default
+   * placeholder is a string which doesn't have the engine name.
+   */
+  initPlaceHolder() {
+    let engineName = Services.prefs.getStringPref("browser.urlbar.placeholderName", "");
+    if (engineName) {
+      // We can do this directly, since we know we're at DOMContentLoaded.
+      this._setURLBarPlaceholder(engineName);
+    }
+  },
+
+  /**
+   * Updates the URLBar placeholder for the specified engine, delaying the
+   * update if required. This also saves the current engine name in preferences
+   * for the next restart.
+   *
+   * Note: The engine name will only be displayed for built-in engines, as we
+   * know they should have short names.
+   *
+   * @param {nsISearchEngine} engine The search engine to use for the update.
+   * @param {Boolean} delayUpdate    Set to true, to delay update until the
+   *                                 placeholder is not displayed.
+   */
+  _updateURLBarPlaceholder(engine, delayUpdate = false) {
+    if (!engine) {
+      throw new Error("Expected an engine to be specified");
+    }
+
+    let engineName = "";
+    if (Services.search.getDefaultEngines().includes(engine)) {
+      engineName = engine.name;
+      Services.prefs.setStringPref("browser.urlbar.placeholderName", engineName);
+    } else {
+      Services.prefs.clearUserPref("browser.urlbar.placeholderName");
+    }
+
+    // Only delay if requested, and we're not displaying text in the URL bar
+    // currently.
+    if (delayUpdate && !gURLBar.value) {
+      // Delays changing the URL Bar placeholder until the user is not going to be
+      // seeing it, e.g. when there is a value entered in the bar, or if there is
+      // a tab switch to a tab which has a url loaded.
+      let placeholderUpdateListener = () => {
+        if (gURLBar.value) {
+          this._setURLBarPlaceholder(engineName);
+          gURLBar.removeEventListener("input", placeholderUpdateListener);
+          gBrowser.tabContainer.removeEventListener("TabSelect", placeholderUpdateListener);
+        }
+      };
+
+      gURLBar.addEventListener("input", placeholderUpdateListener);
+      gBrowser.tabContainer.addEventListener("TabSelect", placeholderUpdateListener);
+    } else {
+      this._setURLBarPlaceholder(engineName);
+    }
+  },
+
+  /**
+   * Sets the URLBar placeholder to either something based on the engine name,
+   * or the default placeholder.
+   *
+   * @param {String} name The name of the engine to use, an empty string if to
+   *                      use the default placeholder.
+   */
+  _setURLBarPlaceholder(name) {
+    let placeholder;
+    if (name) {
+      placeholder = gBrowserBundle.formatStringFromName("urlbar.placeholder",
+        [name], 1);
+    } else {
+      placeholder = gURLBar.getAttribute("defaultPlaceholder");
+    }
+    gURLBar.setAttribute("placeholder", placeholder);
+  },
+
   addEngine(browser, engine, uri) {
     // Check to see whether we've already added an engine with this title
     if (browser.engines) {
       if (browser.engines.some(e => e.title == engine.title))
         return;
     }
 
     var hidden = false;
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -772,16 +772,17 @@
                        cui-areatype="toolbar"
                        aboutHomeOverrideTooltip="&abouthome.pageTitle;"/>
         <toolbarspring cui-areatype="toolbar" class="chromeclass-toolbar-additional"/>
         <toolbaritem id="urlbar-container" flex="400" persist="width"
                      removable="false"
                      class="chromeclass-location" overflows="false">
             <textbox id="urlbar" flex="1"
                      placeholder="&urlbar.placeholder2;"
+                     defaultPlaceholder="&urlbar.placeholder2;"
                      focused="true"
                      type="autocomplete"
                      autocompletesearch="unifiedcomplete"
                      autocompletesearchparam="enable-actions"
                      autocompletepopup="PopupAutoCompleteRichResult"
                      completeselectedindex="true"
                      shrinkdelay="250"
                      tabscrolling="true"
--- a/browser/base/content/test/urlbar/browser.ini
+++ b/browser/base/content/test/urlbar/browser.ini
@@ -90,16 +90,20 @@ uses-unsafe-cpows = true
 [browser_urlbarOneOffs.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
 [browser_urlbarOneOffs_searchSuggestions.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
+[browser_urlbarPlaceholder.js]
+support-files =
+  searchSuggestionEngine.xml
+  searchSuggestionEngine.sjs
 [browser_urlbarPrivateBrowsingWindowChange.js]
 [browser_urlbarRaceWithTabs.js]
 [browser_urlbarRevert.js]
 [browser_urlbarSearchSingleWordNotification.js]
 [browser_urlbarSearchSuggestions.js]
 support-files =
   searchSuggestionEngine.xml
   searchSuggestionEngine.sjs
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/urlbar/browser_urlbarPlaceholder.js
@@ -0,0 +1,93 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * This test ensures the placeholder is set correctly for different search
+ * engines.
+ */
+
+"use strict";
+
+const TEST_ENGINE_BASENAME = "searchSuggestionEngine.xml";
+
+const originalEngine = Services.search.currentEngine;
+const expectedString = gBrowserBundle.formatStringFromName("urlbar.placeholder",
+  [originalEngine.name], 1);
+var extraEngine;
+var tabs = [];
+
+add_task(async function setup() {
+  extraEngine = await promiseNewSearchEngine(TEST_ENGINE_BASENAME);
+
+  // Force display of a tab with a URL bar, to clear out any possible placeholder
+  // initialization listeners that happen on startup.
+  let urlTab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:mozilla");
+  BrowserTestUtils.removeTab(urlTab);
+
+  registerCleanupFunction(() => {
+    Services.search.currentEngine = originalEngine;
+    for (let tab of tabs) {
+      BrowserTestUtils.removeTab(tab);
+    }
+  });
+});
+
+add_task(async function test_change_default_engine_updates_placeholder() {
+  tabs.push(await BrowserTestUtils.openNewForegroundTab(gBrowser));
+
+  Services.search.currentEngine = extraEngine;
+
+  await TestUtils.waitForCondition(
+    () => gURLBar.getAttribute("placeholder") == gURLBar.getAttribute("defaultPlaceholder"),
+    "The placeholder should match the default placeholder for non-built-in engines.");
+
+  Services.search.currentEngine = originalEngine;
+
+  await TestUtils.waitForCondition(
+    () => gURLBar.getAttribute("placeholder") == expectedString,
+    "The placeholder should include the engine name for built-in engines.");
+});
+
+add_task(async function test_delayed_update_placeholder() {
+  // Since we can't easily test for startup changes, we'll at least test the delay
+  // of update for the placeholder works.
+  let urlTab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:mozilla");
+  tabs.push(urlTab);
+
+  // Open a tab with a blank URL bar.
+  let blankTab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
+  tabs.push(blankTab);
+
+  // Pretend we've "initialized".
+  BrowserSearch._updateURLBarPlaceholder(extraEngine, true);
+
+  Assert.equal(gURLBar.getAttribute("placeholder"), expectedString,
+    "Placeholder should be unchanged.");
+
+  // Now switch to a tab with something in the URL Bar.
+  await BrowserTestUtils.switchTab(gBrowser, urlTab);
+
+  await TestUtils.waitForCondition(
+    () => gURLBar.getAttribute("placeholder") == gURLBar.getAttribute("defaultPlaceholder"),
+    "The placeholder should have updated in the background.");
+
+  // Do it the other way to check both named engine and fallback code paths.
+  await BrowserTestUtils.switchTab(gBrowser, blankTab);
+
+  BrowserSearch._updateURLBarPlaceholder(originalEngine, true);
+
+  Assert.equal(gURLBar.getAttribute("placeholder"), gURLBar.getAttribute("defaultPlaceholder"),
+    "Placeholder should be unchanged.");
+
+  await BrowserTestUtils.switchTab(gBrowser, urlTab);
+
+  await TestUtils.waitForCondition(
+    () => gURLBar.getAttribute("placeholder") == expectedString,
+    "The placeholder should include the engine name for built-in engines.");
+
+  // Now check when we have a URL displayed, the placeholder is updated straight away.
+  BrowserSearch._updateURLBarPlaceholder(extraEngine);
+
+  Assert.equal(gURLBar.getAttribute("placeholder"), gURLBar.getAttribute("defaultPlaceholder"),
+    "Placeholder should be the default.");
+});
--- a/browser/locales/en-US/chrome/browser/browser.dtd
+++ b/browser/locales/en-US/chrome/browser/browser.dtd
@@ -429,17 +429,16 @@ These should match what Safari and other
 
 <!-- LOCALIZATION NOTE (moreMenu.label) This label is used in the new Photon
     app (hamburger) menu. When clicked, it opens a subview that contains
     secondary commands. -->
 <!ENTITY moreMenu.label "More">
 
 <!ENTITY openCmd.commandkey           "l">
 <!ENTITY urlbar.placeholder2          "Search or enter address">
-<!ENTITY urlbar.placeholder3          "Enter search terms and addresses here">
 <!ENTITY urlbar.accesskey             "d">
 <!-- LOCALIZATION NOTE (urlbar.extension.label): Used to indicate that a selected autocomplete entry is provided by an extension. -->
 <!ENTITY urlbar.extension.label       "Extension:">
 <!ENTITY urlbar.switchToTab.label     "Switch to tab:">
 
 <!-- LOCALIZATION NOTE (urlbar.searchSuggestionsNotification.hintPrefix): Shown just before the suggestions opt-out hint. -->
 <!ENTITY urlbar.searchSuggestionsNotification.hintPrefix "Tip:">
 <!-- LOCALIZATION NOTE (urlbar.searchSuggestionsNotification.hint): &#x1F50E; is the magnifier icon emoji, please don't change it. -->