Bug 91761 – [CSS Exclusions] SVG shape errors should invalidate exclusion shapes

• Component: CSS
• Status: RESOLVED FIXED

[Bear Travis]

SVG shapes are not valid with certain values (eg, negative width/height)
These values should invalidate CSS exclusion shapes as well

For the exclusions spec, see:
http://dev.w3.org/csswg/css3-exclusions/#shapes-from-svg-syntax

For the SVG spec, see:
http://www.w3.org/TR/SVG/shapes.html

Invalid values
Rectangle: negative width, height, rx, ry
Circle: negative r
Ellipse: negative rx, ry

[Bem Jones-Bey]

Created attachment 153547 [details]
Bug fix

CSS exclusion shapes that are specified with negative radiuses or height/width are now considered invalid and ignored.

[Early Warning System Bot]

Comment on attachment 153547 [details]
Bug fix

Attachment 153547 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13307321

[Early Warning System Bot]

Comment on attachment 153547 [details]
Bug fix

Attachment 153547 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13315278

[WebKit Review Bot]

Comment on attachment 153547 [details]
Bug fix

Attachment 153547 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13313276

[Bem Jones-Bey]

Created attachment 153553 [details]
Corrected Bug Fix

Updated patch that should pass with -Werror=undefined. Maybe the Mac build doesn't have that flag?

[Bem Jones-Bey]

Created attachment 153609 [details]
Updated patch

Fix tests.

[Andreas Kling]

Comment on attachment 153609 [details]
Updated patch

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

r=me though we should really add comments to make it clear why we're selectively disallowing negative numbers.

> Source/WebCore/css/CSSParser.cpp:4470
> +        if (argumentNumber > 1) 
> +            unitFlags = unitFlags | FNonNeg;

Could use a comment - something simple like "// Disallow negative values for width/height."

> Source/WebCore/css/CSSParser.cpp:4526
> +        if (argumentNumber == 2)
> +            unitFlags = unitFlags | FNonNeg;

Ditto.

> Source/WebCore/css/CSSParser.cpp:4573
> +        if (argumentNumber > 1)
> +            unitFlags = unitFlags | FNonNeg;

Ditto.

[Bem Jones-Bey]

Created attachment 153819 [details]
Updated patch

Add requested comments.

[Bem Jones-Bey]

Created attachment 153867 [details]
Updated patch

Make comments sentences.

[Alexandru Chiculita]

Comment on attachment 153867 [details]
Updated patch

Adding CQ+.

[WebKit Review Bot]

Comment on attachment 153867 [details]
Updated patch

Rejecting attachment 153867 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/13332120

[Bem Jones-Bey]

Created attachment 153887 [details]
Updated patch

Automated adding of reviewer didn't work, adding manuallty.

[Alexandru Chiculita]

Comment on attachment 153887 [details]
Updated patch

Cq+ again.

[WebKit Review Bot]

Comment on attachment 153887 [details]
Updated patch

Clearing flags on attachment: 153887

Committed r123405: <http://trac.webkit.org/changeset/123405>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.