Closed Bug 247156 Opened 18 years ago Closed 18 years ago

support non-jarred extensions

Categories

(Toolkit :: Add-ons Manager, enhancement)

All
Other
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: myk, Assigned: bugs)

References

Details

(Keywords: fixed-aviary1.0)

Attachments

(3 files, 2 obsolete files)

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>
Attached patch patch (obsolete) — Splinter Review
I think other than extension authors
will pay no attension to this bug at all.
Attachment #151399 - Attachment mime type: application/octet-stream → application/x-xpinstall
Attachment #151395 - Flags: review?(bugs)
There seems to be two bugs for this feature both with patches, see Bug 247735
for the other one.
(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() ?
Attached patch patch v2 (obsolete) — Splinter Review
Removed zipReader from _getProviderNames()
By far the better, imho.
*** Bug 247735 has been marked as a duplicate of this bug. ***
Depends on: 248298
good solution :) congrats
(i posted the other bug mentioned)

cheers
avih
Attachment #151395 - Flags: review?(bugs)
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.
Attachment #151518 - Flags: review?(mconnor)
Comment on attachment 151518 [details] [diff] [review]
patch v2

looks good to me.  r=mconnor@steelgryphon.com
Attachment #151518 - Flags: review?(mconnor) → review+
Flags: blocking-aviary1.0PR?
Flags: blocking-aviary1.0?
Attached patch Patch v2.1Splinter Review
Thank you for the review.
To make the patch up-to-date.
Attachment #151395 - Attachment is obsolete: true
Attachment #151518 - Attachment is obsolete: true
Have you tested with jar'ed chrome? tried uninstalling? Please let me know the
different things you've tested with. 
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).
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.
Not critical for PR, setting blocking 1.0 
Flags: blocking-aviary1.0PR?
Flags: blocking-aviary1.0PR-
Flags: blocking-aviary1.0?
Flags: blocking-aviary1.0+
(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 on attachment 155682 [details] [diff] [review]
Patch v2.1

Ben verbally approved this patch for the aviary branch.
Attachment #155682 - Flags: approval-aviary+
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
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Keywords: fixed-aviary1.0
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.