Bug 36718

Summary: SVG no AnimateColor for stroke or fill if they are set to none on target
Product: WebKit Reporter: Dirk Schulze <krit>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: charles.wei, robin.webkit, staikos, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Test. The rect should turn from red to green smoothly during a second.
none
Patch
none
Updated patch none

Dirk Schulze
Reported 2010-03-28 01:23:15 PDT
Created attachment 51853 [details] Test. The rect should turn from red to green smoothly during a second. If stroke is not set or 'none', the animation of stroke doesn't work. The color, defined in 'from', is used for the whole animation. The same for fill, if fill is set to 'none' on the target.
Attachments
Test. The rect should turn from red to green smoothly during a second. (406 bytes, image/svg+xml)
2010-03-28 01:23 PDT, Dirk Schulze 		no flags
Details
Patch (19.08 KB, patch)
2010-04-12 23:55 PDT, Robin Cao 		no flags
DetailsFormatted DiffDiff
Updated patch (42.33 KB, patch)
2010-04-20 03:04 PDT, Robin Cao 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Robin Cao
Comment 1 2010-04-12 23:55:05 PDT
Created attachment 53228 [details] Patch Patch
Dirk Schulze
Comment 2 2010-04-13 00:28:16 PDT
Why is the property type String? And why are both strings empty?
Robin Cao
Comment 3 2010-04-13 01:00:15 PDT
(In reply to comment #2) > Why is the property type String? And why are both strings empty? The property is set to "StringProperty" in SVGAnimateElement::resetToBaseValue, because there is no valid color for a none target. The check for empty is to ensure that this animate element is not indeed type String. I admit this patch looks ugly. Maybe the none target should be handled appropriately in SVGAnimateElement::resetToBaseValue. Do you have any suggestions?
Dirk Schulze
Comment 4 2010-04-13 01:18:15 PDT
(In reply to comment #3) > (In reply to comment #2) > > Why is the property type String? And why are both strings empty? > > The property is set to "StringProperty" in SVGAnimateElement::resetToBaseValue, > because there is no valid color for a none target. The check for empty is to > ensure that this animate element is not indeed type String. > > I admit this patch looks ugly. Maybe the none target should be handled > appropriately in SVGAnimateElement::resetToBaseValue. Do you have any > suggestions? Yes, if the property Color is set to String in resetToBaseValue, it should be fixed there.
Robin Cao
Comment 5 2010-04-20 03:04:47 PDT
Created attachment 53791 [details] Updated patch Update patch according to Dirk's advice. Many thanks to Dirk for the review.
Dirk Schulze
Comment 6 2010-04-20 06:40:55 PDT
Comment on attachment 53791 [details] Updated patch Great patch! r=me.
Dirk Schulze
Comment 7 2010-04-20 10:27:58 PDT
(In reply to comment #6) > (From update of attachment 53791 [details]) > Great patch! r=me. Land the patches manually soon.
Dirk Schulze
Comment 8 2010-04-20 10:32:23 PDT
Comment on attachment 53791 [details] Updated patch Clearing flags on attachment: 53791 Committed r57896: <http://trac.webkit.org/changeset/57896>
Dirk Schulze
Comment 9 2010-04-20 10:32:34 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.