Closed Bug 1082108 Opened 10 years ago Closed 10 years ago

Add a way to toggle the devedition theme from customize mode

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 36
Tracking Status
firefox35 --- verified
firefox36 --- verified

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Attached patch devedition-customize.patch (obsolete) — Splinter 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
Status: NEW → ASSIGNED
Attachment #8504254 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8504254 [details] [diff] [review]
devedition-customize.patch

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/customizeMode.inc.css
@@ +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+
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: https://tbpl.mozilla.org/?tree=Try&rev=a18346db5ee2.
Attachment #8504254 - Attachment is obsolete: true
Attachment #8504824 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1d1e647773f1
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
Comment on attachment 8504824 [details] [diff] [review]
devedition-customize.patch

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?
Attachment #8504824 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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: RESOLVED → VERIFIED
Depends on: 1094138
Group: mozilla-employee-confidential
You need to log in before you can comment on or make changes to this bug.