Bug 12430

Summary: SVG has two transform parsers when it should have one.
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
First attempt
none
slightly improved version of rob's patch, fixing one bug oliver: review+

Description Eric Seidel (no email) 2007-01-26 19:21:19 PST
SVGTransform SVGAnimateTransformElement::parseTransformValue(const String& data) const

and 

SVGTransformable::parseTransformAttribute

should find a way to share code.  Having two just means we'll parse some things wrong, and be open to more possible parser-related security holes.
Comment 1 Rob Buis 2007-06-02 07:38:09 PDT
Created attachment 14840 [details]
First attempt

This patch works in general, not really tested for the animation stuff.
Cheers,

Rob.
Comment 2 Eric Seidel (no email) 2007-06-02 11:05:45 PDT
Comment on attachment 14840 [details]
First attempt

The assignment here doesn't ned to be on separate lines:

+        SVGTransformable::parseTransformValue(m_type, ptr, ptr + m_from.length(), t);
+        m_fromTransform = t;

You could just pass m_fromTransform (if you're not gonna check the bool return anyway)

Personally I might give required/optional longer names.  Like requireParamsForTransformType or even requiredForType or something.

Neither of those are functional issues.  I'm perhaps still too asleep give this a careful review.   More later.
Comment 3 Eric Seidel (no email) 2007-06-05 16:33:01 PDT
Created attachment 14873 [details]
slightly improved version of rob's patch, fixing one bug
Comment 4 Eric Seidel (no email) 2007-06-06 00:38:06 PDT
Landed on the feature-branch as r22021.