1061747 - Restore Defaults does not reset the current theme

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

STR:
Open customization mode
Choose a lightweight theme from the recommended themes list
Click on "Restore Defaults"

Expected:
The theme is switched back to Default

Actual:
The theme is unchanged

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Note also that when this bug is fixed, "Undo Restore Default" will also need to be supported that restores the previously selected theme.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Katie, would you like to take this bug next?

Comment 3

[Katie Broida [:ktbee]]

Sure thing!

Comment 4

[Katie Broida [:ktbee]]

Attachment: MozReview Request: Bug 1061747 - Makes the "Restore Defaults" button reset the browser theme. Also adds test to check whether this button resets to the default theme successfully.r?jaws

Review commit: https://reviewboard.mozilla.org/r/51423/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51423/

Comment 5

[Katie Broida [:ktbee]]

I've submitted a patch for making sure the "Restore Defaults" button resets the browser theme to the default. However, I noticed another place a fix might be needed. If you enter customize mode and only change the theme, you're not able to click "Restore Defaults." This button is only available once you change another unrelated setting. Should I add to this patch to make sure that changing the theme will allow you to use the "Restore Defaults" button? Or should that be a separate bug report?

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8750452 [details]
MozReview Request: Bug 1061747 - Makes the "Restore Defaults" button reset the browser theme. Also adds test to check whether this button resets to the default theme successfully.r?jaws

https://reviewboard.mozilla.org/r/51423/#review48259

Looks good, and thanks for updating the test. We should also restore the previous theme if the user hits "Undo" after the Restore Defaults. See browser_970511_undo_restore_defaults.js for the test that will need updating too.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Katie Broida from comment #5)
> Should I add to this patch to
> make sure that changing the theme will allow you to use the "Restore
> Defaults" button? Or should that be a separate bug report?

That should be part of this patch.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

In CustomizableUI.jsm, you can follow how state is added to gUIStateBeforeReset so "Undo" can re-set it.

Comment 9

[Katie Broida [:ktbee]]

Comment on attachment 8750452 [details]
MozReview Request: Bug 1061747 - Makes the "Restore Defaults" button reset the browser theme. Also adds test to check whether this button resets to the default theme successfully.r?jaws

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51423/diff/1-2/

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8750452 [details]
MozReview Request: Bug 1061747 - Makes the "Restore Defaults" button reset the browser theme. Also adds test to check whether this button resets to the default theme successfully.r?jaws

https://reviewboard.mozilla.org/r/51423/#review51086

Looks great! Sorry I didn't get to this sooner.

Comment 11

[Carsten Book [:Tomcat]]

seems there are problems to apply:

applying 6962e21b4433
patching file browser/components/customizableui/CustomizableUI.jsm
Hunk #2 FAILED at 147
1 out of 6 hunks FAILED -- saving rejects to file browser/components/customizableui/CustomizableUI.jsm.rej
patching file browser/components/customizableui/CustomizeMode.jsm
Hunk #3 FAILED at 1456
1 out of 3 hunks FAILED -- saving rejects to file browser/components/customizableui/CustomizeMode.jsm.rej
patching file browser/components/customizableui/test/browser_1007336_lwthemes_in_customize_mode.js
Hunk #1 FAILED at 46
1 out of 1 hunks FAILED -- saving rejects to file browser/components/customizableui/test/browser_1007336_lwthemes_in_customize_mode.js.rej
patch failed to apply
abort: fix up the working directory and run hg transplant --continue

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/a11b2a2868026b88fe75827f19719ade76eca1bd
Bug 1061747 - Restore Defaults should reset the current theme. r=jaws

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a11b2a286802

Comment 14

[Mihai Boldan, Desktop QA [:mboldan]]

I managed to reproduce this issue on Firefox 48.0b1 and on Windows 10 x64.
The issue is no longer reproducible on Firefox 49.0a2 (2016-06-16) or on Firefox 50.0a1 (2016-06-15).
The tests were performed on Windows 10 x64, Mac OS X 10.11.1 and on Ubuntu 16.04 x64.