Bug 1460419 - Remove geoSpecificPref code from search service. r?florian
MozReview-Commit-ID: 1OfikXSwtAJ
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -372,61 +372,16 @@ function isPartnerBuild() {
return distroID && !distroID.startsWith("mozilla");
}
// Method to determine if we should be using geo-specific defaults
function geoSpecificDefaultsEnabled() {
return Services.prefs.getBoolPref("browser.search.geoSpecificDefaults", false);
}
-// Some notes on countryCode and region prefs:
-// * A "countryCode" pref is set via a geoip lookup. It always reflects the
-// result of that geoip request.
-// * A "region" pref, once set, is the region actually used for search. In
-// most cases it will be identical to the countryCode pref.
-// * The value of "region" and "countryCode" will only not agree in one edge
-// case - 34/35 users who have previously been configured to use US defaults
-// based purely on a timezone check will have "region" forced to US,
-// regardless of what countryCode geoip returns.
-// * We may want to know if we are in the US before we have *either*
-// countryCode or region - in which case we fallback to a timezone check,
-// but we don't persist that value anywhere in the expectation we will
-// eventually get a countryCode/region.
-
-// A method to determine if we are in the United States (US) for the search
-// service.
-// It uses a browser.search.region pref (which typically comes from a geoip
-// request) or if that doesn't exist, falls back to a hacky timezone check.
-function getIsUS() {
- // Regardless of the region or countryCode, non en-US builds are not
- // considered to be in the US from the POV of the search service.
- if (getLocale() != "en-US") {
- return false;
- }
-
- // If we've got a region pref, trust it.
- try {
- return Services.prefs.getCharPref("browser.search.region") == "US";
- } catch (e) {}
-
- // So we are en-US but have no region pref - fallback to hacky timezone check.
- let isNA = isUSTimezone();
- LOG("getIsUS() fell back to a timezone check with the result=" + isNA);
- return isNA;
-}
-
-// Helper method to modify preference keys with geo-specific modifiers, if needed.
-function getGeoSpecificPrefName(basepref) {
- if (!geoSpecificDefaultsEnabled() || isPartnerBuild())
- return basepref;
- if (getIsUS())
- return basepref + ".US";
- return basepref;
-}
-
// A method that tries to determine if this user is in a US geography.
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.
@@ -435,21 +390,17 @@ function isUSTimezone() {
// 600 minutes = 10 hours (UTC-10), which is
// Hawaii-Aleutian Standard Time (http://www.timeanddate.com/time/zones/hast)
let UTCOffset = (new Date()).getTimezoneOffset();
return UTCOffset >= 150 && UTCOffset <= 600;
}
-// A less hacky method that tries to determine our country-code via an XHR
-// geoip lookup.
-// If this succeeds and we are using an en-US locale, we set the pref used by
-// the hacky method above, so isUS() can avoid the hacky timezone method.
-// If it fails we don't touch that pref so isUS() does its normal thing.
+// A method that tries to determine our country-code via an XHR geoip lookup.
var ensureKnownCountryCode = async function(ss) {
// If we have a country-code already stored in our prefs we trust it.
let countryCode = Services.prefs.getCharPref("browser.search.countryCode", "");
if (!countryCode) {
// We don't have it cached, so fetch it. fetchCountryCode() will call
// storeCountryCode if it gets a result (even if that happens after the
// promise resolves) and fetchRegionDefault.
@@ -492,16 +443,22 @@ var ensureKnownCountryCode = async funct
});
}
// If gInitialized is true then the search service was forced to perform
// a sync initialization during our XHRs - capture this via telemetry.
Services.telemetry.getHistogramById("SEARCH_SERVICE_COUNTRY_FETCH_CAUSED_SYNC_INIT").add(gInitialized);
};
+// Some notes on countryCode and region prefs:
+// * A "countryCode" pref is set via a geoip lookup. It always reflects the
+// result of that geoip request.
+// * A "region" pref, once set, is the region actually used for search. In
+// most cases it will be identical to the countryCode pref.
+
// Store the result of the geoip request as well as any other values and
// telemetry which depend on it.
function storeCountryCode(cc) {
// Set the country-code itself.
Services.prefs.setCharPref("browser.search.countryCode", cc);
// And set the region pref if we don't already have a value.
if (!Services.prefs.prefHasUserValue("browser.search.region")) {
Services.prefs.setCharPref("browser.search.region", cc);
@@ -2756,19 +2713,18 @@ SearchService.prototype = {
if (!defaultEngine) {
// We only allow the old defaultenginename pref for distributions
// We can't use isPartnerBuild because we need to allow reading
// of the defaultengine name pref for funnelcakes.
if (distroID) {
let defaultPrefB = Services.prefs.getDefaultBranch(BROWSER_SEARCH_PREF);
let nsIPLS = Ci.nsIPrefLocalizedString;
- let defPref = getGeoSpecificPrefName("defaultenginename");
try {
- defaultEngine = defaultPrefB.getComplexValue(defPref, nsIPLS).data;
+ defaultEngine = defaultPrefB.getComplexValue("defaultenginename", nsIPLS).data;
} catch (ex) {
// If the default pref is invalid (e.g. an add-on set it to a bogus value)
// use the default engine from the list.json.
// This should eventually be the common case. We should only have the
// defaultenginename pref for distributions.
// Worst case, getEngineByName will just return null, which is the best we can do.
defaultEngine = this._searchDefault;
}
@@ -3611,19 +3567,18 @@ SearchService.prototype = {
if (!engine || engine.name in addedEngines)
continue;
this.__sortedEngines.push(engine);
addedEngines[engine.name] = engine;
}
} catch (e) { }
- let prefNameBase = getGeoSpecificPrefName(BROWSER_SEARCH_PREF + "order");
while (true) {
- prefName = prefNameBase + "." + (++i);
+ prefName = `${BROWSER_SEARCH_PREF}order.{++i}`;
engineName = getLocalizedPref(prefName);
if (!engineName)
break;
engine = this._engines[engineName];
if (!engine || engine.name in addedEngines)
continue;
@@ -3760,19 +3715,18 @@ 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++) {
- let prefName = prefNameBase + "." + j;
+ let prefName = `${BROWSER_SEARCH_PREF}order.${j}`;
engineName = getLocalizedPref(prefName);
if (!engineName)
break;
if (!(engineName in engineOrder))
engineOrder[engineName] = i++;
}
}
@@ -4165,20 +4119,19 @@ SearchService.prototype = {
try {
if (result.name == Services.prefs.getCharPref(prefName)) {
sendSubmissionURL = true;
break;
}
} catch (e) {}
}
- let prefNameBase = getGeoSpecificPrefName(BROWSER_SEARCH_PREF + "order");
let i = 0;
while (!sendSubmissionURL) {
- let prefName = prefNameBase + "." + (++i);
+ let prefName = `${BROWSER_SEARCH_PREF}order.{++i}`;
let engineName = getLocalizedPref(prefName);
if (!engineName)
break;
if (result.name == engineName) {
sendSubmissionURL = true;
break;
}
}
--- a/toolkit/components/search/tests/xpcshell/head_search.js
+++ b/toolkit/components/search/tests/xpcshell/head_search.js
@@ -432,43 +432,16 @@ var addTestEngines = async function(aIte
function installTestEngine() {
useHttpServer();
return addTestEngines([
{ name: kTestEngineName, xmlFileName: "engine.xml" },
]);
}
/**
- * Set a localized preference on the default branch
- * @param aPrefName
- * The name of the pref to set.
- */
-function setLocalizedDefaultPref(aPrefName, aValue) {
- let value = "data:text/plain," + BROWSER_SEARCH_PREF + aPrefName + "=" + aValue;
- Services.prefs.getDefaultBranch(BROWSER_SEARCH_PREF)
- .setCharPref(aPrefName, value);
-}
-
-/**
- * Installs two test engines, sets them as default for US vs. general.
- */
-function setUpGeoDefaults() {
- const kSecondTestEngineName = "A second test engine";
-
- setLocalizedDefaultPref("defaultenginename", "Test search engine");
- setLocalizedDefaultPref("defaultenginename.US", "A second test engine");
-
- useHttpServer();
- return addTestEngines([
- { name: kTestEngineName, xmlFileName: "engine.xml" },
- { name: kSecondTestEngineName, xmlFileName: "engine2.xml" },
- ]);
-}
-
-/**
* Returns a promise that is resolved when an observer notification from the
* search service fires with the specified data.
*
* @param aExpectedData
* The value the observer notification sends that causes us to resolve
* the promise.
*/
function waitForSearchNotification(aExpectedData) {
deleted file mode 100644
--- a/toolkit/components/search/tests/xpcshell/test_location_funnelcake.js
+++ /dev/null
@@ -1,13 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- http://creativecommons.org/publicdomain/zero/1.0/ */
-
-add_task(async function run_test() {
- Services.prefs.setCharPref("browser.search.geoip.url", 'data:application/json,{"country_code": "US"}');
- // funnelcake builds start with "mozilla"
- Services.prefs.getDefaultBranch("").setCharPref("distribution.id", "mozilla38");
-
- await setUpGeoDefaults();
- await asyncReInit();
-
- equal(Services.search.defaultEngine.name, "A second test engine");
-});
deleted file mode 100644
--- a/toolkit/components/search/tests/xpcshell/test_location_partner.js
+++ /dev/null
@@ -1,12 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- http://creativecommons.org/publicdomain/zero/1.0/ */
-
-add_task(async function run_test() {
- Services.prefs.setCharPref("browser.search.geoip.url", 'data:application/json,{"country_code": "US"}');
- Services.prefs.getDefaultBranch("").setCharPref("distribution.id", "partner-1");
-
- await setUpGeoDefaults();
- await asyncReInit();
-
- equal(Services.search.defaultEngine.name, "Test search engine");
-});
--- a/toolkit/components/search/tests/xpcshell/xpcshell.ini
+++ b/toolkit/components/search/tests/xpcshell/xpcshell.ini
@@ -42,18 +42,16 @@ support-files =
[test_json_cache.js]
[test_list_json_locale.js]
[test_list_json_searchdefault.js]
[test_list_json_searchdefault_distro.js]
[test_list_json_searchorder.js]
[test_location.js]
[test_location_error.js]
[test_location_malformed_json.js]
-[test_location_partner.js]
-[test_location_funnelcake.js]
[test_location_sync.js]
[test_location_timeout.js]
[test_location_timeout_xhr.js]
[test_nodb_pluschanges.js]
[test_save_sorted_engines.js]
[test_pref.js]
[test_purpose.js]
[test_defaultEngine.js]