Bug 1117186 - fix geo-specific search defaults interaction with the defaultEngine getter/setter and use a helper for getting geo-specific prefs f=gavin,r=markh a=sylvestre
authorMark Finkle <mfinkle@mozilla.com>
Wed, 21 Jan 2015 23:05:48 -0500
changeset 243722 a9eec576bb2c
parent 243721 130e59d97b15
child 243723 c1578e681849
push id4450
push usermfinkle@mozilla.com
push date2015-02-09 17:21 +0000
treeherdermozilla-beta@c1578e681849 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh, sylvestre
bugs1117186
milestone36.0
Bug 1117186 - fix geo-specific search defaults interaction with the defaultEngine getter/setter and use a helper for getting geo-specific prefs f=gavin,r=markh a=sylvestre
toolkit/components/search/nsSearchService.js
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -402,48 +402,58 @@ loadListener.prototype = {
   // FIXME: bug 253127
   // nsIHttpEventSink
   onRedirect: function (aChannel, aNewChannel) {},
   // nsIProgressEventSink
   onProgress: function (aRequest, aContext, aProgress, aProgressMax) {},
   onStatus: function (aRequest, aContext, aStatus, aStatusArg) {}
 }
 
-// Hacky method that tries to determine if this user is in a US geography, and
-// using an en-US build.
-function getIsUS() {
+// Method to determine if we should be using geo-specific defaults
+function geoSpecificDefaultsEnabled() {
   let geoSpecificDefaults = false;
   try {
     geoSpecificDefaults = Services.prefs.getBoolPref("browser.search.geoSpecificDefaults");
   } catch(e) {}
 
   let distroID;
   try {
     distroID = Services.prefs.getCharPref("distribution.id");
   } catch (e) {}
 
-  if (!geoSpecificDefaults || distroID) {
-    return false;
-  }
-
+  return (geoSpecificDefaults && !distroID);
+}
+
+// Hacky method that tries to determine if this user is in a US geography, and
+// using an en-US build.
+function getIsUS() {
   // If we've set the pref before, just return that result.
   let cachePref = "browser.search.isUS";
   try {
     return Services.prefs.getBoolPref(cachePref);
   } catch(e) {}
 
   if (getLocale() != "en-US") {
     Services.prefs.setBoolPref(cachePref, false);
     return false;
   }
   let isNA = isUSTimezone();
   Services.prefs.setBoolPref(cachePref, isNA);
   return isNA;
 }
 
+// Helper method to modify preference keys with geo-specific modifiers, if needed
+function getGeoSpecificPrefName(basepref) {
+  if (!geoSpecificDefaultsEnabled())
+    return basepref;
+  if (getIsUS())
+    return basepref + ".US";
+  return basepref;
+}
+
 function isUSTimezone() {
   // Timezone assumptions! We assume that if the system clock's timezone is
   // between Newfoundland and Hawaii, that the user is in North America.
 
   // This includes all of South America as well, but we have relatively few
   // en-US users there, so that's OK.
 
   // 150 minutes = 2.5 hours (UTC-2.5), which is
@@ -1960,20 +1970,17 @@ Engine.prototype = {
 
     this._urls.push(url);
   },
 
   _isDefaultEngine: function SRCH_ENG__isDefaultEngine() {
     let defaultPrefB = Services.prefs.getDefaultBranch(BROWSER_SEARCH_PREF);
     let nsIPLS = Ci.nsIPrefLocalizedString;
     let defaultEngine;
-    let pref = "defaultenginename";
-    if (getIsUS()) {
-      pref += ".US";
-    }
+    let pref = getGeoSpecificPrefName("defaultenginename");
     try {
       defaultEngine = defaultPrefB.getComplexValue(pref, nsIPLS).data;
     } catch (ex) {}
     return this.name == defaultEngine;
   },
 
   /**
    * Get the icon from an OpenSearch Image element.
@@ -3183,23 +3190,17 @@ SearchService.prototype = {
 
   // Get the original Engine object that belongs to the defaultenginename pref
   // of the default branch.
   get _originalDefaultEngine() {
     let defaultPrefB = Services.prefs.getDefaultBranch(BROWSER_SEARCH_PREF);
     let nsIPLS = Ci.nsIPrefLocalizedString;
     let defaultEngine;
 
-    let defPref;
-    if (getIsUS()) {
-      defPref = "defaultenginename.US";
-    } else {
-      defPref = "defaultenginename";
-    }
-
+    let defPref = getGeoSpecificPrefName("defaultenginename");
     try {
       defaultEngine = defaultPrefB.getComplexValue(defPref, nsIPLS).data;
     } catch (ex) {
       // If the default pref is invalid (e.g. an add-on set it to a bogus value)
       // getEngineByName will just return null, which is the best we can do.
     }
     return this.getEngineByName(defaultEngine);
   },
@@ -3970,22 +3971,20 @@ SearchService.prototype = {
             continue;
 
           this.__sortedEngines.push(engine);
           addedEngines[engine.name] = engine;
         }
       }
       catch (e) { }
 
+      let prefNameBase = getGeoSpecificPrefName(BROWSER_SEARCH_PREF + "order");
       while (true) {
-        prefName = BROWSER_SEARCH_PREF + "order.";
-        if (getIsUS()) {
-          prefName += "US.";
-        }
-        engineName = getLocalizedPref(prefName + (++i));
+        prefName = prefNameBase + "." + (++i);
+        engineName = getLocalizedPref(prefName);
         if (!engineName)
           break;
 
         engine = this._engines[engineName];
         if (!engine || engine.name in addedEngines)
           continue;
 
         this.__sortedEngines.push(engine);
@@ -4136,22 +4135,19 @@ SearchService.prototype = {
         if (!(engineName in engineOrder))
           engineOrder[engineName] = i++;
       }
     } catch (e) {
       LOG("Getting extra order prefs failed: " + e);
     }
 
     // Now look through the "browser.search.order" branch.
+    let prefNameBase = getGeoSpecificPrefName(BROWSER_SEARCH_PREF + "order");
     for (var j = 1; ; j++) {
-      var prefName = BROWSER_SEARCH_PREF + "order.";
-      if (getIsUS()) {
-        prefName += "US.";
-      }
-      prefName += j;
+      let prefName = prefNameBase + "." + j;
       engineName = getLocalizedPref(prefName);
       if (!engineName)
         break;
 
       if (!(engineName in engineOrder))
         engineOrder[engineName] = i++;
     }
 
@@ -4354,17 +4350,17 @@ SearchService.prototype = {
       if (e.hidden && e._isDefault)
         e.hidden = false;
     }
   },
 
   get defaultEngine() {
     this._ensureInitialized();
     if (!this._defaultEngine) {
-      let defPref = BROWSER_SEARCH_PREF + "defaultenginename";
+      let defPref = getGeoSpecificPrefName(BROWSER_SEARCH_PREF + "defaultenginename");
       let defaultEngine = this.getEngineByName(getLocalizedPref(defPref, ""))
       if (!defaultEngine)
         defaultEngine = this._getSortedEngines(false)[0] || null;
       this._defaultEngine = defaultEngine;
     }
     if (this._defaultEngine.hidden)
       return this._getSortedEngines(false)[0];
     return this._defaultEngine;
@@ -4382,17 +4378,18 @@ SearchService.prototype = {
     if (!newDefaultEngine)
       FAIL("Can't find engine in store!", Cr.NS_ERROR_UNEXPECTED);
 
     if (newDefaultEngine == this._defaultEngine)
       return;
 
     this._defaultEngine = newDefaultEngine;
 
-    let defPref = BROWSER_SEARCH_PREF + "defaultenginename";
+    let defPref = getGeoSpecificPrefName(BROWSER_SEARCH_PREF + "defaultenginename");
+
     // If we change the default engine in the future, that change should impact
     // users who have switched away from and then back to the build's "default"
     // engine. So clear the user pref when the defaultEngine is set to the
     // build's default engine, so that the defaultEngine getter falls back to
     // whatever the default is.
     if (this._defaultEngine == this._originalDefaultEngine) {
       Services.prefs.clearUserPref(defPref);
     }