Closed Bug 236389 Opened 20 years ago Closed 17 years ago

Don't set type/creator codes for any saved/downloaded files

Categories

(Core :: Networking: File, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: jaas, Assigned: jaas)

References

Details

(Keywords: fixed1.8.1.15)

Attachments

(2 files, 2 obsolete files)

I have been looking at a couple of bugs related to incorrectly setting
type/creator codes on saved files on Mac OS X. I think we should remove
type/creator code setting from Mozilla altogether. Mac OS X's preferred way to
handle file associations is by extension, and Apple's guidelines do not
recommend setting either field (while they do say it is fine to do so if you
wish). Safari's behavior is to not set a type/creator code for any saved files.
Also, by not setting either, a users finder preferences are honored. Right now
we're setting the fields on some files, not other, and often getting it wrong
when we do set the fields.

See bug 167962 and secondarily 190528 for previous discussion on this issue.
Target Milestone: --- → Camino0.9
When you work on this, be sure to check this out: bug 218603

Also when you say you've been looking at a couple of bugs, maybe you should link
them, or make this one a 'meta-bug'
Component: Downloading → Networking: File
Product: Camino → Browser
Target Milestone: Camino0.9 → mozilla1.9alpha
Version: unspecified → Trunk
*** Bug 190528 has been marked as a duplicate of this bug. ***
Sorry, but I thought AHIG called for apps to *always set all relevant info* when
saving/creating files--filetype, extension, and optionally creator if the app is
really the creator:

Mac OS X - System Overview - Preparing Software for Mac OS X
http://developer.apple.com/documentation/MacOSX/Conceptual/SystemOverview/InstallIntegrate/chapter_11_section_2.html#//apple_ref/doc/uid/20000990/CJBDAFFF
"When your application saves a document file, Apple recommends that it associate
the proper filename extension with it, as well as any defined type and creator
codes. [...] [Y]our application should always apply all valid forms of document
typing, including extensions, when it saves its documents."

and

Mac OS X - The Mac OS X File System - Filename Extensions
http://developer.apple.com/documentation/MacOSX/Conceptual/BPFileSystem/Concepts/FilenameExtensions.html
"In addition to filename extensions, applications should also set a file type
and optionally a creator type for any files they create."

This bug seems to be aimed at doing the opposite of what Apple calls for.
Some apps (PhotoShop?) will refuse to open files with not type/creator, iirc.
Care is required here.
we're also not the app *creating* these files. We're just a middle-man. For us
to pick a type/creator willy-nilly isn't the right thing to do either.
See arguments 19-21 of bug 167962. We've been through this at length, and
Apple's HIG sides with us not setting the creator/type. It is very problematic
for us to do so, and according the the guidelines we should not be doing it anyway.
The documents referenced in bug bug 167962 in support of the "do not set file
type" argument seem no longer to exist.  The references I've found are all in
favor of a app setting the extension, the file type, and, if relevant, the creator.

My interpretation of this is that when saving/downloading a "random" file,
Cm/Sm/Fx/etc. should preserve the file's extension (if present) and also set the
appropriate file type code (but not add any creator codes).  When saving a web
page, however, the browsers should also add their creator code since they are
essentially the creator of that document and, presumably, the preferred viewer.

Right now Camino is saving JPEGs with file type JPEG and creator ogle (the
latter half of that being wrong behavior, particularly since prvw is set in
InternetConfig and LaunchServices to be the default viewer of JPEGs on my Mac)
and is saving web pages without either type or creator (which is certainly wrong
on the file type side and, at least in my interpretation, also wrong on the
creator code part).
I don't have time to dig up the document right now, but I bet it still exists,
and even if it doesn't, I highly doubt Apple reversed their position on
creator/type codes.

That aside, as Pinkerton said, we don't actually create these documents. We
don't have a reliable way to set creator/type correctly. So even if you are
right, which I doubt, we can't do it, so we shouldn't.
The documents I cited/quoted in comment 3 are from May and August of 2004. 
Either Apple has reversed their position or they are saying two different things
in two different sets of documents :-)

I'm completely ignorant of the code, but can't Camino/etc. read the
InternetConfig "database" to get the *type* code of files it is downloading or
saving, or is this what is not reliable?

Having a *type* code set really is important for people working with older
and/or Classic apps, because they do seem to demand a *type* code in order to
open a file.

(*Creator* isn't important and *shouldn't* be set [except, IMO, for saved html
documents, where it's easier to make a case that the browser "creates" the file
and, unlike something like a StuffIt archive, can actually read/view/display the
document.  But creator-for-webpages really is minor/separate])
> InternetConfig "database"

