565064 - Need to always update the selected theme preference when switching builds

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P2)

Description

[Josh Tumath]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a5pre) Gecko/20100511 Minefield/3.7a5pre
Build Identifier: 

I use the Strata40 theme in Firefox 3.6, but when I open the trunk, the theme shouldn't be loaded (the default theme should, instead). However, since the new add-ons manager landed, the Strata40 theme loads anyway.

Since an incompatible theme is used, a lot of problems are caused, such as the add-ons manager having no styling.

Reproducible: Always

Comment 1

[Dave Townsend [:mossop]]

Can you attach a copy of extensions.sqlite from your profile folder.

Comment 2

[Josh Tumath]

Where is the profile folder?

Comment 3

[Dave Townsend [:mossop]]

http://support.mozilla.com/en-US/kb/Profiles

Comment 4

[Josh Tumath]

Attachment: My "extensions.sqlite" file

Here you go. ;)

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

When starting Minefield with the strata40 theme enabled it's marked as non-active in the extensions.sqlite but loaded and applied in any way. With the current status you will not be able to switch to another theme because there is no Enable button shown.

Confirmed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a5pre) Gecko/20100516 Minefield/3.7a5pre ID:20100516030646

Comment 6

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Fairly straightforward, if upgrading and the selected theme is app disabled switch back to the default theme. Also implemented a quicker way to get directly to the add-on object from the internalName.

Comment 7

[Robert Strong (they/them - no direct email)]

Comment on attachment 454311 [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
>@@ -1646,16 +1646,24 @@ var XPIProvider = {
>           changed = this.processFileChanges(state, manifests, aAppChanged,
>                                             migrateData);
>         }
>         catch (e) {
>           ERROR("Error processing file changes: " + e);
>         }
>       }
> 
>+      // When upgrading the app and using a custom skin make sure it is still
>+      // compatible otherwise switch back the default
>+      if (aAppChanged && this.currentSkin != this.defaultSkin) {
>+        let oldSkin = XPIDatabase.getVisibleAddonForInternalName(this.currentSkin);
>+        if (oldSkin && oldSkin.appDisabled)
>+          this.enableDefaultTheme();
Seems like if oldSkin was null that you'd want to enable the default theme as in
if (!oldSkin || oldSkin.appDisabled)
  this.enableDefaultTheme();

r=me with that changed or an explanation why this should stay this way

Comment 8

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/78f9e065bfa9

Comment 9

[Josh Tumath]

Even though this has landed, I'm still experiencing the problem.

Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:2.0b2pre) Gecko/20100701 Minefield/4.0b2pre

Comment 10

[Dave Townsend [:mossop]]

(In reply to comment #9)
> Even though this has landed, I'm still experiencing the problem.
> 
> Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:2.0b2pre)
> Gecko/20100701 Minefield/4.0b2pre

What exactly are you seeing?

Comment 11

[Josh Tumath]

I'm having exactly the same problem. My theme is still being used in Minefield, even though it's not compatible.

Comment 12

[Dave Townsend [:mossop]]

(In reply to comment #11)
> I'm having exactly the same problem. My theme is still being used in Minefield,
> even though it's not compatible.

This fix will only take effect if you switched to a build of a different version and back again.

Comment 13

[Josh Tumath]

(In reply to comment #12)
> This fix will only take effect if you switched to a build of a different
> version and back again.

Yes, that's what I mean. When I close Firefox 3.6 and open the trunk build, the theme is still used; it hasn't reverted.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Correct. The theme is disabled but still applied => Reopen

Comment 16

[Dave Townsend [:mossop]]

I'm unable to reproduce this after the fix landed. Do you by any chance have extension compatibility checking disabled?

Comment 17

[Josh Tumath]

(In reply to comment #16)
> I'm unable to reproduce this after the fix landed. Do you by any chance have
> extension compatibility checking disabled?

Funnily enough, I did download the Add-on compatibility extension. However, I removed it. Although, when I did that, incompatible add-ons were still being shown. I went on about:config and reset some bolded prefs called "extension.compatibility.3.6" or something like that. When I restarted Firefox, those prefs disappeared, and everything was back to normal, so I assume that everything is fine.

Another pref called "extensions.acr.previousCheckCompatibility" is set to true, it's default value, if that has anything to do with it....

Comment 18

[Dave Townsend [:mossop]]

Ok, managed to reproduce this, it works fine the first time you upgrade from 3.6 to trunk, but if you switch back to 3.6, enable the other theme and then start trunk again it doesn't switch back to the default theme.

Comment 19

[Dave Townsend [:mossop]]

Attachment: follow-up patch rev 1

The problem is that at the moment we keep a userDisabled flag for themes in the database and it is possible for that to get out of sync with the selected theme preference. This happens here because the old version of the application sets the selected theme preference. When that happens the code does attempt to enable the default theme however since the database already thinks it is enabled it essentially doesn't do anything.

This code makes sure to sync the userDisabled setting of themes with the preference on every startup of a new version of the application. The test fails without the changes and passes with them.

I have plans to essentially ignore the userDisabled bit in the database for themes in the future and always use the pref but it relies on breaking the interactions between themes and personas which is a while off yet so I'd rather take this fix for now.

Comment 20

[Dave Townsend [:mossop]]

Should be fixed now: http://hg.mozilla.org/mozilla-central/rev/d048a5c7195f

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed with Mozilla/5.0 (Windows; Windows NT 6.1; en-US; rv:2.0b2pre) Gecko/20100718 Minefield/4.0b2pre

Comment 23

[Josh Tumath]

In recent builds, this has started to happen again. Each time I open Minefield, the theme I used in Firefox 3.6 is used in Minefield.

Can someone confirm this?

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Which theme and is it incompatible with Firefox 4.0b2?

Comment 25

[Josh Tumath]

It's called "Stratum Fusion". It's not in AMO yet, but can be found here:

http://spewboy.deviantart.com/journal/32802038/

When I first downloaded this theme, this bug did not occur; it was only recently when it started to happen.

Comment 26

[Robert Strong (they/them - no direct email)]

That theme is only compatible up to 3.7a6pre. Firefox trunk is currently at 4.0b2pre

Comment 27

[Josh Tumath]

Exactly; which is why it seems strange that Minefield is using this theme even though it isn't meant to.

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I can see this problem too and have an idea why it is happening. Let me investigate and file a new bug. It's not theme related but affects all extensions. Could be a regression from bug 576553.

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I was probably wrong. Josh, can you please file a new bug for that particular add-on? It looks like to be a bit more complicated. I have prepared an example profile I could attach then. Thanks.

Comment 30

[Josh Tumath]

The new bug is bug 580664. ;)