As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
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]
:
: Andy McKay [:andym]
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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image Justin Wood (:Callek) 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image Dave Townsend [:mossop] 2010-05-07 14:39:21 PDT
http://hg.mozilla.org/projects/addonsmgr/rev/6fbb50534c71
Comment 18 User image 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 User image Dave Townsend [:mossop] 2010-05-11 11:27:11 PDT
http://hg.mozilla.org/mozilla-central/rev/2a012e312acc
Comment 20 User image 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 User image 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.