Closed Bug 1321321 Opened 7 years ago Closed 7 years ago

When file extensions are hidden in the Save file dialog, some files are saved with the wrong extension

Categories

(Core :: Widget: Cocoa, defect, P2)

50 Branch
x86
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: weci2i, Assigned: stefanh)

References

Details

(Whiteboard: STR in comment #15, [tpi:+])

Attachments

(5 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161104212021

Steps to reproduce:

I am using macOS Sierra. In Firefox versions 49 and prior, the file save dialog would open with the file extension showing as part of the file name. I could change the file extension if I wished. The file would always save with the correct extension. Since version 50, this is no longer the case. The save dialog initially pops up with the correct file name shown. However, when I click the Save File button, the next dialog does not show the extension anymore and files are saved with incorrect extensions.


Actual results:

This happens for a number of file types so far that I have noticed. Any jpeg image file is now saved with a .jpeg extension instead of a .jpg extension, even for images where I can confirm that the correct extension is in fact .jpg on the web server. That is minor. However, other files are saved with completely incorrect extensions. I peformed a database dump through phpMyAdmin on several sites I administer. Instead of being saved with the correct .sql file extension, Firefox is now changing it to .mssql. I regularly flash cyanogenmod updates to an Android device that I use. These are .img files - like the one found here https://download.cyanogenmod.org/get/jenkins/187761/cm-14.1-20161130-NIGHTLY-falcon-recovery.img However, firefox is now saving .img files as .ndif files.


Expected results:

I would like the ability to see and change file extensions in the dialog box again and also to have them saved correctly after that.
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86
I guess this is dupe of bug 1320377.
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Component: Untriaged → Widget: Cocoa
Product: Firefox → Core
Resolution: --- → DUPLICATE
John,
Regarding the problem with the file extension not being shown: I'm not on Sierra, but there should be a Finder preference for showing/hiding the file name extension (in the Advanced section). At least that's the case on El Capitan.
(In reply to Stefan [:stefanh] from comment #2)
> John,
> Regarding the problem with the file extension not being shown: I'm not on
> Sierra, but there should be a Finder preference for showing/hiding the file
> name extension (in the Advanced section). At least that's the case on El
> Capitan.

I have Finder set to show file extensions. The only thing that changed on my system that caused this was the upgrade from firefox version 49 to 50. One day it worked on version 49. The next day I went to do the exact same thing after firefox updated to 50 and it had changed.
Status: RESOLVED → UNCONFIRMED
Resolution: DUPLICATE → ---
Attachment #8816267 - Attachment description: Using the nightly version 53 in safe mode to show the process of saving an image → Using the nightly version 53 in safe mode to show the process of saving a .img file
The added attachments are in reference to a comment made on another bug after this was marked as a duplicate, linked here - https://bugzilla.mozilla.org/show_bug.cgi?id=1320377#c22 It was recommended that I reopen this bug as it contains somewhat of a separate issue. 

Each of these three examples were created using Nightly 53.0a1 (2016-11-30) (64-bit) in safe mode on macOS Sierra.
That's really odd, all these examples works fine for me. This is with El Capitan, though. Hmm, can you try and re-build launch services (google should be able to help you here)? This is just a guess, but it does looks like your issue could be connected to some ind of Launch Services corruption. Note that you'll probably loose some stuff when doing this (like what application should launch when double-clicking a file), so be warned.
If that doesn't help, I'm a bit lost. I do have Sierra partition, so I should be able to try and see if I can reproduce it on Sierra next week when I'm back.
I rebuilt launch services. No luck. This is a nearly clean install anyway. I wiped the hard drive and started fresh with Sierra about a month ago.

I have access to a second MacBook Pro running Yosemite. I checked version 50.0.1 and Nightly 53 on it as well and it is exhibiting the exact same behavior as this one.

I'm reasonably certain that the only major change on my system was the update from firefox version 49 to 50 when this began.
I have now tested this on my Sierra partition (clean install) and here's my results when with a nightly build:
1) File extensions are shown in the Save dialog (if I enable it in the Finder prefs).
2) Saving a .jpg file will save it with the .jpg extension if I choose the "All files" format.
3) The other 2 examples works as expected for me (files saved as .img and .sql)

