Bug 12469 - window.alert from onUnload listener does not show when users close the window
Summary: window.alert from onUnload listener does not show when users close the window
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 419.x
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-29 21:29 PST by George Zhu
Modified: 2008-07-09 11:59 PDT (History)
3 users (show)

See Also:


Attachments
Open the page directly and test. (717 bytes, text/html)
2007-01-29 21:39 PST, George Zhu
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description George Zhu 2007-01-29 21:29:18 PST
The window object has an event onUnload, the Mozilla document describe it as:
The unload event is raised when the document is unloaded.
It is true that now this event fires when the document reload, the same as Firefox and IE. (this was fixed in bug 3402)

But the event fail to fire when user close the window. It is well known that closing window also requires unloading of document, so it would be fine if Safari can also support this feature as it is already implemented in Firefox and IE.
Comment 1 George Zhu 2007-01-29 21:39:06 PST
Created attachment 12774 [details]
Open the page directly and test.

Open the page directly and test.
Comment 2 Julien Chaffraix 2008-02-24 16:00:46 PST
> But the event fail to fire when user close the window. It is well known that
> closing window also requires unloading of document, so it would be fine if
> Safari can also support this feature as it is already implemented in Firefox
> and IE.

I have done some debugging and the event is dispatched when closing the window. The issue is that the listener try to call window.alert and fails in WebChromeClient::runJavaScriptAlert because the UIDelegate is null.
The cause is that Cocoa calls (void)setUIDelegate:delegate (in WebKit/mac/WebView/WebView.mm) to set it to null *before* sending the unload event.
My knowledge of the mac port is quite limited so I do not who to blame (Cocoa or WebKit)
Comment 3 Alexey Proskuryakov 2008-02-26 02:55:27 PST
In my testing, -[WebView setUIDelegate:] is called from WebKit itself:

#0  -[WebView setUIDelegate:] (self=0x37ed4c0, _cmd=0x3e1ab6, delegate=0x0) at /Users/ap/Safari/OpenSource/WebKit/mac/WebView/WebView.mm:2091
#1  0x003949e6 in -[WebView(WebPrivate) _close] (self=0x37ed4c0, _cmd=0x926aa890) at /Users/ap/Safari/OpenSource/WebKit/mac/WebView/WebView.mm:703
...

I'm not sure why this is classified as a major bug - what kind of important consequences does it have? Does showing an alert from onunload serve any purpose except for annoying the user?
Comment 4 Julien Chaffraix 2008-02-26 09:29:03 PST
(In reply to comment #3)
> In my testing, -[WebView setUIDelegate:] is called from WebKit itself:
> 
> #0  -[WebView setUIDelegate:] (self=0x37ed4c0, _cmd=0x3e1ab6, delegate=0x0) at
> /Users/ap/Safari/OpenSource/WebKit/mac/WebView/WebView.mm:2091
> #1  0x003949e6 in -[WebView(WebPrivate) _close] (self=0x37ed4c0,
> _cmd=0x926aa890) at
> /Users/ap/Safari/OpenSource/WebKit/mac/WebView/WebView.mm:703
> ...
> 

Odd, I have another call to setUIDelegate before the one you mention on my machine which originate from Cocoa :

#0  -[WebView setUIDelegate:] (self=0x2554c10, _cmd=0x3e13f8, delegate=0x0) at /Users/julien/Webkit/WebKit/WebKit/mac/WebView/WebView.mm:2093
#1  0x00049d21 in ?? ()
#2  0x925f3059 in -[NSArray makeObjectsPerformSelector:withObject:] ()
#3  0x925f3059 in -[NSArray makeObjectsPerformSelector:withObject:] ()
#4  0x92603d2d in -[NSArray makeObjectsPerformSelector:] ()
(...)

Beside the one you are pointing at is executed after the unload listener : in WebView _close, the unload event is dispatched in mainLoader->detachFromParent() and setUIDelegate is called afterwards.

> I'm not sure why this is classified as a major bug - what kind of important
> consequences does it have? Does showing an alert from onunload serve any
> purpose except for annoying the user?
> 

Updated the summary and the severity as the original bug was that we were not dispatching an unload event when closing the window (which is major).

I agree with you about how annoying alert from onunload listener can be but Firefox and Opera have chosen to display those so it is believed we should do the same.
Comment 5 David Kilzer (:ddkilzer) 2008-06-01 08:50:48 PDT
See Bug 18341.

Comment 6 Vicki Murley 2008-07-09 11:59:41 PDT
I can't reproduce this anymore, using WebKit nightly r34367.