Bug 100299

Created attachment 193027 [details] Patch Checking to see if the bots think that this needs special tweaks for subpixel layout, since the current tweaks don't work for me locally.

Attachment 193027 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/exclusions/resources/rounded-rectangle.js', u'LayoutTests/fast/exclusions/shape-outside-floats/shape-outside-floats-simple-rounded-rectangle-expected.html', u'LayoutTests/fast/exclusions/shape-outside-floats/shape-outside-floats-simple-rounded-rectangle.html']" exit_code: 1 LayoutTests/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] LayoutTests/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] LayoutTests/ChangeLog:10: Line contains tab character. [whitespace/tab] [5] LayoutTests/ChangeLog:16: Line contains tab character. [whitespace/tab] [5] Total errors found: 4 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 193137 [details] Patch After looking at the code (and no complaints from EWS), I think that floats aren't laid out in the same subpixel fashion as the inline text content, so right now, I don't need the subpixel utils to have the test work properly.

Created attachment 193155 [details] Patch Removing comment because it's wrong, but still not using subpixel utils because they shouldn't be needed since I'm using floats to stand in for floats, where in the shape inside case, it has to compensate for the fact that floats aren't the same as shape-inside for positioning purposes.

Comment on attachment 193155 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193155&action=review r=me > LayoutTests/fast/exclusions/resources/rounded-rectangle.js:172 > + if (floatValue == "right") { > + dimensions.shapeX = -dimensions.shapeWidth; > + } The braces are not necessary here.

Comment on attachment 193155 [details] Patch Clearing flags on attachment: 193155 Committed r145847: <http://trac.webkit.org/changeset/145847>

All reviewed patches have been landed. Closing bug.

Comment on attachment 193155 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193155&action=review > LayoutTests/fast/exclusions/resources/rounded-rectangle.js:23 > + if (lineTop < dimensions.shapeHeight && lineBottom > dimensions.shapeHeight) Part of this code is confusing and unneeded: you can't have lineTop >= dimensions.shapeHeight the way you are calling xOutset. > LayoutTests/fast/exclusions/resources/rounded-rectangle.js:141 > + + dimensions.shapeRadiusX + "px, " > + + dimensions.shapeRadiusY + "px); " The specification has a 'curve' keyword before these 2. Couldn't find a bug about it though.

(In reply to comment #8) > (From update of attachment 193155 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=193155&action=review My apologies, I didn't realize this had made it onto your queue, so I got another review for this yesterday. Thanks for the feedback, I'll incorporate it into my patch for Bug 98673. > > > LayoutTests/fast/exclusions/resources/rounded-rectangle.js:23 > > + if (lineTop < dimensions.shapeHeight && lineBottom > dimensions.shapeHeight) > > Part of this code is confusing and unneeded: you can't have lineTop >= dimensions.shapeHeight the way you are calling xOutset. You're right, and I've updated it in my patch for Bug 98673. > > > LayoutTests/fast/exclusions/resources/rounded-rectangle.js:141 > > + + dimensions.shapeRadiusX + "px, " > > + + dimensions.shapeRadiusY + "px); " > > The specification has a 'curve' keyword before these 2. Couldn't find a bug about it though. IIRC, I had the same issue with the spec in the past, and it was explained to me that it isn't actually a keyword, it's just a way of describing the 5th and 6th arguments. I'll bug Alan about that again, though. I'm pretty sure that the syntax we have implemented currently is correct.