STR: 1. Open about:addons in Nightly, determined to try out the new compact themes from bug 1314091. 2. Click "Enable" several times on various themes (default, compact-dark, compact-light), somewhat randomly, to see how they look and compare 3. Open devtools to see how the compact themes look there 4. Start switching between the themes again Actual results: After performing these somewhat chaotic steps for a while, a notice saying that "Compact Light will be enabled after you restart Nightly" appeared on the compact-light theme, and I cannot enable it again. See attached screenshot. I assume that things would get back in order after Firefox restart, but the restart shouldn't be required in the first place, should it?

(In reply to Jarda Snajdr [:jsnajdr] from comment #0) > I assume that things would get back in order after Firefox restart, but the > restart shouldn't be required in the first place, should it? No. Can you get more reliable steps to reproduce ?

(In reply to :Gijs from comment #1) > No. Can you get more reliable steps to reproduce ? I recorded a short screencast here: http://recordit.co/iQSculicwu I'm switching between the themes with devtools opened. The green "restart needed" label appears almost on every switch for a fraction of a second (not visible on the screencast, as it seems to have a low FPS). Sometimes, it stays displayed for several seconds - that's what happens near the end of the screencast - the label was there, and then disappeared. I didn't click on anything.

Does this reproduce on a clean profile? Is this a debug build? I can't reproduce. :-\

I use the official Nightly build. On a clean profile, the bug doesn't reproduce. This is my list of addons when it happens: ADB Helper Valence RDP Inspector Test Pilot Min Vid No More 404s AdBlock Plus I was unable to reduce the list to something shorter. I did some debugging and found this: - when a theme is being disabled, two events are dispatched: "onDisabling" and "onDisabled". Between these two events, a "need to restart to disable" warning is displayed. - when a theme is being enabled, another two events: "onEnabling" and "onEnabled". Between these two events, a "need to restart to enable" is displayed. Most of the time, these labels are not shown. Sometimes, the "onEnabling" label is displayed for several seconds before disappearing. Two or three times (out of hundreds of samples), the label stayed displayed forever. This happened even with devtools closed, so devtools are probably not needed to reproduce. This is all I can find, sorry. Feel free to close the bug as worksforme/incomplete if nobody else is able to reproduce.

(In reply to Jarda Snajdr [:jsnajdr] from comment #4) > I use the official Nightly build. On a clean profile, the bug doesn't > reproduce. > > This is my list of addons when it happens: > ADB Helper > Valence > RDP Inspector > Test Pilot > Min Vid > No More 404s > AdBlock Plus > > I was unable to reduce the list to something shorter. > > I did some debugging and found this: > - when a theme is being disabled, two events are dispatched: "onDisabling" > and "onDisabled". Between these two events, a "need to restart to disable" > warning is displayed. > - when a theme is being enabled, another two events: "onEnabling" and > "onEnabled". Between these two events, a "need to restart to enable" is > displayed. > > Most of the time, these labels are not shown. > Sometimes, the "onEnabling" label is displayed for several seconds before > disappearing. > Two or three times (out of hundreds of samples), the label stayed displayed > forever. > > This happened even with devtools closed, so devtools are probably not needed > to reproduce. > > This is all I can find, sorry. Feel free to close the bug as > worksforme/incomplete if nobody else is able to reproduce. I think this is fair enough, even if I can't repro, we just shouldn't be showing that notification at all when switching between a theme that supports lwthemes and different lwthemes. The add-on manager has enough information for this. It might still end up wontfix given that we're going to be messing with themes anyway... but let's ask first. :-) Andrew, am I right in thinking you're doing stuff with the addons manager these days? Any idea what's going on here?

Oh boy, themes are a dark corner of the add-ons manager that I don't understand well. But even with the new theme API work, a bunch of the logic involved in this bug (e.g., deactivating the current theme when enabling a new theme) isn't going anywhere so lets get to the bottom of this. I think these notifications come from the onDisabling and onEnabling events, based on the "needsRestart" flag. But my first big question is why we're getting these events in the first place. "Compact Light" and "Compact Dark" are built-in themes, they are not actually stored in XPIs, so we shouldn't be getting those events. I'm unable to reproduce this but Jarda, it sounds like you can and like you are proficient with the Browser Toolbox. Can you capture a full stack trace for an onDisabling or onEnabling event on a non-default theme? Also cc'ing Mossop to straighten me out in case I'm completely barking up the wrong tree...

Tweaking the summary to make it clearer that the theme is immediately applied - it's just the messaging that's wrong.

(In reply to Andrew Swan [:aswan] from comment #6) > I'm unable to reproduce this but Jarda, it sounds like you can and like you are > proficient with the Browser Toolbox. Can you capture a full stack trace for an > onDisabling or onEnabling event on a non-default theme? This is what I got after setting a breakpoint in the _updateState method in extensions.xml: _updateState extensions.xml:1180 onDisabling extensions.xml:1609 delegateAddonEvent extensions.js:622 initialize/this[event] extensions.js:509 set_userDisabled extensions.xml:1085 oncommand about:addons:1 _updateState extensions.xml:1180 onDisabled extensions.xml:1615 delegateAddonEvent extensions.js:622 initialize/this[event] extensions.js:509 set_userDisabled extensions.xml:1085 oncommand about:addons:1 _updateState extensions.xml:1180 onEnabling extensions.xml:1597 delegateAddonEvent extensions.js:622 initialize/this[event] extensions.js:509 set_userDisabled extensions.xml:1085 oncommand about:addons:1 _updateState extensions.xml:1180 onEnabled extensions.xml:1603 delegateAddonEvent extensions.js:622 initialize/this[event] extensions.js:509 set_userDisabled extensions.xml:1085 oncommand about:addons:1 The breakpoint hits four times when switching from one theme to another. Between hits 1-2, a "restart to disable" warning is shown. Between hits 3-4, a "restart to enable" is shown. I'm not sure how complete and useful these stack traces are. It's XBL code, and I don't know how much is the new debugger proficient working with that. Sorry it took me a few days to respond.

Paging Andrew for comment #8.

Hm. I have to admit that the relationship between LightweightThemeManager.jsm and XPIProvider.jsm and how they are meant to work together is making my head spin. It looks like the needsRestart flag is set to true if a particular pref has a non-default setting: http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/toolkit/mozapps/extensions/LightweightThemeManager.jsm#664-665 Can you check the value of general.skins.selectedSkin in about:config? (it may change as you select new themes...) But when I try this locally, that preference never seems to change, it seems that LightweightThemeManager uses the preference lightweightThemes.selectedThemeID. There is code in XPIProvider.jsm that sets the general.skins.selectedSkin preference but as I read it, it should only apply when you switch to a theme that is stored in an XPI, but (I don't think that) that is the case here. Can you grab the full list of extensions you have installed (from about:support) and include it as well as the answer to the above question?

(In reply to Andrew Swan [:aswan] from comment #10) > It looks like the needsRestart flag is set to true if a particular pref has > a non-default setting: > http://searchfox.org/mozilla-central/rev/ > 30fcf167af036aeddf322de44a2fadd370acfd2f/toolkit/mozapps/extensions/ > LightweightThemeManager.jsm#664-665 > Can you check the value of general.skins.selectedSkin in about:config? (it > may change as you select new themes...) general.skins.selectedSkin has the default value for me ("classic/1.0") and never changes. When setting a breakpoint on the line that evaluates needsRestart, it always evaluates to false. It's the "lightweightThemes.selectedThemeID" pref that stores the selected theme. Looking at about:config, I also see a pref named: services.sync.prefs.sync.lightweightThemes.selectedThemeID set to "true". I'm logged in to Firefox Sync in the profile where this happens. Maybe a clue? Setting this pref to "false" doesn't change anything though - when switching themes, the restart warnings still show up for a moment. > Can you grab the full list of extensions > you have installed (from about:support) and include it as well as the answer > to the above question? My list of extensions from about:support: Name: ADB Helper Version: 0.9.2 Enabled: true ID: adbhelper@mozilla.org Name: Adblock Plus Version: 2.8.2 Enabled: true ID: {d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d} Name: Application Update Service Helper Version: 1.0 Enabled: true ID: aushelper@mozilla.org Name: FlyWeb Version: 1.0.0 Enabled: true ID: flyweb@mozilla.org Name: Form Autofill Version: 1.0 Enabled: true ID: formautofill@mozilla.org Name: Min Vid Version: 0.3.4 Enabled: true ID: @min-vid Name: Multi-process staged rollout Version: 1.7 Enabled: true ID: e10srollout@mozilla.org Name: No More 404s Version: 1.7.0 Enabled: true ID: wayback_machine@mozilla.org Name: Pocket Version: 1.0.5 Enabled: true ID: firefox@getpocket.com Name: Presentation Version: 1.0.0 Enabled: true ID: presentation@mozilla.org Name: RDP Inspector Version: 1.2.5 Enabled: true ID: rdpinspector@getfirebug.com Name: SHA-1 deprecation staged rollout Version: 1.0 Enabled: true ID: disableSHA1rollout@mozilla.org Name: Shield Recipe Client Version: 1.0.0 Enabled: true ID: shield-recipe-client@mozilla.org Name: Test Pilot Version: 0.9.3-dev-e085559 Enabled: true ID: @testpilot-addon Name: Valence Version: 0.3.7 Enabled: true ID: fxdevtools-adapters@mozilla.org Name: Web Compat Version: 1.0 Enabled: true ID: webcompat@mozilla.org Name: WebCompat Reporter Version: 1.0.0 Enabled: true ID: webcompat-reporter@mozilla.org

so we are having a hard time replicating this, though it would be an issue - can someone from QA help with replication scenario steps

I have been unsuccessful in reproducing this issue following the steps (and, with all the add-ons installed) Ica, can you give it a try?

I was unable to reproduce this issue using a clean profile on Firefox 54.0a1 (2017-02-15) under Windows 10 64-bit. But based on the following paragraph mentioned in Comment 11 > set to "true". I'm logged in to Firefox Sync in the profile where this > happens. Maybe a clue? Setting this pref to "false" doesn't change anything > though - when switching themes, the restart warnings still show up for a > moment. I’m suspecting that this is a dupe of Bug 1205612 because I successfully reproduced this issue while being logged into Firefox Account - see screencast: https://www.screencast.com/t/lyvnCu1c4 So my guess is that this is a sync issue and not a compact theme specific bug. Jarda, could you please check if you see the same “Restart” message when enabling a different lightweight theme on the same profile? Such in the following screencast: https://www.screencast.com/t/FuV7wvSeH

(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #14) > Jarda, could you please check if you see the same “Restart” message when > enabling a different lightweight theme on the same profile? Such in the > following screencast: https://www.screencast.com/t/FuV7wvSeH Hi Vasilica, I tried out the "Symphony of Colors" theme and the restart message appears there, too. So it's not specific to the DevEdition themes. > So my guess is that this is a sync issue and not a compact theme specific > bug. I agree that a Sync delay is the most probable cause for this bug.

Thanks Jarda for your reply. Based on Comment 14 and Comment 15 I am marking this bug as Duplicate.

Removing the qawanted keyword since this bug was marked as duplicate.