[webkit-reviews] review denied: [Bug 27719] [Chromium] Regression in r42671 - js event object being hidden. : [Attachment 34587] patch3 - Add second layout test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 11 13:47:00 PDT 2009


Adam Barth <abarth at webkit.org> has denied Nate Chapin <japhet at chromium.org>'s
request for review:
Bug 27719: [Chromium] Regression in r42671 - js event object being hidden.
https://bugs.webkit.org/show_bug.cgi?id=27719

Attachment 34587: patch3 - Add second layout test
https://bugs.webkit.org/attachment.cgi?id=34587&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
Getting close.	A couple points:

1) I don't think you need the with() block in the second test.	It's not really
doing anything at the moment.  You can just say parent.frames[1], etc.

2) You should move the new test to the same folder as the first test.  You're
not really testing the security of aboutBlank like the other tests in that
folder.

3) GetEntered() is also wrong, although you'd need a more complex test to see
that.  The correct window is the one that the setter is being called on.  You
can see how to do this by looking at the location setter:

http://trac.webkit.org/browser/trunk/WebCore/bindings/v8/custom/V8DOMWindowCust
om.cpp#L164

The event getter is probably also wrong in the same way at
http://trac.webkit.org/browser/trunk/WebCore/bindings/v8/custom/V8DOMWindowCust
om.cpp#L149 but we can deal with that in another patch.


More information about the webkit-reviews mailing list