Closed
Bug 837863
Opened 12 years ago
Closed 12 years ago
"ASSERTION: Native should be the target!" with onerror, modified __proto__
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: jruderman, Unassigned)
References
Details
(Keywords: assertion, testcase)
Attachments
(2 files)
###!!! ASSERTION: Native should be the target!: 'native == targetSupp', file dom/base/nsJSEnvironment.cpp, line 1472
|
Reporter | |
Comment 1•12 years ago
|
||
|
||
Comment 2•12 years ago
|
||
Hmm. "native" is an nsHTMLSharedElement (<head>). "targetSupp" is a mozilla::dom::HTMLSpanElement.
When we call nsGenericHTMLElement::GetOnerror, "this" is the <span>, as expected.
But in nsJSContext::JSObjectFromInterface we do this:
1468 nsCOMPtr<nsISupports> targetSupp = do_QueryInterface(aTarget);
1469 nsCOMPtr<nsISupports> native =
1470 nsContentUtils::XPConnect()->GetNativeOfWrapper(mContext,
1471 JSVAL_TO_OBJECT(v));
1472 NS_ASSERTION(native == targetSupp, "Native should be the target!");
and the "native" ends up being the <head>, because <head> is on XPConnect bindings but <span> is not anymore, and GetNativeOfWrapper walks up the proto chain. Note that in this case v.toObject().getClass()->name is "HTMLSpanElement", so we have the right object and everything.
At first glance, this assert is just bogus now. Bobby, Peter, what do you think?
|
|
||
Comment 3•12 years ago
|
||
And and for those who may not want to load the testcase, the important part is:
var j = document.createElement("span");
j.__proto__ = document.createElement("head");
|
||
Comment 4•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #2)
> and the "native" ends up being the <head>, because <head> is on XPConnect
> bindings but <span> is not anymore, and GetNativeOfWrapper walks up the
> proto chain.
Thankfully, that behavior is very close to going away, and will go away in bug 658909, at which point this assertion should be valid again.
Depends on: 658909
|
|
||
Comment 5•12 years ago
|
||
Will it be valid? After that bug I assume GetNativeOfWrapper will return null in this case, no?
|
|
||
Updated•12 years ago
|
Flags: needinfo?(bobbyholley+bmo)
|
|
||
Comment 6•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #5)
> Will it be valid? After that bug I assume GetNativeOfWrapper will return
> null in this case, no?
Hm, why would it return null? We have a native <span> element whose proto has been munged to point to a <head> element, right? As soon as we stop doing the wacky proto climbing behavior, we should just get the <span> native here, right? Or maybe I'm missing something?
Flags: needinfo?(bobbyholley+bmo)
|
|
||
Comment 7•12 years ago
|
||
Oh, GetNativeOfWrapper knows about WebIDL objects? OK.
So what do we do in the meantime? Seems like this assert is interfering with fuzzing, so we should disable it until that bug lands or something...
|
|
||
Comment 8•12 years ago
|
||
Or alternately reorder GetNativeOfWrapper to check for WebIDL objects first?
|
|
||
Updated•12 years ago
|
Flags: needinfo?(bobbyholley+bmo)
|
|
||
Comment 9•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #8)
> Or alternately reorder GetNativeOfWrapper to check for WebIDL objects first?
This might be more sane, but I'd marginally prefer to avoid munging this stuff until I land. Let's just disable or annotate the assert for now if it's causing problems?
Flags: needinfo?(bobbyholley+bmo)
|
|
||
Comment 10•12 years ago
|
||
Jesse, which works better for you?
|
|
||
Updated•12 years ago
|
Flags: needinfo?(jruderman)
|
|
Reporter | |
Comment 11•12 years ago
|
||
This assertion isn't especially interfering with fuzzing. It's non-fatal, so I can just ignore it until this bug is fixed.
Flags: needinfo?(jruderman)
|
|
Reporter | |
Comment 12•12 years ago
|
||
WFM on mozilla-central. (Fix by bug 658909?)
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WORKSFORME
|
Assignee | |
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•