Bug 12430 - SVG has two transform parsers when it should have one.
Summary: SVG has two transform parsers when it should have one.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-26 19:21 PST by Eric Seidel (no email)
Modified: 2007-06-06 00:38 PDT (History)
0 users

See Also:


Attachments
First attempt (13.75 KB, patch)
2007-06-02 07:38 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
slightly improved version of rob's patch, fixing one bug (15.84 KB, patch)
2007-06-05 16:33 PDT, Eric Seidel (no email)
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.