[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