Bug 3402

Summary:

unload event doesn't fire

Product:

WebKit

Reporter:

Brad <brkemper>

Component:

DOM

Assignee:

Maciej Stachowiak <mjs>

Status:

RESOLVED CONFIGURATION CHANGED

Severity:

Major

CC:

ahmad.saleem792, ap, bfulgham, bugs-webkit, cdumez, darin, emacemac7, eric, george.zhu, ian, jsp, karlcow, kingsinsear6, laszlo.gombos, rniwa, rwlbuis, slewis

Priority:

Version:

312.x

Hardware:

Mac

OS:

OS X 10.3

Attachments:

Description	Flags
Mondo test case	none
Bug3402 test page	none
Bug3402 test page (unzipped)	none
Initial patch	none
Patch	rniwa: review-

Brad

Reported 2005-06-09 21:13:14 PDT

most people are familiar with attaching javascript to the loading of a page by having an onLoad parameter in the body tag of a page. There is also a parameter called onUnload that is supposed to work the same way, except it is supposed to fire when the page is closed (such as when a window is closed). In Safari it doesn't. Non of the JavaScript code that is in the onUnload parameter of a body tag will be executed when the window is closed. But it should.

Attachments
Mondo test case (6.71 KB, text/html) 2005-06-29 06:09 PDT, Jesse Pelton	no flags	Details
Bug3402 test page (397 bytes, application/x-zip-compressed) 2007-01-05 01:16 PST, George Zhu	no flags	Details
Bug3402 test page (unzipped) (504 bytes, text/html) 2007-01-05 06:27 PST, Jesse Pelton	no flags	Details
Initial patch (2.89 KB, patch) 2008-05-08 12:58 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Patch (3.60 KB, patch) 2011-10-11 05:41 PDT, Rob Buis	rniwa: review-	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Jesse Pelton

Comment 1 2005-06-29 06:09:27 PDT

Created attachment 2701 [details] Mondo test case Safari is not alone in handling unload events unevenly. This test case uses multiple script-based approaches to setting an unload event handler on window and body objects. I tested with IE 6, Firefox 1.0.4, Opera 8.0, Konqueror 3.4, and Safari 2.0. Only Opera supported all the tested methods, but it invokes body unload handlers multiple times. Unfortunately, there is no single method that works for all browsers. I think that the ideal would be for all browsers to support all the tested methods so script authors don't have to work out which method works where, so I'll send this test case to the developers of all the browsers I tested. Safari is, from my point of view, the worst offender. All the other tested browsers support setting an unload event on the window object (as allowed in JavaScript since at least version 1.3), but Safari does not. While neither HTML 4.01 nor DOM 2 specifies an unload event for the window object, I think it should nonetheless be supported: Netscape's JavaScript specification is the de facto standard for the window object (since it's outside the domain of the W3C specs), and other browsers implement it.

Brad

Comment 2 2005-06-29 09:07:50 PDT

That is a very useful table in the attachment. In all the variants shown, the onunload handler is set within a JavaScript block. One other case however, is having the handler built into the body tag as a parameter, like this: <body onUnload="someFunction()"> Theoretically, it would be handled the same as Variant 4 (body.onunload), but my experience shows that it does not fire at all in Safari when it is set that way and the window closes (at least in cases where the entire window is closed, not just the page).

Alexey Proskuryakov

Comment 3 2006-02-08 11:56:23 PST

The attached test case doesn't currently work because of bug 7143.

Alexey Proskuryakov

Comment 4 2006-04-05 09:41:02 PDT

