Re: PATCH: Add configurable key grab sequences (v3)

[Michal Novotny <minovotn redhat com>]

Hi Daniel,
as I've been asked to tell you my comments and whether it looks sane to 
me, I did the review. It looks good but some comments are inline...

On 06/18/2010 12:29 AM, Daniel P. Berrange wrote:
> Allow the key grab sequence to be configured by applications.
> The grab sequence is represented by a new boxed data object
>
>    VncGrabSequence
>
> This can be created from an array of keysyms
>
>    guint keys[] = { GDK_Control_L, GDK_Alt_L };
>    guint len = sizeof(keys)/sizeof(keys[0]);
>    vnc_grab_sequence_new(len, keys);
>
> Or from a string format
>
>    vnc_grab_sequence_new_from_string("Control_L+Alt_L");
>
> The gvncviewer.c example program shows a user interface for
> configuring key sequences. The gvncviewer.py example program
> demonstrates access from the python binding.
>
>    

Ok, looking good and maybe it's good to have option to have it both by 
sequence definition and by string. I've been thinking about this 
originally too but then I found my definition by string redundant and 
maybe even useless provided the fact there will be the key recorded, 
e.g. like the example user interface for configuring key sequences in 
gvncviewer.c code.

> This patch is heavily based on the original proposal from
> Michal Novotny
>    

Thanks for mentioning me in the patch credits ;)

> ---
>   examples/gvncviewer.c      |  182 +++++++++++++++++++++++++++++++++++++++++---
>   examples/gvncviewer.py     |   14 +++-
>   src/Makefile.am            |    3 +
>   src/libgtk-vnc_sym.version |   10 +++
>   src/vnc.override           |   57 ++++++++++++++
>   src/vncdisplay.c           |   82 +++++++++++++++++++-
>   src/vncdisplay.h           |    3 +
>   src/vncgrabsequence.c      |  121 +++++++++++++++++++++++++++++
>   src/vncgrabsequence.h      |   58 ++++++++++++++
>   9 files changed, 515 insertions(+), 15 deletions(-)
>   create mode 100644 src/vncgrabsequence.c
>   create mode 100644 src/vncgrabsequence.h
>
> diff --git a/examples/gvncviewer.c b/examples/gvncviewer.c
> index c3fbd74..1799e32 100644
> --- a/examples/gvncviewer.c
> +++ b/examples/gvncviewer.c
> @@ -43,23 +43,33 @@ static const GOptionEntry options [] =
>
>   static GtkWidget *vnc;
>
> +typedef struct {
> +	GtkWidget *label;
> +	guint curkeys;
> +	guint numkeys;
> +	guint *keysyms;
> +	gboolean set;
> +} VncGrabDefs;
> +
>   static void set_title(VncDisplay *vncdisplay, GtkWidget *window,
>   	gboolean grabbed)
>   {
> -	const char *name;
> -	char title[1024];
> -	const char *subtitle;
> +	const gchar *name = vnc_display_get_name(VNC_DISPLAY(vncdisplay));
> +	VncGrabSequence *seq = vnc_display_get_grab_keys(vncdisplay);
> +	gchar *seqstr = vnc_grab_sequence_as_string(seq);
> +	gchar *title;
>
>   	if (grabbed)
> -		subtitle = "(Press Ctrl+Alt to release pointer) ";
> +		title = g_strdup_printf("(Press %s to release pointer) %s - GVncViewer",
> +					seqstr, name);
>   	else
> -		subtitle = "";
> -
> -	name = vnc_display_get_name(VNC_DISPLAY(vncdisplay));
> -	snprintf(title, sizeof(title), "%s%s - GVncViewer",
> -		 subtitle, name);
> +		title = g_strdup_printf("%s - GVncViewer",
> +					name);
>
>   	gtk_window_set_title(GTK_WINDOW(window), title);
> +
> +	g_free(seqstr);
> +	g_free(title);
>   }
>
>   static gboolean vnc_screenshot(GtkWidget *window G_GNUC_UNUSED,
> @@ -216,6 +226,145 @@ static void do_scaling(GtkWidget *menu, GtkWidget *vncdisplay)
>   		vnc_display_set_scaling(VNC_DISPLAY(vncdisplay), FALSE);
>   }
>
> +static void dialog_update_keysyms(GtkWidget *window, guint *keysyms, guint numsyms)
> +{
> +	gchar *keys;
> +	int i;
> +
> +	keys = g_strdup("");
> +	for (i = 0; i<  numsyms; i++)
> +	{
> +		keys = g_strdup_printf("%s%s%s", keys,
> +			(strlen(keys)>  0) ? "+" : " ", gdk_keyval_name(keysyms[i]));
> +	}
> +
> +	gtk_label_set_text( GTK_LABEL(window), keys);
> +}
> +
> +static gboolean dialog_key_ignore(int keyval)
> +{
> +	switch (keyval) {
> +		case GDK_Return:
> +		case GDK_Escape:
> +			return TRUE;
> +	}
> +
> +	return FALSE;
> +}
>    

