Last Comment Bug 562919 - When migrating with multiple themes all will be marked as enabled in the database
: When migrating with multiple themes all will be marked as enabled in the data...
Status: VERIFIED FIXED
[AddonsRewriteTestday][rewrite]
: testcase
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: All All
: P1 major (vote)
: mozilla1.9.3a5
Assigned To: Dave Townsend [:mossop]
:
Mentors:
: 563116 (view as bug list)
Depends on:
Blocks: 461973 SMAddonMgr
  Show dependency treegraph
 
Reported: 2010-04-30 08:21 PDT by Alfred Kayser
Modified: 2010-05-20 07:46 PDT (History)
16 users (show)
dtownsend: in‑testsuite+
dtownsend: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Enable Themes button (115.36 KB, image/png)
2010-04-30 11:52 PDT, Tony Chung [:tchung]
no flags Details
Screenshot showing the missing 'enable' button on themes (66.70 KB, image/gif)
2010-04-30 14:00 PDT, Alfred Kayser
no flags Details
test profile (215.03 KB, application/force-download)
2010-04-30 16:06 PDT, Henrik Skupin (:whimboo)
no flags Details
Busted extensions.sqlite (21.00 KB, application/octet-stream)
2010-05-02 09:25 PDT, neil@parkwaycc.co.uk
no flags Details
patch rev 1 (8.52 KB, patch)
2010-05-07 14:38 PDT, Dave Townsend [:mossop]
robert.strong.bugs: review+
Details | Diff | Splinter Review

Description Alfred Kayser 2010-04-30 08:21:16 PDT
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.
Comment 1 u88484 2010-04-30 08:25:16 PDT
Enable on the default theme (I only have one so just using that as an example) will unload the persona.
Comment 2 Alfred Kayser 2010-04-30 08:29:33 PDT
There is no 'enable' button on other full themes. That is the real issue here.
Comment 3 Henrik Skupin (:whimboo) 2010-04-30 09:42:44 PDT
I see an enable button. Can you please give exact steps and tell us your complete build id? Thanks.
Comment 4 Tony Chung [:tchung] 2010-04-30 11:52:12 PDT
Created attachment 442773 [details]
Enable Themes button

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?
Comment 5 Alfred Kayser 2010-04-30 14:00:32 PDT
Created attachment 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.
Comment 6 Alfred Kayser 2010-04-30 14:02:21 PDT
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 Henrik Skupin (:whimboo) 2010-04-30 15:34:43 PDT
I have seen it too a couple of minutes ago. I have no steps yet but will keep an eye on it.
Comment 8 Henrik Skupin (:whimboo) 2010-04-30 16:04:48 PDT
I have still no idea why that happens but I have a simplified profile ready which I will attach to this bug now.
Comment 9 Henrik Skupin (:whimboo) 2010-04-30 16:06:28 PDT
Created attachment 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.
Comment 10 Jim Jeffery not reading bug-mail 1/2/11 2010-04-30 17:35:58 PDT
*** Bug 563116 has been marked as a duplicate of this bug. ***
Comment 11 Justin Wood (:Callek) (Away until Aug 29) 2010-04-30 19:16:03 PDT
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
Comment 12 neil@parkwaycc.co.uk 2010-05-02 09:25:30 PDT
Created attachment 442994 [details]
Busted extensions.sqlite

This extensions.sqlite was created by running an about:addons build once on an existing profile.
Comment 13 Dave Townsend [:mossop] 2010-05-05 10:28:41 PDT
(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.
Comment 14 Dave Townsend [:mossop] 2010-05-05 10:34:06 PDT
(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.
Comment 15 Dave Townsend [:mossop] 2010-05-07 14:36:49 PDT
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.
Comment 16 Dave Townsend [:mossop] 2010-05-07 14:38:49 PDT
Created attachment 444177 [details] [diff] [review]
patch rev 1

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.
Comment 17 Dave Townsend [:mossop] 2010-05-07 14:39:21 PDT
http://hg.mozilla.org/projects/addonsmgr/rev/6fbb50534c71
Comment 18 Robert Strong [:rstrong] (use needinfo to contact me) 2010-05-10 10:50:56 PDT
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
Comment 19 Dave Townsend [:mossop] 2010-05-11 11:27:11 PDT
http://hg.mozilla.org/mozilla-central/rev/2a012e312acc
Comment 20 Henrik Skupin (:whimboo) 2010-05-18 07:09:40 PDT
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?
Comment 21 Dave Townsend [:mossop] 2010-05-20 07:46:52 PDT
Should be fine here.

Note You need to log in before you can comment on or make changes to this bug.