Closed
Bug 562919
Opened 14 years ago
Closed 14 years ago
When migrating with multiple themes all will be marked as enabled in the database
Categories
(Toolkit :: Add-ons Manager, defect, P1)
Toolkit
Add-ons Manager
Tracking
()
VERIFIED
FIXED
mozilla1.9.3a5
People
(Reporter: alfredkayser, Assigned: mossop)
References
Details
(Keywords: testcase, Whiteboard: [AddonsRewriteTestday][rewrite])
Attachments
(5 files)
In the new addons UI introduced by 554007, the 'Use Theme' button is missing. Only 'Personas' can be enabled, but then they cannot be disabled.
Enable on the default theme (I only have one so just using that as an example) will unload the persona.
Reporter | ||
Comment 2•14 years ago
|
||
There is no 'enable' button on other full themes. That is the real issue here.
Comment 3•14 years ago
|
||
I see an enable button. Can you please give exact steps and tell us your complete build id? Thanks.
Comment 4•14 years ago
|
||
I also see the enable button on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a5pre) Gecko/20100430 Minefield/3.7a5pre. It's right there for your walnut theme, yay! Perhaps this is OS dependent issue?
Reporter | ||
Comment 5•14 years ago
|
||
This is with build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100430 Minefield/3.7a5pre GTB5 This is in Windows XP, classic theme.
Reporter | ||
Comment 6•14 years ago
|
||
To be more precise, the .addon-control.enable button has: hidden="true". So, somehow the logic of the addon manager makes it hidden.
Comment 7•14 years ago
|
||
I have seen it too a couple of minutes ago. I have no steps yet but will keep an eye on it.
Comment 8•14 years ago
|
||
I have still no idea why that happens but I have a simplified profile ready which I will attach to this bug now.
Summary: With the new addons UI, there is no way to use a theme. → Under some conditions it's not possible to enable other themes (personas)
Version: unspecified → Trunk
Updated•14 years ago
|
Keywords: testcase
Whiteboard: [AddonsRewriteTestday] → [AddonsRewriteTestday][rewrite]
Comment 9•14 years ago
|
||
Just create a fresh profile and replace its content. Then open the Addons Manager and select the Themes pane. As you can see there is no Enable button for the Default theme.
Comment 11•14 years ago
|
||
This sounds like a manifestation of my [suite] issue. Where if there is more than one "app-installed" theme, there is no enable button on the other. I suspect the manager is making some assumption about "App Theme is Enabled" and does not provide buttons for the _other_ app themes. If this is a separate issue I can split it out if need be
Blocks: SMAddonMgr
Comment 12•14 years ago
|
||
This extensions.sqlite was created by running an about:addons build once on an existing profile.
Assignee | ||
Updated•14 years ago
|
Priority: -- → P1
Assignee | ||
Updated•14 years ago
|
Assignee | ||
Comment 13•14 years ago
|
||
(In reply to comment #9) > Created an attachment (id=442850) [details] > test profile > > Just create a fresh profile and replace its content. Then open the Addons > Manager and select the Themes pane. As you can see there is no Enable button > for the Default theme. I think this profile is causing a different bug which I've filed as bug 563971. The others here seem to be seeing something different.
Assignee | ||
Comment 14•14 years ago
|
||
(In reply to comment #5) > Created an attachment (id=442793) [details] > Screenshot showing the missing 'enable' button on themes > > This is with build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; > rv:1.9.3a5pre) Gecko/20100430 Minefield/3.7a5pre GTB5 > > This is in Windows XP, classic theme. For some reason here the manager thinks that Walnut is enabled, but I presume the default theme is the one that is showing? The other themes there don't have enable buttons because they are incompatible with the app I think.
Assignee | ||
Comment 15•14 years ago
|
||
Figured this out. This is a problem when migrating multiple themes from the old extensions.rdf. We never used to make a userDisabled property in the rdf for themes, the preference was all that was used. The current code assumes a lack of userDisabled property means the add-on was enabled which is wrong for themes.
Summary: Under some conditions it's not possible to enable other themes (personas) → When migrating with multiple themes all will be marked as enabled in the database
Assignee | ||
Comment 16•14 years ago
|
||
This solves the problem at migration time and adds an automated test for it. It doesn't currently fix the problem for existing users that have hit it, need to consider the cost of that vs. just asking them to delete the extensions.sqlite from their profile.
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Attachment #444177 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 17•14 years ago
|
||
http://hg.mozilla.org/projects/addonsmgr/rev/6fbb50534c71
Flags: in-testsuite+
Whiteboard: [AddonsRewriteTestday][rewrite] → [AddonsRewriteTestday][rewrite][fixed-in-addonsmgr][needs-review]
Comment 18•14 years ago
|
||
Comment on attachment 444177 [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 >@@ -1383,17 +1383,20 @@ var XPIProvider = { > // Update the AddonInternal properties. > newAddon._installLocation = aInstallLocation; > newAddon.visible = !(newAddon.id in visibleAddons); > newAddon.installDate = aAddonState.mtime; > newAddon.updateDate = aAddonState.mtime; > > // If there is migration data then apply it. > if (aMigrateData) { >- newAddon.userDisabled = aMigrateData.userDisabled; >+ // Theme disabled state is determined by the selected theme preference >+ // and will have been set in loadManifestFromRDF Perhaps something along these lines? // A theme's disabled state is determined by the selected theme preference // which is read in loadManifestFromRDF >+ if (newAddon.type != "theme") >+ newAddon.userDisabled = aMigrateData.userDisabled; > if ("installDate" in aMigrateData) > newAddon.installDate = aMigrateData.installDate; > if ("targetApplications" in aMigrateData) > newAddon.applyCompatibilityUpdate(aMigrateData, true); > } > > // Update the database. > XPIDatabase.addAddonMetadata(newAddon, aAddonState.descriptor); r=me with the wording fixed up
Attachment #444177 -
Flags: review?(robert.bugzilla) → review+
Updated•14 years ago
|
Whiteboard: [AddonsRewriteTestday][rewrite][fixed-in-addonsmgr][needs-review] → [AddonsRewriteTestday][rewrite][fixed-in-addonsmgr][needs-landing]
Assignee | ||
Comment 19•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/2a012e312acc
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [AddonsRewriteTestday][rewrite][fixed-in-addonsmgr][needs-landing] → [AddonsRewriteTestday][rewrite]
Target Milestone: --- → mozilla1.9.3a5
Comment 20•14 years ago
|
||
Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a5pre) Gecko/20100517 Minefield/3.7a5pre Dave, do the automated test cover all of the possible cases or do we also need manual tests?
Status: RESOLVED → VERIFIED
Flags: in-litmus?
You need to log in
before you can comment on or make changes to this bug.
Description
•