Closed Bug 68454 Opened 24 years ago Closed 22 years ago

Crash when trying to save an image that opens in a javascipt popup - M1BR Trunk [@ comdlg32.dll]


(Core Graveyard :: File Handling, defect)

Not set


(Not tracked)



(Reporter: matxdr, Assigned: danm.moz)




(Keywords: crash, testcase, topcrash+, Whiteboard: [adt3])

Crash Data


(2 files, 2 obsolete files)

Using build 2001020904.

Go to  Click on any
picture, a pop up will open with the piicture in it.  Right click in that pop
up, select save as.

Result:  Crash
Expected:  Save image as dialog
in my place it doesn't crash, but indeed there's no 'save image as' dialog, just
'save (page) as'...
WORKSFORME on NT4 with build 2001020904
Sent TB26149258G.  
talkback says:

   COMDLG32.DLL + 0x1adb0 (0x7fe1adb0)    

I think that's a windows library. not very informative :(
Assignee: asa → vishy
Component: Browser-General → XP Apps
QA Contact: doronr → sairuh
interesting. i'm using 2001.02.15.08 comm bits on linux --i don't get a crash. i
do get the Save File dialog, go thru the motions to save the file [select dir,
supply filename (fwiw, i saved it as gif), click OK], but when i check the
target dir, nothing is saved!

over to bill [tentatively] --is what i'm seeing a dup?

is anyone else able to reproduce either the crash, or the inability to save the
file? thx!
Assignee: vishy → law
Ever confirmed: true
I can reproduce this (more or less) with a recent debug build.

Those popup pages are nasty.  They have an onblur handler that closes the
window.  Apparently, dismissing a context menu triggers that handler, which
closes the window.  When the context menu itself triggers actions that need the
window (e.g., Save As...), things go haywire.  At least that's my guess.
Probably an event/window kind of thing (for saair or danm), but I'll investigate

This is pretty severe (content can encourage the user to crash the browser);
nominating for nsbeta1.
Keywords: nsbeta1-
Keywords: nsbeta1-nsbeta1
I attached the call stack I get to when I recreate this in the debugger.  
Something is going very wrong as a result of the file picker's parent being 
closed out from under it.

Handing off to Dan Matejka (who used to be a big Cyndi Lauper fan, I hear).
Assignee: law → danm
Target Milestone: --- → mozilla0.9.2
->0.9.3 unless this is more widespread (Sorry, Cyndi!)
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Keywords: crash
Keywords: nsenterprise
*** Bug 89131 has been marked as a duplicate of this bug. ***
bug 89131 has a good test case.
OS: Windows ME → All
Hardware: PC → All
Adding talkback keyword topcrash, sig in the summary and cc'ing some talkback 
folks. Clutter everywhere.
Keywords: topcrash
Summary: Crash when trying to save an image that opens in a javascipt popup → Crash when trying to save an image that opens in a javascipt popup [@ nsVoidArray::RemoveElement]
Removing topcrash+signature. I think you've got the wrong bug.
Keywords: topcrash
Summary: Crash when trying to save an image that opens in a javascipt popup [@ nsVoidArray::RemoveElement] → Crash when trying to save an image that opens in a javascipt popup
Works for me on Windows. Anyone else?
I crash, current branch or trunk on win2k, for the steps to reproduce at the 
beginning of this bug.
Target Milestone: mozilla0.9.3 → mozilla0.9.4
I get this problem, too, only I am able to repeat on any arbitary image. I tried
it, in fact, on the main Mozilla banner at and the crash still happened.

Interestingly enough, I saved the image using IE 5.5, and Mozilla refused to
read it. I then tried to save the image using Netscape 4.7 and everything worked
fine. I'm not sure if that new information helps any.
Target Milestone: mozilla0.9.4 → mozilla0.9.5
I was digging around in Talkback data and came across a comment mentioning this
bug.  Here is the info:

===> 4   0x00000000 8560629a ===> 0x00000000 
     Crash date range: 2001-09-15 to 2001-09-18
     Min/Max Seconds since last crash: 157 - 150102
     Min/Max Runtime: 157 - 150102
     Count       Platform List 
   1 Linux 2.4.5
   2 Linux 2.4.9
   1 Windows NT  5.1 build 2600
     Count      Build Id List 
   4 2001091311
 Comment Keywords Count 
 Offset and and their stack traces 
===> 0x00000000 8560629a <===

     (35549279)	URL:
