After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 541276 - XDG directories should have their own icons
XDG directories should have their own icons
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
2.17.x
Other All
: Normal minor
: ---
Assigned To: Alexander Larsson
gtkdev
Depends on: 584286 585534
Blocks:
 
 
Reported: 2008-07-02 17:13 UTC by Nicolò Chieffo
Modified: 2010-02-16 21:17 UTC
See Also:
GNOME target: ---
GNOME version: 2.23/2.24


Attachments
icons patch (1.59 KB, patch)
2008-07-02 17:16 UTC, Nicolò Chieffo
none Details | Review
xdg-directory-icons (1.71 KB, patch)
2008-07-03 13:55 UTC, Nicolò Chieffo
none Details | Review
xdg-directory-icons.patch (1.73 KB, patch)
2008-10-09 12:49 UTC, Nicolò Chieffo
none Details | Review
glib-xdg-cache-reloaded.patch (359 bytes, patch)
2008-10-09 18:19 UTC, Nicolò Chieffo
rejected Details | Review
xdg_user_dirs_icons_with_emblems.patch (2.23 KB, patch)
2009-02-26 11:59 UTC, Nicolò Chieffo
rejected Details | Review
move_user_special_dirs.patch (1.71 KB, patch)
2009-02-27 18:14 UTC, Nicolò Chieffo
none Details | Review
move_user_special_dirs.patch (1.71 KB, patch)
2009-02-27 18:15 UTC, Nicolò Chieffo
needs-work Details | Review
reload_cache.patch (2.20 KB, patch)
2009-04-10 12:50 UTC, Nicolò Chieffo
none Details | Review
XGD directories with an emblem icon for each folder (103.65 KB, image/png)
2009-04-13 18:30 UTC, Martin Lettner
  Details
attach-icons-to-xdg-folders.patch (1.68 KB, patch)
2009-05-31 18:53 UTC, Nicolò Chieffo
none Details | Review
icons-glib-patch.patch (2.29 KB, patch)
2009-06-12 09:03 UTC, Nicolò Chieffo
none Details | Review
icons-with-auto-fallback.patch (1.77 KB, patch)
2009-06-12 14:04 UTC, Nicolò Chieffo
committed Details | Review
Screenshot Desktop VS Download (9.26 KB, image/png)
2009-06-22 16:50 UTC, Michael Monreal
  Details

Description Nicolò Chieffo 2008-07-02 17:13:43 UTC
new that we have XDG directories we should attach icons to them, like the desktop icon.

Other information:
Comment 1 Nicolò Chieffo 2008-07-02 17:16:45 UTC
Created attachment 113870 [details] [review]
icons patch

This could be a start. Don't look at the icons I chose, since there are no default icons for these directories
Comment 2 Matthias Clasen 2008-07-02 18:40:56 UTC
I don't think this is adaequate. 

If you really think this is the right thing to do, proper names for those places should be added to the icon-naming spec.
Comment 3 Nicolò Chieffo 2008-07-02 19:15:56 UTC
What do you mean with "I don't think this is adequate?"
I cannot understand if you think that XDG directories should not have different icon than normal directory, or if you think that this is not the correct place.

If the latter, this should be the right place to insert icons, since in the line before the patch, there is the statement to attach "user-desktop" icon is attached to the desktop directory.
P.S. - Nautilus shows the icons without any additional change!
Comment 4 Matthias Clasen 2008-07-02 20:41:55 UTC
I meant that  the chosen icon names are not adaequate. Thats why I said there should be proper names like user-desktop. A bookmark-new icon may not look 
at all like a place to store templates...

But I'm also somewhat unconvinced that it is really desirable to have special icons for _all_ of those. home and desktop are somewhat special
Comment 5 Nicolò Chieffo 2008-07-02 21:16:36 UTC
I am convinced that at least documents, video, music, pictures and downloads should have different icons.
Maybe public and templates should keep the normal one, but since they are special directories maybe the user should be visually informed about this, so he does not have the idea to remove or rename them.
What do you think?

I will repost the patch with new icon names soon.
Comment 6 Nicolò Chieffo 2008-07-03 13:55:32 UTC
Created attachment 113922 [details] [review]
xdg-directory-icons

