Bug 135923

Summary: [CSS Shapes] the sandbag JS utility code isn't producing the correct expected result in the shape-image gradient tests
Product: WebKit Reporter: Rebecca Hauck <rhauck>
Component: Tools / TestsAssignee: Bem Jones-Bey <bjonesbe>
Status: RESOLVED FIXED    
Severity: Normal CC: bjonesbe, buildbot, commit-queue, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Rebecca Hauck 2014-08-13 19:09:12 PDT
The following tests imported from the W3C CSS Shapes test suite are failing because they use the sandbag JS utility code to estimate the text flow around the shape. The expected results generated by the code isn't matching the actual, so we should first look at the utility code to make sure it's current.

LayoutTests/css3/shapes/shape-outside/shape-image/gradients/shape-outside-radial-gradient-001.html
LayoutTests/css3/shapes/shape-outside/shape-image/gradients/shape-outside-radial-gradient-002.html
LayoutTests/css3/shapes/shape-outside/shape-image/gradients/shape-outside-radial-gradient-003.html
LayoutTests/css3/shapes/shape-outside/shape-image/gradients/shape-outside-radial-gradient-004.html

If it turns out that the util code needs to be updated, the change must first be updated in the W3C repo and re-imported.
Comment 1 Bem Jones-Bey 2014-11-04 11:47:28 PST
I don't think this is a fault of the JS utility code. I replaced the JS code in the expected file with a circle() shape outside (in the 001 test), and the JS code and the circle() both give the same shape that is different from the shape created by the gradient. Examining the shape drawn by the inspector for both the gradient shape case and the circle shape case, it looks to me like the gradient creates a shape, that while circular, has a much rougher contour, so there are places where it is farther our and places where it is closer in than the clean circle.

It may make the most sense to use math like in the JS utility to figure out where the text should be positioned in the ideal case, and then have a delta that it can vary and just check that the text is positioned within that delta. (Making it a testharness test instead of a ref test.)
Comment 2 Bem Jones-Bey 2014-11-07 10:39:53 PST
Created attachment 241191 [details]
Patch
Comment 3 Bem Jones-Bey 2014-11-07 10:57:31 PST
Created attachment 241193 [details]
Patch
Comment 4 Dirk Schulze 2014-11-07 11:15:24 PST
Comment on attachment 241193 [details]
Patch

r=me
Comment 5 WebKit Commit Bot 2014-11-07 12:02:52 PST
Comment on attachment 241193 [details]
Patch

Clearing flags on attachment: 241193

Committed r175755: <http://trac.webkit.org/changeset/175755>
Comment 6 WebKit Commit Bot 2014-11-07 12:02:58 PST
All reviewed patches have been landed.  Closing bug.