Closed Bug 1260596 Opened 4 years ago Closed 3 years ago

Themes panel is empty when all default-shipped lightweight themes (lwts) are removed

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 49
Tracking Status
firefox46 --- affected
firefox47 --- affected
firefox48 --- affected
firefox49 --- verified
firefox-esr38 --- affected
firefox-esr45 --- affected

People

(Reporter: arni2033, Assigned: jaws)

References

Details

(Whiteboard: Only aurora and nightly channels are unaffected)

Attachments

(2 files)

Read Note (1) immediately. Detailed STR in comment 1

STR_1:
1. Open Release/Beta, Enter Customize mode, apply all recomended themes 1 by 1
2. Open about:addons, delete all themes, close about:addons
3. Enter Customize mode, open Themes panel

AR:  Themes panel is empty!!!
ER:  The panel should display Default theme

Notes:    (ノ◕ヮ◕)ノ  *:・゚✧*:・゚✧
1) You can't reproduce Release/Beta effect on Nightly/DE because DE theme interferes
2) If you actually launch the same Nightly/DE profile in Release/Beta, it exposes the bug
>>>   My Info:   Win7_64, Nightly 48.0a1,    32bit, ID 20160326030430
>>>                       DevEdition 45.0a2, 32bit, ID 20151222004010
>>>                       Release 45.0.1,    32bit, ID 20160315153207
STR_1_detailed:
0. Open new profile OR delete all themes, open about:config?filter=them and reset those prefs
1. Open http://example.org/
2. Enter Customize mode
3. While there're recommended themes, do: Open Themes panel, then apply a recommended theme
4. Open Themes panel again to check what's in there, then close it
5. Exit Customize mode
6. Open about:addons -> Appearance, delete all themes. Close about:addons
7. Enter Customize mode
8. Open Themes panel

AR_detailed:
 Nightly/DE:   After Step 4 there's no Dev.Edition theme
 Release/Beta: After Step 8 Themes panel is empty!!!
 
ER:
 Nightly/DE:   After Step 4 Dev.Edition theme should be displayed in Themes panel
 Release/Beta: After Step 8 Themes panel should display Default theme
