RESOLVED FIXED 106003
[CSS Regions] Add tests for widows and orphans
https://bugs.webkit.org/show_bug.cgi?id=106003
Summary [CSS Regions] Add tests for widows and orphans
Andrei Bucur
Reported 2013-01-03 00:43:16 PST
There are no regions tests for the widows/orphans feature that landed in http://trac.webkit.org/changeset/137200 .
Attachments
Patch (8.80 KB, patch)
2013-01-03 07:35 PST, Andrei Bucur
no flags
Patch (8.46 KB, patch)
2013-01-18 06:19 PST, Andrei Bucur
no flags
Andrei Bucur
Comment 1 2013-01-03 07:35:07 PST
Tony Chang
Comment 2 2013-01-03 09:48:07 PST
Comment on attachment 181174 [details] Patch 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.
Andrei Bucur
Comment 3 2013-01-03 09:54:51 PST
(In reply to comment #2) > (From update of attachment 181174 [details]) > 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. Thx for the review! Will fix right away.
Andrei Bucur
Comment 4 2013-01-18 06:19:39 PST
WebKit Review Bot
Comment 5 2013-01-18 06:42:43 PST
Comment on attachment 183439 [details] Patch Clearing flags on attachment: 183439 Committed r140139: <http://trac.webkit.org/changeset/140139>
WebKit Review Bot
Comment 6 2013-01-18 06:42:46 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.