Maybe we may define some other in the future. However the main reason 
for this is that usually when starting Fedora (11) that I used for 
testing I've noticed that when the sequence is being set as Ctrl + Enter 
it grabs the enter and boots the entry in the Grub bootloader. For Ctrl 
+ Esc it returns to the main Grub bootloader menu when editing the grub 
boot commands.

> +
> +static gboolean dialog_key_press(GtkWidget *window G_GNUC_UNUSED,
> +        GdkEvent *ev, VncGrabDefs *defs)
> +{
> +	gboolean keySymExists;
> +	int i;
> +
> +	if (dialog_key_ignore(ev->key.keyval))
> +		return FALSE;
> +
> +	if (defs->set&&  defs->curkeys)
> +		return FALSE;
> +
> +	/* Check whether we already have keysym in array - i.e. it was handler by something else */
> +	keySymExists = FALSE;
> +	for (i = 0; i<  defs->curkeys; i++) {
> +		if (defs->keysyms[i] == ev->key.keyval)
> +			keySymExists = TRUE;
> +	}
> +
> +	if (!keySymExists) {
> +		defs->keysyms = g_renew(guint, defs->keysyms, defs->curkeys + 1);
> +		defs->keysyms[defs->curkeys] = ev->key.keyval;
> +		defs->curkeys++;
> +	}
> +
> +	dialog_update_keysyms(defs->label, defs->keysyms, defs->curkeys);
> +
> +	if (!ev->key.is_modifier) {
> +		defs->set = TRUE;
> +		defs->numkeys = defs->curkeys;
> +		defs->curkeys--;
> +	}
> +
> +	return FALSE;
> +}
> +
>    

Just a note about this one since this is my original code: This expects 
user to press modifiers and end it with some non-modifier key. Like Ctrl 
+ Alt + G for example. It doesn't accept Ctrl + Alt only since both the 
keys are modifiers. Nevertheless, since we have a good version of the 
patch now we can play with this when having time for that so we can 
watch for the curkeys counter count and if the value decremented this 
means one key has been released and therefore the sequence with this key 
could be the one to be set, e.g. press and release Ctrl + Alt and this 
will be registered/triggered:

1. press Ctrl (curkeys = 0 + 1 => 1)
2. press Alt  (curkeys = 1 + 1 => 2)
3. release Ctrl (curkeys = 2 -1 => 1)
4. curkeys number decremented, trigger
   4.1. save the remaining key in the sequence (Alt)
   4.2. save the key that's been released now (Ctrl)
5. save the sequence to some list and use it for vnc_set_grab_keys()

What do you think about this Daniel?