It isn't reliable, at least since we don't have good lookup data. MIME types are
notoriously bad, and if we do lookups based on file extension, then if the
extension is good we wouldn't need creator/type. If the extension or the lookup
data is bad we get a bad creator/type. Its very problematic and we will not get
it correct to an acceptable degree. This is sort of vague, but you're really
going to have to think this out yourself in order to get exactly what I'm
saying. Try to come up with a specific, acceptable system using whatever data
you want from a downloading POV (MIME, extension). If you come up with one, let
me know.

We cannot compensate for the fact that some old applications need to be updated
for Mac OS X by doing the impossible or sketchy. It isn't our responsibility,
and we can't do it reliably anyway.
Blocks: 233375
*** Bug 343229 has been marked as a duplicate of this bug. ***
I can confirm as a user that Mozilla *is* getting it wrong here. It's setting the creator type of downloaded zip files to a completely different app than the one I've set as the default, and it's very, very annoying. Not setting the creator type would help a lot here, and would also better honour user preferences should they ever decide to change the app used for some file type.

So basically: Mozilla is doing the wrong thing right now, and doing nothing would fix the problem. Furthermore, it would match the behaviour of Safari.
InternetConfig cannot reliably map a MIME type to a type/creator code on modern OS X.  If you cannot reliably determine the type/creator then you should absolutely not set the type/creator code in a file, because type/creator takes precedence over the mechanism that modern OS X applications use to map file extensions to applications.

What you get is this: a user, one time, runs a crusty old Classic app that happens to be mapped to the type/creator corresponding to application/octet-stream.  Now what you see is Firefox, by act of setting the type/creator, associates all application/octet-stream files, like .dmg and .iso,  with this obsolete Classic application rather than the using correct modern mapping for .dmg and .iso files.  

I've had this problem with ZIP and DMG files for years because I happened to have some old Classic apps kicking around.  Even after migrating to an Intel MacBook, files downloaded with Firefox are still associated with these old Classic applications;  these apps are incapable of running on the Intel platform, but just their presence on my system is enough to mess up Firefox, apparently.  

You can't install an application on OS X without downloading a DMG or ZIP file these days.  It should be considered a serious problem if Firefox can't handle these file types correctly.

Apple's documentation explicitly states that a web browser application should not set a creator type for files that are downloaded and saved: (presumably it's OK to set the file type, however)

http://developer.apple.com/documentation/MacOSX/Conceptual/BPFileSystem/Articles/FilenameExtensions.html#//apple_ref/doc/uid/20002297-110671

"Applications that are not a primary editor for documents of a given type should not set a creator type for those documents. For example, an Internet browser may download and save files of many different types, but that does not mean it owns all of those files."

