Created attachment 73267 [details] [review]
Patch to updated themed icon handling

There seems to be some legacy code, which handles themed icons. With the new Gtk+ and GNOME versions, this code isn't needed any longer and is removed by this patch. This patch also updates the icon names according to the 'icon naming specification' (Tango) which is supported in gnome-icon-theme.

(In reply to comment #1)
> Created an attachment (id=73267) [edit]
> Patch to updated themed icon handling
> 
> There seems to be some legacy code, which handles themed icons. With the new
> Gtk+ and GNOME versions, this code isn't needed any longer and is removed by
> this patch. This patch also updates the icon names according to the 'icon
> naming specification' (Tango) which is supported in gnome-icon-theme.

Actually, I don't want that. Currently the only upp'ed GNOME dependencies are older libgnomeui ones, and Totem still works on older systems with an updated GTK+, and I don't want to lose that. Whether the code should be extended to also handle newer icons in preference is another matter.

Well, the best way is to completely deprecate rhymthbox-* icons because as Jakub Steiner said there is no best visual metaphor for those icons. Just like Rhythmbox, they don't use icons for volume up/down or playing. It'd be nice if you can remove those icons for next version. This would not affect the compatibility for older GNOME machines.

Created attachment 73524 [details] [review]
Add support for legacy icons to the previous patch

A different approach to support legacy icons.

I just check CVS, they have removed rhymthbox-volume-up and down. Still left rythmbox-playing untouched.

Created attachment 75568 [details] [review]
Add support for high-contrast themes.

Prefer stock-icons to support high-contrast themes.

Created attachment 75569 [details] [review]
Add missing files to provious patch

Hey Jan,

I've looked through the patch, and it sounds like a bad idea to support the legacy stuff. The problem is that either we provide our own icons, as we do now, or use the icon theme ones. For most of the icons, that means that we'd be left with the (not too wide) selection from the GTK+ stock icons.

So it would be easier to just require a gnome-icon-theme version, and make sure Totem uses that all over. 2.16.0 would suit me.

Also, bear in mind that sound-juicer and Rhythmbox are cut'n'pasting the bacon-volume code, so we can't just make it depend on Totem stuff.

Created attachment 75737 [details] [review]
Patch to use gnome-icon-theme icons.

Regression with gnome-icon-theme 2.16: no icon for the screenshot menu item.

This patch uses the gnome-icon-theme 2.16 (Tango) icon names.

The Tango icon "camera-photo" for the screenshot menu item is not yet avaiable in gnome-icon-theme, but I would prefer no icon over the only avaiable alternative GTK_STOCK_CONVERT.

Furthermore this patch caches the pixbufs in BaconVolume and TotemPlaylist.

Thanks very much for the work :)

Updated in libbacon:

2006-11-01  Bastien Nocera  <hadess@hadess.net>

        * src/bacon-volume.c: (bacon_volume_button_init),
        (bacon_volume_button_dispose), (bacon_volume_button_new),
        (bacon_volume_scale_value_changed), (bacon_volume_theme_changed),
        (bacon_volume_load_icons):
        * src/bacon-volume.h: Patch from Jan Arne Petersen <jpetersen@jpetersen.org>
        to make use of the named icons in gnome-icon-theme instead of our own
        copies (Closes: #357143)

and Totem HEAD:

2006-11-01  Bastien Nocera  <hadess@hadess.net>

        * data/Makefile.am:
        * data/playlist-playing.png:
        * data/stock_media_next.png:
        * data/stock_media_pause.png:
        * data/stock_media_play.png:
        * data/stock_media_previous.png:
        * data/totem.glade:
        * src/Makefile.am:
        * src/bacon-volume.c: (bacon_volume_button_init),
        (bacon_volume_button_dispose), (bacon_volume_button_new),
        (bacon_volume_scale_value_changed), (bacon_volume_theme_changed),
        (bacon_volume_load_icons):
        * src/bacon-volume.h:
        * src/totem-menu.c: (on_recent_file_item_activated),
        (totem_recent_manager_changed_callback),
        (totem_ui_manager_connect_proxy_callback):
        * src/totem-playlist.c: (totem_playlist_set_reorderable),
        (load_icon), (icon_theme_changed), (totem_playlist_save_playlist),
        (set_playing_icon), (init_columns), (init_treeview),
        (totem_playlist_realize), (totem_playlist_init),
        (totem_playlist_finalize), (totem_playlist_new),
        (totem_playlist_add_one_mrl), (totem_playlist_set_playing):
        * src/totem-stock-icons.c:
        * src/totem-stock-icons.h:
        * src/totem.c: (totem_action_exit), (totem_callback_connect),
        (main):

        Big patch from Jan Arne Petersen <jpetersen@jpetersen.org> to make
        use of named icons instead of stock icons (Closes: #357143)

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

+  theme = gtk_icon_theme_get_default ();

Just FYI, this isn't multi-screen safe.