FWIW, a current build of WebKit (http://nightly.webkit.org) has the same behavior as Firefox 1.5 - tests 1 and 5 (and only those) pass.

Jesse Pelton

Comment 5 2006-04-05 11:04:38 PDT

That's progress of a sort, and it's probably sufficient unless the goal is complete support for W3C standards. At least there's now one variant (the first) that works for all the browsers indicated in the test case. While it would be cool if Safari could support variants 2, 3 and 4, only Opera supports 3 and 4, even though those appear to be mandated by the DOM standard. (It turns out that Opera does not support variant 2, contrary to what the test case says. I was misled by the browser's caching strategy. Reloading the page before running variant 2 does not result in the unload handler being called.)

Brad

Comment 6 2006-04-06 08:07:17 PDT

(In reply to comment #4) > FWIW, a current build of WebKit (http://nightly.webkit.org) has the same > behavior as Firefox 1.5 - tests 1 and 5 (and only those) pass. > Actually, #5 would be closest to setting explicitly in the HTML attribute. I just tested to see if having it in the HTML works now, and in the latest version (Version 2.0.3 (417.9.2)) it does! That is indeed good news. I personally think that is the most important one (along with being able to to the analogous "setAttribute()" in pure JavaScript (variant 5). Along with variant 1, this is enough for me as a developer/designer, and I think covers compatibility with the vast number of sites that use an unLoad event.

Brad

Comment 7 2006-04-06 08:09:26 PDT

Oh, BTW, when I say "latest version (Version 2.0.3 (417.9.2)) ", I mean Safari, not WebKit. So this is already in production (yeah!).

George Zhu

Comment 8 2007-01-05 01:16:40 PST

Created attachment 12237 [details] Bug3402 test page Bug3402 test page

George Zhu

Comment 9 2007-01-05 01:19:51 PST

Comment on attachment 12237 [details] Bug3402 test page I test it with Safari 2.0.4(419.3), onunload event fail to activate while in Firefox and IE, it works well. Is it a regression?

Jesse Pelton

Comment 10 2007-01-05 06:27:37 PST

Created attachment 12240 [details] Bug3402 test page (unzipped) Unzipped the test case to simplify testing.

Jesse Pelton

Comment 11 2007-01-05 06:29:12 PST

George's test case works for me with the same version of Safari.

George Zhu

Comment 12 2007-01-28 23:16:54 PST

(In reply to comment #11) > George's test case works for me with the same version of Safari. > Hi, Jesse Pelton, I finally find the difference, if I reload the document in Safari, yes, the event is activated. But if I click red close button on left top corner of the page, the event is not activate, which behaves differently from IE and Firefox... So the problem can be describe as: onUnload event does not fire when user close the window.

Jesse Pelton

Comment 13 2007-01-29 05:12:11 PST

It might or might not be a regression. My "mondo" test case, while it has its uses, ignores this case, and seems to have focused attention on the cases it does cover.

George Zhu

Comment 14 2007-01-29 21:54:02 PST

Ok, you are right, the problem mentioned here has already resolved, how to close it? On the other hand, I create a new bug 12469, you can find it here: http://bugs.webkit.org/show_bug.cgi?id=12469

Rob Buis

Comment 15 2008-05-08 12:58:47 PDT

Created attachment 21025 [details] Initial patch Just a first stab at this bug. In fact it only is meant for the third problem in the Mondo testcase, using addEventListener to register an unload event listener and then testing that it is called. I used the Mondo testcase to verify that this now works using the patch, however I would like to get some early feedback to a) get to know whether the approach is sane and b) whether there is a need to support the 2nd and 4th variant (which are the same problem). *If* the approach is sane there may be some other help elements that may use the same technique, ie. all that use setHTMLWindowEventListener should have this third problem. Cheers, Rob.

Darin Adler

Comment 16 2008-05-24 23:09:59 PDT

Comment on attachment 21025 [details] Initial patch The patch needs a test case for each of the events you're patching here; and we need to double check with other browsers behavior too. Also, I don't think this is the correct approach. I don't think we want to add these listeners to both the body element and the window object. Instead we want to dispatch these events to both the window object and the body element.

Maciej Stachowiak

Comment 17 2008-05-24 23:50:03 PDT

