421396 - Extension installation fails with "item is null" in nsExtensionManager.js

Status: RESOLVED FIXED

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

Description

[Jaime Mitchell (use [e-mail] for email)]

Attachment: Extensions.log

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b5pre) Gecko/2008030607 Minefield/3.0b5pre ID:2008030607

Steps to reproduce:

1. Try and install extension such as http://www.kaply.com/operator/activities.xpi
2. Select Allow in the info bar
3. Click Install Now in the count down prompt
4. Error is displayed in prompt in Addons Manager

Minefield could not install the file at 

http://www.kaply.com/operator/activities.xpi

because: Unexpected installation error
Review the Error Console log for more details.
-203

Error console gives:
Item is null on the following line
var installLocation = this.getInstallLocation(item.id);

Relevant profile files are attached.

Comment 1

[Jaime Mitchell (use [e-mail] for email)]

Attachment: Extensions.ini

Comment 2

[Jaime Mitchell (use [e-mail] for email)]

Attachment: Extensions.cache

Comment 3

[Jaime Mitchell (use [e-mail] for email)]

Attachment: Extensions.rdf

Comment 4

[Dave Townsend [:mossop]]

It looks like the inspector extension's old directory is still present which is confusing the EM somewhat. I'm not positive but it might be related to what is happening here, either way it is not a good thing as the EM log shows.

Looks like the auto-updater is only set to remove the inspector's files, not the folders, can it remove folders too? Otherwise on startup the EM attempts to remove them, then fails because it doesn't have write access to the folder and so barfs which cannot be good for its health.

Comment 5

[Nick Thomas [:nthomas] (UTC+12)]

The updater can't remove directories at the moment (bug 386760). In testing Fx2->3 updates, I'd noticed the newly-empty Talkback directories get removed on starting up Fx3, but I would have had write access to the appdir.

Comment 6

[Dave Townsend [:mossop]]

(In reply to comment #5)
> The updater can't remove directories at the moment (bug 386760). In testing
> Fx2->3 updates, I'd noticed the newly-empty Talkback directories get removed on
> starting up Fx3, but I would have had write access to the appdir.

Yeah the extension manager would delete bad extension directories on startup if it was able to.

Comment 7

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

note: for talkback we have a workaround in the win32 installer to remove the obsolete directory. I'm less inclined to use this workaround for DOMi unless the version distributed by AMO uses a different id so we won't be removing it by accident when it is intentionally installed into the app dir.

Comment 8

[Dave Townsend [:mossop]]

(In reply to comment #7)
> note: for talkback we have a workaround in the win32 installer to remove the
> obsolete directory. I'm less inclined to use this workaround for DOMi unless
> the version distributed by AMO uses a different id so we won't be removing it
> by accident when it is intentionally installed into the app dir.

The problem is that right now we delete all DOMi's files on every app update anyway, so you are already unable to install it into the app dir.

We should fix this, it is going to slow down app startup for all users it affects (the EM goes through its slow path attempting to remove the dir on every startup from what I can see)

A few options:

We could make the EM able to deal with an inability to delete the bad directory and just start ignoring it

We could properly delete the DOMi directory properly on update (requires either updater patch or some custom hack) or stop deleting the files so that users can actually install DOMi there if they like.

Comment 9

[Jaime Mitchell (use [e-mail] for email)]

(In reply to comment #4)
> It looks like the inspector extension's old directory is still present which is
> confusing the EM somewhat. I'm not positive but it might be related to what is
> happening here, either way it is not a good thing as the EM log shows.

I deleted the directory manually and then restarted firefox. Since then I have not had this error and have successfully installed a few extensions, so it looks like this diagnosis is correct.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

My instinct is that this should block. This will affect anyone who did a custom install of Firefox with a non-administrator user account. I'm not terribly sure what size of our user population that is, but breaking add-ons for them is removing a core feature, and there's no workaround that I can really see.

Connor: do we have to remove DOMi? If we didn't remove it, while this bug would still exist, it wouldn't be encountered as frequently and IMO this wouldn't block anymore.

Comment 12

[Jean-Michel Reghem]

I had this problem on my vista business
it is now working for me by renaming extensions.log, extensions.rdf and extensions.ini in the profile folder into .bak file
Restarting firefox: all is now working

Comment 13

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Simple fix, crazy unit test.

The fix is just to check for the presence of an install.rdf after an uninstall fails. If there isn't one then the item is damaged beyond repair and we should just ignore it by removing from the startup cache. It won't get re-added without an install.rdf in it.

The testcase is somewhat complex. It creates a custom install location with an add-on present in it. Starts the extension manager to detect  and install it. It then starts returning an empty undeletable directory from the install location, restarts the em and makes sure the extension has gone. Then goes on to check we can safely install extensions.

Comment 14

[u294409]

Does this test with INSTALLATION? You mentioned UNINSTALLATION.

Comment 15

[Dave Townsend [:mossop]]

Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v  <--  nsExtensionManager.js.in
new revision: 1.284; previous revision: 1.283
done
RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/test_bug421396.js,v
done
Checking in toolkit/mozapps/extensions/test/unit/test_bug421396.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/test_bug421396.js,v  <--  test_bug421396.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/data/test_bug421396/chrome.manifest,v
done
Checking in toolkit/mozapps/extensions/test/unit/data/test_bug421396/chrome.manifest;
/cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/data/test_bug421396/chrome.manifest,v  <--  chrome.manifest
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/data/test_bug421396/install.rdf,v
done
Checking in toolkit/mozapps/extensions/test/unit/data/test_bug421396/install.rdf;
/cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/data/test_bug421396/install.rdf,v  <--  install.rdf
initial revision: 1.1
done

Comment 16

[Brian King [:kinger]]

Was this checked in before the beta 5 cut-off? I am still seeing it there.

Comment 17

[Nick Thomas [:nthomas] (UTC+12)]

No, the cutoff for b5 was March 25th, plus the Target Milestone on this is not 3.0b5.