Closed
Bug 1121862
Opened 10 years ago
Closed 10 years ago
Enable MLS geolocation for Firefox desktop
Categories
(Core :: DOM: Geolocation, defect)
Core
DOM: Geolocation
Tracking
()
People
(Reporter: cpeterson, Assigned: cpeterson)
References
Details
Attachments
(1 file)
2.01 KB,
patch
|
garvan
:
review+
dougt
:
superreview+
hschlichting
:
feedback+
|
Details | Diff | Splinter Review |
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•10 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)
Comment 2•10 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+
Attachment #8549437 -
Flags: review?(gkeeley) → review+
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.
Updated•10 years ago
|
tracking-firefox37:
--- → +
Comment 4•10 years ago
|
||
Please don't make this change without sign-off from Firefox product owners (Chad/Madhava/Gavin).
Comment 5•10 years ago
|
||
Comment on attachment 8549437 [details] [diff] [review]
enable-mls.patch
lgtm!
Attachment #8549437 -
Flags: superreview?(dougt) → superreview+
Assignee | ||
Comment 6•10 years ago
|
||
Comment 7•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Comment 8•10 years ago
|
||
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•10 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)
Comment 10•10 years ago
|
||
Chad/Madhava/I signed off on Nightly testing of MLS in Firefox.
Flags: needinfo?(gavin.sharp)
Comment 11•10 years ago
|
||
We decided not to take the MLS change in 37 or ESR 31. Marking those releases as wontfix.
Comment 12•9 years ago
|
||
[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.
tracking-firefox43:
--- → ?
Comment 13•9 years ago
|
||
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!
status-firefox43:
--- → affected
status-firefox44:
--- → affected
tracking-firefox44:
--- → +
Flags: needinfo?(telliott)
Flags: needinfo?(lmandel)
Comment 14•9 years ago
|
||
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•9 years ago
|
||
(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)
Comment 16•9 years ago
|
||
(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)
Comment 17•9 years ago
|
||
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•9 years ago
|
||
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•9 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•9 years ago
|
||
Bug 1197211 made MLS the default geolocation provider in 43 and 44.
Comment 21•8 years ago
|
||
(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•8 years ago
|
||
(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.
Description
•