583818 - Can't ever change the extension of a file to be saved

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: WIP of buggy code for the nsSaveHeaderSniffer path

I broke this when fixing bug 576978.  It's now impossible to change the extension of a file you're saving (e.g., .tif to .tiff, .jpeg to .jpg, .htm to .html, etc.).

Ideally we'd fix this in a way that also fixes bug 359373 (and doesn't regress bug 169789) and without causing the warning alluded to by Wevah in bug 576978 comment 2.

I've been poking at this, switching to the new-in-10.3 setAllowedFileTypes: (and removing setAllowsOtherFileTypes: [which causes the warning]), but the problem is that I somehow break the accessory view->file extension synchronization for saving HTML file types, no matter how well I try to if things.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Saving stuff like bug 578421 really becomes a pain with this bug :(

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I see this also on misconfigured MS-servers somewhat frequently (foo.aspx?image=barbaz.jpg), so I download an image as foo.aspx and have to go several rounds with the Finder to allow me to change the extension to .jpg.

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #0)
> problem is that I somehow break the accessory view->file extension
> synchronization for saving HTML file types, no matter how well I try to if
> things.

Maybe something like Stuart's fix for bug 520407 (which fixed what I broke the last time I tried to customize a Save dialogue :P) is what I need here; I'll check when I get back to code bugs after a1.

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #3)
> (In reply to comment #0)
> > problem is that I somehow break the accessory view->file extension
> > synchronization for saving HTML file types, no matter how well I try to if
> > things.
> 
> Maybe something like Stuart's fix for bug 520407 (which fixed what I broke the
> last time I tried to customize a Save dialogue :P) is what I need here; I'll
> check when I get back to code bugs after a1.

For future reference (e.g., bug 359373), in bug 259903 kreeger added a corresponding setNewSaveOption: IBAction for this filter view, but it seems like it's never been called?

I'm going to punt on that problem (bug 359373) and just fix the part of this regression that bugs me, though.

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Regression fix

This patch returns us to the status quo ante, without regressing the actual fix in bug 576978 (make only files’ names be selected) or causing "You are changing the filename extension" warnings to appear.

(It technically regresses one other inadvertent "fix" from bug 576978; with that patch, we started forcing HTML files saved by "Download Link Target" to have the "html" extension; with this patch, we return to not requiring it.  That probably should be fixed properly by making sure HTML files coming via "Download Link Target" end up in the isHTML check.)

The other change in this patch is that you can now use either "html" or "htm" as the file extension for HTML files that are handled by the isHTML check's code; this lets users rename "foo.htm" to "foo.html" when saving, which was a regression from my last fix that bothered me.  (When switching back and forth between formats, we'll still prefer "html", however.)

Er, I've actually fixed bug 359373 (and verified that deleting the extension entirely and saving, or using a random extension, still works, both with straight HTML source and a bug's--CGI HTML--source).  I'm a little surprised, but I guess it doesn't have an HTML MIME type, which is why we had to append the extension in the first place.

Some day I'd still like to allow us to save regular HTML files with varied extensions (I think there's a bug on that, but right now I can't find it), but it's too much for right now, and I'd like to just fix the regression I caused which has annoyed me ever since :P

The insane whitespace et al. in this file is already on file as bug 482163, which cl was going to fix two years ago ;)

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

For reference, some tests for things where you might like to change the extension:

http://drupal.org/files/issues/sample.jpeg
http://css-class.com/test/css/3/transform-rotation-rolling2.htm
https://bugzilla.mozilla.org/attachment.cgi?id=468173

Invoke "Download Link Target" on the links on http://caminobrowser.org/download/releases/0.8.5/

view-source:http://css-class.com/test/css/3/transform-rotation-rolling2.htm
view-source:https://bugzilla.mozilla.org/show_bug.cgi?id=583818
view-source:http://www.google.com

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #6)
> view-source:��0

that was https://bugzilla.mozilla.org/show_bug.cgi?id=583818 !

Comment 8

[Stuart Morgan]

Comment on attachment 506317 [details] [diff] [review]
Regression fix

>+        // Require the default file extension for HTML and HTML-as-text.
>+        [savePanel setAllowedFileTypes:[NSArray arrayWithObjects:[file pathExtension], @"htm", @"html", nil]];
...
>         if (filterIndex == eSaveFormatPlainText) {
>           file = [file stringByDeletingPathExtension];
>           file = [file stringByAppendingPathExtension:@"txt"];
>+          [savePanel setAllowedFileTypes:[NSArray arrayWithObject:[file pathExtension]]];

It's weird to set it twice in the plain text case; move the first chunk here into an |else| so it's only done once either way.

Also, it probably wouldn't hurt to allow xhtml as an extension, given that xhtml in generally served with an HTML mime type.

sr=smorgan with those changes.

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

http://hg.mozilla.org/camino/rev/113dcfecad64 with those changes.