Closed Bug 1339754 Opened 8 years ago Closed 7 years ago

Compact themes are not applied after restart if you try to enable them with a complete theme installed

Categories

(Toolkit :: Add-ons Manager, defect, P5)

defect

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- affected
firefox54 --- affected

People

(Reporter: cgeorgiu, Unassigned)

References

Details

(Whiteboard: triaged)

[Affected versions]: - latest Nightly 54.0a1 (2017-02-14) - latest Aurora 53.0a2 (2017-02-14) [Affected platforms]: - Windows 10 x64 - macOS 10.12 (Sierra) - Ubuntu 16.04 x64 LTS [Steps to reproduce]: 1. Launch Firefox 2. Go to: https://addons.mozilla.org/En-us/firefox/complete-themes/ and install a complete theme, e.g "LavaFox V2" 3. Restart the browser in order to install the complete theme 4. Go to about:addons and select "Appearance" (if it's not already in focus) 5. Enable Compact Dark theme and click "Restart now" to apply the selected theme [Expected result]: - Compact Dark theme is enabled after restart and the other themes are displayed as Disabled [Actual result]: - Compact Dark theme is not enabled and all themes are displayed as Disabled [Regression range]: - This is not a regression as it is reproducible on Nightly 53.0a1 (20170115030210) where this feature first landed [Additional notes]: - both Compact Dark and Compact Light are affected - see screenshot with about:addons after browser restart: http://imgur.com/a/MNl2N
Has STR: --- → yes
Component: Theme → Add-ons Manager
Product: Firefox → Toolkit
Does this affect other lightweight themes besides the compact ones we're now shipping? Andrew, do you have time to look at this?
Flags: needinfo?(ciprian.georgiu)
Flags: needinfo?(aswan)
(In reply to :Gijs from comment #1) > Does this affect other lightweight themes besides the compact ones we're now > shipping? No, it only affects compact themes.
Flags: needinfo?(ciprian.georgiu)
This seems to be an old issue which reproduces constantly on Nightly and Dev Edition branches. See Bug 1194638.
(In reply to :Gijs from comment #1) > Andrew, do you have time to look at this? I'll put it in my queue, hopefully can get to it this week...
(In reply to Andrew Swan [:aswan] from comment #5) > (In reply to :Gijs from comment #1) > > Andrew, do you have time to look at this? > > I'll put it in my queue, hopefully can get to it this week... That would be great. While I do not think this should block compact themes ( not many people use complete themes ) I would support this fix being uplifted to 53. Please needinfo me if you need help with that.
Priority: -- → P3
I think the basic problem here is that when we switch from a complete them to a lightweight theme, the code that applies the change after a restart is here: http://searchfox.org/mozilla-central/rev/4039fb4c5833706f6880763de216974e00ba096c/toolkit/mozapps/extensions/LightweightThemeManager.jsm#328 Now, the add-ons manager itself is started indirectly from, of all things, nsXREDirProvider::DoStartup, which gets called from here: http://searchfox.org/mozilla-central/rev/4039fb4c5833706f6880763de216974e00ba096c/toolkit/xre/nsAppRunner.cpp#4307 But built-in themes don't get added to the LWT manager until the final-ui-startup event which happens a bit later: http://searchfox.org/mozilla-central/rev/4039fb4c5833706f6880763de216974e00ba096c/toolkit/xre/nsAppRunner.cpp#4307 The simplest fix would be to somehow move the addition of built-in lightweight themes earlier but I'm in over my head on the whole startup sequence. We could also add code to the LWT manager to handle this sequence (ie, if getUsedTheme() returns null, remember the theme we want to apply and then apply it in addBuiltinTheme() ) but that sounds gross to me. I could use some guidance here...
Flags: needinfo?(aswan) → needinfo?(dtownsend)
I think what you want to do is to move the calls to addBuiltinTheme to just before the add-ons manager gets started up. You can add a listener for the profile-do-change topic to browserblue's init function (https://dxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#389). profile-do-change is called immediately before the add-ons manager startup (http://searchfox.org/mozilla-central/source/toolkit/xre/nsXREDirProvider.cpp#1150). At first glance that should all be safe but you might find you have to tinker with the LWT code to make sure all the data structures it needs for those calls to succeed are initialised early enough.
Flags: needinfo?(dtownsend)
Whiteboard: triaged
Complete themes are deprecated and will no longer be loaded in Firefox 57. From what I can see so far, this looks like quite a tricky bug for just a few releases. The number of complete theme users is small, is this worth the effort at this stage? Suggesting moving down to a P5 or won't fixing.
(In reply to Andy McKay [:andym] from comment #10) ... > Suggesting moving down to a P5 or won't fixing. I ust assumed P3 would relegate this to the ages.
Priority: P3 → P5
We no longer support complete themes -> wontfix.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.