Bug 116745 – [CSS Shapes] rectangle and inset-rectangle do not properly handle rx and ry

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Bem Jones-Bey]

According to the SVG spec (http://www.w3.org/TR/SVG/shapes.html#RectElement), the default value for rx and ry when one is unspecified is the value of the other. The current implementation always sets an unspecified rx or ry to 0.

[Bem Jones-Bey]

It looks like the behavior of rx and ry is more incorrect than originally thought, they should follow all of the rules in the SVG spec for the rx and ry parameters of rect.

[Bem Jones-Bey]

Created attachment 203981 [details]
Patch

Fix to repsect the rules in the spec, if ry is omitted, it defaults to rx, and if rx > width/2, rx = width/2, and if ry > height/2, ry = height/2

[Build Bot]

Comment on attachment 203981 [details]
Patch

Attachment 203981 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/772585

New failing tests:
css3/masking/clip-path-animation.html

[Build Bot]

Created attachment 204003 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Build Bot]

Comment on attachment 203981 [details]
Patch

Attachment 203981 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/772668

New failing tests:
css3/masking/clip-path-animation.html

[Build Bot]

Created attachment 204040 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3

[Bem Jones-Bey]

*sigh* I take this as a sign that I'm doing way too many different things. Will be fixing. Than you EWS for catching my fail.

[Bem Jones-Bey]

Created attachment 204067 [details]
Patch

Update missed test

[Bem Jones-Bey]

Created attachment 204454 [details]
Patch

Change for new spec behavior: the radii get now get scaled the as defined for border radius, not clamped as in SVG.

[Dirk Schulze]

Comment on attachment 204454 [details]
Patch

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

r=me but please add the assertions to be sure.

> Source/WebCore/css/BasicShapeFunctions.cpp:-53
> -        if (!rectangle->cornerRadiusX().isUndefined()) {

Was skeptical about about removing this if clause. Bem wants to add an ASSERT, since he says it can never be undefined. He should be right, since we always set the value on style resolving.

> Source/WebCore/css/BasicShapeFunctions.cpp:-105
> -        if (!rectangle->cornerRadiusX().isUndefined()) {

Ditto.

[Bem Jones-Bey]

Created attachment 204464 [details]
Patch

Update to add asserts

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 204464 [details]:

media/video-played-collapse.html bug 58630 (authors: [e-mail], [e-mail], [e-mail], and [e-mail])
media/audio-garbage-collect.html bug 117555 (author: [e-mail])
transitions/color-transition-rounding.html bug 114182 (author: [e-mail])
transitions/cubic-bezier-overflow-svg-length.html bug 114183 (author: [e-mail])
transitions/interrupt-zero-duration.html bug 114184 (authors: [e-mail], [e-mail], and [e-mail])
transitions/multiple-background-transitions.html bug 114185 (author: [e-mail])
transitions/cubic-bezier-overflow-color.html bug 114186 (author: [e-mail])
transitions/mismatched-shadow-transitions.html bug 114188 (author: [e-mail])
transitions/color-transition-all.html bug 114189 (authors: [e-mail] and [e-mail])
transitions/cubic-bezier-overflow-shadow.html bug 114191 (author: [e-mail])
transitions/min-max-width-height-transitions.html bug 114192 (author: [e-mail])
transitions/cancel-transition.html bug 114193 (authors: [e-mail], [e-mail], and [e-mail])
transitions/border-radius-transition.html bug 114194 (author: [e-mail])
transitions/flex-transitions.html bug 114195 (author: [e-mail])
transitions/mixed-type.html bug 114196 (author: [e-mail])
transitions/multiple-mask-transitions.html bug 114197 (author: [e-mail])
transitions/color-transition-premultiplied.html bug 114198 (author: [e-mail])
transitions/mismatched-shadow-styles.html bug 114199 (author: [e-mail])
transitions/mask-transitions.html bug 114200 (authors: [e-mail], [e-mail], and [e-mail])
transitions/cubic-bezier-overflow-length.html bug 114201 (author: [e-mail])
transitions/multiple-background-size-transitions.html bug 114202 (authors: [e-mail] and [e-mail])
transitions/clip-transition.html bug 114203 (authors: [e-mail], [e-mail], and [e-mail])
transitions/cubic-bezier-overflow-transform.html bug 114204 (author: [e-mail])
transitions/interrupted-accelerated-transition.html bug 56242 (authors: [e-mail], [e-mail], and [e-mail])
transitions/background-transitions.html bug 114206 (author: [e-mail])
http/tests/security/mixedContent/redirect-https-to-http-iframe-in-main-frame.html bug 114208 (authors: [e-mail] and [e-mail])
http/tests/inspector/inspect-element.html bug 78869 (author: [e-mail])
fast/repaint/table-cell-collapsed-border-scroll.html bug 117529 (author: [e-mail])
fast/loader/javascript-url-in-object.html bug 114210 (authors: [e-mail] and [e-mail])
platform/mac/editing/deleting/deletionUI-single-instance.html bug 114181 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 204464 [details]
Patch

Clearing flags on attachment: 204464

Committed r151517: <http://trac.webkit.org/changeset/151517>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.