17666 - nsFileSpecWithUIImpl.cpp:ChooseOutputFile returns OK when CANCELLED

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[Andrew Stone]

Found in M10:
PutFile returns nsFileDlgResults_Cancel if the user hits the cancel
button.  If you look at the following code (without the *ADD* line)
you can see that on cancel, it falls into the first if stmt and then
right back out, returning the result of mBaseFileSpec->SetFromFileSpec(spec).
For me this was NS_OK.
Please add the line denoted by *ADD* into the source tree so
that the cancel selection is passed to the caller.  Do you think
that NS_ERROR_ABORT is the appropriate response?


nsFileDlgResults result = fileWidget->PutFile(nsnull, winTitle, spec);
if (result != nsFileDlgResults_OK)
    {
*ADD*  if (result == nsFileDlgResults_Cancel) return NS_ERROR_ABORT;
    if (spec.Exists() && result != nsFileDlgResults_Replace)
        return NS_FILE_FAILURE;
    }
return mBaseFileSpec->SetFromFileSpec(spec);

Comment 1

[Peter Trudelle]

Thanks!  reassigning to sdagley for review and checkin.

Comment 2

[Steve Dagley]

Doh!  Missed this one when it first arrived, will check in today.  The
NS_ERROR_ABORT error code looks reasonable to me although NS_FILE_FAILURE seems
to be the catch all not OK value in nsFileSpecWithUIImpl.cpp.

Comment 3

[Steve Dagley]

Code checked in.  Andrew, can you describe a test case for this?

Comment 4

[Andrew Stone]

Oops, sorry for the delay.  Here's a test case.  Right click on any link
or in the main display window.  Click on "save page as" or "save link as".
The Save window should come up.  Now enter a valid file name and directory,
but click on "cancel" instead of "save".  If the bug is NOT fixed, you should
see the downloading window pop up (sometimes is there and gone really fast), and
a file will be created with the name you chose (do a dir or ls in the directory
to check for it).  If it IS fixed, the won't be there, and a message will appear
in the apprunner window saying something like "SelectFileAndTransferLocationSepc
failed, rv = { Exception... "component returned.." blah blah blah"

Comment 5

[Paul MacQuiddy]

[e-mail] to verify