1082584 - Pref on Developer Edition theme

Status: RESOLVED DUPLICATE of bug 1072181

(Firefox :: Theme, defect)

Description

[Brian Grinstead [:bgrins]]

Once the theme lands and is uplifted, we should uplift a patch to the Developer Edition channel's firefox.js that deafaults the following prefs to true:

pref("browser.devedition.theme.enabled", true);
pref("browser.devedition.theme.showCustomizeButton", true);

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: devedition-theme-defaults.patch

Patch that updates Aurora branding to set defaults

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

We can land this now in m-c, right? I think it would be best to land stuff as early as possible in m-c/fx-team to make sure nothing breaks, uplift all the disabled-by-default stuff ASAP and then file a bug to coordinate the uplift of a specific set of patches that alter the behavior of Aurora.

Comment 3

[Joe Walker [:jwalker]]

(In reply to Panos Astithas [:past] (overloaded, please needinfo) from comment #2)
> We can land this now in m-c, right? I think it would be best to land stuff
> as early as possible in m-c/fx-team to make sure nothing breaks, uplift all
> the disabled-by-default stuff ASAP and then file a bug to coordinate the
> uplift of a specific set of patches that alter the behavior of Aurora.

That would mean that anyone on Nightly would see the new theme, right - The 'turn on' patches should probably stay on Gum and land on Nightly/Aurora at the same time.

Comment 4

[Joe Walker [:jwalker]]

Comment on attachment 8509595 [details] [diff] [review]
devedition-theme-defaults.patch

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

This patch will conflict with https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1072181&attachment=8510155 It's not a hard problem to solve, but maybe we should have a dependency to help anyone wanting to test?

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

So will the patch from bug 1080082. I could probably pick an order when I rebase them for pushing to gum and set the bug dependencies accordingly?

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Joe Walker [:jwalker] (overloaded - needinfo me or ping on irc) from comment #3)
> (In reply to Panos Astithas [:past] (overloaded, please needinfo) from
> comment #2)
> > We can land this now in m-c, right? I think it would be best to land stuff
> > as early as possible in m-c/fx-team to make sure nothing breaks, uplift all
> > the disabled-by-default stuff ASAP and then file a bug to coordinate the
> > uplift of a specific set of patches that alter the behavior of Aurora.
> 
> That would mean that anyone on Nightly would see the new theme, right - The
> 'turn on' patches should probably stay on Gum and land on Nightly/Aurora at
> the same time.

This could land on m-c since the branding changes are limited to the Aurora release.  We would just want to hold off on an uplift until it's ready to be seen.  I'm not sure about the ordering of uplifts, but my gut feeling is that the pref uplifts should be last and the preffed off features should uplift ASAP.  This way we can test the changes on Aurora by manually flipping the prefs, which seems safer because a simple pref uplift is a lot less likely to go wrong than some of the features we are working on.

Comment 7

[Brian Grinstead [:bgrins]]

Comment on attachment 8509595 [details] [diff] [review]
devedition-theme-defaults.patch

Here are the branding changes needed to turn on the devedition theme

Comment 8

[Brian Grinstead [:bgrins]]

(In reply to Panos Astithas [:past] (overloaded, please needinfo) from comment #5)
> So will the patch from bug 1080082. I could probably pick an order when I
> rebase them for pushing to gum and set the bug dependencies accordingly?

Do we actually need these individual pref patches when the patch from bug 1072181 looks to include all of them?  https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1072181&attachment=8510155

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

I'm happy to incorporate all pref changes in that patch if you want.

Comment 10

[:Gijs (he/him)]

Comment on attachment 8509595 [details] [diff] [review]
devedition-theme-defaults.patch

rs=me for the code...

...but I thought we were going to do something next to Aurora, based on the aurora codebase, with different branding. Will this now fully replace Aurora? Where was this decision taken/discussed?

(in other words: I don't think we should flip the default for the normal aurora users if we keep them instead of 'forcing' them to use the 'dev edition' browser or beta/nightly)

Comment 11

[Brian Grinstead [:bgrins]]

(In reply to Panos Astithas [:past] (overloaded, please needinfo) from comment #9)
> I'm happy to incorporate all pref changes in that patch if you want.

It looks like it already has devedition.theme.enabled, so it should either include browser.devedition.theme.showCustomizeButton as well or not have either one.

I guess the question is if there would be value in having finer grained control for preffing things on.  Seems like keeping them separate may make backing out one particular thing easier, but then again it makes rebasing and uplifting more complicated with the possibility of conflicts.  So if doing it in one patch in Bug 1072181 is easiest then we should probably include everything there (including the stuff here and the feedback url change in bug 1080082).

Comment 12

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: devedition-feedback-url-1080082.patch

Adding patch from bug 1080082 at jwalker's request.

Comment 13

[Paul Rouget [:paul]]

Attachment: More prefs.

These prefs require bug 1067337 and bug 1056923 to land in aurora.

Comment 14

[:Gijs (he/him)]

As I've said elsewhere, I think we should confirm with jmaher and other talos-watching folks how to ensure consistent talos results - probably by ensuring the default talos profiles pref off the devedition-specific stuff.

Comment 15

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: devedition-theme-defaults.patch

jwalker asked me to merge the feedback URL fix from bug 1080082.

Comment 16

[Panos Astithas (he/him) [:past] (please ni?)]

Putting the feedback URL in the branding overrides is probably fine, but for the rest Gavin suggested in bug 1072181 that we should #ifdef them in firefox.js instead. That patch already includes Brian's prefs, so you should just land the feedback URL change in fx-team and we'll do the uplift at the last minute.

Comment 17

[Tim Nguyen :ntim]

Comment on attachment 8514111 [details] [diff] [review]
More prefs.

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

::: browser/branding/aurora/pref/firefox-branding.js
@@ +40,5 @@
>  
>  // Developer Edition defaults
>  pref("browser.devedition.theme.enabled", true);
>  pref("browser.devedition.theme.showCustomizeButton", true);
> +pref("devtools.timeline.enabled", true);

The storage inspector and the toolbox frames button seem worth to turn on as well.