129979 - If Save filename altered, add original extension (jpg) instead of filetype default (jpeg)

Status: RESOLVED INCOMPLETE

(Core Graveyard :: File Handling, defect)

Description

[Skewer]

I can confirm that this behavior is occuring in Windows XP. Here is the relevant
info from bug 57113:

------- Additional Comment #20 From Jonas Jørgensen 2002-03-10 04:15 -------

0.9.9 branch build 2002-03-09-13, Win2k:

* Go to http://home.snafu.de/tilman/mozilla/mozilla-uber-alles.jpg
* Right-click the image
* Choose "Save Image"
* Type "abc" in the "File name" field and press Enter

Expected results: File is saved as "abc.jpg".
Actual results:   File is saved as "abc.jpeg".

Should I reopen this bug or file a new?



------- Additional Comment #21 From Boris Zbarsky 2002-03-10 07:26 -------

That is in fact the design behavior... .jpeg and .jpg are both valid JPEG
extensions and .jpeg is the more correct one.  In the same way, we save HTML
files as .html instead of .htm

Does this mess things up on Windows?



------- Additional Comment #22 From Jonas Jørgensen 2002-03-10 10:15 -------

It doesn't mess things up, but this behavior is against the platform standard.
If the file name field is changed from something with an extension to something
without an extension, the exact same extension should be applied. Even if .jpeg
is more correct than .jpg, it should apply ".jpg" if that is the extension of
the original file.

So, should I reopen this bug or file a new one?

Comment 1

[Skewer]

Note that we should only use the original extension if it is valid for the
content-type used. For example, if a file is image/jpeg, here's what should
happen to the extension:

pic.jpg -> abc.jpg
pic.jpeg -> abc.jpeg
pic.jpe -> abc.jpe
pic.jfif -> abc.jfif
pic.cgi?id=123 -> abc.jpg (note, I would use this as the default extension for
sure on any 8.3 platform)
file00001 -> abc.jpg

And in the last two cases, we should tack on the correct extension anyway, but
that may be another bug.

Comment 2

[Boris Zbarsky [:bzbarsky]]

What you describe in comment 1 as the desired behavior is _exactly_ what I see
happening with linux build 2002-03-08-07, trunk.  As I asked in bug 57113,
comment 23, please test with a trunk build and see whether the bug is present
there.

Comment 3

[Skewer]

This *was* tested on the trunk.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Which exact trunk build?  Which exact image?  I can't fix this bug if I can't
reproduce it....

Comment 5

[Warner Young]

Boris, I just tried with build 2002030908, and I can see the behavior described.

Comment 6

[Boris Zbarsky [:bzbarsky]]

OK, does someone here build on windows (or at least know how to modify files in
their chrome and then use the modified versions)?  Can you please add the line:

  alert("The default extension: '"+defaultExtension+"'");

right after 

  var defaultExtension = getDefaultExtension(defaultFileName,
aSniffer.uri,                                                   contentType);

in contentAreaUtils.js?  Then go to the image page describe in the opening
comment of this bug, do "save image" and see what Mozilla is using as the
default extension?  With a linux 2002-03-10 trunk build I see "jpg" as the
default extension, but anyone with a windows build seems to be able to reproduce
this....

There are a few places where this bug could be coming in:

1)  The Windows implementation of nsIMIMEService::GetFromMIMEType could be
    messing up (most likely), but I'm not sure how that can happen for this
    particular type...
2)  The Windows implementation of standard URLs could be confused when
    initialized with a raw filename (though we should _still_ be getting the
    correct extension from the URL)
3)  Something could be going wrong in XP code ... just not on Linux.

If someone would be willing to test this by adding some alerts to strategic
locations, thus helping me narrow the problem down to one of those three
possibilities, that would be much appreciated....

Comment 7

[Skewer]

Note that in Windows the decision for what file extension to use lies heavily on
the native Save As dialog and its Files of Type selection. For example, I might
put a filter on this dialog similar to this:

JPEG Image                                 | *.jpeg; *.jpg; *.jpe; *.jfif