Summary: Themes panel looses items after some actions (sometimes it's empty) → Themes panel is empty when all default-shipped lightweight themes (lwts) are removed
(In reply to arni2033 from comment #1)
> 3. While there're recommended themes, do: Open Themes panel, then apply a
> recommended theme
> 4. Open Themes panel again to check what's in there, then close it

> AR_detailed:
>  Nightly/DE:   After Step 4 there's no Dev.Edition theme

We only show the last 5 themes that have been in use, mixed with the default themes, and the devedition theme is treated as just another lightweight theme, so this doesn't really strike me as a bug per se.
I can't reproduce with the steps from comment #1 on current nightly. Maybe this got fixed by the fix from bug 1260595? Or maybe I'm missing something? Arni, can you confirm if you're still seeing this?
Flags: needinfo?(arni2033)
(In reply to :Gijs Kruitbosch from comment #4)
> I can't reproduce with the steps from comment #1 on current nightly.
> Maybe this got fixed by the fix from bug 1260595?
It's nothing like bug 1260595, but it could easily be fixed by some intervention. In my view that bug only fixed the way how reset function is called, not the function itself...   From comment 0:
There's no way to test _this_ on Nightly, unless you know a way to delete dev theme.
But that way should be checked on Nightly 47- as well, to avoid false-positive results (obviously)

I knew there're bugs that are only reproducible on Release channel, and here it is. Quite scary.
Flags: needinfo?(arni2033)
Whiteboard: Only aurora and nightly channels are unaffected
(In reply to arni2033 from comment #5)
> (In reply to :Gijs Kruitbosch from comment #4)
> > I can't reproduce with the steps from comment #1 on current nightly.
> > Maybe this got fixed by the fix from bug 1260595?
> It's nothing like bug 1260595, but it could easily be fixed by some
> intervention. In my view that bug only fixed the way how reset function is
> called, not the function itself...   From comment 0:
> There's no way to test _this_ on Nightly, unless you know a way to delete
> dev theme.

Hm, but the steps from comment #1 (rather than comment #0) do include Nightly and devedition? And I do keep seeing the devedition theme on nightly now...
Flags: needinfo?(arni2033)
(In reply to :Gijs Kruitbosch from comment #6)
> Hm, but the steps from comment #1 (rather than comment #0) do include Nightly and devedition?
> And I do keep seeing the devedition theme on nightly now...
Come on, it was you who wrote comment 3, saying that expectations for Nightly/DE are invalid.
I agree with that comment, and it means that my attempt to detect this bug on alpha versions failed.
It's only reproducible on Release/Beta. Bug 1260595 changed nothing at all (on Nightly).

Interestingly, it's reproducible on Nightly 2015-03-26, but not on 2015-03-27 - fixed by bug 1094821
So the best solution is to finally enable dev theme on Release, because it was deleted with a REALLY weak reason - "it applies dev theme on Release via sync... so let's kill it" (bug 1180163 comment 13)
Flags: needinfo?(arni2033)
See Also: → 1094821
Reproducible since 2014-08-23, therefore it was missed in bug 1007336. Pushlog:
> https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0b9dd32d1e16&tochange=64c4ec2df3d4
Blocks: 1007336
(In reply to arni2033 from comment #7)
> So the best solution is to finally enable dev theme on Release, because it
> was deleted with a REALLY weak reason - "it applies dev theme on Release via
> sync... so let's kill it" (bug 1180163 comment 13)

That really wasn't the only reason, and still isn't. At least for me, maintenance (ie making sure the theme works well enough on release) is a much bigger issue. In any case, the dropdown just shouldn't end up being empty and omitting the default theme. :-\
Assignee: nobody → jaws
Status: NEW → ASSIGNED
https://bugzilla.mozilla.org/show_bug.cgi?id=1007336#c35 requested we hide the My Themes section "if the only theme there is the default theme AND the recommended section is empty." This causes the empty menu. We don't show the Themes button if a complete theme is selected.
Comment on attachment 8753883 [details]
MozReview Request: Bug 1260596 - Themes panel is empty when all default-shipped lightweight themes (lwts) are removed. r?gijs

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53542/diff/1-2/
Attachment #8753883 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8753883 [details]
MozReview Request: Bug 1260596 - Themes panel is empty when all default-shipped lightweight themes (lwts) are removed. r?gijs

https://reviewboard.mozilla.org/r/53542/#review50288

::: browser/components/customizableui/test/browser_1007336_lwthemes_in_customize_mode.js:66
(Diff revision 1)
> +  yield endCustomizing();
> +  Services.prefs.setCharPref("lightweightThemes.usedThemes", "[]");
> +  Services.prefs.setCharPref("lightweightThemes.recommendedThemes", "[]");
> +  info("Removed all recommended themes");

Does this remove the devedition theme as well, when present? If so, we should make extra sure these prefs get reset when the test has run. How does this work in aurora when the devedition theme is the default?
(In reply to :Gijs Kruitbosch from comment #13)
> Does this remove the devedition theme as well, when present? If so, we
> should make extra sure these prefs get reset when the test has run. How does
> this work in aurora when the devedition theme is the default?

The devedition theme is removed at the beginning of the test by the call to
>   LightweightThemeManager.clearBuiltInThemes();

I just built with ac_add_options --with-branding=browser/branding/aurora and the test passes fine in devedition. During the test the menu shows up with only one entry (the default theme).
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/52c96fb34b72
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Affected:   Win7_64, Beta 48.0b10, 32bit, ID 20160721144529
Fixed on:   Win7_64, Beta 49.0b2,  32bit, ID 20160808002253
See Also: → 1327560
You need to log in before you can comment on or make changes to this bug.