There are two issues with the animation of the orient attribute: See http://jsfiddle.net/dodgeyhack/fajLbp48/8/ for a test 1. Animation logic from 'auto' to an angle is missing. 2. The animated value of orientType is not used within the renderer so although the animation is running and is reflected in the DOM, it is not rendered. This affects animations that have a differing type for the initial value and the animated from/to values. Current tests (listed below) just query the DOM via JS so have not picked this up. Current tests: LayoutTests/svg/animations/animate-marker-orient-from-angle-to-angle.html LayoutTests/svg/animations/animate-marker-orient-from-angle-to-auto.html LayoutTests/svg/animations/animate-marker-orient-to-angle.html
Created attachment 240242 [details] Patch
Currently in the process of trying to upload a new patch but webkit-patch is throwing an exception. Watch this space...
Created attachment 240585 [details] Patch
Attachment 240585 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 240609 [details] Patch
Comment on attachment 240609 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240609&action=review Patch looks great in general. Just some snippets. > Source/WebCore/svg/SVGAnimatedAngle.cpp:99 > + // Discrete animation - no linear interpolation possible between values (e.g. auto to angle) Period at the end. > Source/WebCore/svg/SVGAnimatedAngle.cpp:106 > + } else { add a return here and don't check for else. > LayoutTests/svg/animations/animate-marker-orienttype-2.html:21 > +<script> > +if (testRunner) { > + testRunner.waitUntilDone(); > + window.setTimeout(function() {testRunner.notifyDone()}, 1500); > +} > +</script> > +<svg xmlns="http://www.w3.org/2000/svg" viewbox="0 0 150 150" width="500" height="500"> > + <defs> > + <marker id="arrow" orient="auto" markerWidth="10" markerHeight="10" style="overflow:visible"> > + <path d="M5,0 L 0,-5 L0,5 z" fill="green" opacity="0.5" /> > + <animate attributeName="orient" from="auto" to="80" begin="0s" dur="1s" fill="freeze"/> > + </marker> > + </defs> > + <path d="M 20,20 L 80,80" marker-start="url(#arrow)" stroke="black"/> > +</svg> We use a different kind of tests for animations. Please take a look at the other tests in the same folder. Especially, take a look at the script-test folder and create a js file there. Then create a boiler plate file in the svg/animation folder. The animation tests test different values during the animation (so if the SVG DOM returns the right values) as well as rendering and repainting.
Created attachment 240766 [details] Patch
As discussed with Dirk: The pixel test portion of repaint tests has been deprecated. Therefore these tests aren't appropriate here (repaint tests wouldn't show orientation of the marker). We're not looking to verify the animation at various time points - that is handled by the js script animation tests which exist. These tests are verifying that the animated value is used when rendering.
Comment on attachment 240766 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240766&action=review r=me > Source/WebCore/svg/SVGAnimatedAngle.cpp:113 > + if (toAngleAndEnumeration.second == SVGMarkerOrientAngle) > + animatedAngleAndEnumeration.first = toAngleAndEnumeration.first; > + else > + animatedAngleAndEnumeration.first.setValue(0); > + return; We use early returns if there is a return right after the else. Can be fixed later though.
Comment on attachment 240766 [details] Patch Clearing flags on attachment: 240766 Committed r175525: <http://trac.webkit.org/changeset/175525>
All reviewed patches have been landed. Closing bug.