The nsIFilePicker does not provide the means for callers to specify a default file extension on platforms such as Win32. This affects embedders who show a save as dialog, filter on some file type (e.g. *.foo) and expect .foo to be automatically appended in the filename returned by the call. Add a defaultFileExt attribute to nsIFilePicker in trunk & 0.9.4 branch and ensure that Win32 version of file picker uses it.

Work in progress patch. Feedback please.

The patch in bug 117269 does this....

This fix is needed on the 0.9.4 branch as well. I'll use this bug to cover the branch and the other more comprehensive fix for bug 117269 can go in the trunk when it's ready. Both appear to do more or less the same thing as far as the file picker is concerned, adding a "defaultExtension" attribute to nsIFilePicker and using it in the Win32 nsFilePicker.cpp implementation. Gus, can you test the patch when you have the time and see if it fixes your issue? Thanks

can we get some review action: law, mscott?

Patch works... oddly, on win2k atleast, I can save a .exe file as .zip, .dll, .doc, and probably any known or registered file extensions, but if i try .foo it adds .exe to the end. (this is identical behavior in IE and other standard file dialogs) If I enter a file name and add a . the the file is saved w/o an extension.

Do all nsIFilePicker C++ implementations inherit from nsBaseFilePicker? Also, the JS nsFilePicker for Linux will need some code added, I think. I'm starting to have misgivings about using this feature of the Win32 file picker. The problem is that the user can change the file type, in which case the default extension is wrong. You don't show any of the code that would use this feature, but if we specified "htm" as the default extension when we show the file picker for "save page as...", and the user selects "plain text", then wouldn't we end up saving the file as plainText.htm? Maybe we need to embellish the file picker with code that detects the user changing the file type and resetting the default extension when that happens (e.g., they choose "plain text", we reset the default extension to "txt")?

Hm, perhaps I'm misunderstanding the problem here. Why does this functionality need to be in the filepicker - i.e. why can't the caller of nsIFilePicker append the extension after the filepicker is dismissed?

In response to comment #7: Two reasons: 1) If a user enters "filename." we get "filename" so we would be adding an extension when the user wanted none. 2) If the file exists (after default extension processing) the user is prompted for overwrite. We would have to add the same logic on our side.

Ok. Re: Bill's comment #6, that would just involve grabbing the first file extension from the selected filter, wouldn't it? (as passed to AppendFilter).

Brian, the basic idea is that the extension thing should only happen on Windows (and maybe OS2). Thefore making callers (some of which are JS, where figuring out the platform can be somewhat problematic) handle the extension stuff seems wrong... see bug 117269 for the consequences.

The caller could append the extension after filepicker returns but it breaks the overwrite confirmation behaviour. If a file called "blah.foo" exists on the disk then they should be shown a "The file blah.foo already exists do you want to replace it?" dialog whether they type "blah" or "blah.foo" in the filename field.

Bill, the OS/2, Windows, Mac, Gtk & BeOS filepickers all derive from nsBaseFilePicker, hence the reason I put the stock implementation in there. I forgot the js implementation but I can implement that easily enough.

Updated patch includes patch for nsFilePicker.js and uses diff -b to hide some whitespace noise.

> + if (mDefaultExtension.Length() > 0) { Please use |if (!mDefaultExtension.IsEmpty()) {| > + nsCAutoString ext; ext.AssignWithConversion(mDefaultExtension); AssignWithConversion converts to ASCII, iirc. Are there encoding issues that need to be addressed here?

Win32 accepts a 3 character maximum ascii string as the default extension so I don't think it makes any difference what character conversion I use here. I can change the Length() > 0 test to !IsEmpty()

I should note that the lpstrDefExt is actually TCHAR so in theory this is not always true, however we compile Mozilla with UNICODE undefined meaning it is ascii for us.

Marking it edt0.9.4+ (i.e., approval for 0.9.4 check-in). When could we get r= and sr=?

Comment on attachment 70408 [details] [diff] [review] Same as last except for Length test is replaced by !IsEmpty test sr=rpotts@netscape.com

Comment on attachment 70408 [details] [diff] [review] Same as last except for Length test is replaced by !IsEmpty test r=alecf but I wish there was a way to do this without the AssignWithConversion() bit...(i.e. so we don't loose non-ascii extensions) but that is probably a rare case.

Fix checked into 0.9.4 branch...

closing as fixed. I believe the trunk is addressed w/ the other patch in the other bug. re-open if I'm wrong please.

Reopening. :) Looking over this patch and my patch in the other bug I like this one more. This one also has all the requisite reviews. Could you land it on the trunk please? Then the other bug can focus on using this new attribute to solve the real problem over there. Thanks, Boris

Adam, just a slight correction to your comment. While a nsFilePicker.cpp for gtk that derives from nsBaseFilePicker might exist on the 0.9.4 branch, that's not what is used, the JS filepicker in xpfe/components/filepicker is.

Patch for the trunk, the same except for calling the new global ToNewUnicode function.

a=roc+moz for 0.9.9

Closing again. Just discovered patch for the other bug got checked into the trunk last night.

over to embed qa [load-balancing]...

See Bugscape bug: http://bugscape.netscape.com/show_bug.cgi?id=12174