1462015 - Unify browser.search.region and browser.search.countryCode

Status: RESOLVED FIXED

(Firefox :: Search, enhancement, P3)

Description

[Mike Kaply [:mkaply]]

We currently have two preferences for a user's location, browser.search.countryCode and browser.search.region.

There was a time when they could have been different based on the old preference isUS, but since we did the reset of all of these preferences for Firefox 57, these two preferences have always been the same.

We should pick one and use it in all places in preparation for moving the region selection code outside of search.

My preference is browser.search.region since that is what is being used outside of the search service (activity stream, telemetry, url formatter).

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Bug 1462015 - Remove browser.search.countryCode pref.

Review commit: https://reviewboard.mozilla.org/r/255360/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/255360/

Comment 2

[Florian Quèze [:florian]]

Comment on attachment 8990323 [details]
Bug 1462015 - Remove browser.search.countryCode pref.

https://reviewboard.mozilla.org/r/255360/#review262834

Two more occurences https://searchfox.org/mozilla-central/source/testing/profiles/common/user.js#12 and https://searchfox.org/mozilla-central/search?q=search.countryCode&case=false&regexp=false&path=google

::: toolkit/components/search/nsSearchService.js:404
(Diff revision 1)
> -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
> +var ensureKnownRegion = async function(ss) {
> +  // If we have a region already stored in our prefs we trust it.
> +  let region = Services.prefs.getCharPref("browser.search.region", "");
> +
> +  if (!region) {
> +    // We don't have it cached, so fetch it. fetchREgion() will call

fetchR*e*gion

Comment 3

[Mike Kaply [:mkaply]]

Attachment: Bug 1462015 - Remove browser.search.countryCode pref.

Comment 4

[Florian Quèze [:florian]]

I don't see any easy way to do an interdiff between mozreview and phabricator. Was there a reason to switch from one to the other within this bug?

Comment 5

[Mike Kaply [:mkaply]]

> I don't see any easy way to do an interdiff between mozreview and phabricator. Was there a reason to switch from one to the other within this bug?

No, I had just reconfigured everything for phabricator so I switched. I'll move this one back to mozreview.

Sorry about that.

Comment 6

[Mike Kaply [:mkaply]]

Attachment: Bug 1462015 - Remove browser.search.countryCode pref.

Comment 7

[Mike Kaply [:mkaply]]

florian:

There is no way to bring back a discarded diff so what I did was create a Phabricator revision for the original patch then applied the requested changes on top of that.

You can see the diff here:

https://phabricator.services.mozilla.com/D2187?vs=5325&id=5329&whitespace=ignore-most#toc

Sorry for the confusion.

Comment 8

[Phabricator Automation]

Comment on attachment 8992657 [details]
Bug 1462015 - Remove browser.search.countryCode pref.

Florian Quèze [:florian] has approved the revision.

https://phabricator.services.mozilla.com/D2187

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0866ebeda09d
Remove browser.search.countryCode pref. r=florian

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/0866ebeda09d

Comment 11

[Julien Cristau [:jcristau]]

I'm assuming this doesn't require uplift.  Feel free to reset status flag and do the request if I'm mistaken.