I changed all the icons to "xdg-directory-*" (with * the name of the xdg directory).
As regards the possibility to not change the icon for some special directories the solution is simple: create a theme in which the special icon is linked to the "folder" icon file.
What do you think?
Comment 7 Matthias Clasen 2008-07-03 14:09:26 UTC
you need to go to xdg-list and propose adding names for these places to the icon naming spec. 
Comment 8 Nicolò Chieffo 2008-07-03 15:30:15 UTC
I've forwarded the request to the xdg ML
Comment 9 Nicolò Chieffo 2008-07-03 18:29:39 UTC
Maybe I didn't request the right thing... they told me that they prefer that nautilus implements it using emblems, but I tried to point out that if it is implemented here, every GIO filemanager will benefit of this feature without adding complexity to all filemanagers to do the same thing.

They replied that they have no idea on which is the way to request adding new icons. do you know it?
Comment 10 Matthias Clasen 2008-07-03 18:39:34 UTC
Sending mail to xdg-list is the right way. Just give it some time...
Comment 11 Nicolò Chieffo 2008-07-08 11:49:10 UTC
5 days passed without any response. Showld I try something else?
thanks
Comment 12 Nicolò Chieffo 2008-09-05 08:23:52 UTC
1 month passed now.
Matthias, can you suggest me something else?
Comment 13 Alexander Larsson 2008-10-09 11:56:32 UTC
Getting icon names in any spec seems sort of tricky atm. 

I don't think using custom icons for these locations is a bad idea. In fact, i prefer it over emblems, since emblems cannot do some of the things you can do with full custom icons (for instance, the desktop folder icon can't be done as an emblem, and the home one is problematic too).

However, i'm a bit worried about how this will work when the xdg directories change. Nautilus handles this, and in fact it automatically updates the prefs when the user moves or renames on of the dirs. We would need a way to invalidate the glib cache of the config for this to work.

Also, the patch needs to handle the g_get_user_special_dir calls returning NULL.
Comment 14 Nicolò Chieffo 2008-10-09 12:48:10 UTC
You are right, the glib spec on xdg dirs says that you cannot change xdg dirs at runtime. you need to log out.

"Depending on the platform, the user might be able to change the path of the special directory without requiring the session to restart; GLib will not reflect any change once the special directories are loaded."

This is of course really a limit!

