[webkit-reviews] review denied: [Bug 63727] animate-dom-02-f.html test from SVG1.1SE testsuite fails : [Attachment 99416] Patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has denied Rob Buis
<rwlbuis at gmail.com>'s request for review:
Bug 63727: animate-dom-02-f.html test from SVG1.1SE testsuite fails
https://bugs.webkit.org/show_bug.cgi?id=63727

Attachment 99416: Patch
https://bugs.webkit.org/attachment.cgi?id=99416&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=99416&action=review

This still needs more tests I fear: I'm quoting the relevant text from the
beginEvent spec:
"This event is raised when the element local timeline begins to play. It will
be raised each time the element begins the active duration (i.e. when it
restarts, but not when it repeats). It may be raised both in the course of
normal (i.e. scheduled or interactive) timeline play, as well as in the case
that the element was begun with the beginElement or beginElementAt methods.
Note that if an element is restarted while it is currently playing, the element
will raise an end event and another begin event, as the element restarts"

That means we want a test for:
- beginEvent/endEvent fired by beginElement/endElement calls (a simple test,
you added such a test already)
- beginEvent/endEvent fired for a regular anim that has begin="0.1s" end="0.2s"
(a simple test)
- endEvent/beginEvent re-fired when restarting an element when the animation is
still running (aka. begin/endElement calls while a regular anim is running)

Another part of that spec:
Bubbles: No
Cancelable: No

You should write test to make sure these events aren't cancelable, nor do they
bubble.
Looking forward to them! :-)

Note the same applies to endEvent/repeatEvent as well, we should at least make
sure the basic situations covered by the spec are implemented exactly like
that.

> LayoutTests/svg/custom/animate-dom-02-f.svg:27
> +    <d:testDescription xmlns="http://www.w3.org/1999/xhtml"
href="http://www.w3.org/TR/SVG11/animate.html#InterfaceSVGAnimationElement">
> +	      <p>
> +		This tests that the methods on the ElementTimeControl
> +		interface return the undefined value, since the IDL
> +		operations are declared to return void.
> +	      </p>
> +	      <p>
> +	       After the loading the document, a rectangle is shown
> +	       indicating whether all four methods from the ElementTimeControl
> +	       interface returned undefined when invoked.  The rectangle
> +	       is black if the test did not run, red if the test failed
> +	       and green if the test succeeded.
> +			</p>

You should remove the whole boilerplate from this file, its not related to the
original test anymore.