380417 - [FIX]SVG Load Event Doesn't Always Fire, Recent Regression

Status: RESOLVED FIXED

(Core :: SVG, defect, P1)

Description

[Duncan Loveday]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070510 Minefield/3.0a5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070510 Minefield/3.0a5pre

In the attached test case, most of the time the SVG load event doesn't fire. The SVG is imported into the HTML by means of an EMBED written using javascript document.write(). The problem may be timing dependent since removing the alert message from the test case makes it work correctly.

Reproducible: Sometimes

Steps to Reproduce:
1. Open the attached test.html
2. Click the reload button and/or click in the location field and hit enter.
3.
Actual Results:  
In most cases, the following alerts are displayed:
    Loading doc
    HTML load handler
Occasionally, the following alerts are displayed:
    Loading doc
    SVG load handler
    HTML load handler

Expected Results:  
The correct behaviour is to consistently display
    Loading doc
    SVG load handler
    HTML load handler

This works correctly on FF2 and on recent trunk builds, no more than a couple of weeks old.

Comment 1

[Duncan Loveday]

Attachment: test SVG source

Comment 2

[Duncan Loveday]

Attachment: test Javascript source

Comment 3

[Duncan Loveday]

Attachment: test HTML source

Comment 4

[Duncan Loveday]

Attachment: test Javascript source

Unfortunately, I can't recreate the fault with files that work live in the bug, possibly because of time dependency. Reattaching files with relative URLs.

Save the three files test.html, test.js, test.svg in a directory and open test.html.

Comment 5

[Duncan Loveday]

Attachment: test HTML source

Comment 6

[Duncan Loveday]

The latest three attachments recreate the bug consistently for me when saved in a directory. Comment out the "loading doc" alert from test.html and it starts working.

Comment 7

[Ria Klaassen (not reading all bugmail)]

Attachment: the three files zipped up

Comment 8

[Ria Klaassen (not reading all bugmail)]

Regression range for this is http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1178137320&maxdate=1178139779

Comment 9

[Boris Zbarsky [:bzbarsky]]

Ah, yes.  So SVG fires onload events for its elements only if there is already a presentation set up for the document by the time the close tag for the element is parsed.  

Bug 379485 changed things so that in fact there is no presshell in this case, because the frame for the <embed> hasn't been created yet.

The question I have is whether SVG should fire the events independently of presentation existence...

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

I assume that SVG frames do not listen for SVG load events, right?

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

I *think* SVGLoad doesn't need prescontext at all.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 264524 [details] [diff] [review]
Proposed fix

Though, this is ok too, works like normal DOM event dispatching where context is used if there is one.

Comment 13

[tor]

If we're using the event dispatcher, do we want to set the NS_EVENT_FLAG_CANT_BUBBLE flag?

http://www.w3.org/2003/01/REC-SVG11-20030114-errata.html#SVGLoad%20does%20not%20bubble

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

yes.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Oh, that is just proposed errata. Does that mean that we should wait until it has been somehow accepted or ?

Comment 16

[tor]

It seems the errata is just to simplify the language, not to change the actual behavior.

Comment 17

[Boris Zbarsky [:bzbarsky]]

I can add that, sure.  Do you want to see an updated patch?

Comment 18

[tor]

Comment on attachment 264524 [details] [diff] [review]
Proposed fix

sr=tor with bubble flag added.

Comment 19

[Jonathan Watt [:jwatt]]

Comment on attachment 264524 [details] [diff] [review]
Proposed fix

Sorry for the delayed review! r=jwatt, but please do add the NS_EVENT_FLAG_CANT_BUBBLE flag.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Checked in.