Bug 12001 - marker shorthand property only sets marker-start
Summary: marker shorthand property only sets marker-start
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2006-12-27 10:30 PST by Rémi Zara
Modified: 2006-12-30 14:10 PST (History)
1 user (show)

See Also:

the marker shorthand property sets start mid and end (1.79 KB, patch)
2006-12-27 10:37 PST, Rémi Zara
no flags Details | Formatted Diff | Diff
Updated patch (54.52 KB, patch)
2006-12-27 14:57 PST, Rémi Zara
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rémi Zara 2006-12-27 10:30:14 PST
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
Comment 1 Rémi Zara 2006-12-27 10:37:16 PST
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.
Comment 2 Eric Seidel (no email) 2006-12-27 13:59:15 PST
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?
Comment 3 Rémi Zara 2006-12-27 14:57:02 PST
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.
Comment 4 Darin Adler 2006-12-27 22:23:37 PST
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.

Comment 5 Alexey Proskuryakov 2006-12-28 11:01:06 PST
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)
Comment 6 Alexey Proskuryakov 2006-12-28 11:16:33 PST
Actually, the test gives me a crash even without this patch (r18452).
Comment 7 Alexey Proskuryakov 2006-12-28 11:54:00 PST
Filed bug 12015 about the crash.
Comment 8 Rob Buis 2006-12-30 14:10:46 PST
Landed in r18488.