817567 - Unhelpful error message thrown too late

Status: RESOLVED INACTIVE

(Core :: XPConnect, defect)

Description

[Jesse Ruderman]

The testcases in bug 809674 and bug 805879 now throw:

> IDL methods marked with [implicit_jscontext] or [optional_argc] may not be 
> implemented in JS

Who in the what now?  The line that throws doesn't even contain a function call!

I think the error should be be thrown on an earlier line, and be something that's helpful to fuzzers and humans, like:

> Argument 1 to initTimeEvent must be a native Window object

[Split from bug 805879 comment 3.  Thanks for noticing this, Andrew.]

Comment 1

[Boris Zbarsky [:bzbarsky]]

Well, so we should make Window [builtinclass].

But the real issue here is that presumably we're calling some Window method that's throwing from XPCWrappedJS now (where, exactly?) and the caller doesn't propagate the error?

Comment 2

[Andrew McCreight [:mccr8]]

I can poke around to figure out where it is originally coming from, as I got off the hook too easily in bug 805879. ;)

Comment 3

[Andrew McCreight [:mccr8]]

So, basically, the scenario is, initMutationEvent has a Node as its 4th argument, but you can pass in any random JS object, and XPConnect doesn't care, it will just turn it into an XPCWrappedJS node.

Should it immediately check that the argument is really a real Node? That seems to be what Jesse is suggesting, but I don't know if it really makes sense or not. If you never touch the Node you pass in, then it doesn't really matter, so potentially producing an error early would break existing code. So I'm not sure if there's anything to do here.

Do you have any opinion, Olli? Besides perhaps that you wish mutation events would go away. ;)

Comment 4

[Andrew McCreight [:mccr8]]

Looking at nsIDOMNode, it is listed as [scriptable], so it seems perfectly reasonable to allow somebody to pass in a JS object for a Node argument, we're just in a weird scenario where a JS object can't really implement all of Node, despite the fact that Node is scriptable. So this seems like a wontfix to me.

Comment 5

[Boris Zbarsky [:bzbarsky]]

We should make nsIDOMNode [builtinclass].  I can't recall the exact reason that hasn't happened yet, if there is one...

Comment 6

[Bobby Holley (:bholley)]

This error message was basically a low-risk stopgap measure when we realized that XPCWrappedJS doesn't properly implement [implicit_jscontext] and [optional_argc] semantics (it totally screws up the argument order).

Ideally, the presence of either of those two identifiers would cause the XPIDL compiler to automatically mark the interface as [builtinclass] (that is to say, unimplementable by JS). But that breaks various things - in particular, some of our test code relies on script implementing nsIDOMWindow.

If someone wants to improve the situation here, I'd suggest doing the above, and then fixing any code that breaks.

Comment 7

[Jesse Ruderman]

> Ideally, the presence of either of those two identifiers would cause the XPIDL 
> compiler to automatically mark the interface as [builtinclass]

Or report an error saying you need to mark the interface as [builtinclass].  I think that would lead to fewer surprises.

Comment 8

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.