Last Comment Bug 1061747 - Restore Defaults does not reset the current theme
: Restore Defaults does not reset the current theme
Status: VERIFIED FIXED
[outreachy-12]
:
Product: Firefox
Classification: Client Software
Component: Toolbars and Customization (show other bugs)
: 34 Branch
: All All
-- normal (vote)
: Firefox 49
Assigned To: Katie Broida [:ktbee]
:
: :Gijs
Mentors: Jared Wein [:jaws] (please needinfo? me)
Depends on: 1306561 1306584
Blocks:
  Show dependency treegraph
 
Reported: 2014-09-02 08:38 PDT by Jared Wein [:jaws] (please needinfo? me)
Modified: 2016-10-07 02:29 PDT (History)
4 users (show)
jaws: firefox‑backlog+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: 3
Has Regression Range: ---
Has STR: ---
affected
affected
verified
verified

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
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 (58 bytes, text/x-review-board-request)
2016-05-09 13:24 PDT, Katie Broida [:ktbee]
jaws: review+
Details | Review

Description User image Jared Wein [:jaws] (please needinfo? me) 2014-09-02 08:38:12 PDT
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 User image Jared Wein [:jaws] (please needinfo? me) 2014-09-02 08:39:54 PDT
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 User image Jared Wein [:jaws] (please needinfo? me) 2016-05-02 09:07:33 PDT
Katie, would you like to take this bug next?
Comment 3 User image Katie Broida [:ktbee] 2016-05-02 09:18:41 PDT
Sure thing!
Comment 4 User image Katie Broida [:ktbee] 2016-05-09 13:24:14 PDT
Created 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 commit: https://reviewboard.mozilla.org/r/51423/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51423/
Comment 5 User image Katie Broida [:ktbee] 2016-05-09 13:28:19 PDT
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 User image Jared Wein [:jaws] (please needinfo? me) 2016-05-09 17:31:10 PDT
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 User image Jared Wein [:jaws] (please needinfo? me) 2016-05-09 19:46:17 PDT
(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 User image Jared Wein [:jaws] (please needinfo? me) 2016-05-10 06:04:33 PDT
In CustomizableUI.jsm, you can follow how state is added to gUIStateBeforeReset so "Undo" can re-set it.
Comment 9 User image Katie Broida [:ktbee] 2016-05-17 15:14:35 PDT
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 User image Jared Wein [:jaws] (please needinfo? me) 2016-05-22 08:37:26 PDT
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 User image Carsten Book [:Tomcat] 2016-05-24 09:32:53 PDT
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 User image Jared Wein [:jaws] (please needinfo? me) 2016-05-24 17:07:39 PDT
https://hg.mozilla.org/integration/fx-team/rev/a11b2a2868026b88fe75827f19719ade76eca1bd
Bug 1061747 - Restore Defaults should reset the current theme. r=jaws
Comment 13 User image Carsten Book [:Tomcat] 2016-05-25 13:16:05 PDT
https://hg.mozilla.org/mozilla-central/rev/a11b2a286802
Comment 14 User image Mihai Boldan, QA [:mboldan] 2016-06-16 06:35:22 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.