Closed Bug 229695 Opened 19 years ago Closed 19 years ago
Icon in download dialog is based on extension, not MIME type
When you download a file and you get the dialog, the icon is based on the extension of the file, not the MIME type of the file. It should be based on the MIME type (even if there wasn't one; in those cases we would internally guess one, not necessarily only on the extension, and that should match the icon).
See http://lxr.mozilla.org/seamonkey/source/embedding/components/ui/helperAppDlg/nsHelperAppDlg.js#265 and http://lxr.mozilla.org/mozilla/source/modules/libpr0n/decoders/icon/nsIconURI.cpp#404 and http://lxr.mozilla.org/mozilla/source/modules/libpr0n/decoders/icon/nsIconURI.cpp#128 I would guess that we need to synthesize a filename for the moz-icon URI with the "right" extension (based on the MIMEInfo's MIME type, somehow, though we need to be a little careful because for image/jpeg we should use the "default" extension for the type, from the MIME info, whereas for application/octet-stream we should not use an extension from the MIME info). Perhaps the logic should be: 1) Get type for default extension on mime info 2) If that matches the type on mime info, use that default extension 3) If not, get default extension for the mime info's type 4) Synthesize a filename with that extension and use it. This is assuming we want the icon to match the type; I have no preference either way on that question.
the relevant code is at http://lxr.mozilla.org/mozilla/source/modules/libpr0n/decoders/icon/win/nsIconChannel.cpp#249 I don't think it would be a good idea to change nsIconURI, why can't the icon channels decide that for themselves?
Creating a fake URI assumes that all implementations of the icon service use extensions for working out the icon. This might not always be true. Couldn't we just pass the MIME type? "moz-icon:?mime=text/plain" or something. We know what the MIME type is by that point.
We already do, but the windows icon channel prefers to use the extension...
Sounds like a bug in the Windows icon channel then, rather than a bug in the way it is called. :-)
Ian, this is not the only consumer of icon URIs. Perhaps we just need to clearly respec what icon URIs _should_ do. Then either fix the helper app dialog or the impls as needed.
Sure -- my point was just that as I see it, _if_ a MIME type is specified on the moz-icon: URI, then that should take priority over the extension. If one is not specified, then the extension should be used. Why would that break other call sites? (And why would you want to specify the MIME type on the URI if it isn't used? I'm having difficulty understanding why this is controversial.)
It's not controversial. It just needs someone to look over all callsites to make sure nothing breaks.
ok, I basically have a patch... Hixie: Consider the case that windows (this patch is windows specific. this bug actually does not occur on linux, where we do not show any icon :) ) knows nothing about a specific mime type, consequently does not have an icon for it. Should we: a) Show no icon b) Show the icon matching the file extension c) Show some generic icon d) do something else
OS: Linux → Windows 2000
That is a good question. The thing to avoid, IMHO, is us saying: File type: Unknown type (application/x-foo-bar) File icon: [the HTML file icon] ...because then we look silly. Maybe we could show a generic icon based on the top-level MIME type? e.g.: unknown text/* - show text/plain icon unknown application/* - show application/octet-stream icon unknown image/* - show an image icon unknown audio/* - show a sound icon ...etc. If that's too hard, though, I'm not strongly against just showing the extension's icon. After all, we don't know _what_ icon is relevant, and at that point the icon is just prettiness, right? And presumably using the extension's icon would match the icon of the default application, too.
ok, summary of the irc discussion between hixie and me: o) The icon we show is the icon of the file (i.e. of the mime type), not necessarily the application we open it with o) Hence, if we can't show an icon matching the mime type, we should show the generic icon I'll attach a patch for that.
Assignee: file-handling → cbiesinger
Target Milestone: --- → mozilla1.7alpha
Status: NEW → ASSIGNED
this patch now only uses the content type if the file is not local. for local files, it just calls SHGetFileInfo on the file path and gets the real icon for the file.
Attachment #138310 - Attachment is obsolete: true
Attachment #138312 - Flags: superreview?(bz-vacation)
Comment on attachment 138312 [details] [diff] [review] patch v2 Sure, looks OK. sr=bzbarsky. Does MacOS need similar changes?
Attachment #138312 - Flags: superreview?(bz-vacation) → superreview+
macos doesn't seem to need them... http://lxr.mozilla.org/mozilla/source/modules/libpr0n/decoders/icon/mac/nsIconChannel.cpp#325 however, OS/2 probably does. I have no idea if asking the OS for an icon for "." works there though... cc'ing mkaply
patch checked in. leaving bug open for os/2
I changed my mind and filed Bug 235018 about os/2. this bug is fixed.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.