129250 - MRJ plugin can't use XPCOM interfaces using AUTF8String and ACString parameters

Status: VERIFIED FIXED

(Core Graveyard :: Java: OJI, defect)

Description

[Darin Fisher]

i ran into some trouble while trying to land my patch for bug 124042.  the
changes make some of the necko interfaces use the new AUTF8String XPIDL string
type.  this causes problems for the MRJ plugin since it doesn't link to anything
mozilla.  i guess we either need a separate API for MRJ or MRJ needs to know
about our string interfaces.

added #ifdef NOT_BROKEN_BY_DARIN around string-fu that wouldn't link in
LiveConnectNativeMethods.cpp.  this most likely breaks the MRJ plugin.

Comment 1

[Mike Shaver (:shaver emeritus)]

And because string depends on intl, it can't just link against string, right? 
What a tangled web we weave...

Glue library to the rescue?

Comment 2

[Patrick C. Beard]

Even worse, this means we have a binary incompatibility with the plugin 
and existing browsers. Luckily the MRJ plugin for Mac OS 9 is built with 
the browser, but the MRJ plugin for Mac OS X isn't and this is a bug that's 
going to bite us soon.

I need to be able to build a plugin that works with both NS 6.2.1 and later 
browsers on Mac OS X. This looks to be impossible now. My fault for 
using a non-frozen interface.

Comment 3

[Darin Fisher]

we have a couple of options:

1) add more dependencies (statically linked into) the MRJ plugin.
2) eliminate necko, do the URL parsing ourselves.
3) use nsIURLParser that still works w/ |string|.

i'd like to avoid bloating the MRJ plugin, so (1) seems bad.  if we go w/ (3)
then we're in the same boat... depending on a non-frozen interface.

so, moving forward option (2) might be best... especially considering that the
MRJ plugin only wants to parse out the prePath from an URL.

but all of these options seem to require some sort of runtime version/interface
checking that i simply don't know how to satisfy :(

Comment 4

[Alec Flett]

I'd lean towards (3) and freeze nsIURLParser, personally.. we'd bloat the mrj
plugin slightly, but not as bad as (2)
(1) seems bad all together. we COULD have people link against libstring_s.lib or
whatever it's called, but again, it would add bloat.

Comment 5

[Darin Fisher]

i really don't want to freeze nsIURLParser :(  also, in order to use
nsIURLParser we have to make it non-scriptable... since making it scriptable
would require using AUTF8String parameters, which wouldn't help the MRJ plugin
any.  i suppose this isn't much of an issue since there isn't any JS code
calling into nsIURLParser (or else it would be broken on i18n sites).  hmm... it
wouldn't be that much code to parse out the prepath.

Comment 6

[Doug Turner (:dougt)]

Most embedders and component developers may already need to link with both
xpcom/glue and strings.  Sure this is bloat, but that is the price ya gotta pay
until both things are frozen.  

exactly how much crap will be pulled in if we wanted to support AUTF8String and
ACString parameters?  

Comment 7

[Darin Fisher]

well.. the MRJ plugin only needs:

 nsDependentCString - for setting AUTF8String attribute values
 nsCAutoString      - for getting AUTF8String attribute values

IOW if the code factors nicely, we might be able to get away with few additional
dependencies.

Comment 8

[Darin Fisher]

patrick: where does the url string come from that we're wanting to parse?  if it
comes via the plugin API, then chances are that it has already been
canonicalized.  in which case, it should be fairly trivial to parse the prePath
manually.  just search for the third '/' or if no '/' after the first ':' then
use the entire url string as the security context origin.  sound good?

Comment 9

[Patrick C. Beard]

Reassigning to bnesse.

Comment 10

[Patrick C. Beard]

Attachment: patch for MRJPluginCarbon v1

This patch fixes the bustage in the Mac OS X Java plugin, and stops using the
unfrozen nsIIOService & nsIURI interfaces. Instead, it assumes the URL it
stores in its mLocation field is in canonical form, and NULL terminates the
string at the 3rd slash.

The patch also fixes the plugin so that it uses nsIComponentManagerObsolete
instead of nsIComponentManager, so that will be binary compatible with Mozilla
0.9.4 and later.

Comment 11

[Patrick C. Beard]

Brian, please migrate this patch to the classic MRJ plugin.

Comment 12

[Patrick C. Beard]

Nominating for nsbeta1.

Comment 13

[Darin Fisher]

patrick: noticed one thing... allocating mLocation w/ ::strdup and deallocating
with delete[].  might this be a problem?

Comment 14

[Patrick C. Beard]

No, this strdup is part of the plugin source code in

mozilla/plugin/oji/MRJCarbon/plugin/Source/StringUtils.cpp

which uses |operator new()| to allocate the string.

Comment 15

[Darin Fisher]

fair enough :)

Comment 16

[Patrick C. Beard]

I checked in the MRJPluginCarbon patch, since it is needed to fix bustage.

Comment 17

[Darin Fisher]

you might want to check this in on the 0.9.9 branch as well since my changes for
bug 124042 landed there.

Comment 18

[Brian Nesse (gone)]

Attachment: Patch to fix MRJ Plugin and remove dependencies.

This patch removes incorporates beards changes, removes Darins additions, and
eliminates dead project dependencies.

Comment 19

[Darin Fisher]

Comment on attachment 73280 [details] [diff] [review]
Patch to fix MRJ Plugin and remove dependencies.

sr=darin

Comment 20

[Brian Nesse (gone)]

Attachment: Patch for the 0.9.9 branch.

Same patch, except for the 0.9.9 branch.

Comment 21

[Darin Fisher]

Comment on attachment 73296 [details] [diff] [review]
Patch for the 0.9.9 branch.

sr=darin

Comment 22

[Patrick C. Beard]

Comment on attachment 73280 [details] [diff] [review]
Patch to fix MRJ Plugin and remove dependencies.

r=beard

Comment 23

[Patrick C. Beard]

Comment on attachment 73296 [details] [diff] [review]
Patch for the 0.9.9 branch.

r=beard

Comment 24

[Asa Dotzler [:asa]]

Comment on attachment 73280 [details] [diff] [review]
Patch to fix MRJ Plugin and remove dependencies.

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 25

[Asa Dotzler [:asa]]

Comment on attachment 73296 [details] [diff] [review]
Patch for the 0.9.9 branch.

a=asa (on behalf of drivers) for checkin to the 0.9.9 branch

Comment 26

[Brian Nesse (gone)]

Fix checked into the trunk.

Comment 27

[Brian Nesse (gone)]

Not currently planned to land on the branch. closing as fixed (on trunk.)

Comment 28

[Patty Mac]

Verified the patches.

Comment 29

[Dawn Endico]

I'm removing this release note item for this bug from the Mozilla 1.0 and
future versions of the release notes because this bug is marked fixed.

Mail me if you think this item should be re-added.