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 154467 - Notification applet to indicate when desktop sharing is active
Notification applet to indicate when desktop sharing is active
Status: RESOLVED FIXED
Product: vino
Classification: Applications
Component: Server
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Jonh Wendell
Vino Maintainer(s)
: 167712 306690 381190 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2004-10-04 14:29 UTC by Mark McLoughlin
Modified: 2006-12-08 17:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (28.69 KB, patch)
2006-11-16 00:22 UTC, Jonh Wendell
needs-work Details | Review
The second try (21.88 KB, patch)
2006-12-07 13:02 UTC, Jonh Wendell
needs-work Details | Review
Third try (22.53 KB, patch)
2006-12-07 20:29 UTC, Jonh Wendell
committed Details | Review

Description Mark McLoughlin 2004-10-04 14:29:16 UTC
From Fedora:

  After turning on desktop sharing there is no visual indication that
  desktop sharing is on. I think a notification icon that at a minimum
  indicates when desktop sharing is active would aid in users from
  engaging in activity they would otherwise refrain from while desktop
  sharing was enabled. The goal here is a request to implement a visual
  reminder, notification icon was the first thing that came to mind.

  Let me take a moment and prioritize a list of features i would like to
  see for any mechanism to be adopted as a visual reminder that desktop
  sharing is active:

  1) indication that sharing is active
  2) be able to call up the preference dialog from a right click menu on
     the icon
  3) number of remote clients access
  4) ability to review the ipaddress/hostname of remote connections
  5) ability to disconnect/refuse specific remote connections

There's some more details on how I thought this should work in:

  http://www.gnome.org/~markmc/remote-desktop.html
Comment 1 Mark McLoughlin 2004-10-04 14:29:44 UTC
Forgot the link to the Fedora bug:

  http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=134475
Comment 2 Teppo Turtiainen 2005-07-17 19:38:29 UTC
*** Bug 306690 has been marked as a duplicate of this bug. ***
Comment 3 Sven J. 2005-07-17 23:25:33 UTC
I build a simple tray icon that does the job.
But it is build in C# with mono and runs separate from the vino server.

get it at http://poweroftwo.de/vinotrayicon.tar

u need to place the two pictures at
/usr/share/pixmaps/desk_remotable.png
/usr/share/pixmaps/desk_viewable.png
Comment 4 Michaël Arnauts 2005-07-27 22:07:47 UTC
*** Bug 167712 has been marked as a duplicate of this bug. ***
Comment 5 Steven Yuchao Zhang 2006-02-28 10:55:15 UTC
This is a very useful feature. 
Comment 6 Brice Goglin 2006-07-16 00:30:35 UTC
Is there any chance we get these features soon in the future?
Thanks in advance, I'd really appreciate it!
Comment 7 Jonh Wendell 2006-08-01 00:23:36 UTC
This feature is not only useful, but it is necessary!
Today, for example, i've received a conection and i didn't get know when the remote side has disconnected. How to know if the remote side is watching you?
Comment 8 Jonh Wendell 2006-11-16 00:22:41 UTC
Created attachment 76691 [details] [review]
Proposed patch

My second patch to vino. I hope you can use it. Feel free to adapt it if i made some mistakes.
Comment 9 Jonh Wendell 2006-11-16 14:47:31 UTC
Screenshots for this patch (text in pt_BR):
http://www.bani.com.br/?p=45
Comment 10 Mark McLoughlin 2006-11-17 16:37:52 UTC
Cool! Good stuff John.

Comments:

  - I don't think we want the always_show_tray_icon preference. The status 
    icon doesn't really do anything useful when no-one is connected, so
    the default of only showing the icon when someone is connected is fine

  - We don't need this "enabled" property on the server ... the only thing
    that governs whether the icon should be displayed is whether there are
    any clients connected. So, even if the server is disabled (or, rather,
    "on hold") ... then we still want the icon displayed if there are clients
    connected

  - I prefer to stick with the term "status icon" rather than "tray icon"
    since that's what gtk uses

  - I think we want we want to do is to put the status icon into it's own
    module ... so something like this:

    vino-status-icon.[ch]:

      VinoStatusIcon *vino_status_icon_new (VinoServer *server);

      void vino_status_icon_add_client    (VinoStatusIcon *icon,
                                           VinoClient     *client);
      void vino_status_icon_remove_client (VinoStatusIcon *icon,
                                           VinoClient     *client);

    vino-server.[ch]:

      typedef struct _VinoClient VinoClient;

      void vino_server_disconnect_client (VinoServer *server,
                                          VinoClient *client);

      const char *vino_client_get_hostname (VinoClient *client);

    VinoStatusIcon would derive from GtkStatusIcon

    Note, there is a VinoServer for each GdkScreen ... so, there should be
    a status icon per screen. So, that implies you'd have something like:

      VinoStatusIcon *
      vino_status_icon_new (VinoServer *server)
      {
        GdkScreen *screen;

        screen = vino_server_get_screen (server);

        return g_object_new (VINO_TYPE_STATUS_ICON,
                             "screen", screen,
                             "server", server,
                             NULL);
      }

    However, GtkStatusIcon doesn't currently have a "screen" property. It
    should, so I'll try and fix that this weekend. For now, just add a
    FIXME about it in vino_status_icon_new()

    The status icon would look after its visibility based on whether clients
    are connected.

  - You want to use gdk_spawn_command_line_on_screen() since Vino may be
    running on multiple screens

  - There's something weird going on with the menu ... I got it into a state
    where clicking on the icon only caused a flicker as if the menu was being
    shown and hidden really quickly

  - Coding style:

       - have a good look around at the coding style of the rest of Vino
         and try and copy it

       - keep stuff hidden behind module APIs as much as possible - so e.g.
         there's no reason for the popup callbacks to defined in the header
         file and, also, there's no reason to expose the contents of the
         VinoServerClient structure in the header file

       - don't use c++ style comments

       - all messages for users need to be marked for translation with _()

       - rather than e.g. vino_tray_icon_popup_menu_cb(), I prefer the likes
         of vino_status_icon_show_menu() and 
         vino_status_icon_disconnect_client()