However, if I choose to not show the file extension the .img file will be saved with the .ndif extension. I can't explain that, though.
So I think we have 2 problems here:
1) File extension not shown in the save dialog, regardless of you having enabled that in Finder prefs.
2) Nightly altering the file extensions when the file is saved. Sometimes this seems to depend on the fact that file extensions are not shown in the save dialog (.img saved as .ndif) or what format you choose (.jpg saved as .jpeg).

1) Is a bit tricky since I can't reproduce it. There must be some kind of preference that's lurking behind this... What does 'defaults read NSGlobalDomain AppleShowAllExtensions' in Terminal give you? 
2) It looks like we might have an issue here with us changing the file extension during certain conditions. This will take some time to sort out, though.
My Nightly updated to the 12-7 just now. This is strange. I'm now seeing what appears to be expected behavior with this most recent update.

1. File extension is now being shown in the save dialog (the setting is still enabled in Finder prefs). Since you asked, 'defaults read NSGlobalDomain AppleShowAllExtensions' yields 1.

2. I just tested the nightly CyanogenMod .img as I had before and it saves correctly as .img. .jpg are now .jpg again instead of .jpeg. However, if I were ever to disable showing file extensions in Finder prefs (or for people/users who don't even have any idea about such things), I would hope that file extensions turn out correctly after the save. I did a test since we're discussing it. When file extensions are hidden, firefox still chooses the wrong one for all my original examples.
Thanks for the update - then we're left with one problem. I'll confirm this and change the description a bit.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: The File Save Dialog no Longer Shows the File Extension and Files are Saved with Incorrect Extensions → When file extensions are hidden in the Save file dialog, files are saved with the wrong extension
Summary: When file extensions are hidden in the Save file dialog, files are saved with the wrong extension → When file extensions are hidden in the Save file dialog, some files are saved with the wrong extension
STR:

1) In Finder preferences, make sure that "Show all file extensions" is unchecked
2) Set your "downloads" preference to "Always ask me where to save files"
3) Visit https://download.cyanogenmod.org/?device=falcon and click one of the "CyanogenMod Recovery" links (".img" files)
4) Choose to save the file
5) In the Save dialog, choose a location and click "Save".

--> Note that the file you saved now has an ".ndif" extension, instead of ".img".
Whiteboard: STR in comment #15
For the record, I wouldn't say the patch in bug 426680 caused this since without that patch we're not appending any extension at all if "Show all file extensions" is checked.
OK, so there are 2 problems here.

1) If you hide the extension the thePanel.nameFieldStringValue will return a file name without an extension. See https://github.com/codefromabove/NSSavePanel-Accessory-View-Tutorial/blob/master/SavePanelAccessoryView/SaveFromCFunction.mm#L55

2) Now, .img files are actually files that MacOS recognize. The UTI type identifier is ”com.apple.disk-image-ndif” and looking at the declaration we get:
 {
    UTTypeConformsTo = "com.apple.disk-image";
    UTTypeDescription = "Classic Disk Image";
    UTTypeIdentifier = "com.apple.disk-image-ndif";
    UTTypeTagSpecification =     {
        "com.apple.ostype" =         (
            dimg,
            hdro,
            rohd,
            hdcm
        );
        "public.filename-extension" =         (
            ndif,
            img
        );
    };
}

