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 682101 - Provide a way to get a symbolic icon for a device
Provide a way to get a symbolic icon for a device
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks: 681457 681458
 
 
Reported: 2012-08-17 13:40 UTC by William Jon McCann
Modified: 2012-09-05 10:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add symbolic icon support to drive, volume, and mount (22.50 KB, patch)
2012-08-29 04:59 UTC, William Jon McCann
accepted-commit_now Details | Review
Add symbolic icon support to gfileinfo (11.15 KB, patch)
2012-08-29 04:59 UTC, William Jon McCann
accepted-commit_now Details | Review
Add ability to get symbolic icon for content type (7.16 KB, patch)
2012-08-29 04:59 UTC, William Jon McCann
none Details | Review
Add ability to get symbolic icon for content type (8.68 KB, patch)
2012-08-29 17:55 UTC, William Jon McCann
accepted-commit_now Details | Review
Add symbolic icon support to drive, volume, and mount (23.03 KB, patch)
2012-08-29 22:09 UTC, William Jon McCann
committed Details | Review
Add symbolic icon support to gfileinfo (12.45 KB, patch)
2012-08-29 22:09 UTC, William Jon McCann
committed Details | Review
Add ability to get symbolic icon for content type (9.20 KB, patch)
2012-08-29 22:09 UTC, William Jon McCann
committed Details | Review

Description William Jon McCann 2012-08-17 13:40:05 UTC
At various times we would like to be able to get a symbolic icon for a device. Specifically, we need them for the Nautilus sidebar. See bug 681457.

It doesn't seem right for the gvfs backends to return symbolic icons when a caller asks for just an icon since there are still many uses for the full color icons.

Perhaps we need a G_FILE_ATTRIBUTE_STANDARD_SYMBOLIC_ICON ?
Comment 1 David Zeuthen (not reading bugmail) 2012-08-20 15:21:42 UTC
Sounds good to me. You'd also need to add

 g_drive_get_symbolic_icon()
 g_volume_get_symbolic_icon()
 g_mount_get_symbolic_icon()

methods to the API since that's what GVfs' computer:// backend would use to populate the GFile attributes in question.

(You'd also need to add a lot of glue in GVfs including mount daemons and remote volume monitors to handle the g_{drive,volume,mount}_get_symbolic_icon() data... Cosimo recently did that for another API addition - it's a lot of work.)
Comment 2 William Jon McCann 2012-08-29 04:59:25 UTC
Created attachment 222708 [details] [review]
Add symbolic icon support to drive, volume, and mount

We need symbolic icon support for display in Nautilus.
Comment 3 William Jon McCann 2012-08-29 04:59:29 UTC
Created attachment 222709 [details] [review]
Add symbolic icon support to gfileinfo
Comment 4 William Jon McCann 2012-08-29 04:59:31 UTC
Created attachment 222710 [details] [review]
Add ability to get symbolic icon for content type
Comment 5 Cosimo Cecchi 2012-08-29 13:07:52 UTC
Review of attachment 222708 [details] [review]:

Looks generally good to me

::: gio/gunixmounts.c
@@ +1936,2 @@
       else
+        icon_name = use_symbolic ? "media-removable-symbolic" : "media-flash";

Is it worth filing a bug requesting a media-flash-symbolic for this?

::: gio/gwin32mount.c
@@ +204,3 @@
+  case DRIVE_REMOTE : return use_symbolic ? "folder-remote-symbolic" : "folder-remote";
+  case DRIVE_CDROM : return use_symbolic ? "drive-optical-symbolic" : "drive-optical";
+  default : return use_symbolic ? "folder-symbolic" : "folder";

Not completely sure about changing these icon names for win32 - ideally these would be provided inside GTK+ itself, but it looks like even previously we weren't using available stock icons, so maybe it's fine.
Comment 6 Cosimo Cecchi 2012-08-29 13:15:48 UTC
Review of attachment 222709 [details] [review]:

Looks good to me
Comment 7 Cosimo Cecchi 2012-08-29 13:25:46 UTC
Review of attachment 222710 [details] [review]:

::: gio/gcontenttype.c
@@ +472,3 @@
+ */
+GIcon *
+ * Returns: (transfer full): symbolic #GIcon corresponding to the content type.

Not a huge fan of duplicating all of the code between g_content_type_get_icon() and this, since the bulk of the function seems to be the same.
Maybe you can factor out a get_icon_internal private method?

::: gio/glocalfileinfo.c
@@ +1695,2 @@
                     }
                 }

Could get_icon_name() become get_gicon() and all these condition checks be moved into that shared function? (since they appear to be basically the same for symbolic and non-symbolic cases).
Comment 8 William Jon McCann 2012-08-29 17:14:16 UTC
Review of attachment 222708 [details] [review]:

::: gio/gunixmounts.c
@@ +1936,2 @@
       else
+        icon_name = use_symbolic ? "media-removable-symbolic" : "media-flash";

Jakub didn't want to add that.

::: gio/gwin32mount.c
@@ +204,3 @@
+  case DRIVE_REMOTE : return use_symbolic ? "folder-remote-symbolic" : "folder-remote";
+  case DRIVE_CDROM : return use_symbolic ? "drive-optical-symbolic" : "drive-optical";
+  default : return use_symbolic ? "folder-symbolic" : "folder";

Yeah those names made no sense anyway. I think we should use the correct names and copy them into the build for Win32 if we think that is necessary. But I'd rather do that separately from this patch since their is no regression here since those icons don't exist already.
Comment 9 William Jon McCann 2012-08-29 17:55:23 UTC
Created attachment 222814 [details] [review]
Add ability to get symbolic icon for content type
Comment 10 Cosimo Cecchi 2012-08-29 18:53:22 UTC
Review of attachment 222814 [details] [review]:

Looks good to me
Comment 11 Cosimo Cecchi 2012-08-29 21:34:31 UTC
Review of attachment 222708 [details] [review]:

This looks good to commit then, with the following fix

::: docs/reference/gio/gio-sections.txt
@@ +1131,3 @@
 g_drive_get_name
 g_drive_get_icon
+g_drive_get_symbolic_icon

Should also add the new methods for g_unix_mount* here
Comment 12 Cosimo Cecchi 2012-08-29 21:36:02 UTC
Review of attachment 222709 [details] [review]:

Good to commit, with the following fixes

::: gio/gfileinfo.h
@@ +172,3 @@
+ * Since: 2.34
+ **/
+ * Corresponding #GFileAttributeType is %G_FILE_ATTRIBUTE_TYPE_OBJECT.

Should also add this, and the two methods below to gio-sections.txt

::: gio/gio.symbols
@@ +423,3 @@
 g_file_info_get_edit_name
 g_file_info_get_icon
+g_file_info_get_symbolic_icon

Missing _set_symbolic_icon here
Comment 13 Cosimo Cecchi 2012-08-29 21:37:00 UTC
Review of attachment 222814 [details] [review]:

Good to commit with the following fix

::: gio/gio.symbols
@@ +162,3 @@
 g_content_type_get_mime_type
 g_content_type_get_icon
+g_content_type_get_symbolic_icon

Should also add this to gio-sections.txt
Comment 14 William Jon McCann 2012-08-29 22:09:18 UTC
Created attachment 222880 [details] [review]
Add symbolic icon support to drive, volume, and mount

We need symbolic icon support for display in Nautilus.
Comment 15 William Jon McCann 2012-08-29 22:09:21 UTC
Created attachment 222881 [details] [review]
Add symbolic icon support to gfileinfo
Comment 16 William Jon McCann 2012-08-29 22:09:23 UTC
Created attachment 222882 [details] [review]
Add ability to get symbolic icon for content type
Comment 17 Cosimo Cecchi 2012-08-30 14:00:03 UTC
Review of attachment 222880 [details] [review]:

++
Comment 18 Cosimo Cecchi 2012-08-30 14:00:20 UTC
Review of attachment 222881 [details] [review]:

++
Comment 19 Cosimo Cecchi 2012-08-30 14:00:39 UTC
Review of attachment 222882 [details] [review]:

++
Comment 20 William Jon McCann 2012-08-30 15:05:13 UTC
Attachment 222880 [details] pushed as a2dca48 - Add symbolic icon support to drive, volume, and mount
Attachment 222881 [details] pushed as a15a071 - Add symbolic icon support to gfileinfo
Attachment 222882 [details] pushed as 40b4fae - Add ability to get symbolic icon for content type
Comment 21 David Zeuthen (not reading bugmail) 2012-09-04 17:44:07 UTC
These patches do not appear to include any changes to monitor/udisks2/*.c ... can anyone tell me what the symbolic icons for G{Drive,Volume,Mount} instances for e.g. USB sticks and hard disks are? (gvfs-mount(1) output, please)
Comment 22 David Zeuthen (not reading bugmail) 2012-09-04 17:59:18 UTC
(In reply to comment #21)
> These patches do not appear to include any changes to monitor/udisks2/*.c ...
> can anyone tell me what the symbolic icons for G{Drive,Volume,Mount} instances
> for e.g. USB sticks and hard disks are? (gvfs-mount(1) output, please)

Gathered on IRC that this will trigger the fallback in the interface wrappers. Good enough for now, we can always implement this in the udisks2 volume monitor if needed. Thanks.