Closed Bug 565064 Opened 12 years ago Closed 12 years ago

Need to always update the selected theme preference when switching builds

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla2.0b2
Tracking Status
blocking2.0 --- beta2+

People

(Reporter: josh.tumath+bugzilla, Assigned: mossop)

References

()

Details

(Whiteboard: [rewrite])

Attachments

(3 files)

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
Blocks: 550048
Version: unspecified → Trunk
Can you attach a copy of extensions.sqlite from your profile folder.
Whiteboard: [rewrite]
Where is the profile folder?
Here you go. ;)
Priority: -- → P2
Summary: Incompatible themes are used even though they aren't compatible → Need to always update the selected theme preference when switching builds
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
blocking2.0: --- → beta1+
blocking2.0: beta1+ → final+
blocking2.0: final+ → beta2+
Attached patch patch rev 1Splinter Review
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.
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Attachment #454311 - Flags: review?(robert.bugzilla)
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
Attachment #454311 - Flags: review?(robert.bugzilla) → review+
Landed: http://hg.mozilla.org/mozilla-central/rev/78f9e065bfa9
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3
Target Milestone: mozilla1.9.3 → mozilla1.9.3b2
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
(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?
I'm having exactly the same problem. My theme is still being used in Minefield, even though it's not compatible.
(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.
(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.
Correct. The theme is disabled but still applied => Reopen
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Duplicate of this bug: 576452
I'm unable to reproduce this after the fix landed. Do you by any chance have extension compatibility checking disabled?
(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....
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.
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.
Attachment #455777 - Flags: review?(robert.bugzilla)
Attachment #455777 - Flags: review?(robert.bugzilla) → review+
Should be fixed now: http://hg.mozilla.org/mozilla-central/rev/d048a5c7195f
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Duplicate of this bug: 344697
Verified fixed with Mozilla/5.0 (Windows; Windows NT 6.1; en-US; rv:2.0b2pre) Gecko/20100718 Minefield/4.0b2pre
Status: RESOLVED → VERIFIED
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?
Which theme and is it incompatible with Firefox 4.0b2?
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.
That theme is only compatible up to 3.7a6pre. Firefox trunk is currently at 4.0b2pre
Exactly; which is why it seems strange that Minefield is using this theme even though it isn't meant to.
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.
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.
The new bug is bug 580664. ;)
You need to log in before you can comment on or make changes to this bug.