> +static gboolean dialog_key_release(GtkWidget *window G_GNUC_UNUSED,
> +        GdkEvent *ev, VncGrabDefs *defs)
> +{
> +	int i;
> +
> +	if (dialog_key_ignore(ev->key.keyval))
> +		return FALSE;
> +
> +	if (defs->set) {
> +		if (defs->curkeys == 0)
> +			defs->set = FALSE;
> +		if (defs->curkeys)
> +			defs->curkeys--;
> +		return FALSE;
> +	}
> +
> +	for (i = 0; i<  defs->curkeys; i++)
> +	{
> +		if (defs->keysyms[i] == ev->key.keyval)
> +		{
> +			defs->keysyms[i] = defs->keysyms[defs->curkeys - 1];
> +			defs->curkeys--;
> +			defs->keysyms = g_renew(guint, defs->keysyms, defs->curkeys);
> +		}
> +	}
> +
> +	dialog_update_keysyms(defs->label, defs->keysyms, defs->curkeys);
> +
> +	return FALSE;
> +}
> +
> +static void do_set_grab_keys(GtkWidget *menu G_GNUC_UNUSED, GtkWidget *window)
> +{
> +	VncGrabDefs *defs;
> +	VncGrabSequence *seq;
> +	GtkWidget *dialog, *content_area, *label;
> +	gint result;
> +
> +	dialog = gtk_dialog_new_with_buttons ("Key recorder",
> +						GTK_WINDOW(window),
> +						GTK_DIALOG_MODAL | GTK_DIALOG_DESTROY_WITH_PARENT,
> +						GTK_STOCK_OK,
> +						GTK_RESPONSE_ACCEPT,
> +						GTK_STOCK_CANCEL,
> +						GTK_RESPONSE_REJECT,
> +						NULL);
> +
> +	label = gtk_label_new("Please press desired grab key combination");
> +	defs = g_new(VncGrabDefs, 1);
> +	defs->label = label;
> +	defs->keysyms = 0;
> +	defs->numkeys = 0;
> +	defs->curkeys = 0;
> +	defs->set = FALSE;
> +	g_signal_connect(dialog, "key-press-event",
> +			G_CALLBACK(dialog_key_press), defs);
> +	g_signal_connect(dialog, "key-release-event",
> +			G_CALLBACK(dialog_key_release), defs);
> +	gtk_widget_set_size_request(dialog, 300, 100);
> +	content_area = gtk_dialog_get_content_area( GTK_DIALOG(dialog) );
> +	gtk_container_add( GTK_CONTAINER(content_area), label);
> +	gtk_widget_show_all(dialog);
> +
> +	result = gtk_dialog_run(GTK_DIALOG(dialog));
> +	if (result == GTK_RESPONSE_ACCEPT) {
> +		/* Accepted so we make a grab sequence from it */
> +		seq = vnc_grab_sequence_new(defs->numkeys,
> +					    defs->keysyms);
> +
>    

Well, most of this code is the same like I wrote except few 
modifications obviously - mainly the sequence definition function 
renamed to vnc_grab_sequence_new() etc. Looking good.

> +		vnc_display_set_grab_keys(VNC_DISPLAY(vnc), seq);
> +		set_title(VNC_DISPLAY(vnc), window, FALSE);
> +		vnc_grab_sequence_free(seq);
> +	}
> +	g_free(defs);
> +	gtk_widget_destroy(dialog);
> +}
> +
>   static void vnc_credential(GtkWidget *vncdisplay, GValueArray *credList)
>   {
>   	GtkWidget *dialog = NULL;
> @@ -353,7 +502,7 @@ int main(int argc, char **argv)
>   	GtkWidget *window;
>   	GtkWidget *layout;
>   	GtkWidget *menubar;
> -	GtkWidget *sendkey, *view;
> +	GtkWidget *sendkey, *view, *settings;
>   	GtkWidget *submenu;
>   	GtkWidget *caf1;
>   	GtkWidget *caf2;
> @@ -367,6 +516,7 @@ int main(int argc, char **argv)
>   	GtkWidget *cab;
>   	GtkWidget *fullscreen;
>   	GtkWidget *scaling;
> +	GtkWidget *showgrabkeydlg;
>   	const char *help_msg = "Run 'gvncviewer --help' to see a full list of available command line options";
>
>   	/* Setup command line options */
> @@ -441,6 +591,16 @@ int main(int argc, char **argv)
>
>   	gtk_menu_item_set_submenu(GTK_MENU_ITEM(view), submenu);
>
> +	settings = gtk_menu_item_new_with_mnemonic("_Settings");
> +	gtk_menu_shell_append(GTK_MENU_SHELL(menubar), settings);
> +
> +	submenu = gtk_menu_new();
> +
> +	showgrabkeydlg = gtk_menu_item_new_with_mnemonic("_Set grab keys");
> +	gtk_menu_shell_append(GTK_MENU_SHELL(submenu), showgrabkeydlg);
> +
> +	gtk_menu_item_set_submenu(GTK_MENU_ITEM(settings), submenu);
> +
>   #if WITH_LIBVIEW
>   	ViewAutoDrawer_SetActive(VIEW_AUTODRAWER(layout), FALSE);
>   	ViewOvBox_SetOver(VIEW_OV_BOX(layout), menubar);
> @@ -516,6 +676,8 @@ int main(int argc, char **argv)
>   			 G_CALLBACK(send_cad), vnc);
>   	g_signal_connect(cab, "activate",
>   			 G_CALLBACK(send_cab), vnc);
> +	g_signal_connect(showgrabkeydlg, "activate",
> +			 G_CALLBACK(do_set_grab_keys), window);
>   	g_signal_connect(fullscreen, "toggled",
>   			 G_CALLBACK(do_fullscreen), window);
>   	g_signal_connect(scaling, "toggled",
> diff --git a/examples/gvncviewer.py b/examples/gvncviewer.py
> index 9a74268..c5769f3 100644
> --- a/examples/gvncviewer.py
> +++ b/examples/gvncviewer.py
> @@ -28,8 +28,15 @@ if len(sys.argv) != 2 and len(sys.argv) != 3:
>
>   def set_title(vnc, window, grabbed):
>       name = vnc.get_name()
> +    keys = vnc.get_grab_keys()
>    

Oh, ok, so vnc.get_grab_keys() is returning a list of all the key codes 
defined. Good idea. Programmer have to consider using the 
gtk.gdk.keyval_name(value) like used below and in fact I was not certain 
about the definition of gtk.gdk.keyval_name() in python but looking 
good. Originally my idea was to put:

seq = [ gtk.gdk.Alt_L, gtk.gdk.Control_L ]

or something similar to the python binding but obviously it was not 
working since gtk.gdk.Alt_L is not being defined and I was not 
investigating this further and I have rather chosen the implementation 
like I did. But to provide a list of keycodes is a good idea too.

> +    keystr = None
> +    for k in keys:
> +        if keystr is None:
> +            keystr = gtk.gdk.keyval_name(k)
> +        else:
> +            keystr = keystr + "+" + gtk.gdk.keyval_name(k)
>       if grabbed:
> -        subtitle = "(Press Ctrl+Alt to release pointer) "
> +        subtitle = "(Press %s to release pointer) " % keystr
>       else:
>           subtitle = ""
>
> @@ -173,6 +180,11 @@ layout.add(vnc)
>   vnc.realize()
>   vnc.set_pointer_grab(True)
>   vnc.set_keyboard_grab(True)
> +
> +# Example to change grab key combination to Ctrl+Alt+g
> +grab_keys = [ gtk.keysyms.Control_L, gtk.keysyms.Alt_L, gtk.keysyms.g ]
>    

Oh. Thanks. This is what I wrote above that I've been looking for. It's 
not being defined as "gtk.gdk.Alt_L" but like "gtk.keysyms.Alt_L" 
instead. Thanks for this code. It may be useful to know that some time.

> +vnc.set_grab_keys(grab_keys)
> +
>   #v.set_pointer_local(True)
>
>   if len(sys.argv) == 3:
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 393d76a..745ca99 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -85,6 +85,7 @@ gtk_vnc_includedir = $(includedir)/gtk-vnc-1.0/
>   gtk_vnc_include_HEADERS = \
>   			vncdisplay.h \
>   			vncdisplayenums.h \
> +			vncgrabsequence.h \
>   			vncimageframebuffer.h
>
>   libgtk_vnc_1_0_la_SOURCES = \
> @@ -92,6 +93,7 @@ libgtk_vnc_1_0_la_SOURCES = \
>   			vncdisplay.h vncdisplay.c \
>   			vncdisplayenums.h vncdisplayenums.c \
>   			vncdisplaykeymap.h vncdisplaykeymap.c \
> +			vncgrabsequence.h vncgrabsequence.c \
>   			vncmarshal.h vncmarshal.c
>
>   vncdisplayenums.c: vncdisplay.h
> @@ -204,6 +206,7 @@ GVNC_INTROSPECTION_SRCS = \
>   GTK_VNC_INTROSPECTION_SRCS = \
>   			$(srcdir)/vncimageframebuffer.h $(srcdir)/vncimageframebuffer.c \
>   			$(srcdir)/vncdisplay.h $(srcdir)/vncdisplay.c \
> +			$(srcdir)/vncgrabsequence.h $(srcdir)/vncgrabsequence.c \
>   			$(builddir)/vncdisplayenums.h $(builddir)/vncdisplayenums.c
>
>   GVnc-1.0.gir: libgvnc-1.0.la $(G_IR_SCANNER) Makefile.am
> diff --git a/src/libgtk-vnc_sym.version b/src/libgtk-vnc_sym.version
> index b6f5347..dc219d4 100644
> --- a/src/libgtk-vnc_sym.version
> +++ b/src/libgtk-vnc_sym.version
> @@ -68,6 +68,16 @@
>       vnc_image_framebuffer_new;
>       vnc_image_framebuffer_get_image;
>
> +# grab key settings support
> +    vnc_display_set_grab_keys;
> +    vnc_display_get_grab_keys;
> +    vnc_grab_sequence_new;
> +    vnc_grab_sequence_new_from_string;
> +    vnc_grab_sequence_copy;
> +    vnc_grab_sequence_free;
> +    vnc_grab_sequence_as_string;
> +    vnc_grab_sequence_get_type;
> +
>     local:
>         *;
>   };
> diff --git a/src/vnc.override b/src/vnc.override
> index dfc346e..2098358 100644
> --- a/src/vnc.override
> +++ b/src/vnc.override
> @@ -52,3 +52,60 @@ _wrap_vnc_display_send_keys(PyGObject *self,
>       Py_INCREF(Py_None);
>       return Py_None;
>   }
> +%%
> +override vnc_display_get_grab_keys kwargs
> +static PyObject*
> +_wrap_vnc_display_get_grab_keys(PyGObject *self,
> +                            PyObject *args, PyObject *kwargs)
> +{
> +    VncGrabSequence *seq;
> +    PyObject *keyList;
> +    int i;
> +
> +    seq = vnc_display_get_grab_keys(VNC_DISPLAY(self->obj));
> +
> +    keyList = PyList_New(0);
> +    for (i = 0 ; i<  seq->nkeysyms ; i++)
> +       PyList_Append(keyList, PyInt_FromLong(seq->keysyms[i]));
>    

Ok, appending the keycodes (instead of the keyval names) to the list is 
a good idea like I wrote above.

> +
> +    return keyList;
> +}
> +%%
> +override vnc_display_set_grab_keys kwargs
> +static PyObject*
> +_wrap_vnc_display_set_grab_keys(PyGObject *self,
> +                            PyObject *args, PyObject *kwargs)
> +{
> +    static char *kwlist[] = {"keys", NULL};
> +    PyObject *keyList;
> +    int i;
> +    guint nkeysyms;
> +    guint *keysyms;
> +    VncGrabSequence *seq;
> +
> +    if (!PyArg_ParseTupleAndKeywords(args, kwargs,
> +                                     "O|I:VncDisplay.set_grab_keys", kwlist,
> +&keyList))
> +        return NULL;
> +
> +    if (!PyList_Check(keyList))
> +        return NULL;
> +
> +    nkeysyms = PyList_Size(keyList);
> +    keysyms = g_new0(guint, nkeysyms);
> +
> +    for (i = 0 ; i<  nkeysyms ; i++) {
> +        PyObject *val = PyList_GetItem(keyList, i);
> +        keysyms[i] = (guint)PyInt_AsLong(val);
> +    }
> +
> +    seq = vnc_grab_sequence_new(nkeysyms, keysyms);
> +    g_free(keysyms);
> +
> +    vnc_display_set_grab_keys(VNC_DISPLAY(self->obj), seq);
> +
> +    vnc_grab_sequence_free(seq);
> +
> +    Py_INCREF(Py_None);
> +    return Py_None;
> +}
> diff --git a/src/vncdisplay.c b/src/vncdisplay.c
> index 71e158b..8eeb243 100644
> --- a/src/vncdisplay.c
> +++ b/src/vncdisplay.c
> @@ -85,6 +85,9 @@ struct _VncDisplayPrivate
>
>   	GSList *preferable_auths;
>   	const guint8 const *keycode_map;
> +
> +	VncGrabSequence *vncgrabseq; /* the configured key sequence */
> +	gboolean *vncactiveseq; /* the currently pressed keys */
>    

