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

RESOLVED FIXED in Firefox 63

Status

()

enhancement
P3
normal
RESOLVED FIXED
Last year
11 months ago

People

(Reporter: mkaply, Assigned: mkaply)

Tracking

Trunk
Firefox 63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

Last year
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).
Assignee

Updated

Last year
Priority: -- → P3
Comment hidden (mozreview-request)

Comment 2

11 months ago
mozreview-review
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-
Assignee

Updated

11 months ago
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)
Assignee

Comment 5

11 months ago
> 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)

Updated

11 months ago
Attachment #8991472 - Attachment is obsolete: true
Assignee

Comment 7

11 months ago
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+

Comment 9

11 months ago
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/0866ebeda09d
Remove browser.search.countryCode pref. r=florian

Comment 10

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0866ebeda09d
Status: ASSIGNED → RESOLVED
Closed: 11 months 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.