Enable MLS geolocation for Firefox desktop

VERIFIED FIXED in Firefox 38

Status

()

defect
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

unspecified
mozilla38
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox35 wontfix, firefox36 wontfix, firefox37+ wontfix, firefox38 fixed, firefox43+ fixed, firefox44+ fixed, firefox-esr31 wontfix)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Switch Firefox desktop platforms (Windows, OS X, Linux, and Metro) from GLS to MLS for testing in the Nightly channel.

AFAICT, the "geo.provider.use_mls" pref is not needed on desktop because it is only used for B2G builds that want to override an OEM's custom location provider.
Attachment #8549437 - Flags: superreview?(dougt)
Attachment #8549437 - Flags: review?(gkeeley)
(Assignee)

Comment 1

4 years ago
Comment on attachment 8549437 [details] [diff] [review]
enable-mls.patch

Hanno: are there any server changes you would like to make before enabling MLS for testing in the Nightly channel?
Attachment #8549437 - Flags: feedback?(hschlichting)

Updated

4 years ago
Depends on: 1113606

Comment 2

4 years ago
Comment on attachment 8549437 [details] [diff] [review]
enable-mls.patch

Looks good to me, #1113606 is pretty much done, so a proper API key should be there for desktop builds.

If possible, I'd like to wait with landing this until Tuesday next week (Monday being a holiday in the US). We already got stumbling in Fennec launched to release channel earlier this week and the new country lookup service in Desktop beta as of later today. I'd rather have a couple of days in between adding another new client, so correlating/isolating problems gets a bit easier.
Attachment #8549437 - Flags: feedback?(hschlichting) → feedback+

Updated

4 years ago
Attachment #8549437 - Flags: review?(gkeeley) → review+

Comment 3

4 years ago
Chris, regarding your comment about ESR Firefox. I *think* this is the only patch that needs to be backported to switch it to MLS.
Although NetworkGeolocationProvider.js has had a bunch of work post-31, it was for B2G.
(Assignee)

Updated

4 years ago
Blocks: 1122393
Please don't make this change without sign-off from Firefox product owners (Chad/Madhava/Gavin).
Comment on attachment 8549437 [details] [diff] [review]
enable-mls.patch

lgtm!
Attachment #8549437 - Flags: superreview?(dougt) → superreview+
https://hg.mozilla.org/mozilla-central/rev/1ef39d9f57c5
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
I don't see explicit sign-off from Firefox product owners in this bug. Did Chad/Madhava/Gavin sign off on this change on Nightly?

We are also tracking 37. Should this change be uplifted to Aurora? Is that still unclear?

ESR was marked as fixed but I think that was accidental (fixed was meant for 38 instead) in comment 6 where this change landed on inbound.
Flags: needinfo?(cpeterson)
(Assignee)

Comment 9

4 years ago
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #8)
> I don't see explicit sign-off from Firefox product owners in this bug. Did
> Chad/Madhava/Gavin sign off on this change on Nightly?

Gavin, can you please confirm that this GLS change (to MLS, OS X CoreLocation, and Windows 7+ Location API) is OK? You gave a "go for it" in the "Testing Mozilla Location Service (MLS) on Firefox desktop? (bug 1121862)" email thread.

Should I follow-up with Chad and Madhava, too? dougt is geolocation module owner and he sr+'d this change.


> We are also tracking 37. Should this change be uplifted to Aurora? Is that
> still unclear?

TBD. We will need to uplift to 37 and ESR 31 (to avoid geolocation service outage) iff we do not renew our GLS contract. Our current GLS contract ends April 30. Firefox 38 won't hit release until May 19.


> ESR was marked as fixed but I think that was accidental (fixed was meant for
> 38 instead) in comment 6 where this change landed on inbound.

Yes; that was a mistake. Thanks.
Flags: needinfo?(cpeterson) → needinfo?(gavin.sharp)
Chad/Madhava/I signed off on Nightly testing of MLS in Firefox.
Flags: needinfo?(gavin.sharp)
We decided not to take the MLS change in 37 or ESR 31. Marking those releases as wontfix.

Updated

4 years ago
See Also: → 1136976
[Tracking Requested - why for this release]: This has been live in aurora since 38, and we were originally going into 42 beta, but missed the window. Servers are ready, and we'd like the move forwards.
Blocks: 1197211
Toby (and maybe Lawrence): do you know why we decided not to take this change back in February? 

I'd like to know two more things. How do we know this is ready?  And, can you describe how to test this for QE to have a run through now and then again when 43 is in beta?  Thanks!
Flags: needinfo?(telliott)
Flags: needinfo?(lmandel)
Ah, I see now. The patches here and in some other bugs landed already and weren't backed out so there's nothing to land here. Wherever the change is that you need to make, please request uplift in that bug.
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #13)
> Toby (and maybe Lawrence): do you know why we decided not to take this
> change back in February? 

IIRC, the service wasn't really ready and the timing wasn't right for the change.
Flags: needinfo?(lmandel)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #13)
> Toby (and maybe Lawrence): do you know why we decided not to take this
> change back in February? 
> 

There was a belief at the time that the Google location deal might not be renewed. A few weeks later it was, so the pressure was lifted. It was then targetted at 42 (as part of the privacy push), but got lost in the shuffle.


> I'd like to know two more things. How do we know this is ready? 

Well, it's been running in Aurora. It serves up more traffic to external queries now than we expect this change to add.

> And, can
> you describe how to test this for QE to have a run through now and then
> again when 43 is in beta? 

Hanno's favorite test is just to visit http://html5demos.com/geo
Flags: needinfo?(telliott)
Seems like a nice thing to call out for our users. Without knowing much about this, it seems like there's a privacy benefit to using MLS. 

What's the purpose of the switch and what's the benefit for the user?
You got it. We don't use the information you send to us. 

I'm hopeful we'll also have an addon available around the same point that'll let you figure out your location without talking to a server at all (you predownload a map of your city).

Comment 19

4 years ago
Some notes on purpose:

- remove 3rd party dependancy, which removes risks such as
-- service changes out of our control that break older ffox versions
-- contract not being renewed and scrambling to find a geolocation provider

- build up the health of the service for fxos use
-- fxos is dependant on MLS where QC's geoservice can't be used, and the belief is that a service that is put to greater use will be healthier
(Assignee)

Comment 20

4 years ago
Bug 1197211 made MLS the default geolocation provider in 43 and 44.
Status: RESOLVED → VERIFIED
(In reply to Chris Peterson [:cpeterson] from comment #20)
> Bug 1197211 made MLS the default geolocation provider in 43 and 44.

"You are not authorized to access bug 1197211"
Is there any reason why I cannot access to this bug?
(In reply to Vincent (caméléon) from comment #21)
> Is there any reason why I cannot access to this bug?

I've cleared the moco-only security flag from that bug, as there was indeed nothing confidential in it. Especially since the real reason we didn't use MLS was already in the public bug at https://bugzilla.mozilla.org/show_bug.cgi?id=1216662#c0 where we decided to roll back the changes.
You need to log in before you can comment on or make changes to this bug.