GNOME Bugzilla – Bug 606419
Support for application-indicators/StatusNotifierIcon
Last modified: 2011-04-16 10:39:47 UTC
We would like to support application-indicators for Vino as proposed at this page: https://wiki.ubuntu.com/DesktopExperienceTeam/ApplicationIndicators The Canonical DX team will provide a patch to vino, but the work has not started, so I wanted to open this bug as a placeholder and to get feedback on the feature, thanks! The distro URL for this work is here: https://bugs.launchpad.net/ubuntu/+source/vino/+bug/497883
Created attachment 154779 [details] [review] Support for Application Indicators This is a first pass at adding support for the new applications indicators. It was recommended (see the comments on the Launchpad bug linked above by Jorge) that I run this by the Vino developers before we commit to it in Ubuntu. As it stands, I realize this patch has some problems. I chose to do it this way, as the least invasive option I could come up with. My goals were: - support application indicators - make the patch acceptable to upstream - in that vein, make it maintainable for both the Canonical team (realizing we would maintain the patch for Lucid, as it is passed the Gnome release deadline) and upstream developers - support not removing any existing functionality (namely, the current implementations using GtkStatusIcon) if the particular target does not have the application indicator libraries. I had problems meeting those goals to my satisfaction. The way I came up with implementing this was copy and pasting the code for the existing VinoStatusIcon and VinoTubeStatusIcon implementations and then changing it to have the application indicator specific functionality. I do not like that I did not re-use the existing common code between the VinoStatusIcon and VinoAppIndicator classes. It did not make sense to inherit from VinoStatusIcon because VinoAppIndicator is not an instance of GtkStatusIcon. I wasn't sure what other means of abstraction I had available to do this. I bring this to your review for a few reasons: 1) what can I do to make this more acceptable for inclusion in gnome upstream (even if eventually) and, related, 2) what is the best way to handle the common code in this case and have it be minimally invasive to the rest of the vino code base?
As suggested by Martin Pitt (https://bugs.launchpad.net/ubuntu/+source/vino/+bug/497883/comments/4), it would be better if this patch were separated into the string changes as suggested by Matthew Paul Thomas and then the Application Indicator Support. I believe the string changes in the .ui file are general enough to apply to both the GtkStatusIcon version and the AppIndicator version. I'm attaching two patches, one for just the string changes and one for the app indicator specific changes.
Created attachment 154991 [details] [review] Support for App Indicators only, separate from UI string changes.
Created attachment 154992 [details] [review] String/UI changes for the preferences capplet
Comment on attachment 154992 [details] [review] String/UI changes for the preferences capplet I pushed a modified commit as 472747356a7a56c93015a4e51a23b29f58ea4788.
Comment on attachment 154991 [details] [review] Support for App Indicators only, separate from UI string changes. Sorry, I'm not interested in application indicator support in Vino.