Note that using a filter like this, the program wouldn't be able to tell whether
I deliberately specified "filename.jpeg" by using quote marks, or whether I just
entered "filename" and it got tacked on by the malformed filter. So this may
have nothing to do with the default extension which is passed to the frontend
that you refer to in Linux. The filter we would want with a file named .jpg is
something more like this:

JPEG Image (*.jpeg, *.jpg, *.jpe, *.jfif)  | *.jpg; *.jpeg; *.jpe; *.jfif
All Files (*.*)                            | *.*

(The other two changes are bug 23471 and bug 130025.) In other words, the filter
should actually be rearranged dynamically so that the default extension on it is
the file's actual extension. If we had a .jpe file, we'd need to change the
filter accordingly for that too.

Comment 8

[Jonas Jørgensen]

What you described as the correct behavior in comment 1 is the correct behavior 
for what should be the default file name when the Save dialog *opens*. That is 
*not* what this bug is about. This bug is about the fact that if the file name 
is changed to something without an extension, the very same extension and 
nothing but that extension should be readded to the filename. (BTW, Win32 is 
not an 8.3 platform.)

Boris: I'll add the line you mentioned in comment 6 when I get home (I'm at 
school right now).

Comment 9

[Boris Zbarsky [:bzbarsky]]

I fail to see the relevance of comment 7.  The extension that the filepicker
should be appending is the default extension we set for the filepicker, which
is, for good or bad, pretty much unrelated to the filter list.

Let's try to confine comments on this bug to a determination of what Mozilla
_is_ doing and figure that out before moving on to what it _should_ do.

Comment 10

[Jonas Jørgensen]

defaultExtension == "jpg" before showing the Save dialog.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Ok.  So.. let me get this straight.  We set a defaultExtension of "jpg" on the
filepicker.  We open the filepicker.  You type "abc".  The file gets saved as
"abc.jpeg".  Is that what you're seeing?

Comment 12

[Jonas Jørgensen]

Yup. After fp.show() is called a few lines further down, fp.file.leafName is
"abc.jpeg", and that's what the file is saved as.

Comment 13

[Boris Zbarsky [:bzbarsky]]

OK.  In that case, this is almost certainly a bug in Windows (or rather in the
standard Windows filepicker widget)....

Jonas, do you build?  If so, could you please take a look at
http://lxr.mozilla.org/seamonkey/source/widget/src/windows/nsFilePicker.cpp#188
and add:

printf("The default extension: '%s'\n", extensionBuffer);

right before the | ofn.lpstrDefExt = extensionBuffer; | line?  What does that
show?

Comment 14

[Bill Law]

>OK.  In that case, this is almost certainly a bug in Windows (or rather in the
>standard Windows filepicker widget)....

Not a bug, but a feature (and one that we actually benefit from, I suspect).

I played around some with the Win32 file picker the other day, to try to figure
out if our use of lpstrDefExt made sense in cases where we offered multiple
"file types" which had different extensions.

Basically, I wanted to find out if we set lpstrDefExt to "html" and the user
chose the "plain text" option in the file picker, would the file picker still
tack on "html" for the extension.

The answer was no.  It seemed to treat lpstrDefExt as a boolean flag indicating
whether or not to attach the file extension matching the selected filter.

That's a good thing for "plain text", I think.  But it appears as if it picks
the *first* extension in the filter list.  That makes sense, since it has to
pick one of 'em.

So I think we need to re-order the filter list extensions to put the actual
extension first, or, simply put .jpg first (since that is much less likely to
lead to surprises).

Comment 15

[Jonas Jørgensen]

> Jonas, do you build?

