[webkit-reviews] review denied: [Bug 38424] add support for text/html-sandboxed on sandboxed iframes : [Attachment 64832] Updated patch with more tests

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 26 23:25:28 PDT 2010


Adam Barth <abarth at webkit.org> has denied Patrik Persson
<patrik.j.persson at ericsson.com>'s request for review:
Bug 38424: add support for text/html-sandboxed on sandboxed iframes
https://bugs.webkit.org/show_bug.cgi?id=38424

Attachment 64832: Updated patch with more tests
https://bugs.webkit.org/attachment.cgi?id=64832&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
Generally, this looks good.  Can you add a test for loading a
text/html-sandboxed document inside a plain iframe in a sandboxed iframe?

Thanks!

LayoutTests/http/tests/security/html-sandboxed-mainwindow.html:7
 +	layoutTestController.setCanOpenWindows();
Do we still need to call closeRemainingWindowsWhenDone to clean up properly? 
(I might have gotten the name wrong.)

LayoutTests/http/tests/security/html-sandboxed-mainwindow.html:15
 +  var nbrTrustedWindows = 0;
nbr => generally we don't like these sorts of abbreviations.

LayoutTests/http/tests/security/html-sandboxed-mainwindow.html:31
 +	for (var i = 0; i < 5; i++) {
Why do we open so many windows?  That seems like of silly.


More information about the webkit-reviews mailing list