Comment 11 Mark McLoughlin 2006-11-17 23:51:29 UTC
See bug #376502 for a patch which implements gtk_status_icon_set_screen()

Note, we can't use this yet because GNOME 2.17 will use gtk+ 2.10. This API will only be available from with gtk+ 2.12
Comment 12 Jonh Wendell 2006-11-22 13:41:34 UTC
Hi. My comments:

> I don't think we want the always_show_tray_icon preference. The status 
>   icon doesn't really do anything useful when no-one is connected, so
>    the default of only showing the icon when someone is connected is fine

I think this option is very useful, because it lets the people know when their desktop sharing is active or not. An improvement would be change the icon (a little change) when exists someone connected. Another improvement would be show the server IP as the icon tooltip.

I work on an environment where most of time we need to access the users desktops, but we don't know their IP's. So, by phone we tell: Please, point your mouse at that blue icon near to clock and say me the numbers you see.


> We don't need this "enabled" property on the server ... the only thing
>    that governs whether the icon should be displayed is whether there are
>    any clients connected. So, even if the server is disabled (or, rather,
>    "on hold") ... then we still want the icon displayed if there are clients
>    connected

I think this behavior would confuse the user. "My desktop sharing is disabled, why there is someone connected to my machine?"


>I think we want we want to do is to put the status icon into it's own
>    module ... so something like this:

I agree with you, surely the code gets better, but definitely i have to study how to do this. I'm novice about "classes" in GTK/C. Can you help me with this?


About the other points, i agree and i'm going to do the possible to get the changes in.
Comment 13 John Richard Moser 2006-11-28 05:47:29 UTC
(In reply to comment #10)
> Comments:
> 
>   - I don't think we want the always_show_tray_icon preference. The status 
>     icon doesn't really do anything useful when no-one is connected, so
>     the default of only showing the icon when someone is connected is fine
> 
>   - We don't need this "enabled" property on the server ... the only thing
>     that governs whether the icon should be displayed is whether there are
>     any clients connected. So, even if the server is disabled (or, rather,
>     "on hold") ... then we still want the icon displayed if there are clients
>     connected

I think that the icon should be shown whenever vino is configured.  It should be possible to click the icon and have it disable the server, rather than wade through two menus (System::Preferences) and a configuration applet.  I would feel safer having the server off when I'm not connecting, but not having to go through the trouble of fishing through the menus when I am about to leave.

It would be EXTREMELY nice to be able to click the icon and hit "Disable server," "Disconnect session," etc.  I had a connection for 16 hours from some guy in Europe I didn't know I had until I checked via netstat as a guess to why X was lagging so horribly.  I either missed the "Someone is trying to connect to you..." dialog, or misclicked, or he got around it; but in any case he got in and I should have noticed this 16 hours ago and shouldn't have to 'killall vino-server' to kick him out.
Comment 14 Jonh Wendell 2006-12-01 10:33:05 UTC
*** Bug 381190 has been marked as a duplicate of this bug. ***
Comment 15 birger 2006-12-04 13:49:44 UTC
As I said in bug 381190 that got linked here, I have been told that this applet would be mandatory according to Norwegian law if we want to deploy gnome with the vnc server set up and operative by default.

The popup where the user must accept the connection is also mandatory, but that one already exists and can be set up as mandatory using sabayon. :-)

Of course we want to deploy with the vnc server set up so our support people can connect to users calling in with problems. We need this issue fixed... Or we may be breaking the law. Users must be able to see some notification as long as the helpdesk is connected to their display.
 