Comments: Clicking around caused a crash.  Site uses a lot of Flash and JavaScript.
     (35521442)	Comments: Just looking at the preferences. Didn't change anything!
     (35466168)	URL:
Comments: Crash in response to bug #68454Mozilla 0.9.4 (build 2001091303) on
WindowsXP Pro
     (35417357)	Comments: in the preferences - in the "tree":expanded the whole tree
compressed the first folder againbang

Incident 35466168 will probably be of most interest.  The last comment about
preferences I think is actually a crash represented by bug 86723.  I can't see
this bug and that bug being related, but Talkback reported both of them with the
same stack signature and offset...weird.
Target Milestone: mozilla0.9.5 → mozilla0.9.6
spam: over to File Handling. i have not changed the assigned developer [or the
other fields for that matter], so if anyone realizes that a bug should have a
more appropriate owner, go ahead and change it. :)
Component: XP Apps → File Handling
Blocks: 104166
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Target Milestone: mozilla0.9.7 → mozilla0.9.8
*** Bug 115542 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla0.9.8 → ---
Target Milestone: --- → mozilla1.0.1
No crash with Linux 2002032306. (No option in the popup to save the pic either.)
I still crash when I use old builds. Has this been fixed ?
*** Bug 134406 has been marked as a duplicate of this bug. ***
Crash with 2002040903/Win2K -> TB5004684W and TB5004607X.
Target Milestone: mozilla1.0.1 → mozilla1.1beta
*** Bug 132532 has been marked as a duplicate of this bug. ***

See bug 159793 for an always reproducible test case.

This bug happens only on Windows. It crashes in the "file save as dialog"
provided by the windows operating system. We dereference a null pointer. It
seems, the dialog inside comdlg.dll received a pointer from Mozilla - but
because the Mozilla window closes, the common dialog crashes.
Keywords: nsbeta1
What I saw (bug 132532) was that the parent window handle specified was
destroyed or otherwise invalid.
Adding M1BR Trunk [@ comdlg32.dll] so summary and topcrash+, testcase keywords.
 This is a topcrasher on the Gecko 1.0 branch and there are also incidents on
the MozillaTrunk.  A reproducible case is noted in comment #25.
Keywords: testcase, topcrash+
Summary: Crash when trying to save an image that opens in a javascipt popup → Crash when trying to save an image that opens in a javascipt popup - M1BR Trunk [@ comdlg32.dll]
(er, noting that, on the 1.0 branch, 4 talkback incidents is enough to grab
the seventh spot on the talkback top of the pops, and, er, kaie submitted the
requisite 4 incidents.).
Marking as adt3 based on Comment #28 From John Morrison. 4 incidents, submitted
byt the same person, doesn't seem like a topcrasher IMHO. Are there any other
recent incidents other than kaie's in talkback?
Whiteboard: [adt3] [ETA Needed]
Hmm, didn't want to discourage a fix. It's been a long-standing known problem
that bringing up a dialog from the context window for a window that sets 
'onblur="self.close()"' will crash.
Actually, when I went to the brief report on climate it only showed the four
entries by kaie. But when I ran this query myself, I got 143 crashes in 
comdlg32.dll since 07/01 on all builds (Gecko1.0, Trunk, 6.x). Not all the 
crashes are this one (some refer to printing) but many refer to saving an 
image (often also mentioning popup). 

I still think doing 'onblur="self.close()"' is a little sick, but it is a not
totally uncommon trap for the unwary.
the screenshot on will also crash
when trying to 'Save Image As...'

QA Contact: sairuh → petersen
Moving milestone out to future since we've obviously missed the 1.1beta.

Dan can let us know when he expects this will get fixed when he gets around to
looking at it again.
Target Milestone: mozilla1.1beta → Future
*** Bug 176824 has been marked as a duplicate of this bug. ***
*** Bug 177312 has been marked as a duplicate of this bug. ***
*** Bug 180056 has been marked as a duplicate of this bug. ***
*** Bug 162601 has been marked as a duplicate of this bug. ***
No talkback incidents from recent builds, and not showing up on any talkback
reports.  Marking fixed.
Closed: 22 years ago
Resolution: --- → FIXED
Still there in Mozilla 1.2.1

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.2.1) Gecko/20021130

Reproduce as desribed.