Ok, you did rename it to vncactiveseq. That's good. But what is this 
gboolean? Shouldn't this be the currently active sequence defined by 
keys pressed and numkeys/curkeys to identify number of keys being 
currently pressed?

>   };
>
>   G_DEFINE_TYPE(VncDisplay, vnc_display, GTK_TYPE_DRAWING_AREA)
> @@ -104,7 +107,8 @@ enum
>     PROP_SCALING,
>     PROP_SHARED_FLAG,
>     PROP_FORCE_SIZE,
> -  PROP_DEPTH
> +  PROP_DEPTH,
> +  PROP_GRAB_KEYS,
>   };
>
>   /* Signals */
> @@ -198,6 +202,9 @@ vnc_display_get_property (GObject    *object,
>         case PROP_DEPTH:
>           g_value_set_enum (value, vnc->priv->depth);
>   	break;
> +      case PROP_GRAB_KEYS:
> +	g_value_set_boxed(value, vnc->priv->vncgrabseq);
> +	break;
>    

Honestly I don't know what does this one do. Could you please explain?

>         default:
>   	G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
>   	break;
> @@ -241,6 +248,9 @@ vnc_display_set_property (GObject      *object,
>         case PROP_DEPTH:
>           vnc_display_set_depth (vnc, g_value_get_enum (value));
>           break;
> +      case PROP_GRAB_KEYS:
> +	vnc_display_set_grab_keys(vnc, g_value_get_boxed(value));
> +	break;
>    

Same as above.

>         default:
>           G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
>           break;
> @@ -673,6 +683,37 @@ static gboolean motion_event(GtkWidget *widget, GdkEventMotion *motion)
>   	return TRUE;
>   }
>
> +
> +static gboolean check_for_grab_key(GtkWidget *widget, int type, int keyval)
> +{
> +	VncDisplayPrivate *priv = VNC_DISPLAY(widget)->priv;
> +	int i;
> +
> +	if (!priv->vncgrabseq->nkeysyms)
> +		return FALSE;
> +
> +	if (type == GDK_KEY_RELEASE) {
> +		/* Any key release resets the whole grab sequence */
> +		memset(priv->vncactiveseq, 0,
> +		       sizeof(gboolean)*priv->vncgrabseq->nkeysyms);
>    

Resets the whole grab sequence? Shouldn't it remove only the keysym 
that's currently being pressed? But this is OK. This should work as well.

> +
> +		return FALSE;
> +	} else {
> +		/* Record the new key press */
> +		for (i = 0 ; i<  priv->vncgrabseq->nkeysyms ; i++)
> +			if (priv->vncgrabseq->keysyms[i] == keyval)
> +				priv->vncactiveseq[i] = TRUE;
> +
> +		/* Return if any key is not pressed */
> +		for (i = 0 ; i<  priv->vncgrabseq->nkeysyms ; i++)
> +			if (priv->vncactiveseq[i] == FALSE)
> +				return FALSE;
>    

Now I see what you mean by vncactiveseq gboolean. It defines whether the 
sequence recording is active. Looking good then.

> +
> +		return TRUE;
> +	}
> +}
> +
> +
>   static gboolean key_event(GtkWidget *widget, GdkEventKey *key)
>   {
>   	VncDisplayPrivate *priv = VNC_DISPLAY(widget)->priv;
> @@ -752,9 +793,7 @@ static gboolean key_event(GtkWidget *widget, GdkEventKey *key)
>   		}
>   	}
>
> -	if (key->type == GDK_KEY_PRESS&&
> -	    ((keyval == GDK_Control_L&&  (key->state&  GDK_MOD1_MASK)) ||
> -	     (keyval == GDK_Alt_L&&  (key->state&  GDK_CONTROL_MASK)))) {
> +	if (check_for_grab_key(widget, key->type, key->keyval)) {
>   		if (priv->in_pointer_grab)
>   			do_pointer_ungrab(VNC_DISPLAY(widget), FALSE);
>   		else if (!priv->grab_keyboard || !priv->absolute)
> @@ -1472,6 +1511,10 @@ static void vnc_display_finalize (GObject *obj)
>   		g_object_unref (priv->gc);
>   		priv->gc = NULL;
>   	}
> +	if (priv->vncgrabseq) {
> +		vnc_grab_sequence_free(priv->vncgrabseq);
> +		priv->vncgrabseq = NULL;
> +	}
>
>   	g_slist_free (priv->preferable_auths);
>
> @@ -1640,6 +1683,17 @@ static void vnc_display_class_init(VncDisplayClass *klass)
>   								G_PARAM_STATIC_NAME |
>   								G_PARAM_STATIC_NICK |
>   								G_PARAM_STATIC_BLURB));
> +	g_object_class_install_property (object_class,
> +					 PROP_GRAB_KEYS,
> +					 g_param_spec_boxed( "grab-keys",
> +							     "Grab keys",
> +							     "The key grab sequence",
> +							     VNC_TYPE_GRAB_SEQUENCE,
> +							     G_PARAM_READWRITE |
> +							     G_PARAM_CONSTRUCT |
> +							     G_PARAM_STATIC_NAME |
> +							     G_PARAM_STATIC_NICK |
> +							     G_PARAM_STATIC_BLURB));
>    

Ok, good. Just a question? How and where is the property installed this 
way used?

Thanks,
Michal

--
Michal Novotny<minovotn redhat com>, RHCE
Virtualization Team (xen userspace), Red Hat