Bug 63727 – animate-dom-02-f.html test from SVG1.1SE testsuite fails

• Component: SVG
• Status: NEW

[Rob Buis]

This fails because no events are fired at all during animation (onbegin/onrepeat/onend)

[Rob Buis]

Created attachment 99313 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 99313 [details]
Patch

Attachment 99313 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8958728

[WebKit Review Bot]

Comment on attachment 99313 [details]
Patch

Attachment 99313 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8957778

New failing tests:
svg/W3C-SVG-1.1-SE/animate-dom-02-f.svg

[WebKit Review Bot]

Created attachment 99322 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Rob Buis]

Created attachment 99324 [details]
Patch

[Rob Buis]

Comment on attachment 99324 [details]
Patch

I forgot to add some files, getting used to git again :/

[Rob Buis]

Created attachment 99345 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 99345 [details]
Patch

Attachment 99345 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8963579

[Nikolas Zimmermann]

Comment on attachment 99345 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=99345&action=review

This seems like a good start, but you should at least add some more test -- that's tricky stuff! :-) r- as I had some comments and it doesn't build on gtk at present.

> Source/WebCore/svg/animation/SMILTimeContainer.cpp:268
> +            RefPtr<TimeEvent> event = TimeEvent::create(eventNames().beginEvent, animation->document()->defaultView(), 0);
> +            animation->dispatchEvent(event.release());

Why the refcounting round trip? just use animation->dispatchEvent(TimeEvent::create(...)

> Source/WebCore/svg/animation/SMILTimeContainer.cpp:298
> +            RefPtr<TimeEvent> event = TimeEvent::create(eventNames().endEvent, animation->document()->defaultView(), 0);
> +            animation->dispatchEvent(event.release());

Ditto.

> Source/WebCore/svg/animation/TimeEvent.h:47
> +protected:

Why protected, you mean private.

> Source/WebCore/svg/animation/TimeEvent.idl:30
> +        readonly attribute DOMWindow            view;
> +        readonly attribute long                 detail;
> +        
> +        [OldStyleObjC] void initUIEvent(in DOMString type, 
> +                                        in DOMWindow view, 
> +                                        in long detail);

No need to wrap lines, nor to line up the argument names.

[Rob Buis]

Created attachment 99361 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 99361 [details]
Patch

Attachment 99361 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8959793

[WebKit Review Bot]

Comment on attachment 99361 [details]
Patch

Attachment 99361 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8965550

[WebKit Review Bot]

Comment on attachment 99361 [details]
Patch

Attachment 99361 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8965575

New failing tests:
svg/W3C-SVG-1.1-SE/animate-dom-02-f.svg

[WebKit Review Bot]

Created attachment 99370 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[WebKit Review Bot]

Comment on attachment 99361 [details]
Patch

Attachment 99361 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8970200

[Collabora GTK+ EWS bot]

Comment on attachment 99361 [details]
Patch

Attachment 99361 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8970216

[Rob Buis]

Created attachment 99416 [details]
Patch

[WebKit Review Bot]

Comment on attachment 99416 [details]
Patch

Attachment 99416 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8959895

New failing tests:
svg/W3C-SVG-1.1-SE/animate-dom-02-f.svg

[WebKit Review Bot]

Created attachment 99423 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[WebKit Review Bot]

Comment on attachment 99416 [details]
Patch

Attachment 99416 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8962678

[WebKit Review Bot]

Comment on attachment 99416 [details]
Patch

Attachment 99416 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8960793

[Nikolas Zimmermann]

Comment on attachment 99416 [details]
Patch

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.

[Nicolas Hoizey]

I don't really know if it will help build test cases, but I created a really simple example showing addEventListerner('endEvent', …) doesn't work:

http://codepen.io/nhoizey/details/ydsqm

[Florin Malita]

*** Bug 81995 has been marked as a duplicate of this bug. ***

[jay]

what is the delay on this?
18 months for something in the SVG test/harness?

[Clément Ducerf]

any updates on this ?
safari is the last major browser that doesn't fire beginEvent, endEvent, repeatEvent