Add a way to toggle the devedition theme from customize mode

VERIFIED FIXED in Firefox 35



3 years ago
2 years ago


(Reporter: bgrins, Assigned: bgrins)


Firefox 36
Dependency tree / graph

Firefox Tracking Flags

(firefox35 verified, firefox36 verified)



(1 attachment, 1 obsolete attachment)



3 years ago
As discussed in Bug 1053434, we need to have a way to toggle the devedition theme from customize mode.

I've talked with shorlander about this, and he suggests to add a checkbox button next to the themes dropdown that allows this to be toggled.  This button will only be visible by default in the devedition channel.

Comment 1

3 years ago
Created attachment 8504254 [details] [diff] [review]

Here are the changes that I think are necessary to add this button the customize page.  You can see it by setting browser.devedition.theme.showCustomizeButton to true, and the button will control the browser.devedition.theme.enabled pref.  I've modeled the changes, including 'restore defaults' and 'undo', off of the 'Title Bar' button since it is very similar.  Note that the text of the button says "Use Nightly Theme", but this branding name will be changed for the dev edition channel (which is the only place the button will show up by default).

Please reassign the review if you think there is someone else who should do it.
Assignee: nobody → bgrinstead
Attachment #8504254 - Flags: review?(gijskruitbosch+bugs)

Comment 2

3 years ago
Comment on attachment 8504254 [details] [diff] [review]

Review of attachment 8504254 [details] [diff] [review]:

::: browser/components/customizableui/test/browser_970511_undo_restore_default.js
@@ +121,5 @@
> +
> +  yield gCustomizeMode.reset();
> +  ok(restoreDefaultsButton.disabled, "Restore defaults button should be disabled after reset");
> +  is(deveditionThemeButton.hasAttribute("checked"), defaultValue, "devedition theme button should reflect default value after reset");
> +  is(Services.prefs.getBoolPref(prefName), defaultValue, "Reset should reset drawInTitlebar");

Nit: forgot to update this string

::: browser/themes/shared/customizableui/
@@ +129,5 @@
>                inset 0 1px rgba(255, 255, 255, 0.5);
>    -moz-appearance: none;
>  }
> +.customizationmode-button[checked] {

Have you checked that this overrides all the requisite styles on Windows/Linux?

It would be slightly less risky to split the existing rule and use two id+attribute selectors instead of this class+attribute selector, as it doesn't change specificity...
Attachment #8504254 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 3

3 years ago
Created attachment 8504824 [details] [diff] [review]

Luckily the test harness caught a leak in linux because the uninit function in CustomizeMode.jsm was only being called if CAN_DRAW_IN_TITLEBAR was true.  I've updated the patch based on your comments and also made sure the unload handler is observed in all cases, not just windows / osx.  New try push:
Attachment #8504254 - Attachment is obsolete: true
Attachment #8504824 - Flags: review+


3 years ago
Keywords: checkin-needed

Comment 4

3 years ago
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Last Resolved: 3 years ago
status-firefox36: --- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36

Comment 6

3 years ago
Comment on attachment 8504824 [details] [diff] [review]

Approval Request Comment
[Feature/regressing bug #]: Developer Edition Channel
[User impact if declined]: We would like to uplift this addition to the Customize UI as it provides the UI to toggle off the Developer Edition theme (1053434) for the Aurora channel
[Describe test coverage new/current, TBPL]: Test coverage for this button (and restore default/undo functionality) is added in customizableui tests
[Risks and why]:  The button wouldn't make sense on other channels.  However, it will not show up unless if a pref (browser.devedition.theme.showCustomizeButton) is set to true.  This pref will only defaulted to true on the aurora channel (in bug 1082584).
[String/UUID change made/needed]:
Attachment #8504824 - Flags: approval-mozilla-aurora?
status-firefox35: --- → affected
Attachment #8504824 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox35: affected → fixed
Confirming that changing the "browser.devedition.theme.showCustomizeButton" to true will display the button in Customize Mode. The devedition theme is correctly disabled/enabled by the button in Firefox Developer Edition.

Verified on:
- Firefox Developer Edition, build ID: 20141028155607
- Latest Nightly, build ID: 20141029030207
status-firefox35: fixed → verified
status-firefox36: fixed → verified


3 years ago
Depends on: 1094138


3 years ago
Group: mozilla-employee-confidential
Depends on: 1271626
You need to log in before you can comment on or make changes to this bug.