Closed Bug 1122132 Opened 9 years ago Closed 9 years ago

remove #ifdef NIGHTLY from polaris prefs

Categories

(Core :: DOM: Security, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: mmc, Assigned: mmc)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Assignee: nobody → mmc
Status: NEW → ASSIGNED
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.
Attachment #8549780 - Flags: review?(gavin.sharp)
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.
Attachment #8549780 - Flags: review?(gavin.sharp) → review-
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #3)
> My understanding of bug 1090535

I meant bug 1119891.
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
We can easily give Bill branded try builds from other channels with ifdefs removed to solve that problem.
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.
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?
Flags: needinfo?(gavin.sharp)
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.
Flags: needinfo?(gavin.sharp)
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] 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.
Gavin, please consider the arguments in comment #10. This is an opportunity to get more feedback and make the feature better for later release.
Flags: needinfo?(gavin.sharp)
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.
Flags: needinfo?(gavin.sharp)
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
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: