Patch looks good to me.

Release Note Request (optional, but appreciated) [Why is this notable]: We will switch to MLS [Affects Firefox for Android]: I don't know? I guess we use Android service. [Suggested wording]: Firefox uses Mozilla Location Service for geolocation [Links (documentation, blog post, etc)]: I guess we will have one Should ship in 53

Recommend suggested wording be "Firefox Beta uses Mozilla Location Service for geolocation" for clarity. This does not affect Android, which uses the Android geolocation APIs.

Comment on attachment 8840184 [details] Bug 1341897 - Harmonize Geolocation providers. https://reviewboard.mozilla.org/r/114664/#review118908

Pushed by mdesimone@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f1327d3dcdcb Harmonize Geolocation providers. r=jdm

I'm surprised to see this use #ifdef RELEASE. We don't use that anywhere else. I'm not sure we set it anywhere. Beta and release builds should be identical...

(In reply to Julien Cristau [:jcristau] from comment #9) > I'm surprised to see this use #ifdef RELEASE. We don't use that anywhere > else. I'm not sure we set it anywhere. Beta and release builds should be > identical... We need to keep the non-release provider configuration different while other matters get sorted out. What would be the right way to do just that?

Judging from a quick query may it be "RELEASE_BUILD" [1]? [1] http://searchfox.org/mozilla-central/source/devtools/client/inspector/webpack/prefs-loader.js#40

RELEASE_BUILD no longer exists, it's called RELEASE_OR_BETA. Bug 1304829.

(In reply to Julien Cristau [:jcristau] from comment #12) > RELEASE_BUILD no longer exists, it's called RELEASE_OR_BETA. Bug 1304829. Let me get this straight so there's no misunderstanding: we can't, in any way, differentiate the release configuration alone from the other channels?

There's a macro that's defined for early beta (first half of the cycle basically), but later builds are identical to release as far as I know.

(In reply to Julien Cristau [:jcristau] from comment #14) > There's a macro that's defined for early beta (first half of the cycle > basically) EARLY_BETA_OR_EARLIER in case you want to use it

Toby, what shape are we in for 53 beta and MLS? Is this still planned to be part of 53 or are we planning to switch in 55?

We're fine for 53 early beta as long as Michelangelo is happy with it. From the server side I have no concerns. Whether it ever goes further is completely up in the air :)

I think at this point we don't want to do any uplifts, but instead let this ride the trains. Which means this will reach beta in 55. Since we are using the early-beta flag, this wouldn't have any effect for the current beta 53 anymore, as I think we are about past mid-cycle on it.

Hanno, the Firefox 55 release notes [1] for this bug currently read: "Firefox uses Mozilla Location Service for geolocation". AFAICT, we only use MLS for Linux Firefox builds on the Nightly and Beta channels [2]. Is that correct? Should the release note be changed to something like: "Firefox for Linux uses Mozilla Location Service for geolocation on the Nightly and Beta channels." [1] https://www.mozilla.org/en-US/firefox/55.0a1/releasenotes/ [2] https://hg.mozilla.org/mozilla-central/file/f1327d3dcdcb/browser/app/profile/firefox.js#l1271

(In reply to Chris Peterson [:cpeterson] from comment #19) > Hanno, the Firefox 55 release notes [1] for this bug currently read: > "Firefox uses Mozilla Location Service for geolocation". AFAICT, we only use > MLS for Linux Firefox builds on the Nightly and Beta channels [2]. Is that > correct? Should the release note be changed to something like: "Firefox for > Linux uses Mozilla Location Service for geolocation on the Nightly and Beta > channels." That release note entry looks good to me. It's simple enough and highlights the change - pretty much everyone might end up using MLS now. Details can than be found in this bug. This bug after all changes, which geo providers are used. After this bugs rides the 55 train, 55 beta will use MLS as a geolocation provider on all OS platforms. It will also use native OS providers on macOS and Windows were available. But on both macOS and Windows MLS is still used, even if those native providers get used as well. To complicate things further, this will only be true for the early (first three weeks) period of beta, after which all platforms move back to GLS only. GLS only (no MLS / no native) will be what's shipped in late beta and release. Whether or not we want to change anything about late beta and release is being discussed several levels above my pay-grade ;)

Note, the second link you provided shows the first version of the patch, the current state on moz-central uses the early_beta flag: https://hg.mozilla.org/mozilla-central/file/tip/browser/app/profile/firefox.js#l1308

(In reply to Hanno Schlichting [:hannosch] from comment #20) > This bug after all changes, which geo providers are used. After this bugs > rides the 55 train, 55 beta will use MLS as a geolocation provider on all OS > platforms. It will also use native OS providers on macOS and Windows were > available. But on both macOS and Windows MLS is still used, even if those > native providers get used as well. Thanks. I didn't see the later code changes. I also didn't realize that we use both MLS and native OS providers simultaneously on Mac and Windows. Which location result do we return the JavaScript: MLS or native provider?

(In reply to Chris Peterson [:cpeterson] from comment #22) > I also didn't realize that we > use both MLS and native OS providers simultaneously on Mac and Windows. > Which location result do we return the JavaScript: MLS or native provider? It depends - one or the other :) Sometimes the native providers do not return results or do not return results in a reasonable time. Sometimes they should be present, based on what we can determine about the OS version, but than fail to work at runtime. In those cases MLS is used as a fallback. A typical example is macOS, where the Core Location provider for desktop requires WiFi. If WiFi is disabled or no WiFi networks are nearby, Core Location returns a "Not Found" - as Apple doesn't provide a GeoIP based fallback as part of Core Location. IIRC the logic is something like this: 1. Call the native provider 2. After a couple seconds check to see if it returned a result, if not start the MLS fallback query 3. Both providers are now in a callback race, and the one that returns a successful result first, is passed on to the user 4. If the second provider comes in with a result, but is late, drop it This is for the "single shot" / "one lookup" API. I don't remember the details for watch position anymore. But it gets more complicated, as there are caches involved. And we don't want to jump between the result from one provider to the other, as that could cause the location to jump around on an actual end user visible map.

I don't think this was ever put into the release notes. I don't see it in the notes for 55 release.