JAR protocol handler should be moved out of necko and into libjar

RESOLVED FIXED in mozilla1.8alpha4

Status

()

--
minor
RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: darin.moz, Assigned: Biesinger)

Tracking

Trunk
mozilla1.8alpha4
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

15 years ago
JAR protocol handler should be moved out of necko and into libjar.

Justification:

The JAR protocol handler requires CAPS to get the channel owner from
the script security context in form of an nsIPrincipal.

I've always wondered why the JAR protocol even lives in Necko.  It seems
like it could be moved into libjar.  Then libjar could depend on necko, etc.

Necko doesn't have much use for the JAR protocol internally.  It's on par
with the CHROME protocol, which is not included in Necko either.
(Reporter)

Updated

15 years ago
Keywords: helpwanted
Target Milestone: --- → Future
-> me and targeting optimistically
Assignee: darin → cbiesinger
Keywords: helpwanted
Target Milestone: Future → mozilla1.8alpha2
Created attachment 151828 [details] [diff] [review]
patch

I'll attach a patch showing the changes I made to the moved files separately
Created attachment 151829 [details]
changes to moved files
Attachment #151828 - Flags: review?(bsmedberg)
Status: NEW → ASSIGNED
(Reporter)

Comment 4

15 years ago
Comment on attachment 151829 [details]
changes to moved files

>+    nsCOMPtr<nsIStandardURL> stdURL(do_CreateInstance("@mozilla.org/network/standard-url;1"));

use NS_STANDARDURL_CONTRACTID instead?	it makes life easier if we ever
wish to change consumers over to another nsIStandardURL implementation
while maintaining the old one.
(Reporter)

Comment 5

15 years ago
Comment on attachment 151828 [details] [diff] [review]
patch

one comment: make sure that libjar still builds correctly in standalone mode. 
we probably do not want standalone libjar to include the jar protocol handler.
standalone libjar just uses the files listed at
http://lxr.mozilla.org/seamonkey/source/modules/libjar/objs.mk#38 afaict... so
it should continue to work. any idea how I can build it?
(Reporter)

Comment 7

15 years ago
cc'ing dveditz

dan: can you confirm that this is not going to cause problems for the standalone
version of libjar?  thanks!
The "standalone" version for the installer is really just the basic nsZipArchive
code, should be fine to move all this stuff into the Mozilla component.
urg, this patch is incomplete, it doesn't remove the dirs from allmakefiles.sh,
and the necko_jar.xpt file from various installer file lists

I'll make a new patch sometime during the next few days, feel free to review the
rest of the patch in the meantime :)
Target Milestone: mozilla1.8alpha2 → mozilla1.8beta

Comment 10

15 years ago
Comment on attachment 151828 [details] [diff] [review]
patch

With allmakefiles.sh changes, packaging xpt files, and darin's comments, r=me
Attachment #151828 - Flags: review?(bsmedberg) → review+
Created attachment 158758 [details] [diff] [review]
changes to existing files
Attachment #151828 - Attachment is obsolete: true
Attachment #151829 - Attachment is obsolete: true
Moved files:
netwerk/protocol/jar/src/nsJARProtocolHandler.cpp
netwerk/protocol/jar/src/nsJARURI.cpp
netwerk/protocol/jar/src/nsJARURI.h
netwerk/protocol/jar/src/nsJARChannel.h
netwerk/protocol/jar/src/nsJARProtocolHandler.h
netwerk/protocol/jar/src/nsJARChannel.cpp
netwerk/protocol/jar/public/nsIJARURI.idl
netwerk/protocol/jar/public/nsIJARProtocolHandler.idl
netwerk/protocol/jar/public/nsIJARChannel.idl

All with an identical name to modules/libjar.
Created attachment 158774 [details] [diff] [review]
changes to moved files, v2

updated per darin's comment
Comment on attachment 158774 [details] [diff] [review]
changes to moved files, v2

In addition to the two patches here, I'll remove all of netwerk/modules/jar,
and get someone to copy the files as described above.
Attachment #158774 - Flags: superreview?(darin)
Attachment #158758 - Flags: superreview?(darin)
(Reporter)

Comment 15

14 years ago
Comment on attachment 158758 [details] [diff] [review]
changes to existing files

looks like you also need to remove the "#undef NECKO_PROTOCOL_jar" line from
necko-config.h.in, right?
Attachment #158758 - Flags: superreview?(darin) → superreview+
(Reporter)

Updated

14 years ago
Attachment #158774 - Flags: superreview?(darin) → superreview+
Repository copy per comment 12 is done now.

/be
patch checked in.

leaving open as a reminder to remove the old files, once tinderboxes are green
tboxes are green, old files removed.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
sorry about the camino breakage :( should be fixed now, thanks to pinkerton.
Target Milestone: mozilla1.8beta → mozilla1.8alpha4
You need to log in before you can comment on or make changes to this bug.