[webkit-reviews] review denied: [Bug 29240] iframes keep getting scrollbars with scrolling="no" : [Attachment 70026] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Oct 7 07:28:20 PDT 2010
Ojan Vafai <ojan at chromium.org> has denied Mike Lawther
<mikelawther at chromium.org>'s request for review:
Bug 29240: iframes keep getting scrollbars with scrolling="no"
https://bugs.webkit.org/show_bug.cgi?id=29240
Attachment 70026: Patch
https://bugs.webkit.org/attachment.cgi?id=70026&action=review
------- Additional Comments from Ojan Vafai <ojan at chromium.org>
r- for the tab characters in the test. Otherwise, the test looks fine. I prefer
if someone more familiar with the C++ side review the rest if possible.
View in context: https://bugs.webkit.org/attachment.cgi?id=70026&action=review
> LayoutTests/fast/frames/iframe-scrolling-attribute-overflowscroll.html:17
> + var content =
> + '<html><head><style type="text/css">' + scrolltype + ' {
overflow:scroll; }</style></head>' +
> + '<body><div style="width:380px; height:400px;
background-color:green"></div></body></html>';
> +
> + var doc = document.getElementById(frameId).contentDocument;
Looks like some tabs crept in here and below.
> LayoutTests/fast/frames/iframe-scrolling-attribute-overflowscroll.html:32
> + return "expected (" + frame.clientWidth + "," +
frame.clientHeight + "), " +
Nit: might want to include the frameId in the output so people can know which
frame broke.
> LayoutTests/fast/frames/iframe-scrolling-attribute-overflowscroll.html:47
> + if (window.layoutTestController) {
> + layoutTestController.dumpAsText();
> + }
One-line if statements should not have curly braces.
More information about the webkit-reviews
mailing list