1346172 - Fix Geolocation prefs

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, defect)

Comment 1

[Michelangelo De Simone [Offline] [:mds]]

Attachment: Bug 1346172 - Fix Geolocation prefs.

Review commit: https://reviewboard.mozilla.org/r/119008/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/119008/

Comment 2

[Sylvestre Ledru [:Sylvestre]]

You are mixing the changes, why that?

Comment 3

[Michelangelo De Simone [Offline] [:mds]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #2)

> You are mixing the changes, why that?

I don't know what you're saying here: unless I'm totally mistaken by the way "EARLY_BETA_OR_EARLIER" [1] works this should be exactly as it is supposed to be.

[1] from the outcome of our chat, EARLY_BETA_OR_EARLIER should ALSO include DEV_EDITION and NIGHTLY, right?

Comment 4

[Chris Peterson [:cpeterson]]

Comment on attachment 8845864 [details]
Bug 1346172 - Fix Geolocation prefs.

https://reviewboard.mozilla.org/r/119008/#review121124

::: browser/app/profile/firefox.js:1257
(Diff revision 1)
>  //
> -
> -// Geolocation preferences for the RELEASE channel.
> +// Geolocation preferences for Nightly/Aurora/Early Beta.
> +//
>  // Some of these prefs are specified even though they are redundant; they are
> -// here for clarity and end-user experiments.
> -#ifdef RELEASE
> +// here for clarity.
> +#ifdef EARLY_BETA_OR_EARLIER

Seems like your diff would be simpler and easier to review if you just replaced `#ifdef RELEASE` with the inverted logic `#ifndef EARLY_BETA_OR_EARLIER`.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Exactly what Chris said :)

Comment 6

[Michelangelo De Simone [Offline] [:mds]]

Comment on attachment 8845864 [details]
Bug 1346172 - Fix Geolocation prefs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/119008/diff/1-2/

Comment 7

[Michelangelo De Simone [Offline] [:mds]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #5)

> Exactly what Chris said :)

Oh, I see. The explanation is trivial: consistency with the surrounding context. There's no other negative EARLY_BETA_OR_EARLIER check anywhere in the pref file.

Anyhow, it's been updated.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8845864 [details]
Bug 1346172 - Fix Geolocation prefs.

https://reviewboard.mozilla.org/r/119008/#review122516

Thanks, much better!

Comment 9

[Michelangelo De Simone [Offline] [:mds]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #8)

> Thanks, much better!

Thank you!:)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0198076d288f
Fix Geolocation prefs. r=sylvestre

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0198076d288f