Closed Bug 129979 Opened 23 years ago Closed 13 years ago

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

Categories

(Core Graveyard :: File Handling, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: SkewerMZ, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: helpwanted)

Attachments

(1 file)

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?
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.
Keywords: nsCatFood
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.
Blocks: 66836
Keywords: nsCatFood
This *was* tested on the trunk.
Which exact trunk build? Which exact image? I can't fix this bug if I can't reproduce it....
Boris, I just tried with build 2002030908, and I can see the behavior described.
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....
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.
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).
Summary: Save files with the original extension (.jpg) instead of default for that filetype (.jpeg) → If Save filename altered, add original extension (jpg) instead of filetype default (jpeg)
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.
defaultExtension == "jpg" before showing the Save dialog.
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?
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.
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?
>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).
> 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 :)
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).
Attached patch Prototype patch (deleted) — Splinter Review
Jonas, try this and see how it works?
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.
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...
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...
Right. See comment 19. I'll try to work on a more extensive patch later tonight if I finish my homework early enough....
Taking; gonna hope to get this done sometime in the near future (like June)
Assignee: law → bzbarsky
Priority: -- → P2
Target Milestone: --- → mozilla1.2alpha
Depends on: exe-san, 147679
*** Bug 150129 has been marked as a duplicate of this bug. ***
*** Bug 163254 has been marked as a duplicate of this bug. ***
*** Bug 163771 has been marked as a duplicate of this bug. ***
Oops... X bugs are being duped into NT territory. This is "All" or something.
OS: Windows XP → All
Hardware: PC → All
No idea when I'll be able to get to this.
Keywords: helpwanted
Target Milestone: mozilla1.2alpha → mozilla1.4alpha
QA Contact: sairuh → petersen
Blocks: 183221
Priority: P2 → P4
Target Milestone: mozilla1.4alpha → mozilla1.7beta
No longer blocks: 183221
Depends on: 213877
do I misunderstand something, or does bug 163254 say that we already do this?
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).
sigh, I keep forgetting that there are two separate codepaths for this save dialog
No plans to work on this any time in the foreseeable future, so to default owner.
Assignee: bz-vacation → file-handling
QA Contact: chrispetersen → ian
Priority: P4 → --
Target Milestone: mozilla1.7beta → ---
*** Bug 312915 has been marked as a duplicate of this bug. ***
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).
Attachment #73556 - Flags: superreview?(cbiesinger)
Attachment #73556 - Flags: review?(cst)
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.
Attachment #73556 - Flags: superreview?(cbiesinger) → superreview+
biesi, steps to reproduce so I can review this?
Actually this might have been fixed by bug 213877 after all...
Assignee: file-handling → nobody
QA Contact: ian → file-handling
Comment on attachment 73556 [details] [diff] [review] Prototype patch Sorry, I can't do this review.
Attachment #73556 - Flags: review?(cst) → review?
Attachment #73556 - Flags: superreview+
Attachment #73556 - Flags: review?
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.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → INCOMPLETE
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: