Summary: | [CSS Shapes] the spec-examples reftests are off by a few pixels | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rebecca Hauck <rhauck> | ||||
Component: | Tools / Tests | Assignee: | Bem Jones-Bey <bjonesbe> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bjonesbe, clopez, commit-queue | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Rebecca Hauck
2014-08-13 19:22:39 PDT
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 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. Created attachment 241114 [details]
Patch
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? 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. Comment on attachment 241114 [details] Patch Clearing flags on attachment: 241114 Committed r175721: <http://trac.webkit.org/changeset/175721> All reviewed patches have been landed. Closing bug. |