User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4a) Gecko/20030305 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4a) Gecko/20030305 When writing XUL pages with the root XUL elements window/page/dialog, the onload event is fired twice if an iframe exists in it. If no iframe is given, the event fires once. If the iframe itself has an onload handler, three events are processed: one for the iframe, two for the root element. This bug must actually be around at least a year now, it is the reason for bug 147068. Reproducible: Always Steps to Reproduce: (Testcase follows.)

same bug with page/dialog instead of window

Sounds like something broke and the onload event is bubbling....

Bug 147068 has a simple workaround to circumvent this bug here, else bug 195885 would have been stuck. The workaround should be removed as soon as this bug here is resolved.

As Boris stated in a posting to npm.dom: > load events do not bubble, per DOM2 Events. > See http://www.w3.org/TR/2000/REC-DOM-Level-2-Events-20001113/events.html#Events-eventgroupings-htmlevents

The onload events that are firing on the iframe and bubbling up to the window don't have a target! The third event that fires on the window's document is correctly targetted at that document. Question: should you even be able to listen to a load of a frame using an bubbling event handler on the frame's container element?

Per DOM spec, no. Does it work with HTML? I would not be one bit surprised if this is a XUL hack...

OK, so using this html the main document actually loaded first - once... Then the iframe loads, and its body event handler target is its document. Then the iframe element onload handler fires, but with a null target. I can reload the iframe which fires the same two load events.

hmm... That looks like in HTML the load event is in fact not bubbling up to the window. Not sure where the extra event comes from, though.

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

This fixes the testcase in this bug and in bug 155188, by just syncing nsXULElement to nsGenericElement (again, dammit).

Comment on attachment 134745 [details] [diff] [review] Possible patch bryner? jst? What do you think? I have to say I'm not too happy with this fix... I'd prefer to introduce a NS_EVENT_FLAG_CANT_CAPTURE and set that as well as NS_EVENT_FLAG_CANT_BUBBLE on all these load/error events. Then we can just check for those flags here and not have to know about event types... Right now it looks like CANT_BUBBLE events do in fact bubble and just don't get processed during said bubble (I could be wrong here, though). Oh, and as for the event with no target, that's the result of the code athttp://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#883 ( GlobalWindowImpl::HandleDOMEvent). It creates an nsEvent (which hence has no target) and which also has no prescontext (so GetTarget always returns null).

Comment on attachment 134745 [details] [diff] [review] Possible patch Note that NS_IMAGE_LOAD is already "handled" at line 3057 (should the other types go there too?) while the tabbed browser currently requires NS_IMAGE_ERROR to bubble.

In fact, error events do bubble per the DOM spec. Why don't we bubble them up? Is it to prevent window.onerror from firing for broken images?

Although, the spec talks about "error" as applying to script execution only... too bad that's so out of touch with th reality of the past many years. :(

Comment on attachment 134745 [details] [diff] [review] Possible patch I'm really confused now. I think this patch stops error events from bubbling, which is not what I think you want. As for image load events, they can't bubble or be captured (as per my previous reference) but I don't know what you want there.

Neil, at the moment image error events bubble in XUL (and stuff depends on that). They do NOT bubble in non-XUL (and stuff may depend on that too). We need to resolve that issue. I think we all agree this patch is not what we want in the tree. It was just a demonstration of where the problem lies. The question is what behavior we _do_ want. My suggestion is as follows: 1) Load events should capture but not bubble. This requires fixing whatever chrome capturing event listeners we have to stop sucking and check the target (last time I tried making this change Tp skyrocketed). Even so, Tp may suffer due to added event propagation for all those images in webpages. Perhaps load events should not capture up the tree either, and we should fix chrome handlers that depend on them to? (I doubt we have many, or any, since they don't capture up the tree right now.) 2) Error events should capture and bubble. 3) Whether events capture/bubble should be controlled by flags per comment 11, so that the decision on whether it captures/bubbles or not only has to be made once.

bz, I'm all for introducing an NS_EVENT_FLAG_CANT_CAPTURE flag and eliminating the event type checking in all the places where we can. I'd much prefer that over the patch you attached... :-)

Comment on attachment 134745 [details] [diff] [review] Possible patch OK. Should we also change the behavior of the DONT_BUBBLE flag to actually skip bubbling instead of just preventing handling while bubbling?

I don't think we want to change how the bubbling works (bryner probably knows more about this than I do tho), since IIRC there's code that cares about those events during the bubbling stage (even if no registerd handlers are called) up the tree, or in chrome, or somewhere.

Erk. Could we fix that or something? We should really not be having code that cares about bubbling events that don't bubble....

Yeah, of course, assuming there's no good reason for such code (and assuming I'm not dreaming things up here), but shouldn't the bubbling change be a separate bug?

Well... this bug is about the fact that "onload" events bubble in XUL. They should not be doing that. So no, the bubbling change is exactly this bug. The capture change is what's sort of 'extra'.

Um, right. Then yeah, that should be fixed here :-)

Perhaps the right thing to do is to make the DONT_BUBBLE flag work as it does now (bubble up, but don't get processed) and set it on the relevant events... Then consider changing how the flag works in 1.7a? bryner? Thoughts?

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

Confirmed in 1.7b under Linux, OS should be changed to "All".

patching file `nsXULElement.cpp' Assertion failed: hunk, file patch.c, line 321 abnormal program termination

Confirmed to still exist in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040616

(In reply to comment #28) > Confirmed to still exist in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) > Gecko/20040616 Same here, with FireFox 1.0 on XP. My problem is a bit different tho, the onload is triggered when I load content into an iframe...possibly related to the same bug.

(In reply to comment #29) > (In reply to comment #28) > > Confirmed to still exist in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) > > Gecko/20040616 > > Same here, with FireFox 1.0 on XP. My problem is a bit different tho, the onload > is triggered when I load content into an iframe...possibly related to the same bug. > I managed a quick workaround to make it work for now in my project. Just setAttribute the onload to 'null' after the first onload (assuming its calling a js function), that keeps the window onload from triggering every single time I try loading content in an iframe in the same window.

With that patch using the testcase the "window" alert is displayed only once.

Comment on attachment 172627 [details] [diff] [review] Adapted patch for trunk r- See comment #16.

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

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

no approved patch -> moving to beta3 nomination

please contact certain vendors which are known to heavily rely on addEventListener('load',object,true) before you change the api such that all their code breaks permanently.

This should be now fixed (bug 234455), at least the test case wfm. Please reopen if there is still something to fix here.