1122132 - remove #ifdef NIGHTLY from polaris prefs

Status: RESOLVED WONTFIX

(Core :: DOM: Security, defect)

Description

[[:mmc] Monica Chew (no longer reading bugmail)]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1119891#c1. We can't experiment on Beta if the code to enable the UI only runs in Nightly.

Comment 1

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Remove ifdef Nightly from polaris prefs

Comment 2

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8549780 [details] [diff] [review]
Remove ifdef Nightly from polaris prefs

Review of attachment 8549780 [details] [diff] [review]:
-----------------------------------------------------------------

Gavin, please see Javaun's comment https://bugzilla.mozilla.org/show_bug.cgi?id=1119891#c1 about wanting to test in Beta. This change does not enable the UI by default, but makes it possible to enable the UI in later channels.

Comment 3

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

Comment on attachment 8549780 [details] [diff] [review]
Remove ifdef Nightly from polaris prefs

My understanding of bug 1090535 is that it is related to Tracking Protection, not Polaris, so I don't think you need to change any of the Polaris pref #ifdefs.

For tracking protection, instead of removing the #ifdef around privacy.trackingprotection.ui.enabled in firefox.js, you should leave it and instead adjust the _initTrackingProtection code that reads it to not throw an exception if the preference is non-existent (e.g. by adding a check for Services.prefs.getPrefType(...) != Services.prefs.PREF_INVALID before the getBoolPref call). Then you can only remove the ifdefs in privacy.js.

Comment 4

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

(In reply to :Gavin Sharp [email: [e-mail]] from comment #3)
> My understanding of bug 1090535

I meant bug 1119891.

Comment 5

[[:mmc] Monica Chew (no longer reading bugmail)]

Hi Gavin,

The fact that this code is surrounded by ifdefs means that Bill Selman ran a diary study using Nightly instead of GA. This is a bad idea for many reasons: e10s is enabled by default in Nightly and has significant impacts on experience. I also happened to regress Nightly on the day that the study started. It was fixed the next day, but who knows how many participants updated? These kinds of breakages happen all the time and we should not rely on Nightly for UR on regular people. Additionally, at least one study participant was affected by the radically different branding for GA vs. Nightly.

Please reconsider.

Thanks,
Monica

Comment 6

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

We can easily give Bill branded try builds from other channels with ifdefs removed to solve that problem.

Comment 7

[[:mmc] Monica Chew (no longer reading bugmail)]

We could, but we didn't. Every extra step that is required to test outside of regular channels leads to more errors and more overhead to test properly, and that's why I filed this bug.

Comment 8

[Steve Workman [:sworkman] (INACTIVE)]

How about we change the ifdefs in firefox.js so that the prefs are exposed in Aurora and Beta? - no change to default values, as per Monica's current patch. And at the same time make the change to _initTrackingProtection so that it can read user added prefs?

Comment 9

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

I don't think that we want the prefs exposed to Aurora/Beta users at this time. Neither "building an experiment on Beta" nor "getting builds for user testing" require it, and it wouldn't make either one significantly easier.

Comment 10

[Steve Workman [:sworkman] (INACTIVE)]

(In reply to :Gavin Sharp [email: [e-mail]] from comment #9)
> I don't think that we want the prefs exposed to Aurora/Beta users at this
> time. Neither "building an experiment on Beta" nor "getting builds for user
> testing" require it, and it wouldn't make either one significantly easier.

... but getting feedback and iterating on the feature is the motivation behind experiments and user testing, and exposing the prefs on non-release branches would make that significantly easier for the power users and early adopters that we need for that feedback. Flipping an exposed pref is much easier than adding a new one. No change to default values, and we'd still be hidden in about:config, behind a clear warning that the user might break things.

Looks like we've been doing something similar with Media Source Extensions - https://dxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#429. Until recently, it was pref'd off entirely, but the pref exposed, on release builds.

Comment 11

[Steve Workman [:sworkman] (INACTIVE)]

Gavin, please consider the arguments in comment #10. This is an opportunity to get more feedback and make the feature better for later release.

Comment 12

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

We need to balance "getting more feedback" with "drawing more attention to the feature before it's ready" and "implying a particular release schedule", which can have negative effects on our ability to ship it in the long term. The way we've decided to do that is by making exposed UI be Nightly-only for the moment.

Comment 13

[[:mmc] Monica Chew (no longer reading bugmail)]

Someone has updated the SUMO article to reference privacy.trackingprotection.enabled which works in all channels, so this pref doesn't make sense anymore anyway.

https://support.mozilla.org/en-US/kb/tracking-protection-firefox