Closed Bug 1527748 Opened 8 months ago Closed 7 months ago

Enable the geolocation OS API pref for Fx <65 and esr release users

Categories

(Core :: DOM: Geolocation, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr60 --- affected
firefox65 --- unaffected
firefox66 --- unaffected
firefox67 --- unaffected

People

(Reporter: Sylvestre, Unassigned)

References

Details

From 65, with bug 1512161, we are now using the OS API for geoloc.
I would like to do that for ESR 60 and < 65.

For Mac:
geo.provider.use_corelocation=true

For Windows:
geo.provider.ms-windows-location=true

For Linux:
geo.provider.use_gpsd=true

(for esr 60, I have been told that we cannot use normandy, I guess we will have to write a system addons)

Michael, could you please help with that? thanks

Severity: normal → enhancement
Flags: needinfo?(mcooper)
Priority: -- → P1

Normandy is available as a built-in component starting in Firefox 60, including ESR60 (bug 1436113). We also have Normandy as a system add-on in some versions before then, but I don't have the full details yet.

This sounds like something we would use rollout for today, but rollout is only available on >=61. The alternative is to use the preference-experiment action, which should be possible, but it is more fragile. I think it is one of our best options today though.

I propose we use a rollout for Fx >=61, and a use the pref experiment to target version of Firefox <61. We can make sure this targets as many versions of Firefox as possible. This is a complex set of recipes, so it will take some time to prepare. We'll want to make sure that it has maximum compatibility.

Flags: needinfo?(mcooper)

Sounds great, thanks!

Josh, am I correct in comment #0 ? thanks

Flags: needinfo?(josh)

Sylvestre, how far back were you thinking going with this change? Is Firefox 60/ESR60 the oldest you need to target?

Flags: needinfo?(sledru)

That is correct.

Flags: needinfo?(josh)

Michael, as far as possible without introducing a lot of work :)

Flags: needinfo?(sledru)

Due to limitations in Normandy, this change has six recipes associated with it. This is the result of a combination of two factors that we can't combine recipes for: different preferences per OS, and different action types for Firefox versions.

The first factor is preference per OS. Here we have a straightforward one-recipe-per-os.

The second factor is using Preference Rollout for Firefox 61-64, and using Preference Experiment for 60 and below. I didn't put a lower bound on the latter, and we'll see how far it can go.

Here are the recipes:

There are few important things to note about these recipes. I've checked all of these things myself, but it would be good to have more eyes on them.

  • For the preference experiment ones (<=60), it is very important that they be set to "high volume" recipes. This changes how Telemetry handles the data from these recipes.
  • Pref rollout only works on 61 and above, so the pref rollouts should be targeted at only those versions.
  • On the flip side, pref experiments aren't a good tool for this, so we only want to use them where rollouts aren't available (60 and below)
  • Normandy has native OS filtering, but I've avoided using it, since it is only available in Firefox 63 and above.

Besides this, the obvious things of checking that the right prefs and the right versions are used would be great.

Ritu, can you take a look at the recipes?

Sylvestre, I think it would be good to get you access to Delivery Console as well, so you could do this kind of review in the future. If that's interesting to you, you can kick off the process by cloning bug 1527780 (except ask for access to stage and prod) to get access to the Normandy servers.

Flags: needinfo?(rkothari)

Thanks for such a detailed and well explained comment. I reviewed the recipes. The only one I am unsure about is https://delivery-console.prod.mozaws.net/recipe/689/ which is not set as "high volume". Is that by design?

Flags: needinfo?(rkothari) → needinfo?(mcooper)

Recipe 689 is also setup as "preference-rollout" but probably should be "preference-experiment".

I guess I forgot to click save on the changes to 689. I've fixed that now.

Oh, something else I meant to ask, do we want to take these straight to 100%? Or should we take a more gradual rollout approach?

Flags: needinfo?(mcooper)