Currently nautilus is cool, since it automatically changes the file .config/user-dirs.dirs, but of course icons will change after a logout :(

About the NULL return, I really cannot understand why strcmp cannot handle NULL arguments... this is sad :( so we could use g_strcmp0 which handles NULL arguments
Comment 15 Nicolò Chieffo 2008-10-09 12:49:23 UTC
Created attachment 120268 [details] [review]
xdg-directory-icons.patch
Comment 16 Nicolò Chieffo 2008-10-09 18:19:41 UTC
Created attachment 120296 [details] [review]
glib-xdg-cache-reloaded.patch

Ok I could propose this patch to glib to automatically reload xdg data if the asked directory does no more exist.
Unfortunately this adds IO every time someone asks for a xdg_user_special_dir, but this shouldn't be a problem, since when you ask for it you will probably do some IO!
Comment 17 Nicolò Chieffo 2008-10-09 18:21:28 UTC
Anyway it's also very simple to add a glib function

g_invalidate_user_special_dir_cache()
and/or
g_set_user_special_dir()

why is it not present? Is there a motivation?
Comment 18 Matthias Clasen 2008-10-09 18:27:50 UTC
Invalidating userdirs is not really possible, due to memory mgmt issues.
There is no api for setting special dirs, since these are _user_ special dirs, not something for apps to mess with.
Comment 19 Nicolò Chieffo 2008-10-09 19:50:43 UTC
I cannot understand. If they are user special dirs they should not be "read-only". And if not how can a user modify them without an app?
Now they are treated as system special dirs (such as /usr which is of course "read-only").

It seems that the special dirs are stored in a gchar* array. It is possible to free the array entries and than call again load_user_special_dirs().
I don't see any memory management problem, but I've just had a quick look at the code. Are you sure?
Comment 20 Matthias Clasen 2008-10-10 00:35:04 UTC
Yes, I'm sure.

> * Return value: the path to the specified special directory, or %NULL
> *   if the logical id was not found. The returned string is owned by
> *   GLib and should not be modified or freed.

We have no control over how long applications keep the pointers around that are returned by g_get_user_special_dir, so there is no way that we can free them in
the library...
Comment 21 Nicolò Chieffo 2008-10-10 06:56:09 UTC
Ok, thank you very much for the explanation! Now I understood!

Anyway, the xdg-directory-icons.patch to GIO can be considered, even if glib does not let a xdg-directory to be changed runtime. This is not a real problem, because the user will see back its beautiful icons after the logout, if he changes the directory name/path, exactly as it happens with the desktop dir.

Do you think that the new icons will be accepted by xdg-spec? And if so, how to make them answer to this request, since xdg-list ignored it?
Comment 22 Nicolò Chieffo 2008-10-14 22:30:38 UTC
I know this is not an important problem, but this improvement could be very appreciated by users!

Is it possible to commit the patch, so that maybe someone will add the new icons? it has been fixed to handle NULL returns, so there is no problem now.

And there's no need to support dynamyc xdg directories now.
Comment 23 Nicolò Chieffo 2008-11-04 10:03:24 UTC
Any intention to fix it in this unstable release cycle?
Comment 24 Alexander Larsson 2009-02-26 11:51:07 UTC
I think it might be a bit late to change this at this late point in the cycle.

However, the glib-xdg-cache-reloaded.patch patch is just wrong. We can't just keep doing blocking i/o in a function like this. Apps expect it to be fast and not block, so it could be called from any sort of place at any time.

IMHO we should add some invalidation call for this and have nautilus call that when it knows that the settings changed. Then we should take care when we update the internal settings so that we only leak the strings that really needed changing. I don't think such a minor leak in a very uncommon situation matters.

Also, we should probably make the gtk file selector call this cache invalidation function when its displayed so that regular apps can get updated directories.
Comment 25 Nicolò Chieffo 2009-02-26 11:59:55 UTC
Created attachment 129558 [details] [review]
xdg_user_dirs_icons_with_emblems.patch

Ok. In the mean time I'm attaching the new patch with emblems, so new icons will not have to be designed
Comment 26 Alexander Larsson 2009-02-26 12:16:28 UTC
I don't think using emblems for this is a good idea. Its already overloaded with way to many things, and as I said above you can't do what we want for e.g. the desktop and homedir icons with an emblem.
Comment 27 Nicolò Chieffo 2009-02-26 12:38:39 UTC
Emblems seems a good compromise for the rest of xdg directories.
Anyway if you think that plain icons are better, who will draw them? I've already contacted the xdg-list several times, without any answer.
Comment 28 Matthias Clasen 2009-02-26 15:57:01 UTC
Alex proposal in comment #24 sounds ok to me (have an invalidate function that just leaks changing strings)
Comment 29 Nicolò Chieffo 2009-02-26 16:07:39 UTC
like
void g_change_user_special_dir(GUserDirectory directory, const char* new_value)

or

void g_invalidate_user_special_dir(GUserDirectory directory)

?

The former does not require disk access, the latter instead does. But the former might also be not good, because maybe now applications can thing that changing a special directory is something that they can normally

Anyway it's quite easy to implement both...
Comment 30 Nicolò Chieffo 2009-02-27 18:14:57 UTC
Created attachment 129676 [details] [review]
move_user_special_dirs.patch

What about this patch?
Comment 31 Nicolò Chieffo 2009-02-27 18:15:05 UTC
Created attachment 129677 [details] [review]
move_user_special_dirs.patch

What about this patch?
Comment 32 Nicolò Chieffo 2009-02-27 18:17:15 UTC
Attached twice, sorry :(
Comment 33 Matthias Clasen 2009-04-03 05:04:22 UTC
I think what we want here is

void g_invalidate_user_special_dirs (void)

As Alex said, it needs to be careful to only leak the strings that are actually changed.

As for asking for icons, http://live.gnome.org/GnomeArt/ArtRequests is a good place to go.
Comment 34 Nicolò Chieffo 2009-04-03 09:28:06 UTC
After easter I will work on the patch to submit a better one.
In the mean time can you file the art request pointing to this page?

Anyway apps that have the old pointer, won't work. Can you think of a way to handle this? Maybe a signal to which apps will register in glib?
Comment 35 Nicolò Chieffo 2009-04-10 12:50:02 UTC
Created attachment 132458 [details] [review]
reload_cache.patch

Hello, this is me again. I've done another try, tell me how can I improve it.

What I do is the following:
- create a new array
- reload the dirs using load_user_special_dirs()
- compare the old dirs with the new ones
- use the old pointers if the dir didn't change
- keep track of how much memory is leaked
Comment 36 Nicolò Chieffo 2009-04-10 13:43:20 UTC
I filed the artwork request in the page http://live.gnome.org/GnomeArt/ArtRequests

I hope someone will see it
Comment 37 Martin Lettner 2009-04-13 18:30:43 UTC
Created attachment 132606 [details]
XGD directories with an emblem icon for each folder

Used icon theme: gnome-colors by perfectska04
http://www.gnome-look.org/content/show.php/GNOME-colors?content=82562
Comment 38 Martin Lettner 2009-04-13 18:33:10 UTC
because of icons: why don't you just use the "emblems" icons?
"emblem-documents" for the "Documents" folder and so on...

having a folder AND the special type like documents or pictures in front of it
as icon (like windows) may be a nice idea but nobody needs it. everybody knows
it's a folder so why also show it. also the icons in the sidebar are very
small.
the desktop has it's own icon in the "Places" sidebar in nautilus and it is
also a simple folder but there is no folder icon, just the desktop icon.

i attached a mockup to show what i mean. the default gnome icon theme does not
offer enough emblem icons so i used the gnome-colors icon theme which has
beautiful icons for just everything.
Comment 39 Nicolò Chieffo 2009-04-14 11:50:33 UTC
A small reference to a folder image should be included. not everybody knows that it's a folder, as you suppose.
Comment 40 Martin Lettner 2009-04-14 12:03:52 UTC
that may be, but in 16x16 size?

i think it's much cleaner without the folder icon.

also i think the user doesn't really have to know that it's actually a folder. if you want to get to your music, click on music; and not on the music folder.
Comment 41 Nicolò Chieffo 2009-04-14 12:08:44 UTC
Can't you differentiate the icon basing on its size?
Comment 42 Nicolò Chieffo 2009-05-29 09:07:44 UTC
Before this bug "dies" again (it was started in July 2008), and since I can't set it to a "needinfo" status (I don't have the permissions), I would like a comment to my last patch by Matthias Clasen.
Comment 43 Nicolò Chieffo 2009-05-30 08:46:57 UTC
Where can we have a discussion about icons where lots of icon developers are involved, so it's not only me and you two?
Comment 44 Nicolò Chieffo 2009-05-30 14:10:41 UTC
I also filed the icons request in bug #584286 as suggested by Lapo Calamandrei in a private conversation
Comment 45 Nicolò Chieffo 2009-05-31 18:53:39 UTC
Created attachment 135682 [details] [review]
attach-icons-to-xdg-folders.patch

This new patch passed the icon naming spec review.
Icon names are now "folder-foo"
Comment 46 Nicolò Chieffo 2009-06-12 09:03:26 UTC
Created attachment 136405 [details] [review]
icons-glib-patch.patch

This patch adds an important feature:
- fallback "folder" icon when the current theme does not have the requested
icon (nautilus has problems with it, so we need nautilus developers to fix it
before committing this patch)
- changed from "folder-foo" to "user-foo" as it makes more sense (thanks alex)

still need a comment from the icon naming spec developer
Comment 47 Nicolò Chieffo 2009-06-12 11:38:23 UTC
No, it's better to keep to folder-foo to use default fallbacks.
I will update the patch as soon as I'm back home
Comment 48 Nicolò Chieffo 2009-06-12 14:04:26 UTC
Created attachment 136424 [details] [review]
icons-with-auto-fallback.patch

This should be the final.
It is applicable even if the theme does not have the "folder-foo" icons, because it fallsback to "folder" icon using an automatic GIO function.
Comment 49 Alexander Larsson 2009-06-15 11:20:04 UTC
I commited a fixed up version of g_reload_user_special_dirs_cache and code in nautilus to use it.

Comment 50 Michael Monreal 2009-06-22 16:49:52 UTC
Works great, but I wonder if folders XDG folder icons should also show up in the path bar? The special icons for home and Desktop are shown, others are not.
Comment 51 Michael Monreal 2009-06-22 16:50:40 UTC
Created attachment 137184 [details]
Screenshot Desktop VS Download
Comment 52 Nicolò Chieffo 2009-06-22 16:52:21 UTC
I don't know how nautilus handles this, sorry
Comment 53 Michael Monreal 2009-06-22 17:07:52 UTC
Same for GTK's file chooser btw. Do both share the same code/widget or are they individual implementations?
Comment 54 Luca Cavalli 2009-06-23 12:20:09 UTC
Nicolò, for GTK file chooser you have to look in http://git.gnome.org/cgit/gtk+/tree/gtk/gtkpathbar.c, there is a special case for root, home and desktop (see ButtonType enum at the beginning of the source code). For Nautilus http://git.gnome.org/cgit/nautilus/tree/src/nautilus-pathbar.c.
Comment 55 Nicolò Chieffo 2009-06-23 13:16:33 UTC
I'm afraid I don't have time to propose another patch. can someone else fix this? I guess it's very simple.
I'm sorry, but I have to study for a very hard exam.
Comment 56 A. Walton 2010-02-16 21:17:49 UTC
*** Bug 610192 has been marked as a duplicate of this bug. ***