Bug 30219

Summary: SVG Path elements are not correctly updated through pathSegList manipulation on relative values
Product: WebKit Reporter: Alexis Deveria <adeveria>
Component: SVGAssignee: Dirk Schulze <krit>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: davinci, jeffschiller, krit, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Bug Depends on: 10827    
Bug Blocks:    
Attachments:
Description Flags
Demo that shows an example of the issue (works in FF/Opera)
none
Tentative patch zimmermann: review-

Alexis Deveria
Reported 2009-10-08 10:01:09 PDT
Created attachment 40884 [details] Demo that shows an example of the issue (works in FF/Opera) Altering a path using pathSegList methods [1] fails to properly change the element's "d" attribute, nor is the element painted correctly. Note that it does correctly add the segment to the pathSegList, as can be discovered by using .getItem(n) or accessing the numberOfItems attribute. But that's pretty useless as the actual shape is not changed. I have attached an SVG file that attempts to add a segment using insertItemBefore, then checks if the "d" attribute has changed. Until this is fixed, if someone knows a workaround to accomplish the same effect that does not require manipulating the "d" attribute itself, that would be greatly appreciated. This functionality is needed for adding/removing nodes in the SVG-edit project [2], and currently only works in Firefox & Opera. [1] http://www.w3.org/TR/SVG/paths.html#InterfaceSVGPathSegList [2] http://svg-edit.googlecode.com
Attachments
Demo that shows an example of the issue (works in FF/Opera) (1.42 KB, image/svg+xml)
2009-10-08 10:01 PDT, Alexis Deveria 		no flags
Details
Tentative patch (7.89 KB, patch)
2010-03-24 18:54 PDT, David Yonge-Mallo 		zimmermann: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
David Yonge-Mallo
Comment 1 2010-03-24 18:16:40 PDT
I've looked a little bit into this and found the cause. The function SVGPathSegList::toPathData() is not completely implemented. Working on a patch.
David Yonge-Mallo
Comment 2 2010-03-24 18:54:43 PDT
Created attachment 51576 [details] Tentative patch I've made a tentative patch to fix this which adds code for relative coordinates in SVGPathSegList::toPathData(). If the approach looks good, SVGPathSegList::getPathSegAtLength() needs to be similarly updated as well.
David Yonge-Mallo
Comment 3 2010-03-25 07:44:03 PDT
One cause of the problem is bug 26487. It appears that all "Rel" path data commands are internally stored as "Abs" commands. Inserting a "relative lineto" (l - that's a lower case "L") command affects all subsequent data points. The above patch adds hackery to properly compute the coordinates in SVGPathSegList::toPathData(). The correct fix would be to not convert the data path commands to "Abs" to begin with.
Nikolas Zimmermann
Comment 4 2010-03-30 06:15:02 PDT
Comment on attachment 51576 [details] Tentative patch As discussed on webkit-dev, this is probably not the right approach. Did you investigate a bit more, David?
David Yonge-Mallo
Comment 5 2010-03-31 09:45:03 PDT
(In reply to comment #4) > (From update of attachment 51576 [details]) > As discussed on webkit-dev, this is probably not the right approach. Did you > investigate a bit more, David? I investigated a bit more, but I haven't had time to come up with a proper solution.
Dirk Schulze
Comment 6 2010-08-14 23:25:18 PDT
It is now possible to create unaltered SVGPathSegLists. Still use normalized SVGPathSegLists, untill we have a way to synchroinze dAttr, pathSegList and normalizedPathSegList.
Nikolas Zimmermann
Comment 7 2010-11-17 06:28:31 PST
*** This bug has been marked as a duplicate of bug 49580 ***
Note You need to log in before you can comment on or make changes to this bug.