Bug 12001 – marker shorthand property only sets marker-start

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

[Rémi Zara]

The marker shorthand property should set the marker for all vertice. It should accept only one marker reference and use it for start mid and end markers.

Today only start is set, and this fails http://www.w3.org/Graphics/SVG/Test/20061213/svggen/painting-marker-03-f.svg

[Rémi Zara]

Created attachment 12067 [details]
the marker shorthand property sets start mid and end

This patch fixes the test mentioned in comment #0, which will be imported when bug 12000 is fixed.

[Eric Seidel (no email)]

Comment on attachment 12067 [details]
the marker shorthand property sets start mid and end

The patch in general looks fine.

Why is:
+        CSSValue *value = parsedProperties[numParsedProperties-1]->value();

needed instead of just using the already defined "value" local?

[Rémi Zara]

Created attachment 12071 [details]
Updated patch

Fixes a style issue.
Added updated result from tests (imported in bug 12000).

The patch uses the normal processing to compute the CSSValue for marker-start. This is done in a recursive call to parseValue. The value variable is no more in the right scope.

The resulting CSSValue is then affected to marker-mid and marker-end.

[Darin Adler]

Comment on attachment 12071 [details]
Updated patch

This looks right. The code in parse4Values is similar, and I assume you looked there to understand the proper technique.

+        CSSValue *value = parsedProperties[numParsedProperties - 1]->value();

I know the code you copied from is formatted this way, but it should be CSSValue* with the * next to the type.

r=me

[Alexey Proskuryakov]

I tried to land this patch, but was getting a crash each time when trying to open svg/W3C-SVG-1.1/painting-marker-03-f.svg

Please don't use non-ASCII characters in the ChangeLog - unfortunately, the format only supports ASCII.

Thread 0 Crashed:
0   com.apple.WebCore        	0x014b1cf0 WebCore::drawStartAndMidMarkers(void*, WebCore::PathElement const*) + 104 (RenderPath.cpp:388)
1   com.apple.WebCore        	0x014d650c WebCore::CGPathApplierToPathApplier(void*, CGPathElement const*) + 464 (PathCG.cpp:229)
2   com.apple.CoreGraphics   	0x90435c70 CGPathApply + 548
3   com.apple.WebCore        	0x014d6574 WebCore::Path::apply(void*, void (*)(void*, WebCore::PathElement const*)) const + 84 (PathCG.cpp:237)
4   com.apple.WebCore        	0x014b2054 WebCore::RenderPath::drawMarkersIfNeeded(WebCore::GraphicsContext*, WebCore::FloatRect const&, WebCore::Path const&) const + 628 (RenderPath.cpp:424)
5   com.apple.WebCore        	0x014b2684 WebCore::RenderPath::paint(WebCore::RenderObject::PaintInfo&, int, int) + 1528 (RenderPath.cpp:206)

[Alexey Proskuryakov]

Actually, the test gives me a crash even without this patch (r18452).

[Alexey Proskuryakov]

Filed bug 12015 about the crash.

[Rob Buis]

Landed in r18488.