Add a way to toggle the devedition theme from customize mode

VERIFIED FIXED in Firefox 35

Status

()

Firefox
Theme
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: bgrins, Assigned: bgrins)

Tracking

Trunk
Firefox 36
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox35 verified, firefox36 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

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.
(Assignee)

Comment 1

3 years ago
Created attachment 8504254 [details] [diff] [review]
devedition-customize.patch

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 2

3 years ago
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+
(Assignee)

Comment 3

3 years ago
Created attachment 8504824 [details] [diff] [review]
devedition-customize.patch

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+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
(Assignee)

Comment 4

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/1d1e647773f1
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/1d1e647773f1
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox36: --- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
(Assignee)

Comment 6

3 years ago
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?
status-firefox35: --- → affected
Attachment #8504824 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/c35d9f79f1ff
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: RESOLVED → VERIFIED
status-firefox35: fixed → verified
status-firefox36: fixed → verified

Updated

3 years ago
Depends on: 1094138
(Assignee)

Updated

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.