81117 - Passing '0' instead of 'null' to XPConnect gives cryptic error

Status: VERIFIED FIXED

(Core :: DOM: Events, defect, P1)

Description

[Vladimir Ermakov]

To reproduce open the attached testcase and check the javascript console.

Comment 1

[Vladimir Ermakov]

Attachment: testcase

Comment 2

[joki (gone)]

Actually its not really a bug, though the error message is not particularly 
useful in determining that.  The problem is that you can't pass '0' in for 
either the viewArg or relatedTargetArg, these are DOM objects which can be 
'null' but not '0'.  There is a difference in JS.

jband, is there any way to get XPConnect to give more info in the error for 
cases like this?  It pretty hard for the user to figure out what's wrong with 
"uncaught exception: null" being the only javascript error info.

Comment 3

[John Bandhauer]

Thanks for reporting this!

It turns out that (as I expected!) xpconnect was creating a nicely descriptive 
exception. The problem was that the security system was vetoing access to the 
exception object in our attempt to convert the exception to a string for display 
in the console.

In extending the nsISceurityCheckedComponent interface on the nsXPCException 
object to report "AllAccess" in CanCreateWrapper I noticed that we had not 
extended the CanGetProperty method to also allow access to the 'code' property. 
This meant that the DOMException use of nsXPCException was also broken!

I'll attach the patch.

Comment 4

[John Bandhauer]

Attachment: proposed fix for review

Comment 5

[Johnny Stenback (:jst)]

sr=jst

Comment 6

[Mitchell Stoltz (not reading bugmail)]

r=mstoltz, but alternatively, all you would have to do is give it a ClassInfo;
wouldn't that be easier?

Comment 7

[Mitchell Stoltz (not reading bugmail)]

I would prefer using ClassInfo wherever possible instead of
SecurityCheckedComponent.

Comment 8

[John Bandhauer]

mitch? It *has* a ClassInfo. You made that a requirement for even calling  
nsISecurityCheckedComponent, no? What we don't want to do is mark it as a 
DOM_OBJECT. It is not really a dom object

Again, I think the granularity of nsISecurityCheckedComponent is *important*. We 
*can* add some new 'AllAccess for everyting' flag to nsIClassInfo. But in the 
end there should be a mechanism that allows the class to declare that some 
methods/attributes should be exposed to non-secure scripters and others should 
not. Whether this is done on a per-call basis or registered ahead of time is 
another issue. But, the availability of that new system should come *before* 
discouraging people from using nsISecurityCheckedComponent in favor of the easy 
to implement and easy to abuse 'AllAccess for everything' nsIClassInfo flags.

I'll get my patch checked in today.

Comment 9

[John Bandhauer]

fix checked in. thanks.

Comment 10

[Vladimir Ermakov]

verifying fixed, shows a nice long message now...