99820 - document.addEventListener fails on load and unload

Status: RESOLVED WONTFIX

(Core :: DOM: Events, defect, P3)

Description

[Bob Clary [:bc] (inactive)]

Steps to reproduce 

1.  create an HTML Document
2.  create a load / unload event handler
3.  use document.addEventListener to add the load / unload event handler to the
document

Expected Results:

load / unload event handler fires when document loads / unloads
Actual Results:

load event handler does not fire unless the document contains an object that
fires a load event and the event handler was added with capture = true.

unload event never fires.

Test Cases:

#1 document with no images using capture=true.
load does not fire, unload does not fire

#2 document with an image using capture=true
load does fire, unload does not fire

#3 document with no images using capture = false
load does not fire, unload does not fire

#4 document with an image using capture = false
load does not fire, unload does not fire

Note that this implies the HTMLBodyElement has the same bug.

Comment 1

[Bob Clary [:bc] (inactive)]

Attachment: testcase 1: document with no images using capture=true

Comment 2

[Bob Clary [:bc] (inactive)]

Attachment: testcase 2: document with an image using capture=true

Comment 3

[Bob Clary [:bc] (inactive)]

Attachment: testcase 3: document with no images using capture = false

Comment 4

[Bob Clary [:bc] (inactive)]

Attachment: testcase 4: document with an image using capture = false

Comment 5

[Bob Clary [:bc] (inactive)]

Tested with:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.4+) Gecko/20010913

Adding testcase keyword

Comment 6

[Vladimir Ermakov]

Also happens on linux branch 2001-09-10 OS - ALL

Comment 7

[Bob Clary [:bc] (inactive)]

with Mozilla 2001-10-30-03/win2k the load event does not fire in any of the test
cases. 

Comment 8

[seth]

With Mozilla 2001-10-30-08/MacOS the load event doesn't fire in any of the test
cases.

Comment 9

[Asa Dotzler [:asa]]

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)

Comment 10

[Boris Zbarsky [:bzbarsky]]

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

Comment 11

[Hixie (not reading bugmail)]

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

Comment 12

[Hixie (not reading bugmail)]

.

Comment 13

[Vedran Miletic]

retargeting

Comment 14

[Alex Vincent [:WeirdAl]]

Testcases 3 and 4 are invalid.  The DOM 2 Events REC states load and unload
events do not bubble.

http://www.w3.org/TR/2000/REC-DOM-Level-2-Events-20001113/events.html#Events-eventgroupings-htmlevents

Comment 15

[Alex Vincent [:WeirdAl]]

I seriously doubt the validity of testcase 2 as well.  I do not see anything in
the second testcase which is being unloaded.

Testcase 1 may be showing us a valid bug; I have a different testcase which
shows the load event propagating to the <xul:tabbrowser/> element of the Mozilla
browser, but not into its content document.

Comment 16

[Alex Vincent [:WeirdAl]]

Please observe bug 111411 comment 12 for a little more detail (albiet DOM
Inspector specific) on what I'm seeing with the unload event.  Similar behavior
should exist for the load event.

Comment 17

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

Attachment: fire load event also for document and body

What you think about this patch.
Also, see
http://www.w3.org/TR/2003/NOTE-DOM-Level-3-Events-20031107/events.html#Events-eventgroupings-htmlevents

Comment 18

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

Is it actually enough to dispatch the
event only at the body element (at least if the document is an
HTML document)? 
The event capture phase should start at document anyway.
If this is the right thing to do, I can make a new patch
later today.

Comment 19

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

Frames should be handled in
http://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#930

Comment 20

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

And changes to unload could be done in
http://lxr.mozilla.org/seamonkey/source/content/base/src/nsDocumentViewer.cpp#1070

Comment 21

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

Forget my patch, things are even more broken.
The onload="" etc. on body and frameset elements are registered to the 
mDocument->GetScriptGlobalObject(). That is wrong.

Comment 22

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

Attachment: patch for DOM 3 Events: load and unload

This is a bit better patch. Not yet fully tested or anything.
jst, what you think about this approach? The patch is surprisingly small :)

Comment 23

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

Attachment: this is the right one

Comment 24

[Johnny Stenback (:jst)]

Comment on attachment 147350 [details] [diff] [review]
this is the right one

This patch will make Mozilla fire both window.onload and document.body.onload
(in that order) if they're both set, no?

I'm fairly sure we won't be able to get away with that. You'll need to test
what IE does, and we need to match IE pretty closely here since there's a *ton*
of expectations on how this is supposed to work, and if it's wrong, sites will
break (as they do already since Mozilla doesn't quite match IE wrt onload
handling).

Comment 17 in bug 201828 may be of some help for you here.

Comment 25

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