Attached patch fix v1.0 (obsolete) — Splinter Review
Attachment #257375 - Flags: review?(stuart.morgan)
We might want to consider still type and creator codes for files without a file extension, if we can deduce them from the MIME type.
Attachment #257375 - Flags: review?(stuart.morgan)
Attached patch fix v1.1 (obsolete) — Splinter Review
This doesn't ever set the creator, and only sets the type if the file has no extension and the type can be determined from MIME info. Needs more testing.
Attachment #257375 - Attachment is obsolete: true
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.6) Gecko/20070802 SeaMonkey/1.1.4

 This behaviour has been a long-standing irritation for me on Mozilla and now SeaMonkey. I experience the same problem described in Bug 273707 with dmg files (Bug 273707 is probably a duplicate of this bug) and I experience the inappropriate type/creator problem on downloaded PDF files which SeaMonkey always sets to "PDF "/"prvw" despite the fact that I want to use Adobe Acrobat Reader (and have set the default handler for files with extension ".pdf" to Acrobat). I have to manually run [almost] /every/ PDF and dmg file which I download with SeaMonkey through a script which deletes the type and creator, so that I can obtain the behaviour I've set as default in the Finder.

 What is the status of the patch? It seems to have languished :-( Josh, would it be possible for you to find some time to update it (if needs be) and request review?
Attached patch fix v1.2Splinter Review
I think we should push for this for Gecko 1.9. This may not be exactly what everyone wants but we're not going to get everyone to agree and this is definitely a huge step in the right direction. The code change itself is small and contained.
Attachment #257485 - Attachment is obsolete: true
Attachment #283295 - Flags: review?(stuart.morgan)
Attachment #283295 - Flags: review?(stuart.morgan) → review+
Attachment #283295 - Flags: superreview?(benjamin)
Attachment #283295 - Flags: superreview?(benjamin) → superreview+
Attachment #283295 - Flags: approval1.9?
Comment on attachment 283295 [details] [diff] [review]
fix v1.2

a=mconnor on behalf of drivers
Attachment #283295 - Flags: approval1.9? → approval1.9+
landed on trunk
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
This problem also affects attachments saved from Thunderbird and I'd find it helpful if it were applied to the 1.8.1 (TB 2.0) branch. Would there be any difficulty with this?
We won't be applying this to the 1.8.1 branch. We can't make such a significant behavioral change in a dot release like that.
The reason this is a bug is because it breaks all conventions around downloads of files on the Mac, and leads to downloading a file that you cannot open. 

If you're not fixing incorrect behaviour like this in point releases, what are you fixing? It may be a significant behavioural change, but it's changing it to behave in a way _that actually works_. 
(In reply to comment #25)
> If you're not fixing incorrect behaviour like this in point releases, what are
> you fixing? 

Security and stability bugs. This is neither.
(In reply to comment #24)
> We won't be applying this to the 1.8.1 branch. We can't make such a significant
> behavioral change in a dot release like that.

 This is not a "significant behavioural change". Apart from those experiencing the bug, most users will observe no difference.

 There is only a more-than-two-year-old /suggestion/ that some users may be adversely affected and your own response at the end of Comment #10 is, quite rightly, that it's not our problem (if, indeed, it's still a problem at all 2.5 years later).

 Again, this is not a "significant behavioural change".

(In reply to comment #26)
> Security and stability bugs. This is neither.

 Small bugfixes like this have gone into branch updates before, there is no reason this one couldn't as well.

 This fix won't make it into Thunderbird 3 at the moment, either, will it? Is there a good reason for not putting it onto that branch?
If this is using MIME type to set Finder info, then it is potentially a security bug (albeit a minor one) since it changes the application that Finder or LaunchServices will use to open the file.
It is changing the (In reply to comment #30)
> If this is using MIME type to set Finder info, then it is potentially a
> security bug (albeit a minor one) since it changes the application that Finder
> or LaunchServices will use to open the file.
> 

That is exactly what it does. It ignores user settings, and sets the application to open the document in the metadata of the file.
So for example: I use Skim as default PDF viewer, but all PDFs received through thunderbird only open in Preview.
(In reply to comment #30)
> If this is using MIME type to set Finder info

In the above, does "this" mean the patch, or the original bug?

The patch does not set any creator code at all, but will use MIME type to set a file type in the Finder if (and only if) no extension is present. That's the right behaviour, and I don't see any security bug there as Gecko is no longer changing anything to do with preferred applications.

The original behaviour was flawed, which is why it has been fixed.
(In reply to comment #32)

In comment #30, "this" means "The original bug". I was referring to comment #26: since the original behavior could potentially be used in an exploit via LaunchServices, it should be applied to older branches.
Bug 418005 suggests that taking this patch on branch might fix a problem where iTunes rejects M3U files downloaded by Firefox.  Should we do that?
Nominating for 1.8.1.x.  See Bug 426792 for reasoning.
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.15?
Josh: how different will a branch patch be for this problem?
sdwilsh: want to take a stab at the back-port since you thought it was important enough to nominate?
(In reply to comment #38)
> Josh: how different will a branch patch be for this problem?
> sdwilsh: want to take a stab at the back-port since you thought it was
> important enough to nominate?
Looks to me like it will almost apply cleanly.  If we want this for branch, I can do the patch.
(In reply to comment #39)
> Looks to me like it will almost apply cleanly.  If we want this for branch, I
> can do the patch.

Shawn, please create a patch for this bug and request approval1.8.1.15 on it. If there are substantial changes, let's have Josh review it, but from your comment, that's not needed.
Flags: blocking1.8.1.15? → blocking1.8.1.15+
Attached patch branch patchSplinter Review
branch patch - compiling now, but it's gonna take a while since I didn't already have a build for it.
Comment on attachment 322827 [details] [diff] [review]
branch patch

she works
Attachment #322827 - Flags: approval1.8.1.15?
Comment on attachment 322827 [details] [diff] [review]
branch patch

Approved for 1.8.1.15, a=dveditz for release-drivers
Attachment #322827 - Flags: approval1.8.1.15? → approval1.8.1.15+
Whiteboard: [needs checkin on 1.8 branch]
Fix checked into the 1.8 branch
Whiteboard: [needs checkin on 1.8 branch]
Josh, can you verify this with a nightly 1.8 build?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: