Closed Bug 1462015 Opened 2 years ago Closed 2 years ago

Unify browser.search.region and browser.search.countryCode

Categories

(Firefox :: Search, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: mkaply, Assigned: mkaply)

References

Details

Attachments

(1 file, 2 obsolete files)

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).
Priority: -- → P3
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
Attachment #8990323 - Flags: review?(florian) → review-
Attachment #8990323 - Attachment is obsolete: true
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?
Flags: needinfo?(mozilla)
> 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.
Flags: needinfo?(mozilla)
Attachment #8991472 - Attachment is obsolete: true
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 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
Attachment #8992657 - Flags: review+
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/0866ebeda09d
Remove browser.search.countryCode pref. r=florian
https://hg.mozilla.org/mozilla-central/rev/0866ebeda09d
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
I'm assuming this doesn't require uplift.  Feel free to reset status flag and do the request if I'm mistaken.
You need to log in before you can comment on or make changes to this bug.