Closed Bug 485984 Opened 11 years ago Closed 11 years ago

remove OJI from the tree

Categories

(Core Graveyard :: Java: OJI, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.9.2a1

People

(Reporter: jaas, Assigned: jaas)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

We should remove OJI from the tree, as of Sun JRE 6 Update 10 for Windows and Linux it is not necessary (new NPAPI/npruntime Java plugin), and a new Mac OS X Java plugin is on the way.
Flags: wanted1.9.2+
There is a removal patch URL in the URL field for this bug. The patch is ~5.1 MB, too big for posting on bugzilla. This patch builds on Windows, Linux, and Mac OS X, and Java works on Windows and Linux with > JRE 6 Update 10.
Attached file fix v1.0 (obsolete) —
Review placeholder attachment, contains URL of actual patch.
Attachment #370269 - Flags: superreview?(jst)
(In reply to comment #1)
> There is a removal patch URL in the URL field for this bug. The patch is ~5.1
> MB, too big for posting on bugzilla. This patch builds on Windows, Linux, and
> Mac OS X, and Java works on Windows and Linux with > JRE 6 Update 10.

"Working" might not be completely correct. Kenneth Russel inserted a patch into JRE 6 Update 12 to solve a permission problem that Firefox extension that use Java had with plugin2. 

Somebody should check that jar loading actually works without OJI.
The code that worked for years http://simile.mit.edu/wiki/Java_Firefox_Extension does not work with plugin2 (that is without OJI). The example code here https://developer.mozilla.org/en/Java_in_Firefox_Extensions does not work either without OJI.
The following forum thread indicates that with slight syntactic changes it appears possible to use Java within Firefox extensions using the new Java Plug-In.
http://forums.java.net/jive/thread.jspa?threadID=45933&tstart=0
Comment on attachment 370269 [details]
fix v1.0

Josh, with the attached patch we appear to be loading the wrong Java plugin on Windows. With a clean mozilla-central tree we list npjp2.dll as the dll we load for Java, and with this patch, I we list npjpi160_10.dll for Java. That should be npjp2.dll in both cases (or alternatively we could use npjpi160_10.dll in a clean mozilla-central tree if the old java plugin is chosen by the user in the Java console). The reason for that looks to be the changes to nsPluginDirServiceProvider.cpp, where you remove the code relating to the function TryToUseNPRuntimeJavaPlugIn(). We need to leave enough of that code in so that the default in our code is to use the new plugin, unconditionally, rather than just removing the code. Actually, we really need to reverse the code there, and in nsPluginHost.cpp, so that we simply refuse to load and/or use the old Java plugin completely.

Besides that, this patch looks good from what I've seen so far, but I still need to spend a bit more time reading through it some more.
Attachment #370269 - Flags: superreview?(jst) → superreview-
Windows is the one platform I didn't test on myself, I had someone make sure my patch compiled and that Java still worked afterwards. My assumption was that without OJI the old Java plugin couldn't work, thus if Java worked it would be the new plugin working. The person claimed that Java did work with my patch. I'll look into it again.
Yeah, the old Java plugin will happily work w/o OJI and LiveConnect, you just can't talk to it from JS (or the other way around).
(In reply to comment #6)
> Windows is the one platform I didn't test on myself, I had someone make sure my
> patch compiled and that Java still worked afterwards. My assumption was that
> without OJI the old Java plugin couldn't work, thus if Java worked it would be
> the new plugin working. The person claimed that Java did work with my patch.
> I'll look into it again.

Please test with some Firefox extensions that use Java too.
Attached patch fix v1.1 (obsolete) — Splinter Review
fix logic that picks old vs. new plugin on Windows
Attachment #370269 - Attachment is obsolete: true
Attachment #371564 - Flags: superreview?(jst)
Attached patch fix v1.2Splinter Review
Add back some AIX code, remove nsJVMAuthTools completely.
Attachment #371564 - Attachment is obsolete: true
Attachment #371984 - Flags: superreview?(jst)
Attachment #371564 - Flags: superreview?(jst)
Attachment #371984 - Flags: superreview?(jst)
Attachment #371984 - Flags: superreview+
Attachment #371984 - Flags: review+
Comment on attachment 371984 [details] [diff] [review]
fix v1.2

Looks good. I think we'll want to have explicit code for some time to explicitly refuse to use the old Java plugin, but we can add that once we completely remove support for XPCOM plugins.
another bustage fix for static builds pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/3051d82cb64f
I found some more oji references, I assume we can remove them but I don't know what this is.
Attachment #372015 - Flags: review?(benjamin)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Attachment #372015 - Flags: review?(benjamin) → review-
Comment on attachment 372015 [details] [diff] [review]
gre-win fixes, v1.0

Don't bother, this file is older than dead. (We should probably just remove all of embedding/config, I'll file a bug on that.)
Blocks: 432625
Blocks: 431964
This change made it impossible to run crashtest for me; see bug 487842.
verified ...FIXED?

 Minefield does allow me to load: http://www.time.gov/timezone.cgi?Eastern/d/-5/java

on Build: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090505 Minefield/3.6a1pre ID:20090505030940
Status: RESOLVED → VERIFIED
That last comment is improper, Java is removed and the browsers shows the "Click here to download plugin" message as well as the popdown when the site is visited.
Blocks: 489988
Comment on attachment 371984 [details] [diff] [review]
fix v1.2

>http://joshaas.net/mozilla/oji9.patch
How do you expect anyone to be able to comment on this?
Depends on: 512214
Depends on: 514533
Blocks: 521624
Depends on: 517355
Depends on: 521673
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.