Bug 63727 - animate-dom-02-f.html test from SVG1.1SE testsuite fails
Summary: animate-dom-02-f.html test from SVG1.1SE testsuite fails
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL: http://dev.w3.org/SVG/profiles/1.1F2/...
Keywords:
: 81995 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-06-30 08:55 PDT by Rob Buis
Modified: 2014-12-02 06:21 PST (History)
10 users (show)

See Also:


Attachments
Patch (41.00 KB, patch)
2011-06-30 09:22 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-03 (1.45 MB, application/zip)
2011-06-30 10:08 PDT, WebKit Review Bot
no flags Details
Patch (17.09 KB, patch)
2011-06-30 10:14 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (42.42 KB, patch)
2011-06-30 11:44 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (49.06 KB, patch)
2011-06-30 13:17 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-02 (1.25 MB, application/zip)
2011-06-30 14:59 PDT, WebKit Review Bot
no flags Details
Patch (49.06 KB, patch)
2011-06-30 19:20 PDT, Rob Buis
zimmermann: review-
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (1.71 MB, application/zip)
2011-06-30 20:07 PDT, WebKit Review Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2011-06-30 08:55:52 PDT
This fails because no events are fired at all during animation (onbegin/onrepeat/onend)
Comment 1 Rob Buis 2011-06-30 09:22:02 PDT
Created attachment 99313 [details]
Patch
Comment 2 Gustavo Noronha (kov) 2011-06-30 10:00:32 PDT
Comment on attachment 99313 [details]
Patch

Attachment 99313 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8958728
Comment 3 WebKit Review Bot 2011-06-30 10:08:31 PDT
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
Comment 4 WebKit Review Bot 2011-06-30 10:08:37 PDT
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
Comment 5 Rob Buis 2011-06-30 10:14:37 PDT
Created attachment 99324 [details]
Patch
Comment 6 Rob Buis 2011-06-30 10:25:25 PDT
Comment on attachment 99324 [details]
Patch

I forgot to add some files, getting used to git again :/
Comment 7 Rob Buis 2011-06-30 11:44:34 PDT
Created attachment 99345 [details]
Patch
Comment 8 Gustavo Noronha (kov) 2011-06-30 11:56:39 PDT
Comment on attachment 99345 [details]
Patch

Attachment 99345 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8963579
Comment 9 Nikolas Zimmermann 2011-06-30 12:09:06 PDT
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.
Comment 10 Rob Buis 2011-06-30 13:17:16 PDT
Created attachment 99361 [details]
Patch
Comment 11 Gustavo Noronha (kov) 2011-06-30 14:49:13 PDT
Comment on attachment 99361 [details]
Patch

Attachment 99361 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8959793
Comment 12 WebKit Review Bot 2011-06-30 14:54:28 PDT
Comment on attachment 99361 [details]
Patch

Attachment 99361 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8965550
Comment 13 WebKit Review Bot 2011-06-30 14:59:32 PDT
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
Comment 14 WebKit Review Bot 2011-06-30 14:59:38 PDT
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
Comment 15 WebKit Review Bot 2011-06-30 15:55:26 PDT
Comment on attachment 99361 [details]
Patch

Attachment 99361 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8970200
Comment 16 Collabora GTK+ EWS bot 2011-06-30 16:31:17 PDT
Comment on attachment 99361 [details]
Patch

Attachment 99361 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8970216
Comment 17 Rob Buis 2011-06-30 19:20:04 PDT
Created attachment 99416 [details]
Patch
Comment 18 WebKit Review Bot 2011-06-30 20:07:42 PDT
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
Comment 19 WebKit Review Bot 2011-06-30 20:07:48 PDT
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
Comment 20 WebKit Review Bot 2011-06-30 20:56:13 PDT
Comment on attachment 99416 [details]
Patch

Attachment 99416 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8962678
Comment 21 WebKit Review Bot 2011-06-30 22:05:53 PDT
Comment on attachment 99416 [details]
Patch

Attachment 99416 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8960793
Comment 22 Nikolas Zimmermann 2011-07-01 00:21:05 PDT
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.
Comment 23 Nicolas Hoizey 2012-08-01 04:44:57 PDT
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
Comment 24 Florin Malita 2013-01-03 09:52:20 PST
*** Bug 81995 has been marked as a duplicate of this bug. ***
Comment 25 jay 2013-01-03 10:08:29 PST
what is the delay on this?
18 months for something in the SVG test/harness?