Bug 38219 - Add test for NodeIterator prototype change behavior
: Add test for NodeIterator prototype change behavior
Status: RESOLVED FIXED
: WebKit
New Bugs
: 528+ (Nightly build)
: Other Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2010-04-27 14:14 PST by
Modified: 2010-04-27 14:51 PST (History)


Attachments
Patch (3.30 KB, patch)
2010-04-27 14:15 PST, Adam Barth
no flags Review Patch | Details | Formatted Diff | Diff
Patch (3.90 KB, patch)
2010-04-27 14:43 PST, Adam Barth
eric: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-04-27 14:14:00 PST
Add test for NodeIterator behavior change from removing custom bindings
------- Comment #1 From 2010-04-27 14:15:12 PST -------
Created an attachment (id=54455) [details]
Patch
------- Comment #2 From 2010-04-27 14:23:41 PST -------
(From update of attachment 54455 [details])
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?
------- Comment #3 From 2010-04-27 14:28:26 PST -------
> 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.
------- Comment #4 From 2010-04-27 14:29:51 PST -------
Hum...  It passes in release Safari.
------- Comment #5 From 2010-04-27 14:32:48 PST -------
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.
------- Comment #6 From 2010-04-27 14:40:20 PST -------
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.  :(
------- Comment #7 From 2010-04-27 14:43:58 PST -------
Created an attachment (id=54457) [details]
Patch
------- Comment #8 From 2010-04-27 14:47:03 PST -------
(From update of attachment 54457 [details])
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.
------- Comment #9 From 2010-04-27 14:51:39 PST -------
Committed r58339: <http://trac.webkit.org/changeset/58339>