Created attachment 302698 [details] [review]
W32: Special treatment for inode/directory mime/type

This is a hack for GLocalFileInfo to correctly get icons for directories.
Without this change content type for any W32 directory is NULL
(because there's no registry entry for "inode/directory" by default,
and in any way there's no file extension that means "directory" to put there),
and GLocalFileInfo uses content type to grab icons.

A place where such 'hacks' are already centralized is glocalfileinfo.c:get_content_type. Maybe that should ensure some fallback content types on win32 as well ? Or does it already ?

(In reply to Matthias Clasen from comment #2)
> A place where such 'hacks' are already centralized is
> glocalfileinfo.c:get_content_type.
One could argue that this is a W32-specific hack, and thus should go into w32-specific source file (gcontenttype-win32.c), rather than into generic glocalfileinfo.c.

Could go either way.

> Maybe that should ensure some fallback
> content types on win32 as well ? Or does it already ?
I dare not touch glocalfileinfo much. I would rather fix gcontenttype on W32 altogether (see bug 735696), which would also fix this bug (i filed this bug separately because it's a small hack for a user-visible immediate problem - lack of directory icons in filechooser, and i would rather have it fixed than wait (possibly in vain) for bug 735696 to be fixed).

Review of attachment 302698 [details] [review]:

The only concern I would have is whether there are other places in the code that assume the content type is obtained by get_registry_classes_key.
If that is not the case, sure, go ahead.

LRN: i'm not sure what that last comment in 748727 means.

mclasen: I was wondering if the rest of the win32 content-type code can deal with the string "inode/directory"

LRN: um, probably not. "inode/directory" is not the proper format for "content-type", as far as current W32 glib gcontenttype code is concerned. "inode/directory" hack is there for a very specific case of working with glocalfileinfo.  If places are found where it also makes sense to hardcode such a check, these could be patched up later on. Hopefully, gcontenttype-win32 rewrite will land before that is needed though

mclasen: well, if you have glocalfileinfo hand out inode/directory as a content-type, the content-type machinery needs to be prepared to deal with it

LRN: that's fair.
g_content_type_equals()            - works (strcasecmp).
g_content_type_is_a()              - works (calls g_content_type_equals() first thing).
g_content_type_is_unknown()        - does not (and should not) work.
g_content_type_get_description()   - no directory-specific code (non-fatal though - will say "inode/directory filetype" instead of "Directory").
g_content_type_get_mime_type()     - does not work (and probably should).
g_content_type_get_icon()          - works.
g_content_type_can_be_executable() - works.
g_content_type_guess()             - doesn't (should?).

mclasen: sounds good enough

LRN: so _get_description(), _get_mime_type() and _type_guess() don't work but probably should made to. First is non-fatal, the last is weird (who calls that on *directories*?). Second one is unlikely (i.e. you get CT from mime "inode/directory" and later on do the reverse with the result you've got).

Attachment 302698 [details] pushed as 5f0665c - W32: Special treatment for inode/directory mime/type

*** Bug 749489 has been marked as a duplicate of this bug. ***