The tray icon from Sven J above does not help with this issue. It only notifies that the vnc server can be connected to without a popup/password. It doesn't warn that there is an actual connection. Since we enforce the popup, the applet does nothing.

Comment 16 Jonh Wendell 2006-12-04 14:01:49 UTC
Hi, birger.

I hope this feature will get in gnome 2.18. The patch i submitted already does what WE want, but it needs some work. I wish i get time to work on it this week.
Comment 17 Jonh Wendell 2006-12-07 13:02:14 UTC
Created attachment 77886 [details] [review]
The second try

Hi. Can you look now? I've tried to correct all the notes you posted.
Comment 18 Mark McLoughlin 2006-12-07 15:37:03 UTC
Okay, first round of comments:

 - Watch your coding style ... e.g. add a space between a
   function's name and the parenthesis

 - Just because a client is on the client list, doesn't mean
   it should be displayed ... it might not yet have been
   accepted. So, for that reason maybe it does make sense
   for VinoStatusIcon to maintain it's own list of clients

   Perhaps something like:

static void
vino_server_client_accepted (VinoServer *server,
                             VinoClient *client)
{
  if (!server->priv->icon)
    server->priv->icon = vino_status_icon_new (server);

  vino_server_add_client (server, client);
}

static void
vino_server_client_disconnected (VinoServer *server,
                                 VinoClient *client)
{
  if (vino_server_remove_client (server, client))
    {
      g_object_unref (server->priv->icon);
      server->priv->icon = NULL;
    }
}

 - Also, this makes it possible to move the tooltip code into
   vino-status-icon.c


+  number_of_clients = g_slist_length(server->priv->clients);
+
+  if (number_of_clients > 0)

 - a zero-length list will be NULL

+      /* Create the status icon */

 - Comments like this aren't useful.

+        g_string_printf(tooltip, _("There is one person connected to this machine."));

 - Perhaps, "One Person is connected"

+     g_string_prepend(tooltip, "GNOME Remote Access\n");

 - I don't think this is useful

+ * Authors:
+ *      Mark McLoughlin <mark@skynet.ie>

 - You shouldn't add me here :-)

+/* Prototypes for private functions */

 - Again, useless comment

+void vino_status_icon_disconnect_client (GtkMenuItem *menuitem, VinoClient *client);
+void vino_status_icon_preferences       (GtkMenuItem *item,     VinoServer *server);
+void vino_status_icon_help              (GtkMenuItem *item,     VinoServer *server);
+void vino_status_icon_about             (GtkMenuItem *item,     gpointer data);

 - Put each parameter on a new line ... and you probably don't
   need the prototypes if you re-order the function definitions

   Also why is one menu item named "menuitem" and the others
   named "item" :-)

+  GtkMenu *menu;
+  VinoServer *server;

 - Align struct members to make it easier to read e.g.:

+  VinoServer *server;
+  GtkMenu    *menu;

+VinoStatusIcon *
+vino_status_icon_new (VinoServer *server)
+{
+  GdkScreen *screen;

+void
+vino_status_icon_set_server(VinoStatusIcon  *icon,
+                            VinoServer      *server)

 - Since the "server" property is construct-only (i.e. it cannot
   be set once the object has been instantiated), then there should
   be no set_server() function, but rather we should just set the
   pointer in set_property()

 - The screen should actually be a parameter to the function.

+      g_signal_connect_object (G_OBJECT (icon),
+                               "popup_menu",
+                               G_CALLBACK (vino_status_icon_show_menu),
+                               icon->priv->server, 0);
+      g_signal_connect_object (G_OBJECT (icon),
+                               "activate",
+                               G_CALLBACK (vino_status_icon_show_menu2),
+                               icon->priv->server, 0);

 - Since you're inheriting from the class which defines these
   signals, you can connect to the signals by setting the
   "activate" and "popup_menu" pointers in VinoStatusIconClass
   in the class_init() function - also, rename the functions
   which handle these signals to _popup_menu() and _activate()
Comment 19 Jonh Wendell 2006-12-07 20:29:41 UTC
Created attachment 77922 [details] [review]
Third try

I want to believe i'm almost there! As usual, i'm waiting for comments.
Comment 20 Jonh Wendell 2006-12-08 17:03:11 UTC
2006-12-08  Jonh Wendell <jwendell@cvs.gnome.org>
	Initial status icon support (Fixes bug #154467)

	* server/vino-status-icon.[ch]: New files
	
	* server/Makefile.am: Added vino-status-icon.[ch]

	* server/vino-server.[ch]:
	(vino_client_get_hostname),
	(vino_server_disconnect_client): new functions
	(vino_server_handle_prompt_response),
	(vino_server_handle_authenticated_client): Call the new 
	vino_server_client_accepted()
	(vino_server_handle_client_gone): Call the new 
	vino_server_client_disconnected()