GNOME Bugzilla – Bug 692955
[W32] GtkIconCache fails to load non-builtin icons
Last modified: 2013-02-21 18:39:13 UTC
get_directory_index() does strcmp() on two relative directory names. One comes from a index.theme file and uses "/" directory separator for directory names, the other comes from icon-theme.cache file that is generated by gtk-update-icon-cache. gtk-update-icon-cache writes directory names in the same form it gets them from GLib - namely, they will have "\" directory separator on W32. As a result, get_directory_index() returns -1, and later GtkIconCache can't load any icon from files in share/icons/<themename>/<resolution>/<category> Bug in get_directory_index() is present in both 2.x and 3.x branches; i'm not sure whether 3.x is affected - most likely it is. 2.x is affected for sure.
Created attachment 234928 [details] [review] A hack to compare subdirectories dirsep-insensitively Since GLib has no usable-from-outside path normalization routines, and using create-GFile-then-get-its-filename-attribute approach would be an overkill, this hack just does a dumb string comparison, except that it allows mismatching directory separators to be used (everything else must match though - number of separators, character case, etc).
It seems wrong to do the extra work at runtime. We should normalize the strings we put into the cache file instead.
Review of attachment 234928 [details] [review]: according to alex' comment
Created attachment 235304 [details] [review] Make sure icon cache has /-separated subdirs only Alternative version - fixes gtk-update-icon-cache instead. By the way, There's a pathnamecmp() function in gtk/gtkimmulticontext.c, and it does roughly the same that attachment #234928 [details] did, so maybe it isn't as bad as Alex thinks.
Review of attachment 235304 [details] [review]: looks good to me
Review of attachment 235304 [details] [review]: ::: gtk/updateiconcache.c @@ +626,2 @@ path = g_build_filename (dir_path, name, NULL); + replace_backslashes_with_slashes (path); Actually, you should probably only do this on the original base_path, and then use g_build_påth ("/", ... instead of g_build_filename (...
Dunno, i don't understand the code well enough to change the functions around. Just added slash conversion. If you think that switching filename-building functions is ok - well, do it then :)
Well, g_build_filename() is just a call to g_build_path() with the system specific separator. Its stupid to call that and then replace strings in the results rather than just adding the right type of separators in the first place.
Created attachment 237089 [details] [review] Make sure icon cache has /-separated subdirs only (v2 - use g_build_path) Also please backport it to 2.x branch (diff applies cleanly, except for the last part, where g_type_init() is still used).
Comment on attachment 237089 [details] [review] Make sure icon cache has /-separated subdirs only (v2 - use g_build_path) Attachment 237089 [details] pushed as 8e80fd1 - Make sure icon cache has /-separated subdirs only (v2 - use g_build_path)