So, it looks like the OS will append an extension if the file name lacks one, and in this case the first extension is ndif. This would also explain why a .jpg file is saved with the .jpeg extension. I guess .sql files saved with the .mssql extension (which I can’t reproduce) could be caused by an application affecting the default extension for the UTI.
(In reply to Stefan [:stefanh] from comment #16)
> For the record, I wouldn't say the patch in bug 426680 caused this since
> without that patch we're not appending any extension at all if "Show all
> file extensions" is checked.

I was partially wrong here, it looks like we never hide the extension in 49.
Priority: -- → P2
Whiteboard: STR in comment #15 → STR in comment #15, [tpi:+]
For the record, I'm looking at this and the right approach here is probably to use the extension in the file name for setting allowedFileTypes. There's at least one more issue I'd like to address, though: saving a file with multiple extension, like "foo.tar.gz", needs special treatment when file extensions are hidden in the panel. The file name will be displayed as "foo.tar" and the panel will then think that you're trying to save a file with the "tar" extension...
Assignee: nobody → stefanh
(In reply to Stefan [:stefanh] from comment #18)
> (In reply to Stefan [:stefanh] from comment #16)
> > For the record, I wouldn't say the patch in bug 426680 caused this since
> > without that patch we're not appending any extension at all if "Show all
> > file extensions" is checked.
> 
> I was partially wrong here, it looks like we never hide the extension in 49.
Blocks: 426680
Status: NEW → ASSIGNED
Attached patch 1321321.1.diffSplinter Review
This patch:
1) Uses the file extension for the allowed extension instead of going the UTI route (fixes the jpg —> jpeg / img —> ndif problem).
2) Works around a known limitation (mentioned in comment #19): if file extensions are hidden (”Show all file extensions” unchecked in the Finder prefs) and a file with multiple extensions is saved, like ”myFile.tar.gz”, the OS will think that you’re trying to override the default extension (”.gz”) with ”.tar”.

Chrome does it like this: https://chromium.googlesource.com/chromium/src/+/18cbba0aedd9d569b27be8adbd2836c839ae9897%5E!/#F6

They’re checking the extension length, but I’m checking if the extension has a known UTI. Doing that we’re covering more cases (like ”foo.torrent.gz”). Note that according to the documentation it looks like setExtensionHidden is supposed to toggle the ”Hide extension” checkbox, but it obviously do more here.
Attachment #8826969 - Flags: review?(mstange)
Comment on attachment 8826969 [details] [diff] [review]
1321321.1.diff

Review of attachment 8826969 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you, this looks good. Sorry for the horrible delay!

Can you explain to me what the original code was intending by using the UTI?

::: widget/cocoa/nsFilePicker.mm
@@ +480,5 @@
> +    CFStringRef type =
> +      UTTypeCreatePreferredIdentifierForTag(kUTTagClassFilenameExtension,
> +                                            (CFStringRef)otherExtension, NULL);
> +    if (type) {
> +      if (!CFStringHasPrefix(type, CFSTR("dyn."))) {

What's dyn.? Please add a comment.
Attachment #8826969 - Flags: review?(mstange) → review+
(In reply to Markus Stange [:mstange] from comment #23)
> Comment on attachment 8826969 [details] [diff] [review]
> 1321321.1.diff
> 
> Review of attachment 8826969 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thank you, this looks good. Sorry for the horrible delay!

No worries - thanks!

> 
> Can you explain to me what the original code was intending by using the UTI?

That code was added in bug 426680. I think the idea was to allow users to change extension (like jpg instead of jpeg). However, you could not change to an extension not known by the UTI (bug 1320377).


> > +      if (!CFStringHasPrefix(type, CFSTR("dyn."))) {
> 
> What's dyn.? Please add a comment.

FYI: If no UTI is found the function creates a dynamic type with the dyn prefix.
(https://developer.apple.com/reference/coreservices/1448939-uttypecreatepreferredidentifierf?language=objc).
Pushed by stefanh@inbox.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fd458fdbd07
Ensure that we always save files with the given extension / Work-around NSSavePanel limitations when saving files with multiple extensions. r=mstange.
https://hg.mozilla.org/mozilla-central/rev/6fd458fdbd07
Status: ASSIGNED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.