Bug 1265881 - Location Bar Internet Search, Searchbar, and Sidebar Search panel do not respect "Edit => Preferences => Browser => Internet Search" r=IanN a=IanN
authorPhilip Chee <philip.chee@gmail.com>
Mon, 02 Jan 2017 16:57:15 +0100
changeset 26895 b8e96387b63dcbde13619b2d9d8cae3514ee02a7
parent 26894 117d52323f97b2e2acf992df6ee26b6c69922859
child 26896 f2b772feaa852739e21b3d6d2c945e3b0dbd6af3
push id1834
push userclokep@gmail.com
push dateMon, 23 Jan 2017 21:48:40 +0000
treeherdercomm-beta@293cffe83e59 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersIanN, IanN
bugs1265881
Bug 1265881 - Location Bar Internet Search, Searchbar, and Sidebar Search panel do not respect "Edit => Preferences => Browser => Internet Search" r=IanN a=IanN
suite/browser/urlbarBindings.xml
suite/common/pref/pref-search.js
suite/common/pref/pref-search.xul
suite/common/search/search-panel.js
suite/common/search/search.xml
suite/common/src/nsSuiteGlue.js
--- a/suite/browser/urlbarBindings.xml
+++ b/suite/browser/urlbarBindings.xml
@@ -345,23 +345,28 @@
         <xul:treechildren anonid="treebody" class="autocomplete-treebody" flex="1"/>
       </xul:tree>
       <xul:box role="search-box" class="autocomplete-search-box"/>
     </content>
 
     <implementation>
       <constructor><![CDATA[
         // listen for changes to default search engine
-        this.mPrefs.addObserver("browser.search", this.mPrefObserver, false);
-        this.mPrefs.addObserver("browser.urlbar", this.mPrefObserver, false);
+        this.mPrefs.addObserver("browser.search", this.mObserver, false);
+        this.mPrefs.addObserver("browser.urlbar", this.mObserver, false);
+        this.mObSvc.addObserver(this.mObserver,
+                                "browser-search-engine-modified", false);
+        this.updateEngines();
       ]]></constructor>
 
       <destructor><![CDATA[
-        this.mPrefs.removeObserver("browser.search", this.mPrefObserver);
-        this.mPrefs.removeObserver("browser.urlbar", this.mPrefObserver);
+        this.mPrefs.removeObserver("browser.search", this.mObserver);
+        this.mPrefs.removeObserver("browser.urlbar", this.mObserver);
+        this.mObSvc.removeObserver(this.mObserver,
+                                   "browser-search-engine-modified");
       ]]></destructor>
 
       <property name="showSearch" onget="return this.mShowSearch;">
         <setter><![CDATA[
           this.mShowSearch = val;
           if (val) {
             this.updateEngines();
             this.mSearchBox.removeAttribute("hidden");
@@ -376,30 +381,50 @@
                 onget="return this.textbox.getAttribute('defaultSearchEngine') == 'true';"
                 onset="this.textbox.setAttribute('defaultSearchEngine', val); return val;"/>
 
       <field name="mSearchBox">
          document.getAnonymousElementByAttribute(this, "role", "search-box");
       </field>
 
       <field name="mPrefs">
-        var svc = Components.classes["@mozilla.org/preferences-service;1"]
-                            .getService(Components.interfaces.nsIPrefService);
-        svc.getBranch(null);
+        Components.classes["@mozilla.org/preferences-service;1"]
+                  .getService(Components.interfaces.nsIPrefService)
+                  .getBranch(null);
       </field>
 
-      <field name="mPrefObserver"><![CDATA[
+      <field name="mObSvc">
+        Components.classes["@mozilla.org/observer-service;1"]
+                  .getService(Components.interfaces.nsIObserverService);
+      </field>
+
+      <field name="mSearchService">
+        Components.classes["@mozilla.org/browser/search-service;1"]
+                  .getService(Components.interfaces.nsIBrowserSearchService);
+      </field>
+
+      <field name="mObserver"><![CDATA[
         ({
           resultsPopup: this,
 
-          observe: function(aObserver, aBlah, aPref) {
-            if (/^browser\.search\./.test(aPref))
-              this.resultsPopup.updateEngines();
-            else if (/^browser\.urlbar\./.test(aPref))
-              this.resultsPopup.updatePref(aPref);
+          observe: function(aSubject, aTopic, aData) {
+            switch (aTopic) {
+              case "browser-search-engine-modified":
+                if (aData == "engine-current") {
+                  this.resultsPopup.updateEngines();
+                }
+                break;
+              case "nsPref:changed":
+                if (/^browser\.search\./.test(aData))
+                  this.resultsPopup.updateEngines();
+                else if (/^browser\.urlbar\./.test(aData))
+                  this.resultsPopup.updatePref(aData);
+                break;
+              default:
+            }
           }
         });
       ]]></field>
 
       <field name="mInputListener"><![CDATA[
         (function(aEvent) {
           // "this" is the textbox, not the popup
           if (this.mSearchInputTO)
@@ -454,17 +479,17 @@
         <body><![CDATA[
           while (this.mSearchBox.hasChildNodes())
             this.mSearchBox.lastChild.remove();
         ]]></body>
       </method>
 
       <method name="updateEngines">
         <body><![CDATA[
-          var defaultEngine = Services.search.defaultEngine;
+          var defaultEngine = this.mSearchService.defaultEngine;
           if (defaultEngine) {
             this.clearEngines();
             this.addEngine(defaultEngine.name, defaultEngine.iconURI,
                            defaultEngine.searchForm);
           }
 
           this.mEnginesReady = true;
         ]]></body>
@@ -554,16 +579,21 @@
         if (text)
           this.searchValue = text;
       ]]></constructor>
 
       <field name="mSelectedIndex">
         -1
       </field>
 
+      <field name="mSearchService">
+        Components.classes["@mozilla.org/browser/search-service;1"]
+                  .getService(Components.interfaces.nsIBrowserSearchService);
+      </field>
+
       <property name="activeChild"
                 onget="return this.childNodes[this.mSelectedIndex]"/>
 
       <property name="selectedIndex">
         <getter>return this.mSelectedIndex;</getter>
 
         <setter><![CDATA[
           if (this.mSelectedIndex != -1)
@@ -603,17 +633,17 @@
       </method>
 
       <property name="overrideValue" readonly="true">
         <getter><![CDATA[
           var item = this.activeChild;
           if (item) {
             // XXXsearch: using default engine for now, this ignores the following values:
             //            item.getAttribute("searchEngine") and item.getAttribute("searchBarUrl")
-            var engine = Services.search.defaultEngine;
+            var engine = this.mSearchService.defaultEngine;
 
             var submission = engine.getSubmission(this.mSearchValue); // HTML response
 
             // getSubmission can return null if the engine doesn't have a URL
             // with a text/html response type.  This is unlikely (since
             // SearchService._addEngineToStore() should fail for such an engine),
             // but let's be on the safe side.
             if (!submission)
--- a/suite/common/pref/pref-search.js
+++ b/suite/common/pref/pref-search.js
@@ -14,36 +14,47 @@ var SearchObserver = {
     Services.obs.addObserver(this, "browser-search-engine-modified", false);
     window.addEventListener("unload", this, false);
   },
 
   observe: function searchEngineListObj_observe(aEngine, aTopic, aVerb) {
     if (aTopic != "browser-search-engine-modified")
       return;
     MakeList();
-    var pref = document.getElementById("browser.search.defaultenginename");
-    if (pref)
-      pref.updateElements();
   },
 
   handleEvent: function searchEngineListEvent(aEvent) {
     if (aEvent.type == "unload") {
       window.removeEventListener("unload", this, false);
       Services.obs.removeObserver(this, "browser-search-engine-modified");
     }
   }
 };
 
 function MakeList() {
   var menulist = document.getElementById("engineList");
-  while (menulist.hasChildNodes())
-    menulist.lastChild.remove();
+  var currentEngineName = Services.search.currentEngine.name;
+
+  // Make sure the popup is empty.
+  menulist.removeAllItems();
 
   var engines = Services.search.getVisibleEngines();
-  for (let i = 0; i < engines.length; i++) {
-    let name = engines[i].name;
+  for (engine of engines) {
+    let name = engine.name;
     let menuitem = menulist.appendItem(name, name);
     menuitem.setAttribute("class", "menuitem-iconic");
-    if (engines[i].iconURI)
-      menuitem.setAttribute("image", engines[i].iconURI.spec);
-    menuitem.engine = engines[i];
+    if (engine.iconURI)
+      menuitem.setAttribute("image", engine.iconURI.spec);
+    menuitem.engine = engine;
+    if (engine.name == currentEngineName) {
+      // Set selection to the current default engine.
+      menulist.selectedItem = menuitem;
+    }
   }
+  // If the current engine isn't in the list any more, select the first item.
+  if (menulist.selectedIndex < 0)
+    menulist.selectedIndex = 0;
 }
+
+function UpdateDefaultEngine(selectedItem) {
+  Services.search.currentEngine = selectedItem.engine;
+  Services.obs.notifyObservers(null, "browser-search-engine-modified", "engine-current");
+}
--- a/suite/common/pref/pref-search.xul
+++ b/suite/common/pref/pref-search.xul
@@ -5,19 +5,16 @@
 
 <!DOCTYPE overlay SYSTEM "chrome://communicator/locale/pref/pref-search.dtd">
 <overlay xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
   <prefpane id="search_pane"
             label="&pref.search.title;"
             script="chrome://communicator/content/pref/pref-search.js">
 
     <preferences id="search_preferences">
-      <preference id="browser.search.defaultenginename"
-                  name="browser.search.defaultenginename"
-                  type="wstring"/>
       <preference id="browser.search.openintab"
                   name="browser.search.openintab"
                   type="bool"/>
       <preference id="browser.search.opentabforcontextsearch"
                   name="browser.search.opentabforcontextsearch"
                   type="bool"/>
       <preference id="browser.search.opensidebarsearchpanel"
                   name="browser.search.opensidebarsearchpanel"
@@ -27,17 +24,17 @@
     <groupbox>
       <caption label="&legendHeader;"/>
 
       <hbox align="center">
         <label value="&defaultSearchEngine.label;"
                accesskey="&defaultSearchEngine.accesskey;"
                control="engineList"/>
         <menulist id="engineList"
-                  preference="browser.search.defaultenginename"/>
+                  oncommand="UpdateDefaultEngine(this.selectedItem)"/>
       </hbox>
       <hbox pack="end">
         <button id="managerButton"
                 label="&engineManager.label;"
                 oncommand="OpenSearchEngineManager();"/>
       </hbox>
     </groupbox>
 
--- a/suite/common/search/search-panel.js
+++ b/suite/common/search/search-panel.js
@@ -17,35 +17,42 @@ function Startup() {
   if (isPB)
     textbox.searchParam += "|private";
 
   LoadEngineList();
   Services.obs.addObserver(engineObserver, SEARCH_ENGINE_TOPIC, true);
 }
 
 function LoadEngineList() {
+  var currentEngineName = Services.search.currentEngine.name;
   // Make sure the popup is empty.
   menulist.removeAllItems();
 
   var engines = Services.search.getVisibleEngines();
-  for (let i = 0; i < engines.length; i++) {
-    let name = engines[i].name;
+  for (engine of engines) {
+    let name = engine.name;
     let menuitem = menulist.appendItem(name, name);
     menuitem.setAttribute("class", "menuitem-iconic");
-    if (engines[i].iconURI)
-      menuitem.setAttribute("image", engines[i].iconURI.spec);
-    menulist.menupopup.appendChild(menuitem);
-    menuitem.engine = engines[i];
+    if (engine.iconURI)
+      menuitem.setAttribute("image", engine.iconURI.spec);
+    menuitem.engine = engine;
+    if (engine.name == currentEngineName) {
+      // Set selection to the current default engine.
+      menulist.selectedItem = menuitem;
+    }
   }
-  menulist.value = Services.search.currentEngine.name;
+  // If the current engine isn't in the list any more, select the first item.
+  if (menulist.selectedIndex < 0)
+    menulist.selectedIndex = 0;
 }
 
 function SelectEngine() {
   if (menulist.selectedItem)
     Services.search.currentEngine = menulist.selectedItem.engine;
+  Services.obs.notifyObservers(null, SEARCH_ENGINE_TOPIC, "engine-current");
 }
 
 function doSearch() {
   var textValue = textbox.value;
 
   // Save the current value in the form history (shared with the search bar)
   // except when in Private Browsing mode.
 
@@ -67,15 +74,13 @@ function doSearch() {
 }
 
 var engineObserver = {
   QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIObserver,
                                          Components.interfaces.nsISupportsWeakReference]),
 
   observe: function(aEngine, aTopic, aVerb) {
     if (aTopic == SEARCH_ENGINE_TOPIC) {
-      if (aVerb == "engine-current")
-        return;
       // Right now, always just rebuild the list after any modification.
       LoadEngineList();
     }
   }
 }
--- a/suite/common/search/search.xml
+++ b/suite/common/search/search.xml
@@ -119,18 +119,20 @@
           if (!this._engines)
             this._engines = this.searchService.getVisibleEngines();
           return this._engines;
         ]]></getter>
       </property>
 
       <property name="currentEngine">
         <setter><![CDATA[
-          var ss = this.searchService;
-          ss.defaultEngine = ss.currentEngine = val;
+          this.searchService.currentEngine = val;
+          var os = Components.classes["@mozilla.org/observer-service;1"]
+                             .getService(Components.interfaces.nsIObserverService);
+          os.notifyObservers(null, "browser-search-engine-modified", "engine-current");
           return val;
         ]]></setter>
         <getter><![CDATA[
           var currentEngine = this.searchService.currentEngine;
           // Return a dummy engine if there is no currentEngine
           return currentEngine || {name: "", uri: null};
         ]]></getter>
       </property>
--- a/suite/common/src/nsSuiteGlue.js
+++ b/suite/common/src/nsSuiteGlue.js
@@ -237,31 +237,16 @@ SuiteGlue.prototype = {
       case "idle":
         if (this._idleService.idleTime > BOOKMARKS_BACKUP_IDLE_TIME * 1000)
           this._backupBookmarks();
         break;
       case "initial-migration":
         this._initialMigrationPerformed = true;
         break;
       case "browser-search-engine-modified":
-        if (data != "engine-default" && data != "engine-current") {
-          break;
-        }
-        // Enforce that the search service's defaultEngine is always equal to
-        // its currentEngine. The search service will notify us any time either
-        // of them are changed (either by directly setting the relevant prefs,
-        // i.e. if add-ons try to change this directly, or if the
-        // nsIBrowserSearchService setters are called).
-        var ss = Services.search;
-        if (ss.currentEngine.name == ss.defaultEngine.name)
-          return;
-        if (data == "engine-current")
-          ss.defaultEngine = ss.currentEngine;
-        else
-          ss.currentEngine = ss.defaultEngine;
         break;
       case "notifications-open-settings":
         // Since this is a web notification, there's probably a browser window.
         var mostRecentBrowserWindow = Services.wm.getMostRecentWindow("navigator:browser");
         if (mostRecentBrowserWindow)
           mostRecentBrowserWindow.toDataManager("|permissions");
         break;
       case "timer-callback":