When migrating with a lightweight theme enabled the default theme will think it is enabled

VERIFIED FIXED in mozilla2.0b7

Status

()

P2
normal
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: mossop, Assigned: mossop)

Tracking

Trunk
mozilla2.0b7
Points:
---
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: [rewrite][has patch])

Attachments

(1 attachment)

When a lightweight theme is enabled the default theme should be marked as userDisabled but during migration we won't check on the lightweight themes when deciding on the value of userDisabled for the default theme.
(Assignee)

Updated

9 years ago
blocking2.0: --- → beta1+
(Assignee)

Updated

9 years ago
Assignee: dtownsend → nobody
(Assignee)

Updated

9 years ago
blocking2.0: beta1+ → final+
(Assignee)

Updated

9 years ago
blocking2.0: final+ → beta2+
(Assignee)

Updated

9 years ago
Assignee: nobody → dtownsend
Duplicate of this bug: 575779

Updated

9 years ago
Duplicate of this bug: 577194

Updated

9 years ago
Duplicate of this bug: 577251
(Assignee)

Updated

9 years ago
blocking2.0: beta2+ → final+
(Assignee)

Updated

9 years ago
Depends on: 585339
(Assignee)

Updated

9 years ago
Depends on: 520124
No longer depends on: 585339
(Assignee)

Updated

8 years ago
No longer depends on: 520124
Duplicate of this bug: 594386
(Assignee)

Comment 6

8 years ago
Created attachment 477785 [details] [diff] [review]
patch rev 1

I dislike having to make the providers talk like this but this really is the simplest fix.
Attachment #477785 - Flags: review?(robert.bugzilla)
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED
Whiteboard: [rewrite] → [rewrite][has patch]
Comment on attachment 477785 [details] [diff] [review]
patch rev 1

>diff --git a/toolkit/mozapps/extensions/XPIProvider.jsm b/toolkit/mozapps/extensions/XPIProvider.jsm
>--- a/toolkit/mozapps/extensions/XPIProvider.jsm
>+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
>@@ -509,21 +510,25 @@ function loadManifestFromRDF(aUri, aStre
>     }
>     else {
>       platform.os = aPlatform;
>     }
> 
>     addon.targetPlatforms.push(platform);
>   });
> 
>-  // Themes are disabled by default unless they are currently selected
>-  if (addon.type == "theme")
>-    addon.userDisabled = addon.internalName != XPIProvider.selectedSkin;
>-  else
>+  // Themes are disabled if they aren't the selected skin or there is an active
>+  // lightweight theme
nit: the wording here made me think that if there is an active lightweight theme a theme is disabled. Perhaps
// A theme's userDisabled value is true if the theme is not the selected skin or the theme is a lightweight theme and it isn't the current lightweight theme.

Might be a good thing to make a note somewhere that themes can't be softblocked.
Attachment #477785 - Flags: review?(robert.bugzilla) → review+
Important for consumer outreach beta.
blocking2.0: final+ → beta7+
(Assignee)

Comment 9

8 years ago
Landed for b7: http://hg.mozilla.org/mozilla-central/rev/626d22af7eff

Note that this wont actually correct the problem if the user has already migrated their DB, they'll need to just switch to a new theme to fix it.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b7
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101010 Firefox/4.0b8pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.