Closed
Bug 717842
Opened 12 years ago
Closed 12 years ago
New nodelist bindings break cross-window instanceof
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
WONTFIX
Tracking | Status | |
---|---|---|
firefox9 | --- | unaffected |
firefox10 | - | --- |
firefox11 | - | --- |
firefox12 | - | --- |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
(Keywords: regression)
Attachments
(1 file)
5.08 KB,
patch
|
Details | Diff | Splinter Review |
There's a hasInstance hook that's supposed to make it work, but it checks the wrong object for the magic jsclass. Ran into it while trying to document sInterfaceClass at Jason's request. Patch coming up. I think we should probably fix this on aurora and beta too...
Assignee | ||
Comment 1•12 years ago
|
||
This fixes the obvious bug, but I don't understand why we need the loop over prototypes. When will that return true for cases when we have instanceIsProxy true but the |obj| doesn't have sInterfaceClass? And aren't those cases broken for the cross-window case too? Also, I don't understand what ListBase<LC>::hasInstance is doing and why.... It seems fishy to me.
Attachment #588287 -
Flags: review?(peterv)
Comment 2•12 years ago
|
||
Web IDL says: Every platform object is associated with a global environment, just as the initial objects are. It is the responsibility of specifications using Web IDL to state which global environment (or, by proxy, which global object) each platform object is associated with. I doubt any spec does this, though. I would expect the subframe.document.getElementsByTagName() call in your test to return an object that is associated with the subframe's global environment, have subframe.HTMLCollection.prototype in its prototype chain, and thus would evaluate `list2 instanceof HTMLCollection` as false, since Web IDL doesn't have any special [[HasInstance]] behaviour.
Assignee | ||
Comment 3•12 years ago
|
||
If that's all we want, then we don't need the special hasInstance hook on the JSClass there. We may not even need the JSClass. That said, the behavior this patch restores it the pre-new-binding behavior. It's the one all other DOM objects in Gecko have, I believe. So it's possible the goal was to not change behavior when switching to the new bindings, which does not preclude changing it as needed, of course. I did check, and none of Opera, Safari, Chrome return true for cross-window instanceof.
Comment 4•12 years ago
|
||
(In reply to Cameron McCormack (:heycam) from comment #2) > since > Web IDL doesn't have any special [[HasInstance]] behaviour. It used to though (http://www.w3.org/TR/2010/WD-WebIDL-20101021/#es-interface-hasinstance), that's what our implementation followed (modulo the prototype/obj mixup). Since it seems gone from recent drafts I'm fine with redoing our implementation.
Assignee | ||
Comment 5•12 years ago
|
||
If we want to do that for nodelists, then it seems like we should be able to nix all the instanceof-related magic, right? Both the proxy hook and the JSClass bit?
Comment 6•12 years ago
|
||
BTW, I'm a bit confused why |list instanceof HTMLCollection| would ever be true if WebIDL doesn't specify [[HasInstance]] behaviour for its interface objects.
Comment 7•12 years ago
|
||
(In reply to Peter Van der Beken [:peterv] from comment #6) > BTW, I'm a bit confused why |list instanceof HTMLCollection| would ever be > true if WebIDL doesn't specify [[HasInstance]] behaviour for its interface > objects. "Interface objects are always function objects." is what makes it work. Seems the drafts have changed quite a bit in this regard.
Comment 8•12 years ago
|
||
By the way, bug 14869 : "getElementsByTagName() returns HTMLCollection instead of NodeList", so ---- var list = document.getElementsByTagName("*"); ok(list instanceof HTMLCollection, "List should be an HTMLCollection"); ---- It should to the extent that Firefox is not conforming to the spec.
Comment 9•12 years ago
|
||
(In reply to Cameron McCormack (:heycam) from comment #2) > Web IDL says: > > Every platform object is associated with a global environment, just as the > initial > objects are. It is the responsibility of specifications using Web IDL to > state > which global environment (or, by proxy, which global object) each platform > object > is associated with. > > I doubt any spec does this, though. Maybe WebIDL could say that each frame gets a fresh DOM environment, no? Are there use cases where it can be useful for some specs to share script-facing objects across different global environments?
Assignee | ||
Comment 10•12 years ago
|
||
> It should to the extent that Firefox is not conforming to the spec.
The test is meant to catch changes from the behavior we have right now, yes. That's the idea of regression tests.
Comment 11•12 years ago
|
||
Regression in a new feature, tracking for FF10.
Comment 12•12 years ago
|
||
(In reply to David Bruant from comment #9) > Maybe WebIDL could say that each frame gets a fresh DOM environment, no? > Are there use cases where it can be useful for some specs to share > script-facing objects across different global environments? I don't think Web IDL should be talking about "frames". It could maybe have a hook that HTML could use to do this. But the tricky thing is describing exactly which objects returned from operations and attributes are associated with the current window and which things aren't. For example: <!DOCTYPE html> <iframe></iframe> <script> var f = document.getElementsByTagName("iframe")[0]; var w = f.contentWindow; var e = document.createElement("span"); // belongs to this window w.document.body.appendChild(e); w.document.body.lastChild; // belongs to this window, not f </script> So we can't just say that all IDL attributes return objects that belong to the global environment that its interface comes from.
Assignee | ||
Comment 13•12 years ago
|
||
> w.document.body.lastChild; // belongs to this window, not f
I believe at that point it should belong to f. That should generally happen on adoptNode calls.
Comment 14•12 years ago
|
||
OK. I tested with instanceof to see if the node actually still belonged to this window, but I forgot instanceof will return true regardless for us currently. :) But this general pattern is what I mean to point out; operations/attributes for objects in frame B that return objects created in frame A.
Comment 15•12 years ago
|
||
(In reply to Cameron McCormack (:heycam) from comment #12) > (In reply to David Bruant from comment #9) > > Maybe WebIDL could say that each frame gets a fresh DOM environment, no? > > Are there use cases where it can be useful for some specs to share > > script-facing objects across different global environments? > > I don't think Web IDL should be talking about "frames". I'm not expert on the HTML5 terminology. Would "browsing context" be more accurate for this case? > It could maybe have > a hook that HTML could use to do this. But the tricky thing is describing > exactly which objects returned from operations and attributes are associated > with the current window and which things aren't. For example: > > <!DOCTYPE html> > <iframe></iframe> > <script> > var f = document.getElementsByTagName("iframe")[0]; > var w = f.contentWindow; > var e = document.createElement("span"); // belongs to this window > w.document.body.appendChild(e); > w.document.body.lastChild; // belongs to this window, not f > </script> > > So we can't just say that all IDL attributes return objects that belong to > the global environment that its interface comes from. My language was confusing, sorry. You're talking about belonging, but that's not what I meant (though I wrote "gets"). The way I see it, objects do not belong to anyone. However, each "browsing context" or "scripting context" is initialized with a fresh set of built-ins. If 2 scripting contexts can communicate synchronously, then they can access each others built-in, but it's not really that they are /sharing/ what /belongs/ to any of them. It's more that the sharing is de facto and so the notion of belonging is somewhat implicit. Regarding nodes in DOM tree, the "belonging" is not at the language level, but at the " DOM library" level. The DOM "library" defines what a node, an element and a document are; it defines that some nodes belong to some documents, but WebIDL has no role to play here.
Updated•12 years ago
|
Comment 16•12 years ago
|
||
[Triage Comment] No need to track this, we shipped 10 with this and it doesn't appear to consider significant user pain so letting it ride
Assignee | ||
Comment 17•12 years ago
|
||
Wontfixing this, per discussion.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
Whiteboard: [need review]
Updated•12 years ago
|
Attachment #588287 -
Flags: review?(peterv)
Updated•12 years ago
|
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•