GNOME Bugzilla – Bug 606972
Support for application-indicators/StatusNotifierIcon
Last modified: 2010-08-19 00:26:21 UTC
We would like to support application-indicators for rhythmbox as proposed at this page and on the GNOME desktop-devel-list: https://wiki.ubuntu.com/DesktopExperienceTeam/ApplicationIndicators The Canonical DX team will provide a patch, however our patch has caused a crasher that we are still investigating: https://bugs.launchpad.net/ubuntu/+source/rhythmbox/+bug/498588 I wanted to open this bug as a placeholder and to get feedback on the feature, thanks!
Looks like an interesting approach. I haven't looked closely at what features the indicators provide.. do we still get things like scroll events and mouse button presses, or are we limited to the menu we provide? What about tooltips for indicators? Is there a plan for dealing with minimize/close-to-tray (or "icon owns window" as I called it) applications?
I think you've listed three features that honestly, we haven't delt with yet. I think they're all interesting, we're going to have to draw up guidelines and implementation for them. But, I can say they'd be V2 type of things right now. We wanted to start with something simple that got the basic feature out there so we could beat that up first. If you have specific ways that you're thinking about using those I would *love* to see you post on the Ayatana list about those so we could make sure to take those use-cases into account when doing V2. Here's the link to the mailing list: https://launchpad.net/~ayatana
I haven't really put much thought into it - those are just the GtkStatusIcon features that we currently use. I forgot to mention that we use drag and drop too. I'd really like it if the 'close to tray' vs 'actually close' problem could be designed out of existence. Looks like that's being addressed here: https://wiki.ubuntu.com/CustomStatusMenuDesignGuidelines already. I think I'll need to figure out which features truly belong there and which just got shoehorned in over the years because the status icon was a convenient way to interact with rb without having to bring up the main window. You might need to patch out some of the configuration options, since some may not apply to the indicator. Without scroll events, the combo box selecting whether to change volume or skip tracks doesn't make any sense, for example.
Created attachment 152945 [details] [review] Proposed patch
Created attachment 152946 [details] [review] Add missing files Sorry, I forgot to add two files to the last patch. :)
Review of attachment 152946 [details] [review]: ::: old/plugins/status-icon/Makefile.am @@ +8,3 @@ + rb-indicator.h \ + rb-indicator.c +else this will need to be updated to apply to git master, since I removed the eggtrayicon code recently ::: old/plugins/status-icon/rb-status-icon-plugin.c @@ +154,3 @@ N_("Choose music to play"), G_CALLBACK (show_window_cmd) }, +#endif Might as well just remove this? And show_window_cmd too. @@ +291,3 @@ +GtkWidget * +rb_status_icon_get_popup (RBStatusIconPlugin *plugin) this probably belongs in rb-indicator.c, since it's indicator specific and it only needs access to the shell. @@ +476,1 @@ if (rb_tray_icon_is_embedded (plugin->priv->tray_icon) == FALSE) { Maybe this detail should move down to the icon implementation - rb_tray_icon_should_notify (icon, mode) or something. @@ +510,3 @@ +#if defined(HAVE_NOTIFY) && defined(HAVE_APP_INDICATOR) +void +rb_tray_icon_attach_notification (RBTrayIcon *icon, NotifyNotification *notification) This should go in rb-indicator.c
Created attachment 154138 [details] [review] Updated patch
Review of attachment 154138 [details] [review]: If I was using this day to day, I'd probably miss the status icon tooltip pretty badly.. what can we do instead? Add a disabled menu item containing the current track details? ::: plugins/status-icon/rb-status-icon-plugin.c @@ +144,3 @@ + G_CALLBACK (toggle_window_cmd) }, +#ifdef HAVE_APP_INDICATOR + { "TrayShowWindow", NULL, N_("_Show Rhythmbox"), NULL, a lot of the #ifdefs here would go away if we used a different action name for this one and compiled them both in. @@ +1260,3 @@ + TRUE); + gtk_widget_hide (GTK_WIDGET (gtk_builder_get_object (builder, "label5"))); + gtk_widget_set_no_show_all (GTK_WIDGET (gtk_builder_get_object (builder, "label5")), I guess we should give that label a real name, then
Created attachment 155159 [details] [review] Add labels and fix the commented problems
Created attachment 155236 [details] [review] Fix markup in artist label
The updated patch contains a disable menu item with the arist/song. It also has proper names for the widgets used in code.
Sorry for the lack of response, I've been trying to figure out what to do about this for a while. I'm not really happy with how the current patch works, but that's mostly due to the existing structure of the status icon plugin. It was originally designed to paper over the differences between EggTrayIcon and GtkStatusIcon. Now EggTrayIcon is gone and we're trying to make the same abstraction fit something completely different, and it's not working. I'd like to get a clear picture of which parts of the status icon plugin apply to app indicators and which don't. Yes: - notification stuff - most of the config dialog (not the mouse wheel mode bit) No: - tooltip - button press/scroll events I don't know: - the 'close to tray' stuff - icon visibility Another thing to consider is that the gnome-shell design has us (eventually) only using libnotify, with no status icon or app indicator at all. One option would be to split everything up so we have a status icon plugin, a notification plugin, and an app indicator plugin.. I don't think I like that though.
*** Bug 615093 has been marked as a duplicate of this bug. ***
In bug 615093, the reporter suggests that the 'quit' and 'show rhythmbox' menu items are too close together. In the status icon popup menu, there's a separator (and another menu item) between them.
The indicator patch has been removed from the latest rhythmbox package in maverick, so I guess we don't need this any more.