Sent TB16453791H
Confirming and reopening bug. Still reproducible with latest trunk on W2K using
testcase from bug 159793.
Resolution: FIXED → ---
Just a bit of status while I beat on this bug. Kai's testcase (comment 40) still
demonstrates the bug. But the original testcase currently appears fixed because
it's concealed by some very messed up event handling in the trunk. Cyndi's
popups don't crash because they never get blur events at all. See bug 191154.
(Even if you have a special window that gets blur events, it'll miss the first
one. See bug 166748. And once it starts getting events it'll get too many. Ssee
bug 164686.) So anyway, at least I understand why this crash happens less often
than it once did.
Whiteboard: [adt3] [ETA Needed] → [adt3] [ETA: soon. ugly but working patch for Windows in hand]
This is a windows-only version of the fix to solicit reviews on the direction
I'm taking with this. The bug is, windows with blur handlers which close the
window do indeed close as soon as context-menu->SaveImageAs summons the
filepicker dialog. Ugliness ensues. I believe it's best to suppress blur events
from the filepicker. IE behaves this way.

bryner and I figured there were two places to do this, in nsEventStateManager
or in nsWindow. nsESM is probably more correct but that module is pretty much
hidden from nsFilePicker (widget) code. The nsWindow route requires individual
fixes for each platform but involves less lifting of skirts.

This patch fixes the crash on Windows. Barring allergic reactions from widget
people, I intend to check in this patch with some cleanup, along with *nix and
Mac versions. Still I'm more interested in approval of concept than a real
review at this point.
Replacing the original patch. Once again this bug happens because we send blur
events to the window owning a filepicker window. If that window contains a blur
handler that closes itself, the window containing the data referenced by the
filepicker is destroyed out from underneath it.

IE doesn't send blur events in this case, and that seems the way to be. Sadly,
the Windows and gtk builds have completely different behaviour. The gtk build
uses a XUL filepicker; Windows a native one. I've chosen to suppress blur
events on XUL platforms within the DOM window, and on native platforms within
their respective widget libraries.

The two systems are completely independent and parallel. They could be hooked
together. However that would involve changing nsIWidget. There's resistance to
that (though it's not out of the question). It would also require getting at
Views from within the widget code. Again this is not impossible but the system
is designed to be opaque in that direction and I thought it best not to pierce
the veil. Oh, and Mac OSX build doesn't have the bug at all. Yay Mac OSX!

So here it is. It ain't pretty. It works.
Attachment #113553 - Attachment is obsolete: true
Much like the last patch (see comment 43) except the blurSuppression method has
been moved from nsIDOMWindowInternal to nsIBaseWindow.
Attachment #113868 - Attachment is obsolete: true
Comment on attachment 114056 [details] [diff] [review]
suppress blur events from the filepicker

Attachment #114056 - Flags: superreview+
Comment on attachment 114056 [details] [diff] [review]
suppress blur events from the filepicker

ew. nsIBaseWindow strikes again.  I really don't understand why we have this
interface or why we have so many implementations of it.  Anyway, r=me on this
Attachment #114056 - Flags: review+
Dan, Could you factor this code into a separate member function and call it from
both GetParent and BlurEventsSuppressed? 

 // this is GetParent(), but without the mIsTopWidgetWindow check
+  if (mIsDestroying || mOnDestroyCalled)
+    return PR_FALSE;
+  nsWindow *widget = 0;
+  if (mWnd) {
+    HWND parent = ::GetParent(mWnd);
+    if (parent) {
+      widget = GetNSWindowPtr(parent);
+      if (widget)
+        if (widget->mIsDestroying)
+          widget = nsnull;
+        else
+          NS_ADDREF(widget);
+    }
+  }

With that change for the widget module changes.
Whiteboard: [adt3] [ETA: soon. ugly but working patch for Windows in hand] → [adt3]
Target Milestone: Future → mozilla1.4alpha
*** Bug 162821 has been marked as a duplicate of this bug. ***
Patch checked in 24 Feb. PS I didn't refactor GetParent (comment 47) but instead
went with the simple, tight (while ::GetParent(HWND)) loop already in use
throughout the file (seven other places). These (and two other instances that
aren't in a loop) could all probably stand to be rolled up into a GetParent(bool
stop at top window, bool immediate or keep going) method.

But anyway, bug's fixed.
Closed: 22 years ago22 years ago
Resolution: --- → FIXED
Verified in the 2003-03-13-04 Win32 and 2003-03-13-03 Mach-0 OS X Trunk builds.
*** Bug 196806 has been marked as a duplicate of this bug. ***
Crash Signature: [@ comdlg32.dll]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.