Bug 106003 – [CSS Regions] Add tests for widows and orphans

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Andrei Bucur]

There are no regions tests for the widows/orphans feature that landed in http://trac.webkit.org/changeset/137200 .

[Andrei Bucur]

Created attachment 181174 [details]
Patch

[Tony Chang]

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]

(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]

Created attachment 183439 [details]
Patch

[WebKit Review Bot]

Comment on attachment 183439 [details]
Patch

Clearing flags on attachment: 183439

Committed r140139: <http://trac.webkit.org/changeset/140139>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.