Bug 135927 – [CSS Shapes] the spec-examples reftests are off by a few pixels

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Rebecca Hauck]

The following tests imported from the W3C CSS Shapes test suite are failing by 1-2px. More investigation is needed to determine if it's just a test update or an actual bug.

LayoutTests/css3/shapes/spec-examples/shape-outside-010.html
LayoutTests/css3/shapes/spec-examples/shape-outside-011.html
LayoutTests/css3/shapes/spec-examples/shape-outside-012.html
LayoutTests/css3/shapes/spec-examples/shape-outside-013.html
LayoutTests/css3/shapes/spec-examples/shape-outside-014.html
LayoutTests/css3/shapes/spec-examples/shape-outside-015.html
LayoutTests/css3/shapes/spec-examples/shape-outside-016.html
LayoutTests/css3/shapes/spec-examples/shape-outside-017.html
LayoutTests/css3/shapes/spec-examples/shape-outside-018.html
LayoutTests/css3/shapes/spec-examples/shape-outside-019.html

If it turns out the tests need to be updated, it should be done in the W3C repo then they should be re-imported.

[Carlos Alberto Lopez Perez]

The test css3/shapes/spec-examples/shape-outside-014.html pass on platform GTK:

[...]
01:46:53.706 20975   css3/shapes/spec-examples/shape-outside-014.html -> ref test hashes didn't match but diff passed
01:46:53.708 20774 [3063/31564] css3/shapes/spec-examples/shape-outside-014.html passed unexpectedly
01:46:53.706 20975 worker/9 css3/shapes/spec-examples/shape-outside-014.html passed
[...]

Expected to fail, but passed:
  css3/shapes/spec-examples/shape-outside-014.html

[Bem Jones-Bey]

These tests are fragile because it is very hard to accurately compute exactly where text is going to be positioned around a circular shape. It would be best to rewrite these as testharness tests so that they can have a tolerance around the computed number and be much more robust.

[Bem Jones-Bey]

Created attachment 241114 [details]
Patch

[Zoltan Horvath]

Comment on attachment 241114 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=241114&action=review

> LayoutTests/css3/shapes/spec-examples/support/spec-example-utils.js:5
> +        firstLine = document.getElementById(linePrefix + '0');

I saw that the first child of the test div are not always a span. I'm not sure if there is an easier way to select the first span child of the 'test' div. If there was such a way to do that that would be nice though.

> LayoutTests/css3/shapes/spec-examples/support/spec-example-utils.js:10
> +            window.setTimeout(runTest, 5);

Is this timeout sufficient enough?

[Bem Jones-Bey]

Comment on attachment 241114 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=241114&action=review

>> LayoutTests/css3/shapes/spec-examples/support/spec-example-utils.js:5
>> +        firstLine = document.getElementById(linePrefix + '0');
> 
> I saw that the first child of the test div are not always a span. I'm not sure if there is an easier way to select the first span child of the 'test' div. If there was such a way to do that that would be nice though.

I don't see how this matters? I'm not selecting the first child of the test div, I'm selecting the element that has the id of "linePrefix + '0'", which is something like 'line-0'. So it doesn't matter what children are before that span.

>> LayoutTests/css3/shapes/spec-examples/support/spec-example-utils.js:10
>> +            window.setTimeout(runTest, 5);
> 
> Is this timeout sufficient enough?

It's arbitrary. I guess I could make it longer, but it really depends on network speed, etc. So unless there's data to say it isn't good enough, might as well leave it as is.

[WebKit Commit Bot]

Comment on attachment 241114 [details]
Patch

Clearing flags on attachment: 241114

Committed r175721: <http://trac.webkit.org/changeset/175721>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.