Bug 3784 - JavaScript save dialog disappears right away (sheet triggers blur event)
Summary: JavaScript save dialog disappears right away (sheet triggers blur event)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 412
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2005-06-30 14:45 PDT by tacoloco
Modified: 2006-03-21 20:57 PST (History)
3 users (show)

See Also:


Attachments
crash log (25.58 KB, text/plain)
2006-03-12 07:08 PST, Alexey Proskuryakov
no flags Details
patch to avoid giving focus/blur events when sheets appear/disappear (3.41 KB, patch)
2006-03-14 05:21 PST, Darin Adler
no flags Details | Formatted Diff | Diff
better version of focus/blur patch (3.66 KB, patch)
2006-03-19 10:54 PST, Darin Adler
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description tacoloco 2005-06-30 14:45:14 PDT
If a javascript-generated page in a new window contains code to close this window as soon as any click 
happens, and the user has a save dialog box open at this moment, Safari crashes immediately.

Example:
1. Go to http://dfdl.de/galerie_content/DFDL.php?folge=44
2. Click on the first picture
3. Right-click on the picture on the newly opened page, press and hold option and select "Save Image 
As..." -> Crash happens.

This is a reproducible crash, so I have set it to P1.
Comment 1 Oliver Hunt 2005-06-30 23:30:52 PDT
I can't reproduce -- although the save as dialog disappears immediately...
Comment 2 tacoloco 2005-07-01 11:24:43 PDT
Alright, this is strange, as I've created a new user on the same machine and it doesn't crash with this new 
user. Maybe some bad behaving extension? I've tried to pinpoint a bad extension amongst the dozens I've 
got, adding extension after extension to the new user without being able to crash it once. After 2 hours I 
gave up.

Any hints on how to isolate this bug, short of copying all my user files to a new user, then deleting stuff 
from it until it works again? Cause I can't do that, not enough hd space :-( Would a crash report help in 
this occasion?
Comment 3 Oliver Hunt 2005-07-08 03:51:25 PDT
> from it until it works again? Cause I can't do that, not enough hd space :-( Would a crash report help in 
> this occasion?

Yes, even if all it does is clear webkit of responsibility :)
Comment 4 David Storey 2005-07-08 08:30:04 PDT
(In reply to comment #1)
> I can't reproduce -- although the save as dialog disappears immediately...

I can't reproduce either, and get the same problem with the save dialogue.  Maybe that is a bug in itself, 
as I've not noticed this before and it should really let you choose a location when you ctrl-option click.  

Comment 5 Joost de Valk (AlthA) 2005-07-08 10:57:08 PDT
K, i get the same problem with the save dialog, renaming the bug to reflect that, and asking the assignee 
of this bug to check for crashes too, since we can't reproduce them but they probably have happened. 
Confirming the bug, newly named: Javascript save dialog dissapearing instantly.
Comment 6 Darin Adler 2005-09-04 13:24:26 PDT
I can't reproduce the bug described here.
Comment 7 Darin Adler 2005-10-10 08:17:54 PDT
Reducing priority since this is not a reproducible crash.
Comment 8 Alexey Proskuryakov 2006-03-12 07:08:09 PST
Created attachment 7022 [details]
crash log
Comment 9 Alexey Proskuryakov 2006-03-12 07:12:50 PST
I can reproduce the crash with the following steps:

0. Enter the following command in Terminal:
mv ~/Library/Preferences/com.apple.Safari.plist ~/Library/Preferences/com.apple.Safari.plist.backup
defaults write com.apple.Safari NSNavPanelExpandedStateForSaveMode -bool YES
run-safari
1. Go to http://dfdl.de/galerie_content/DFDL.php?folge=44
2. Click on the first picture
3. Right-click on the picture on the newly opened page, press and hold option
and select "Save Image As..." -> Crash happens.

Whether this is a bug in WebKit, Safari or AppKit is a different question, of course.
Comment 10 Darin Adler 2006-03-14 05:13:59 PST
I still can't reproduce a crash. Alexey's steps do reliably reproduce a case where the window is closed right away.

The reason for that is that the site has a blur handler that closes the window. The bug here is that we deliver a blur event when the sheet comes up, which isn't right. The sheet shouldn't count for purposes of the JavaScript blur and focus events.

The source of that trouble is -[WebHTMLView _updateFocusState]. It needs to treat the window as key for purposes of focus and blur events even when there's a sheet up. So instead of just calling -[NSWindow isKeyWindow] it should probably call -[NSApplication keyWindow] and check if the key window is either the WebHTMLView's own window or the attachedSheet of that window. Similarly, if the key window is a modal dialog I don't think we should deliver a blur event.

Two things that might make this tricky to fix:

    1) We probably want to treat the window as not focused for purposes of the window's appearance. So we should be careful to only do the new check for the value for setWindowHasFocus: and leave setDisplaysWithFocusAttributes: alone, still using isKeyWindow.

    2) -[WebHTMLView _updateFocusState] is currently called from -[WebHTMLView windowDidBecomeKey:] and -[WebHTMLView windowDidResignKey:], and we register for those only for the window itself in -[WebHTMLView addWindowObservers]. If we leave the window in a focused state when the sheet becomes the key window, we won't get any notification at all when the sheet resigns key if you click on some other window. And we'd want to send a blur event in that case. One way to fix this would be to register for those notifications on a sheet when the sheet is up. We could register for NSWindowWillBeginSheetNotification and NSWindowDidEndSheetNotification notifications, and then set up and tear down an observer for the sheet. A simpler way would be to register for NSWindowDidBecomeKeyNotification and NSWindowDidResignKeyNotification globally without passing a window pointer, and then check the window pointer in the handler method.

None of this necessarily has to do with the crash; I can't reproduce the crash so I'm not sure. You could probably still reproduce the crash by just having JavaScript code in some other window that closes this window. It doesn't have to be from the blur event. Or you could still trigger the blur event with the save dialog up by clicking in another window.
Comment 11 Darin Adler 2006-03-14 05:21:56 PST
Created attachment 7058 [details]
patch to avoid giving focus/blur events when sheets appear/disappear

Here's a patch implementing my suggestion. Since I can't reproduce the crash, I don't know if this fixes the crash, or perhaps just makes it harder to reproduce (since you can still make the sheet go away if you click in another window).
Comment 12 Alexey Proskuryakov 2006-03-14 09:55:24 PST
I could not reproduce the crash with this patch applied, even when dismissing the popup by clicking in other windows.

With 2.0.3 and ToT, it was reproducible on both machines I tried, G4/1.25DP and G4/867DP (even with one processor disabled).
Comment 13 Darin Adler 2006-03-19 10:54:52 PST
Created attachment 7177 [details]
better version of focus/blur patch
Comment 14 Alice Liu 2006-03-20 06:38:06 PST
<rdar://problem/4483827>
Comment 15 Maciej Stachowiak 2006-03-20 22:45:35 PST
Comment on attachment 7177 [details]
better version of focus/blur patch

r=me