Open Bug 1306561 Opened 3 years ago Updated 2 years ago

Restore Defaults button is enabled and has undesired effects on a clean profile using Aurora

Categories

(Firefox :: Toolbars and Customization, defect)

51 Branch
defect
Not set

Tracking

()

REOPENED
Firefox 53
Tracking Status
firefox51 --- wontfix
firefox52 --- wontfix
firefox53 --- fixed

People

(Reporter: JuliaC, Assigned: dao)

References

Details

(Keywords: regression)

Attachments

(1 file)

[Note]:
- This issue only affects Aurora builds

[Affected versions]:
- latest Aurora 51.0a2 (2016-09-29)
- Aurora 50.0a2
- Aurora 49.0a2 

[Affected platforms]:
- Windows 10 x64
- Ubuntu 16.04 x86
- Mac OS X 10.11.6

[Steps to reproduce]:
1. Launch Firefox with a clean profile
2. Go to about:addons and choose the Appearance tab
  - inspect each displayed theme state
3. Open the Menu Panel, click the Customize button and then open the Themes menu
  - inspect My Themes section
4. Check the state of the Restore Defaults button and then click it
  - inspect each displayed theme state from about:addons -> Appearance tab
  - inspect My Themes section

[Expected result]:
- [step 2] and [step 3] Each theme state is clearly revealed: the Developer Edition Theme is displayed like being enabled and the Default one has the disabled state
- [step 4] The Restore Defaults button is disabled

[Actual result]:
- [step 2] and [step 3] The Default and Developer Edition themes are displayed like both are being enabled in that moment 
- [step 4] The Restore Defaults button is enabled, even if the profile is new and there are no changes / customization made
- [step 5] * The Developer Edition theme is replaced by the Default one
           * Only one theme (the Default one) is now displayed like being enabled 

[Regression range]:
- Last good revision: 5c8f37e61ac7af86accd5d86f6c9d10cd0eadf64
- First bad revision: cea65ca3d0bdd8325dc556f6bfde6277fc83fe6e
- Pushlog:
https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=5c8f37e61ac7af86accd5d86f6c9d10cd0eadf64&tochange=cea65ca3d0bdd8325dc556f6bfde6277fc83fe6e
- Possible regressor - bug https://bugzilla.mozilla.org/show_bug.cgi?id=1061747