If the event bubbles, then dispatching to the body will also dispatch to the window (effectively). I agree with darin that adding the event listeners twice does not seem right. The relevant events should either dispatch to the body and bubble, or dispatch twice (to the body, then the window).

Stephanie Lewis

Comment 18 2008-05-25 00:28:32 PDT

At the moment adding onunload events and onbeforeunload events as anything other than a windowEventListener means that the events will never be dispatched to the element.

Rob Buis

Comment 19 2011-10-11 05:41:06 PDT

Created attachment 110502 [details] Patch

Rob Buis

Comment 20 2011-10-11 05:44:12 PDT

I was looking at this bug again and I don't think much is broken nowadays. The cases 2 and 4 of the mondo testcase actually work, provided a function object, not a string is handed over. Since the other implementations do not accept the string assignment either, but do accept the function object, I think it is correct to be strict here. For case 3 I uploaded a new patch. If this seems like the right direction I can add some tests. Cheers, Rob.

Darin Adler

Comment 21 2011-10-11 09:10:22 PDT

Comment on attachment 110502 [details] Patch Is this change correct? Why no test case?

Ryosuke Niwa

Comment 22 2011-11-28 16:55:33 PST

Comment on attachment 110502 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110502&action=review > Source/WebCore/ChangeLog:6 > + Fix addEventListener("unload") for body elements by propegating to the window object. I'm not sure if want to support document.body.addEventListener. It seems like it makes more sense to support document.body.setAttribute('onunload', ...). > Source/WebCore/ChangeLog:8 > + Reviewed by NOBODY (OOPS!). This line should appear before the description. > Source/WebCore/ChangeLog:10 > + No new tests. (OOPS!) We definitely need a test for this. r- because of this.

Ahmad Saleem

Comment 24 2022-08-09 05:42:06 PDT

Latest test results based on both tests across browsers: Test Case 1 - Bug3402 (unzipped) *** Safari 15.6 on macOS 12.5 *** It shows following in Console: >>> Use of window.alert is not allowed while unloading a page. When I try to click button, it shows following: ReferenceError: Can't find variable: Button1_onclick *** Chrome Canary 106 *** No Console Warning / Error but when I try to click button, it shows same as Safari that "Button1_onclick" is not defined. *** Firefox Nightly 105 *** Same as Chrome - no console warning / error but just "Button1_onclick" not defined when clicking button. ____________ Test Case 2 - Mondo Test case *** Safari 15.6 on macOS 12.5 *** Variant 1 - Open 'about:blank' on top of same tab (Also show same console warning / error as Test Case 1) Variant 2 - Open 'about:blank' on top of same tab Variant 3 - Open 'about:blank' on top of same tab Variant 4 - Open 'about:blank' on top of same tab Variant 5 - Open 'about:blank' on top of same tab (Also show same console warning / error as Test Case 1) *** Chrome Canary 106 *** Variant 1 - Open 'about:blank' on top of same tab Variant 2 - Open 'about:blank' on top of same tab Variant 3 - Open 'about:blank' on top of same tab Variant 4 - Open 'about:blank' on top of same tab Variant 5 - Open 'about:blank' on top of same tab *** Firefox Nightly 105 *** Variant 1 - Open 'about:blank' on top of same tab Variant 2 - Open 'about:blank' on top of same tab Variant 3 - Open 'about:blank' on top of same tab Variant 4 - Open 'about:blank' on top of same tab Variant 5 - Open 'about:blank' on top of same tab ______ Just wanted to share updated testing results, if you need me to test differently or different test case or it is covered via WPT and now working as intended. Please update the bug tag (e.g. RESOLVED CONFIGURATION CHANGED) or leave a comment. Thanks!

Karl Dubost

Comment 25 2024-12-01 21:38:39 PST

That seems to work the same everywhere indeed.

Note You need to log in before you can comment on or make changes to this bug.