Summary: | [CSS Regions] Add tests for widows and orphans | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andrei Bucur <abucur> | ||||||
Component: | WebCore Misc. | Assignee: | Andrei Bucur <abucur> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | dino, WebkitBugTracker, webkit.review.bot | ||||||
Priority: | P2 | Keywords: | AdobeTracked | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 57312 | ||||||||
Attachments: |
|
Description
Andrei Bucur
2013-01-03 00:43:16 PST
Created attachment 181174 [details]
Patch
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. (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. Created attachment 183439 [details]
Patch
Comment on attachment 183439 [details] Patch Clearing flags on attachment: 183439 Committed r140139: <http://trac.webkit.org/changeset/140139> All reviewed patches have been landed. Closing bug. |