44975 - Crash closing "Save File" dialog with WindowManager's X button (file picker)

Status: VERIFIED FIXED

(SeaMonkey :: General, defect, P3)

Description

[Stephen Rasku]

Tested on Linux build 2000070710 on RH 6.2 KDE 1.1.2.  100% reproducible.

To reproduce:

1. Select file for download
2. Select Save File.
3. Before saving file, click on the "X" to close the dialog.  Mozilla crashes.

If you click on "Cancel" instead of the "X", the window closes properly.
Clicking on the "X" should have the same effect as clicking cancel.

Comment 1

[Eliot Landrum]

I am unable to reproduce on Linux 200070708. Tested with Sawfish/GNOME and
KDE/KWM 1.1.2. I'm not going to mark WORKSFORME as my build is a bit old, will
try again later with a fresh build.

Comment 2

[Andreas Franke (gone)]

Confirming bug. Reproduced on PC/Linux, build 2000070721, SuSE6.2, fvwm95-2.
Closing the filepicker with the WindowManager's close button causes a crash:

#0  0x411be80d in nsStreamTransfer::SelectFile ()
   from components/libmozxfer.so
#1  0x411be041 in nsStreamTransfer::SelectFileAndTransferLocation ()
   from components/libmozxfer.so
#2  0x411be224 in nsStreamTransfer::SelectFileAndTransferLocationSpec ()
   from components/libmozxfer.so

Re-summarizing from "Exiting download dialog crashes browser"
Reassigning to law (author of nsStreamTransfer::SelectFile).
Don't know if this is XPToolkit/Widgets ("FilePicker") or Networking.

Comment 3

[Andreas Franke (gone)]

Attachment: stack trace with function symbols (PC/Linux, 7/1 build)

Comment 4

[Jeffrey Baker]

Confirmed on Linux-i686 build 2000070909.  This may be a partial regression of
bug 42048.

Comment 5

[harishd]

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

Comment 6

[Andreas Franke (gone)]

It looks like this has been fixed between 2000071910 and 2000072113.

If I am not mistaken, it has been fixed by this checkin:
07/20/2000 19:10 bryner%uiuc.edu  
mozilla/xpfe/components/filepicker/res/content/filepicker.js 1.24
Fix for dogfood bug 45986. r=akkana.
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/xpfe/components/filepicker/res/content&command=DIFF_FRAMESET&file=filepicker.js&rev1=1.23&rev2=1.24&root=/cvsroot

Especially the following line has been added:
57 bryner 1.24   retvals.buttonStatus = nsIFilePicker.returnCancel;

This seems to have changed the codepath in the formerly crashing code of
nsStreamTransfer::SelectFile in xpfe/components/xfer/src/nsStreamTransfer.cpp:

263                 rv = picker->Show( &rc );
264             }
265 
266             #ifdef DEBUG_law
267             printf( "\nFile picker result = 0x%04X\n\n", (int)rc );
268             #endif
269             if ( rc != nsIFilePicker::returnCancel ) {
270                 // Give result to caller.
271                 nsCOMPtr<nsILocalFile> selection;
272                 if ( NS_SUCCEEDED( picker->GetFile( getter_AddRefs(
selection ) ) ) ) {
273                     nsXPIDLCString selectionPath;
274                     if ( NS_SUCCEEDED( selection->GetPath( getter_Copies(
selectionPath ) ) ) ) {
275                         rv = NS_NewFileSpec( aResult );
276                         if ( NS_SUCCEEDED( rv ) ) {
277                             rv = (*aResult)->SetNativePath( selectionPath );
278                             printf( "\nresult native path = %s\n\n", (const
char *)selectionPath );
279                         }
280                     }
281                 }
282 
283                 if ( NS_SUCCEEDED( rv ) && prefs ) {
284                     // Save selected directory for next time.
285                     rv = (*aResult)->GetParent( getter_AddRefs( startDir )
);
286                     if ( NS_SUCCEEDED( rv ) && startDir ) {
287                         prefs->SetFilePref( "browser.download.dir",
startDir, PR_FALSE );
288                         #ifdef DEBUG_law
289                         printf( "\nbrowser.download.dir has been reset\n\n"
);
290                         #endif
291                     }
292                 }
293             } else if ( NS_SUCCEEDED( rv ) ) {
294                 // User cancelled.
295                 rv = NS_ERROR_ABORT;
296             }

cc bryner: please confirm and mark this one fixed. thanks.

Comment 7

[Brian Ryner (not reading)]

Yes, this should be fixed.

Comment 8

[Stephen Rasku]

Verified on Linux build 20000721113