[Additional notes]:
- Before 49.0a2 (2016-06-06), both themes still seems enabled, the Restore Defaults button is still enabled, but clicking the Restore Defaults button doesn't switch the themes anymore
Depends on: 1306584
Blocks: 1061747
Component: Theme → Toolbars and Customization
Depends on: 1164954
Hi :dolske,
Since it's related to Toolbars, can you help shed some light here or find an owner for this?
Flags: needinfo?(dolske)
See 1164954 dep.
Flags: needinfo?(dolske)
Has bug 1164954 fully fixed this or is there something left to do here?
Flags: needinfo?(iulia.cristescu)
(In reply to Dão Gottwald [:dao] from comment #3)
> Has bug 1164954 fully fixed this or is there something left to do here?

Hello! Bug 1164954 only fixes the both themes selection problem, but the Restore Defaults button is still enabled, even if the profile is new and there are no changes / customization made.
Flags: needinfo?(iulia.cristescu)
:dolske, I tagged this as fix-optional, please make a change if we must have the fix.
Flags: needinfo?(dolske)
Is fixing this just a question of changing the default lwtheme pref to match devedition on those builds where we use it by default?
Per comment 4/5, sounds like there is a bit more to do here?
Flags: needinfo?(dolske) → needinfo?(dao+bmo)
(Err, comment 3/4, I meant.)
Assignee: nobody → dao+bmo
Flags: needinfo?(dao+bmo)
Comment on attachment 8825784 [details]
Bug 1306561 - Make the Restore Default button take into account that the devedition theme is the default in devedition builds.

https://reviewboard.mozilla.org/r/103868/#review104744

Note that this is in a race with bug 1314091.
Attachment #8825784 - Flags: review?(jaws) → review+
Comment on attachment 8825784 [details]
Bug 1306561 - Make the Restore Default button take into account that the devedition theme is the default in devedition builds.

https://reviewboard.mozilla.org/r/103868/#review104744

Also, it would be nice if you updated browser_970511_undo_restore_default.js to cover this case.
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> Note that this is in a race with bug 1314091.

Hmm, I don't see any of those patches touch CustomizableUI.jsm.

> Also, it would be nice if you updated browser_970511_undo_restore_default.js
> to cover this case.

lightweightThemes.selectedThemeID needs to be non-empty by default in order to test this. I don't see how to do that in a browser-chrome test.
Flags: in-testsuite-
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b3761531b00b
Make the Restore Default button take into account that the devedition theme is the default in devedition builds. r=jaws
https://hg.mozilla.org/mozilla-central/rev/b3761531b00b
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
(In reply to Dão Gottwald [:dao] from comment #13)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> > Note that this is in a race with bug 1314091.
> 
> Hmm, I don't see any of those patches touch CustomizableUI.jsm.
> 
> > Also, it would be nice if you updated browser_970511_undo_restore_default.js
> > to cover this case.
> 
> lightweightThemes.selectedThemeID needs to be non-empty by default in order
> to test this. I don't see how to do that in a browser-chrome test.

Assign to the default pref branch using Services.prefs.getDefaultBranch("blah").setCharPref(...);

(and unset in a cleanup function, of course)

Please can we reconsider the in-testsuite- here?
Flags: needinfo?(dao+bmo)
Flags: needinfo?(dao+bmo)
Flags: in-testsuite-
Is this worth uplifting to Aurora? If so, please nominate.
Flags: needinfo?(dao+bmo)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #17)
> Is this worth uplifting to Aurora? If so, please nominate.

No, Aurora will soon become Beta where this bug doesn't apply.
Flags: needinfo?(dao+bmo)
Depends on: 1332612
I backed this out of inbound for causing bug 1332612 (mass bc CUI test orange) which would block the Monday merge to aurora.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Backout by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc5a594e63f2
Backed out changeset b3761531b00b for perma-oranging a lot of tests once 53 merges to aurora, rs=bustage,me,etc.
(In reply to Iulia Cristescu, QA [:IuliaC] from comment #4)
> (In reply to Dão Gottwald [:dao] from comment #3)
> > Has bug 1164954 fully fixed this or is there something left to do here?
> 
> Hello! Bug 1164954 only fixes the both themes selection problem, but the
> Restore Defaults button is still enabled, even if the profile is new and
> there are no changes / customization made.

This bug hasn't been fixed completely, as Iulia and Justin already mentioned in Comment 4 and Comment 8. 

There are now _2 issues_ related to what has been reported at first in Comment 0:

    1. the [Restore Defaults] button is active by default on a new profile
       
         * expected: the button should be disabled by default if there's nothing to restore (new profile)
         * affected: Developer Edition builds, including 20170505205544
         * unaffected: Nightly 55.0a1 (2017-05-09), 54.0b6-build1 (20170508210007), 53.0.2 (20170504175505)


    2. using the [Restore Defaults] button on Developer Edition builds resets the theme to 'Default' instead 
       of 'Compact Dark'

         * expected: Developer Edition builds should be reset to 'Compact Dark' as far as the theme is
           concerned
         * affected: all Developer Edition builds, including 20170505205544


Dao, would you like us to follow separate bugs for these?
Flags: needinfo?(dao+bmo)
No separate bug reports needed for now.
Flags: needinfo?(dao+bmo)
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: REOPENED → RESOLVED
Closed: 3 years ago2 years ago
Resolution: --- → INACTIVE
Status: RESOLVED → REOPENED
Resolution: INACTIVE → ---
You need to log in before you can comment on or make changes to this bug.