247156 - support non-jarred extensions

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, enhancement)

Description

[Myk Melez [:myk] [@mykmelez]]

The new extension manager requires compliant extensions to jar their package,
locale, and theme files.  It should allow extensions to leave those files
unjarred, i.e. something like:

    <!-- Packages, Skins and Locales that this extension registers -->
    <em:file>
      <Description about="urn:mozilla:extension:file:newext2">
        <em:package>content/</em:package>
        <em:locale>locale/en-US/</em:locale>
        <em:skin>skin/classic/</em:skin>
      </Description>
    </em:file>

Comment 1

[O. Atsushi (Torisugari)]

Attachment: patch

I think other than extension authors
will pay no attension to this bug at all.

Comment 2

[O. Atsushi (Torisugari)]

Attachment: testcase non-jarred extension xpi file.

Comment 3

[PikeUK]

There seems to be two bugs for this feature both with patches, see Bug 247735
for the other one.

Comment 4

[O. Atsushi (Torisugari)]

(In reply to comment #3)
> There seems to be two bugs for this feature both with patches, see Bug 247735
> for the other one.

Oops. Which should be marked as dup?
At least, my patch enable you to uninstall non-jarred extensions :p

BTW, do you have any idea why _getProviderNames()
is dynamicly copying the contents.rdf (in jar file) to a temp location,
while there's a jar protocol URI in _registerChrome() ?

Comment 5

[O. Atsushi (Torisugari)]

Attachment: patch v2

Removed zipReader from _getProviderNames()
By far the better, imho.

Comment 6

[Robin Monks]

*** Bug 247735 has been marked as a duplicate of this bug. ***

Comment 7

[Avi Halachmi (:avih)]

good solution :) congrats
(i posted the other bug mentioned)

cheers
avih

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 151518 [details] [diff] [review]
patch v2

Mike, do you have time to review?  To answer your question in bug 247735
comment 2, I asked Ben about it before filing this bug, and he said non-jarred
extensions should be supported.

Comment 9

[Mike Connor [:mconnor]]

Comment on attachment 151518 [details] [diff] [review]
patch v2

looks good to me.  r=[e-mail]

Comment 10

[O. Atsushi (Torisugari)]

Attachment: Patch v2.1

Thank you for the review.
To make the patch up-to-date.

Comment 11

[Ben Goodger (use ben at mozilla dot org for email)]

Have you tested with jar'ed chrome? tried uninstalling? Please let me know the
different things you've tested with. 

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Attachment: non-jarred version of tinderstatus

Comment 13

[Myk Melez [:myk] [@mykmelez]]

I tested this with the latest versions of five jarred extensions, the testcase
attached to this bug, and a non-jarred version of tinderstatus.

GoogleBar:
http://update.mozilla.org/extensions/moreinfo.php?application=firefox&id=33&vid=34

AdBlock:
http://update.mozilla.org/extensions/moreinfo.php?application=firefox&id=10&vid=215

Web Developer:
http://update.mozilla.org/extensions/moreinfo.php?application=firefox&id=60&vid=63

IE View:
http://update.mozilla.org/extensions/moreinfo.php?application=firefox&id=60&vid=63

Mozilla Calendar:
http://ftp.mozilla.org/pub/mozilla.org/extensions/mozilla_calendar/

Testcase: attachment 151399 [details]

Non-jarred tinderstatus: attachment 156467 [details]

Firefox correctly installed all extensions upon request.  They all worked upon
restart, and after uninstalling them and restarting Firefox they were all gone
(both from the extensions manager dialog and from the extensions/ subdirectory
of my profile directory).

Comment 14

[Myk Melez [:myk] [@mykmelez]]

One issue I noticed is that with the patch Firefox still doesn't support setting
        <em:package> to content/ and putting the files into
chrome/<name>/content/.  Instead I have to set <em:package> to content/<name>/
and put the files into chrome/<name>/content/<name>/, which is quite
unnecessarily overcomplicated.  But perhaps this is a separate, chrome registry bug.

Comment 15

[Ben Goodger (use ben at mozilla dot org for email)]

Not critical for PR, setting blocking 1.0 

Comment 16

[O. Atsushi (Torisugari)]

(In reply to comment #11)
> Have you tested with jar'ed chrome? tried uninstalling?
> Please let me know the different things you've tested with. 

When the chrome is jarred:
This patch does not affect anything.
EM generates exactly the same install log and uninstalls fine.

When non-jarred:
EM installs non-jarred extensions with a good install log,
but there's a problem on uninstallation: some folders remain.
However, this bug is reproducible without this patch,
so that I think this is a separated issue.


Summary: If an install log contains more than 2 files
under chrome dir, EM fails to delete the parent folders
on uninstalling that extension.

Steps to reproduce:
Create such a extension

install.rdf
chrome
  /myextension.jar
  /readme.txt


and install it and uninstall it.

Non-jarred extensions automatically include more than 2 files,
(contents.rdf and some xul/js files)
so that this bug is not negligible.

I'm not sure this is a valid bug or not...
If if is, I'll try to fix the problem.


(In reply to comment #14)
<em:package>content/</em:package> WFM.

Comment 17

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 155682 [details] [diff] [review]
Patch v2.1

Ben verbally approved this patch for the aviary branch.

Comment 18

[Myk Melez [:myk] [@mykmelez]]

Checked in to trunk and branch:

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.67; previous revision: 1.66
done

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.5.6.46; previous revision: 1.5.6.45
done