550975 - SMIL tests with SVG embedded in XHTML should trigger initial-pause off of "SVGLoad" event, not "load" event

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Daniel Holbert [:dholbert]]

In SMIL tests that have SVG embedded in XHTML, we should be listening for "SVGLoad" -- not "load" -- to capture the beginning of the document timeline. (see bug 535850 comment 6)

Comment 1

[Daniel Holbert [:dholbert]]

So I tried the following change on some mochitests:
> -  window.addEventListener("load", main, false);
> +  window.addEventListener("SVGLoad", main, false);
but then main never gets called.

One possible reason for that is that the <svg>...</svg> content is all before the <script>, so the <svg> element finishes loading before we've seen any script (or had a chance to listen for it.

So, I tried moving <svg> after the <script>, but that still didn't fix it -- "main" still never gets called.

The only thing that seemed to work was:
 (a) moving <svg> after the <script>
 (b) specifying "main" as onload handler directly on the <svg> element
      e.g. <svg onload="main()">

I'm still confused why I'm not receiving any "SVGLoad" events, though. Jwatt, do you know what might be going wrong?  (BTW, I searched MXR for SVGLoad, to see if we test for that event anywhere, and it seems we don't -- it's only mentioned in .cpp and .h files.)

Comment 2

[Daniel Holbert [:dholbert]]

Ah, it looks like you can't do addEventListener for "SVGLoad" on the _window_ object -- you have to call it on an <svg> element itself.

And of course, you have to do it before the SVG object's closing </svg> tag, or else the SVGLoad event will have already fired.

In order to listen to the SVGLoad event, we'd need to:
 (i)   move the <svg> subtree to after the <script class="testbody">
 (ii)  move the <script class="testbody"> inside the <svg> subtree
 (iii) dynamically generate all <svg> content

Alternately, we could:
 (iv)  just leave everything as-is, except add |onload="this.pauseAnimations()"| to the <svg> element.  (so that we'll immediately pause at time=0, let the rest of the document load, and then start evaluating the rest of our test)

I like (iv) the best.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: fix v1

Here's a fix. It:
 - Adds onload="this.pauseAnimations()" to <svg> elements, in all SVG mochitests that previously contained an initial (or effectively-initial) pause.
 - For the tests that I've added the onload handler to: remove no-longer-needed calls to pauseAnimations() & setCurrentTime(0), and replace them with checks to assert that we're already paused at time 0.
 - In a few cases, move some unnecessarily-global variables into main().

Comment 4

[Daniel Holbert [:dholbert]]

BTW, I've verified that these tests still all pass locally (after applying the fix). I've also got them running on TryServer right now.

Comment 5

[Daniel Holbert [:dholbert]]

pushed: http://hg.mozilla.org/mozilla-central/rev/b7d0174a4d37

Comment 6

[Daniel Holbert [:dholbert]]

When hacking on test_smilRetart.xhtml in Bug 580077, I realized that mochitest could benefit from having this bug's fix applied there, too (in the unlikely case that lots of time goes by between SVGLoad & the document's "load" event firing).  So, I added it:  http://hg.mozilla.org/mozilla-central/rev/c7f09e2013b7