LiveConnect exceptions are not propagated to Java

RESOLVED INCOMPLETE

Status

Core Graveyard
Java: Live Connect
RESOLVED INCOMPLETE
13 years ago
6 years ago

People

(Reporter: Rich Giuli, Assigned: Alfred Peng)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040616
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040616

Here is the code in jsj_JSObject.c:

385     /* Warnings are not propagated as Java exceptions - they are simply
386        ignored.  Ditto for exceptions that are duplicated in the form
387        of error reports. */
388     if (report && (report->flags & (JSREPORT_WARNING | JSREPORT_EXCEPTION)))
389         return;

Note that:
JSREPORT_WARNING = 0x1
JSREPORT_EXCEPTION = 0x2
(JSREPORT_WARNING | JSREPORT_EXCEPTION)=(0x1 | 0x2)=0x3
So, the exception check is checking that (report->flags & 0x3) != 0
Suppose report->flags is 0x2, which does not indicate a warning, but a real
exception.
However, (0x2 & 0x3) is 0x2, which is non-zero. 
So, if report->flags is 0x2 only, which does not indicate a warning, the if
statement is true.

I propose the if statement be changed to:
if (report && (report->flags & JSREPORT_EXCEPTION) && (report->flags &
JSREPORT_WARNING))
        return;

This properly checks that the flag is an exception AND the flag is a warning. My
tests show that making this change allows Javascript errors to be propagated
back into Java.

Reproducible: Always

Steps to Reproduce:
Create a Javscript method which throws an exception. For example:
var foo = null;
function jsThrowEx() {
    return foo.bogusMethod();
}

Then invoke this method from java. Ex:
JSObject obj = ...;
obj.call("jsThrowEx", null);
Actual Results:  
The method returns an no exception is thrown. However, there was a javascript
error so an exception should be thrown stating "foo has no properties".

Expected Results:  
A JSException should have been thrown due to the Javascript error. Making the
change I suggest above causes the exception to be thrown.

Comment 1

13 years ago
Good catch, Rich. But you proposed change allows the JSREPORT_WARNING to be
thrown to java as an exception. I think we should only check the
JSREPORT_WARNING flag.
Assignee: live-connect → kyle.yuan
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

13 years ago
Created attachment 180448 [details] [diff] [review]
patch
Attachment #180448 - Flags: superreview?(brendan)
Attachment #180448 - Flags: review?(brendan)

Comment 3

13 years ago
mass reassign my bugs to Pete Zha.
Assignee: kyle.yuan → pete.zha

Comment 4

12 years ago
mass reassign to Alfred
Assignee: zhayupeng → alfred.peng
(Assignee)

Comment 5

11 years ago
Rich, could you attach one specific test case? With the proposed change, there is still "foo has no properties" in the Error Console and no Exception throw in the Java code.

Updated

11 years ago
Attachment #180448 - Flags: superreview?(brendan)
Attachment #180448 - Flags: review?(brendan)
Attachment #180448 - Flags: review?(alfred.peng)

Updated

8 years ago
Component: Java: Live Connect → Java: Live Connect
Product: Core → Core Graveyard

Comment 6

7 years ago
Firefox code moved from custom Liveconnect code to the NPAPI/NPRuntime bridge a while back. Mass-closing the bugs in the liveconnect component which are likely invalid. If you believe that this bug is still relevant to modern versions of Firefox, please reopen it and move it the "Core" product, component "Plug-Ins".
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → INCOMPLETE
(Assignee)

Comment 7

6 years ago
Comment on attachment 180448 [details] [diff] [review]
patch

Remove old review request which is not needed any more.
Attachment #180448 - Flags: review?(alfred.peng)
You need to log in before you can comment on or make changes to this bug.