Bug 12122 - SVGPathElement should not inherit from SVGPathParser
Summary: SVGPathElement should not inherit from SVGPathParser
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: 2007-01-05 01:09 PST by Eric Seidel (no email)
Modified: 2007-06-08 09:57 PDT (History)
2 users (show)

See Also:

First attempt (4.61 KB, patch)
2007-06-03 00:38 PDT, Rob Buis
eric: review-
Details | Formatted Diff | Diff
Addressing Eric's issues (29.82 KB, patch)
2007-06-07 09:32 PDT, Rob Buis
eric: 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-05 01:09:40 PST
SVGPathElement should not inherit from SVGPathParser

The current design makes it very difficult for things like SVGAnimateMotionElement to parse paths.  They have to create a heavy-weight SVGPathElement, when all they want is something capable of parsing a path into a SVGPathSegList

SVGPathElement is-a SVGPathParser, when it really should have-a SVGPathParser.  all of the "this->createMoveTo" calls would just change to "pathCreator->createMoveTo" calls, and the element could be passed into the SVGPathParser.  There could even be an SVGSegList specific subclass which took a seglist (or created one and then returned it).  SVGPathSegListPathParser perhaps?
Comment 1 Rob Buis 2007-06-03 00:38:01 PDT
Created attachment 14845 [details]
First attempt

This is not a full fix for this bug, but it helps the SVGAnimateMotionElement problem. Let me know if that is enough for this bug and whether SVGPathElement should be changed too.

Comment 2 Eric Seidel (no email) 2007-06-05 15:40:05 PDT
Comment on attachment 14845 [details]
First attempt

It's a rather incomplete fix.  Basically you're just adding another class to codify the existing hack of needing to allocate a class to do the parsing.  If you're going to add this PathBuilder class, i would think you would make SVGPathElement use it too.  I don't think that this fix is really much better than the existing hack.  If you got rid of the other instances of classes inheriting from SVGPathParser, that would be a more complete fix.  We can chat about this on IRC if you like.
Comment 3 Rob Buis 2007-06-07 09:32:39 PDT
Created attachment 14896 [details]
Addressing Eric's issues

This seems somewhat nicer IMHO.

Comment 4 Eric Seidel (no email) 2007-06-08 02:15:55 PDT
Comment on attachment 14896 [details]
Addressing Eric's issues

Looks good to me!  Great work!
Comment 5 Rob Buis 2007-06-08 09:57:16 PDT
Landed on feature branch in r22068.