1121862 - Enable MLS geolocation for Firefox desktop

Status: VERIFIED FIXED

(Core :: DOM: Geolocation, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: enable-mls.patch

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.

Comment 1

[Chris Peterson [:cpeterson]]

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?

Comment 2

[Hanno Schlichting]

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.

Comment 3

[:garvan]

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.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Please don't make this change without sign-off from Firefox product owners (Chad/Madhava/Gavin).

Comment 5

[Doug Turner (:dougt)]

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

lgtm!

Comment 6

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1ef39d9f57c5

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1ef39d9f57c5

Comment 8

[Lawrence Mandel [:lmandel] (use needinfo)]

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.

Comment 9

[Chris Peterson [:cpeterson]]

(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.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Chad/Madhava/I signed off on Nightly testing of MLS in Firefox.

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

We decided not to take the MLS change in 37 or ESR 31. Marking those releases as wontfix.

Comment 12

[Toby Elliott [:telliott]]

[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.

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

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!

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 15

[Lawrence Mandel [:lmandel] (use needinfo)]

(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.

Comment 16

[Toby Elliott [:telliott]]

(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

Comment 17

[Fabio Rios [:frios]]

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?

Comment 18

[Toby Elliott [:telliott]]

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

[:garvan]

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

Comment 20

[Chris Peterson [:cpeterson]]

Bug 1197211 made MLS the default geolocation provider in 43 and 44.

Comment 21

[Vincent (caméléon)]

(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?

Comment 22

[Hanno Schlichting]

(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.