Closed Bug 229695 Opened 19 years ago Closed 19 years ago

Icon in download dialog is based on extension, not MIME type

Categories

(Core Graveyard :: File Handling, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.7alpha

People

(Reporter: ian, Assigned: Biesinger)

References

()

Details

Attachments

(1 file, 1 obsolete file)

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
Attached patch patch (obsolete) — Splinter Review
Attached patch patch v2Splinter Review
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: review+
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
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.