No, sorry. (See http://bugzilla.mozilla.org/show_bug.cgi?id=78037#c27 ;-))

> or, simply put .jpg first (since that is much less likely to
> lead to surprises).

Bad idea -- I'd have to file a bug about .jpeg files being saved as .jpg, then :)

Comment 16

[Boris Zbarsky [:bzbarsky]]

Ugh.  That is _not_ what all the documentation for lpstrDefExt says it does...

OK, the fix then is to modify appendFiltersForContentType() to set the
primaryExtension in the nsIMIMEInfo it uses to create the extension list (it
could just take the defaultExtension as an arg).

Comment 17

[Boris Zbarsky [:bzbarsky]]

Attachment: Prototype patch

Jonas, try this and see how it works?

Comment 18

[Skewer]

Comment #14 is a description of the behavior I explained in comment #7.

As I said, in Windows, you can specify in the Save As dialog ANY extension you
want by putting your filename in quotation marks. When a user does this Mozilla
should and appears to comply with that. However, Windows doesn't report the
difference between '"abc.jpeg"' and 'abc' without any extension where the first
extension in the filter list (in this case, apparently, .jpeg) is tacked on. So
we have to work around this by fiddling with the filter (there are other ways
this could be done, I'm sure, but they would be messy and probably not work well
with Windows' dialogs).

It seems like that's what this patch does. If so, good work.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Yeah, the patch makes sure that for all types except HTML we always list the
correct extension first.  I say "except HTML" because HTML follows a different
code path (due to save page, complete) and would require a lot more code.  I'd
like to know that this approach works before I tackle that...

Comment 20

[Jonas Jørgensen]

With this patch, .jpg files are saved as .jpg and .jpeg files are saved as
.jpeg. Good work! It still wants to turn .html into .htm, though...

Comment 21

[Boris Zbarsky [:bzbarsky]]

Right.  See comment 19.

I'll try to work on a more extensive patch later tonight if I finish my homework
early enough....

Comment 22

[Boris Zbarsky [:bzbarsky]]

Taking; gonna hope to get this done sometime in the near future (like June)

Comment 23

[Boris Zbarsky [:bzbarsky]]

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

Comment 24

[Kevin Chen]

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

Comment 25

[Kevin Chen]

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

Comment 26

[Niklas Dougherty]

Oops... X bugs are being duped into NT territory. This is "All" or something.

Comment 27

[Boris Zbarsky [:bzbarsky]]

No idea when I'll be able to get to this.

Comment 28

[Christian :Biesinger (don't email me, ping me on IRC)]

do I misunderstand something, or does bug 163254 say that we already do this?

Comment 29

[Boris Zbarsky [:bzbarsky]]

This bug and bug 163254 are actually semi-duplicates, except that they concern 
different parts of the code (this bug is about the JS code in contentAreaUtils; 
the other, iirc, is about the C++ in the exthandler).

Comment 30

[Christian :Biesinger (don't email me, ping me on IRC)]

sigh, I keep forgetting that there are two separate codepaths for this save dialog

Comment 31

[Boris Zbarsky [:bzbarsky]]

No plans to work on this any time in the foreseeable future, so to default owner.

Comment 32

[Ryan Flint [:rflint] (ping via IRC for reviews)]

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

Comment 33

[Boris Zbarsky [:bzbarsky]]

ccing some seamonkey folks who might want to do something with that patch.  Or not.

Note that Firefox would need a separate fix (and probably separate bug).

Comment 34

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

I can't reproduce the problem here on XP.  Is this a win2k-only issue?  Can someone provide an exact URL to test with?  I experimented with

data:text/html,<img src="http://ctho.ath.cx/tmp/foo">

and ended up with foo.jpg, using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.7) Gecko/20060830 SeaMonkey/1.0.5 Mnenhy/0.7.4.10000

I tried a few other filenames and never ended up with a ".jpeg" extension.

Comment 35

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

biesi, steps to reproduce so I can review this?

Comment 36

[[e-mail]]

Actually this might have been fixed by bug 213877 after all...

Comment 37

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 73556 [details] [diff] [review]
Prototype patch

Sorry, I can't do this review.

Comment 38

[Justin Dolske [:Dolske]]

Clearing review requests and closing as incomplete -- this patch is ancient and the code has changed significantly. Please reopen or attach a new patch if there's still valid work to be done here.