Is the following line going to cause us trouble if we 
fire the load event only on body. 
http://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#878
Currently the load event (if fired on body) does not go up to the global object, 
it is stopped at the nsGenericElement.cpp. In my patch I changed it so that it 
goes up to the nsDocument but not to the global object. That was the reason for 
two events. 
hmm..maybe it is enough to modify the nsIScriptGlobalObject a bit, one new 
method SetLoaded(PRBool aLoaded) or something.

Comment 26

[Johnny Stenback (:jst)]

Yeah, the window does need to know when its content is done loading (mostly for
the popup blocking code to work right), but yeah, that can be done by adding a
method to nsIScriptGlobalObject or nsPIDOMWindow...

Comment 27

[aaronr]

Attachment: body load handler testcase for IE

run this testcase to see how body events are ordered on IE

Comment 28

[aaronr]

Attachment: body load handler testcase for Mozilla

run this testcase to see how body load/unload events are ordered on Mozilla

Comment 29

[aaronr]

after you run the testcases I attached, you'll see that IE will handle the body
load event and then the window load event.  Mozilla (without this patch) is just
the opposite.  Mozilla will handle the window load event before the body load event.

Also, if you change test1 to work with IE, you'll see that Mozilla currently
behaves the same as IE (I used IE6 for my testing).

I know that DOM3 specifies that a load and unload should be fired at the
document node (DOM2 wasn't that specific), so a fix for this bug should go in,
but I am wondering what level of incompatibility with IE is ok (since we already
have some) and what isn't?  Also, does anyone have a good idea of what web
authors have done to date to keep pages consistent when viewed on IE vs Mozilla
in respects to window, document and body event handling?  I'm asking this in
regard to comment #24 above.

Comment 30

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

Aaron, thanks for your tests. But I think we should extend the
test case to cover also "window.onload = a_handler_function;". 
I checked the safari and khtml code and if I now remember right, it is firing
events on body and window, although the target of the event is wrong.
(and of course the load/unload does not work at all on safari/khtml with XHTML,
because it doesn't really understand XHTML.)
I did some tests also on Opera, and AFAIK it registers window.onload to document.

This is complex, but we just have to decide the desired behaviour(, which
must be "compatible enough" with the Web).

Comment 31

[aaronr]

Attachment: multiple testcases and table of results from running on different browsers

New testcases and a result matrix (matrix.html) of what the results were
running these tests on various browsers.  As you can see, there a HUGE
differences between the various browsers behaviors.

Please also look at a post that I put on n.p.m.dom entitled, "questions - DOM
event ordering for bug99820/XML Events".  I'm looking for feedback as to what
should be the desired affect after this patch goes into place.	Trying to
minimize the impact to legacy web pages, but still honor the specs.  Any input
appreciated.

--Aaron

Comment 32

[Hallvord R. M. Steen]

Attachment: Test case for capturing load events

A capturing load event listener should detect all load events in document. This
will make scripts like http://www.hallvord.com/opera/demo/progress/ work in
Gecko.

Comment 33

[Hallvord R. M. Steen]

(In reply to comment #14)
> Testcases 3 and 4 are invalid.  The DOM 2 Events REC states load and unload
> events do not bubble.

Events that do not bubble still go through the capture phase.
http://www.w3.org/TR/2003/NOTE-DOM-Level-3-Events-20031107/events.html#Events-
phases

Comment 34

[Martijn Wargers (dead)]

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

Comment 35

[Mathias Schäfer]

Please fix this bug. As of DOM 3 Events (WD 13 April 2006), »implementations are required to dispatch this event [i.e. load] at least on the Document node.«

document.addEventListener("load", function () { alert("Hello!"); }, false) doesn't work in Gecko, but I can't understand why. As far as I know, this is the only standardized way to register a load handler. I don't think testcase 3 is invalid.

I think there's an inconsistency. If I use the non-standardized window.addEventListener("load", function (e) { alert(e.target + " " + e.eventPhase); }, false), the target of the event is the document object and eventPhase says target phase. That's correct according to DOM 3 Events, but this is completely illogical, because the handler was registered to the window object. Why isn't it possible to register a load handler directly to the window.document (DOM Document) object (with capture=false)?

Comment 36

[Joao Eiras]

It's a bit incredible how long this bug has been neglected, given it's severity.

Comment 37

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

Hmm, I wonder how many regressions there would be if this was fixed.
(See for example bug 335251, which shows that making load events to 
work as they should is difficult - because of backward compatibility)

But maybe something could be done... 

Comment 38

[Joao Eiras]

The only problem here is that this bug exists for too long, which lead for scripts using true as 3rd parameter erroneously. This however violates the specification, and already breks Opera which is currently the only browser supporting capturing event listeners.

Anyway, there will be lots of improvements, aditions, and corrections to 1.9 which will cause lots of regressions. The problem however doesn't lie within the rendering engine but within the webpages. So you can either choose to stop developing the rendering engine, and let the web stall, or continue to evolve together with the web.

Comment 39

[Anne (:annevk)]

Per the HTML Standard the load and unload events do not dispatch on document.