436207 - Display error when install fails due to missing install.rdf

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Tony Chung [:tchung]]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008052804 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008052804 Minefield/3.0pre

There are still invalid extensions that are giving the appearance that they are getting installed in firefox 3.   They shouldnt, cause this is an plugin xpi example of the old ways to install plugins in Fx2. 

See screenshot

Reproducible: Always

Steps to Reproduce:
1. Install RC1, Go to url
2. allow site to the list
3. click download button, and install
4. Verify plugin seems to install via Addons Manager, but nothing appears on the dialog window.


Actual Results:  
invalid extension gives impression it is installed

Expected Results:  
Its not installed.

Comment 1

[Tony Chung [:tchung]]

Attachment: invalid extension installed screenshot

Comment 2

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

Not blocking Firefox 3 on this, but we might need to relnote it. Tony: anything in the error console?

Comment 3

[Tony Chung [:tchung]]

no errors thrown.   I did talk to dave about it earlier, and he had some concerns.   Nom'ing for 3.0.1 would make more sense for this bugfix.

Comment 4

[Dave Townsend [:mossop]]

The issue is with xpis that do not contain an install.rdf (old style with install.js). For some reason we are no longer flagging an error to the user that the install could not proceed.

Comment 5

[Dave Townsend [:mossop]]

Attachment: patch rev 1

This adds back the check for install.rdf to xpinstall and sends the appropriate error if it doesn't exist. Simple unit test included calls xpinstall to install an item with only install.js and checks the install status is right.

Comment 6

[Daniel Veditz [:dveditz]]

Comment on attachment 327388 [details] [diff] [review]
patch rev 1

I'd prefer moving this with the other checks in "OpenAndValidateArchive()", seems a little cleaner and we only support this one form of install now.

But if you have a reason for doing this check here then it looks good to me.

Comment 7

[Dave Townsend [:mossop]]

Attachment: patch rev 2

That makes more sense yeah

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 327955 [details] [diff] [review]
patch rev 2

r/sr=dveditz

Comment 9

[Dave Townsend [:mossop]]

Landed in changeset f795b4346cec. Will look for branch approval in a few days.

Comment 10

[Dave Townsend [:mossop]]

Comment on attachment 327955 [details] [diff] [review]
patch rev 2

Requesting approval. This fixes a confusing UI issue for faulty or deprecated add-on types, includes tests to verify that xpinstall returns the same errors as it did in Firefox 2.

Comment 11

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

Dave, is this fallout from bug 406807?

Comment 12

[Dave Townsend [:mossop]]

(In reply to comment #11)
> Dave, is this fallout from bug 406807?

Yes that's right.

Comment 13

[Samuel Sidler (old account; do not CC)]

Comment on attachment 327955 [details] [diff] [review]
patch rev 2

Approved for 1.9.0.2. Please land in CVS. a=ss

Comment 14

[Dave Townsend [:mossop]]

RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/test_bug436207.js,v
done
Checking in toolkit/mozapps/extensions/test/unit/test_bug436207.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/test_bug436207.js,v  <--  test_bug436207.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/addons/test_bug436207/install.js,v
done
Checking in toolkit/mozapps/extensions/test/unit/addons/test_bug436207/install.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/addons/test_bug436207/install.js,v  <--  install.js
initial revision: 1.1
done
Checking in xpinstall/src/nsXPInstallManager.cpp;
/cvsroot/mozilla/xpinstall/src/nsXPInstallManager.cpp,v  <--  nsXPInstallManager.cpp
new revision: 1.165; previous revision: 1.164
done

Comment 15

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.2pre) Gecko/2008081904 GranParadiso/3.0.2pre.  The error message is generated as expected.