Bug 37431 – Make comma/whitespace around arc flags optional in SVG path syntax

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Jeff Schiller]

Created attachment 53136 [details]
Test case showing that comma/whitespace is optional between and after arc flags

As per recent W3C SVG WG discussion: http://lists.w3.org/Archives/Public/www-svg/2010Apr/0042.html

All other browsers (Firefox, Opera, Batik, IE9) treat the whitespace/comma between largeArcFlag and sweepFlag in the SVG arc path command syntax as optional.  They also treat the whitespace/comma between sweepFlag and the coordinate pair as optional.  

Finally only values of "0" and "1" are allowed for the largeArcFlag and sweepFlag as per the path grammar: http://dev.w3.org/SVG/profiles/1.1F2/publish/paths.html#PathDataBNF

I am attaching a test case.  This test case renders all paths as green in Firefox, Opera and IE9.  This test case renders all paths as red in WebKit-based browsers. 

I would like to suggest that WebKit align with what other browsers are doing, especially considering that the WG considers this as 'proper' behavior.

[Jeff Schiller]

Created attachment 53139 [details]
Test case showing that comma/whitespace is optional between and after arc flags and that flags should only be 0 or 1

[Jeff Schiller]

Created attachment 53146 [details]
Fix to the parser to only accept 0,1 for arc flags and to make comma/whitespace after the flag be optional

Test file shows all green paths after applying the patch and all LayoutTests pass.

[Dirk Schulze]

Looks good to me. Since this is still in discussion and it is not sure if this moves into SVG1.1SE or SVG2.0, I think we shouldn't land this patch yet. I know, that this could be landed because of compatibility reasons, but let us wait some days for the responses of the SVG WG members.
I'll clear the review flag for the moment. Feel free to set it again after more clearness on this topic later.

[Jeff Schiller]

The SVG 1.1 2nd Edition spec has been updated to support the grammar as outlined in this bug: http://dev.w3.org/SVG/profiles/1.1F2/publish/paths.html#PathDataBNF

SVG WG decision: http://lists.w3.org/Archives/Public/www-svg/2010Apr/0046.html

[Jeff Schiller]

Incidentally, WebKit with this patch correctly displays the following test case from the official SVG 1.1 2nd Edition Test Suite: http://dev.w3.org/SVG/profiles/1.1F2/test/svg/paths-data-20-f.svg

[Dirk Schulze]

Comment on attachment 53146 [details]
Fix to the parser to only accept 0,1 for arc flags and to make comma/whitespace after the flag be optional

LGTM. r=me.

[WebKit Commit Bot]

Comment on attachment 53146 [details]
Fix to the parser to only accept 0,1 for arc flags and to make comma/whitespace after the flag be optional

Clearing flags on attachment: 53146

Committed r57473: <http://trac.webkit.org/changeset/57473>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Dirk Schulze]

This patch causes a break of the SL bot http://build.webkit.org/builders/SnowLeopard Intel Leaks/builds/5883/steps/compile-webkit/logs/stdio

[Dirk Schulze]

(In reply to comment #9)
> This patch causes a break of the SL bot
> http://build.webkit.org/builders/SnowLeopard Intel
> Leaks/builds/5883/steps/compile-webkit/logs/stdio

cc1plus: warnings being treated as errors
/Volumes/Data/WebKit-BuildSlave/snowleopard-intel-leaks/build/WebCore/svg/SVGParserUtilities.cpp:145: warning: no previous prototype for 'bool WebCore::parseArcFlag(const UChar*&, const UChar*, bool&)'

[Jeff Schiller]

Ok, I'm not sure I can test this out as I don't have SL.  Is it just a matter of adding a function prototype to the header?  I declared the function before invoking it, but I guess that's not good enough?

[Dirk Schulze]

Landed SL build bix in http://trac.webkit.org/changeset/57474