Closed Bug 612393 Opened 9 years ago Closed 9 years ago

Extensions get lost when downgrading a profile from Firefox 4.0 to 3.6

Categories

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

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla2.0b9
Tracking Status
blocking2.0 --- final+
blocking1.9.2 --- -
blocking1.9.1 --- -

People

(Reporter: whimboo, Assigned: mossop)

References

Details

(Keywords: regression)

Attachments

(2 files)

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101111 Firefox/4.0b8pre ID:20101111030745

This bug is similar to bug 603611 but shows an inconsistent behavior for extensions with a non GUID. Some extensions get installed, others are removed. 

During the startup of a recent 3.6.13pre build we show all extensions, which are placed as XPI files inside the extensions folder, in the installation dialog during start-up. Somehow some of those extensions aren't getting installed. In the case of the profile which is attached, please check the ACR and Dom Inspector extensions which are getting removed.

Steps:
1. Download and prepare the profile
2. Start a Namoroka build
3. Check the extensions which will be installed
4. Check the add-ons manager

After step 4 you will notice that some extensions haven't been installed. There are no useful logging information available on my machine.
Attachment #490694 - Attachment is patch: false
Attachment #490694 - Attachment mime type: text/plain → application/octet-stream
I'm seeing this issue nearly all the time I start testing the AOM between 3.6 and 4.0. Dave, which importance would have this bug compared to other 4.0 blockers?
blocking2.0: --- → ?
status1.9.2: ? → ---
Whiteboard: [d?]
Benjamin, this problem is only relevant for downgrades from 4.0 to 3.6. So it should more block a further 1.9.2 release as 2.0.
status1.9.2: --- → ?
Then really you need to set blocking1.9.2:?, not status1.9.2. AFAIK nobody triages status1.9.2.
blocking1.9.2: --- → ?
status1.9.2: ? → ---
Ok, figured out what was going on here. This is the situation that will cause this problem (Henrik, let me know if this roughly matches how you created the test profile).

1. Run Firefox 3.6 and install some old versions of extensions
2. Run Firefox 4.0 and receive updates to those extensions
3. Run Firefox 3.6 and it will ask you to reinstall those extensions that updated (and any new ones) but the updated ones will never actually get installed.

What happens it this:

In Firefox 3.6 the extensions install unpacked and Firefox maintains a list of known installed extensions in extensions.cache.
When Firefox 4.0 upgrades the extension however it deletes the unpacked extension and replaces it with the packed form.
Now when Firefox 3.6 starts in its initial scan it sees that the directories for some extensions it was previously tracking have been removed and so marks those extensions for deletion, then it sees the new XPIs and offers to install them, and then once installed it goes back to its list of extensions to delete and does so including the ones that have since been installed.

The fix for this is pretty simple, if we just delete the extensions.cache file in Firefox 4 then Firefox 3.6 won't know what extensions used to exist and so will never delete any. The only downside is it will force 3.6 to do a full re-scan of all installed extensions incurring a minor performance penalty, but the first start of 3.6 will be slow anyway
From comment the fix will actually come in FF4, not the branches. Marking as blocking- for branches.
blocking1.9.1: --- → -
blocking1.9.2: ? → -
blocking2.0: ? → final+
(In reply to comment #5)
> Ok, figured out what was going on here. This is the situation that will cause
> this problem (Henrik, let me know if this roughly matches how you created the
> test profile).

I can't really remember which specific Firefox version I have initially used to install the bulk of add-ons. But your approach seems to be reasonable. Whenever you have a patch and wants to have it tested, feel free to mention the tryserver build location.
Flags: in-litmus?
Attached patch patch rev 1Splinter Review
Spinning this through the try server but we're limited about what can be automatically tested here. I have verified that I can reproduce the problem with the test profile but if I remove extensions.cache from there first then the problem is solved.
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Whiteboard: [d?] → [has patch][waiting on try]
Comment on attachment 498777 [details] [diff] [review]
patch rev 1

One or two code corrections that I spotted in the course included here.
Attachment #498777 - Flags: review?(robert.bugzilla)
Whiteboard: [has patch][waiting on try] → [has patch][needs review rs]
Comment on attachment 498777 [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
>...
>@@ -1754,17 +1755,17 @@ var XPIProvider = {
>             fis.close();
>           }
>         }
> 
>         LOG("Processing install of " + id + " in " + aLocation.name);
>         try {
>           var addonInstallLocation = aLocation.installAddon(id, stageDirEntry,
>                                                             existingAddonID);
>-          if (id in aManifests[aLocation.name])
>+          if (aManifests[aLocation.name][id])
What does this fix? Is it that you want to end up in the catch block?
Attachment #498777 - Flags: review?(robert.bugzilla) → review+
(In reply to comment #11)
> Comment on attachment 498777 [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
> >...
> >@@ -1754,17 +1755,17 @@ var XPIProvider = {
> >             fis.close();
> >           }
> >         }
> > 
> >         LOG("Processing install of " + id + " in " + aLocation.name);
> >         try {
> >           var addonInstallLocation = aLocation.installAddon(id, stageDirEntry,
> >                                                             existingAddonID);
> >-          if (id in aManifests[aLocation.name])
> >+          if (aManifests[aLocation.name][id])
> What does this fix? Is it that you want to end up in the catch block?

A little above we have this: http://hg.mozilla.org/mozilla-central/annotate/e952221c3251/toolkit/mozapps/extensions/XPIProvider.jsm#l1729 so that test always passes and then we attempt to get a property off null which throws an exception. It doesn't really cause anything to break just logs an error when there isn't one.
Landed: http://hg.mozilla.org/mozilla-central/rev/a57900b40a7b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [has patch][needs review rs]
Target Milestone: --- → mozilla2.0b9
Version: 1.9.2 Branch → Trunk
Verified with latest Tinderbox build Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9pre) Gecko/20101221 Firefox/4.0b9pre

For the Litmus test we can use the steps in comment 5.

The only visual glitch I can see is that we do not show an icon in the compatibility check dialog for extensions with a GUID. I will file a new bug on it.
Status: RESOLVED → VERIFIED
Testcase added to litmus as #15032 (https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=15032).
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.