It's Sylvestre's night time so I am choosing (on his behalf) to go with a gradual rollout. Can we start at 25% now and then take it to 100% after a day?

I've updated the recipes to target a 25% sample and requested approval for all of them.

QA help requested in https://moz-pi-test.atlassian.net/browse/PI-24

I think Linux should be out of scope at this point, we don't seem to enable that support in our builds.

Thanks Julien. Is that true Sylvestre?

Flags: needinfo?(sledru)

(In reply to Julien Cristau [:jcristau] from comment #13)

QA help requested in https://moz-pi-test.atlassian.net/browse/PI-24

I think Linux should be out of scope at this point, we don't seem to enable that support in our builds.

That's correct. By default as of 65 (bug 1512161), Firefox uses geolocation APIs provided by the OS on Windows and macOS. Linux doesn't have a standard geolocation API, so we always uses GLS (or MLS) on Linux. Firefox on Linux does have a geo.provider.use_gpsd pref to use the "gpsd" API to connect to a hardware GPS device. But we've never really tested this code and these GPS devices are probably rare for desktop users, so I think we SHOULD NOT enable geo.provider.use_gpsd pref.

Chris, as of yesterday, we are rolling out using geo.provider.use_gpsd=true to Firefox users <65, via Normandy. Should we roll that back?

Flags: needinfo?(cpeterson)

The recipes from comment 12 are all live at 25%. About 225K users have enrolled. However, now Windows users have enrolled yet, so I'm going to revisit the recipe for that to see what's wrong.

I typoed the OS query for Windows, checking for Window_NT instead of Windows_NT. I have corrected the new recipes, and put them back up for approval:

https://delivery-console.prod.mozaws.net/recipe/686/
https://delivery-console.prod.mozaws.net/recipe/689/

Also, after read this bug more closely, I've paused the enrollment for the Linux rollout of >60, and made a change to pause the <=60 version that will also need approved. If this is the right choice, I will fully roll back these changes.

https://delivery-console.prod.mozaws.net/recipe/691/

Ritu, can you review these changes please?

I reviewed the recipe and noticed that recipe 686 shows the correct windows_nt string on https://delivery-console.prod.mozaws.net/recipe/686/ but not on https://delivery-console.prod.mozaws.net/recipe/686/approval_history/ (it still shows window_nt). Mythmon said the API shows the correct string and that this is just a front-end/delivery-console bug. With that, approved both recipes.

done.

(In reply to Michael Cooper [:mythmon] from comment #16)

Chris, as of yesterday, we are rolling out using geo.provider.use_gpsd=true to Firefox users <65, via Normandy. Should we roll that back?

Disabling the geo.provider.use_gpsd pref would be best because our gpsd code is untested, but I don't think it's worth your time to revert the pref change. There should be no harm in enabling the geo.provider.use_gpsd pref on Linux because Mozilla's Firefox builds are not compiled with gpsd support (AFAICT from bug 1250922 comment 59 and 60). So the pref will be ignored in Mozilla's Firefox builds.

The original author of Firefox's gpsd code (:tzimmermann) filed a Ubuntu bug asking them to enable gpsd support in their builds:

https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/1783202

If an eager Linux user wants to enable the MOZ_GPSD configure option in Mozilla's Firefox builds and test with a real GPS device, I don't see any reason we wouldn't accept their patches. :)

Flags: needinfo?(cpeterson)

Folks provided better answers that I would :)
thanks

Flags: needinfo?(sledru)

Hi Mythmon, how has the recipe uptake been for pref-rollout and experiment? Sylvestre suggested we could move to 100% now. Given the lack of any significant bug reports related to this, can we configure the recipe to go at 100%? Thanks!

Flags: needinfo?(mcooper)

The rollout for this looks healthy. I've updated the recipes to target 100%, and they are pending approval.

Flags: needinfo?(mcooper)

Done. Approved all 6 recipes.

I think we are good, many thanks for the help!

Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.