Bug 38219 – Add test for NodeIterator prototype change behavior

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

Add test for NodeIterator behavior change from removing custom bindings

[Adam Barth]

Created attachment 54455 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 54455 [details]
Patch

A script-test is better:

script-tests/node-iterator-prototype.js:

description("This test checks whether DOM wrappers created by NodeIterator have their prototypes attached to the correct objects.  These nodes are from the child frame, so that's where their prototypes should be attached.")

function runTest() {
    shouldBeEqualToString("it.nextNode().foo", "child");
    shouldBeEqualToString("it.previousNode().foo", "child");
}

Node.prototype.foo = "parent"

var iframe = document.createElement("iframe");
iframe.onload = "runTest()";
frame.src = "resources/node-iterator-prototype-frame.html";
document.body.appenChild(iframe);


Also.  How do we know that nextNode() and previousNode() are not previously acceessed and thus cached (thus making this test a no-op?)  Did you make sure it failed in an earlier version of Safari?

Why doesn't this need to be waitUntilDone?

[Adam Barth]

> A script-test is better:

The problem is we need to wait for the iframe to load.  Can a script test do that?

> Also.  How do we know that nextNode() and previousNode() are not previously
> acceessed and thus cached (thus making this test a no-op?)

I'm pretty sure nextNode() is fresh.  However, I bet previousNode() is cached.  I'm not familiar enough with NodeIterator's API to know how to "skip over" a node so that I can previousNode() back to it and have it be fresh.

> Did you make sure it failed in an earlier version of Safari?

I'll check.

> Why doesn't this need to be waitUntilDone?

Because the load event for the main frame waits for the load event of a the child iframes and the testing harness waits for the main frame's load event to fire before ending the test.

[Adam Barth]

Hum...  It passes in release Safari.

[Eric Seidel (no email)]

JSC bindings are cached on a per-window basis iirc.  (you would know better than I).  So if the node has ever been accessed as a binding in any frame it will have a cached binding object with the global object from when it was created.  To test this kind of code we have to be careful to access fresh nodes as you say.  There is no way to clear the cache to my knowledge.

[Adam Barth]

I can get it to fail if I just iterate some more nodes.  The sad part is that it show the bug isn't fixed.  :(

[Adam Barth]

Created attachment 54457 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 54457 [details]
Patch

Wow!  That's awesome.  Thanks.

Would still be better if your test output PASS or FAIL instead of/in addition to parents vs. child.  It's not clear reading the expected output that we're wrong.

[Adam Barth]

Committed r58339: <http://trac.webkit.org/changeset/58339>