[webkit-reviews] review granted: [Bug 106003] [CSS Regions] Add tests for widows and orphans : [Attachment 181174] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 3 09:46:07 PST 2013


Tony Chang <tony at chromium.org> has granted Andrei Bucur <abucur at adobe.com>'s
request for review:
Bug 106003: [CSS Regions] Add tests for widows and orphans
https://bugs.webkit.org/show_bug.cgi?id=106003

Attachment 181174: Patch
https://bugs.webkit.org/attachment.cgi?id=181174&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=181174&action=review


> LayoutTests/fast/regions/regions-widows-and-orphans.html:45
> +    for (var i = 0; i < 3; i++) {

Nit: ++i

> LayoutTests/fast/regions/regions-widows-and-orphans.html:65
> +    for (var i = 1; i <= blocks.length; i++) {

Nit: ++i

> LayoutTests/fast/regions/regions-widows-and-orphans.html:69
> +	   for (var j = 1; j <= numLines; j++) {

Nit: ++j

> LayoutTests/fast/regions/regions-widows-and-orphans.html:104
> +function log(msg) {
> +    if (!results)

Nit: I would have used js-test-pre.js instead of implementing my own logPass
and logFail functions.

> LayoutTests/fast/regions/regions-widows-and-orphans.html:115
> +	   var topOfContainer =
document.getElementById(containerId).getBoundingClientRect().top;
> +    var topOfLine = document.getElementById(containerId + "-block-" +
blockNumber + "-line-" + lineNumber).getBoundingClientRect().top;

Nit: Weird indent.

> LayoutTests/fast/regions/regions-widows-and-orphans.html:210
> +    testRunner.waitUntilDone();

Is this needed? The test harness should wait until the onload handler finishes
running before dumping results.


More information about the webkit-reviews mailing list