[Webkit-unassigned] [Bug 12073] Implement setCurrentTime() and pauseAnimations() on SVGSVGElement

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=12073





--- Comment #27 from Joel Webber <jgw at chromium.org>  2011-11-23 06:35:13 PST ---
(In reply to comment #22)
> (From update of attachment 116137 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=116137&action=review
> 
> I fully agree with Erics review comments. I have some more issues with testing:
> 
> > LayoutTests/svg/animations/script-tests/animate-setcurrenttime.js:31
> > +animate1.setAttribute("to", "200");
> > +animate1.setAttribute("fill", "freeze");
> 
> We need lots more tests, as you can guess :-)
> * Accumulating animations need to be tested
> * Several animations running in parallel
> * Nested time containers, test modifying timeline in a inner <svg> element.
>   <svg>some content <svg width="50" height="50"> rect-with-anim-in-inner-svg... </svg> </svg>
>   Test moving time in outer <svg>, should it affect the inner <svg> timeline?
> * Use pure SVG/HTML5 self-contained tests, not using script-tests/ framework as well
> ...
> I'm sure I could come up with more, once I have some more time to think about that.
> Do you also test your testcases in eg. Opera? Would be nice to have them pass in Opera fully as well.

The most recent patch has a test that I believe covers these cases. I'm still becoming familiar with the "dark corners" of the animation spec, so it's entirely possible that I've missed some cases. Please feel free to throw out any cases that come to mind, and I'll add them.

As for the script-tests/ framework question -- I've modified the test to be more self-contained, and it now uses only "fast/js/resources/js-test-pre.js" (for shouldBe() and the PASS/FAIL stuff), and otherwise uses the layoutTestController directly. I'm not sure if this is precisely what you had in mind. I chose not to do pixel tests because I felt it more important to cover all the intermediate states at various values of setCurrentTime(). But if you feel a pixel test would be helpful, I'm glad to add one.

Also, I jammed all the cases together into one big test-case. If you feel it would be better to split them up, again that's fine by me.

(I also just noticed that I left a couple of commented-out script includes in there -- I'll remove those on the next patch)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.