GNOME.org
 

Patch for Ximian bug #6150 - AltGr does not work if the mouse is not over the control

[Blah, re-sending because of a typo in the address]

[CCing gnome-components-list because this needs their approval as well;
explanation follows]

Ximian bug #6150 exhibits this behavior:

- Switch to a (say) Spanish keymap or anything else that uses the AltGr
key (mode switch).

- Run bonobo/tests/test-focus.

- Click on the topmost entry to give it the focus, and leave the mouse
inside that entry.  Type AltGr-2 so that you get an "@" sign.

- Now move the mouse so that it is over the second entry.  Type AltGr-2
again.  Instead of getting an "@" sign on the topmost entry, you'll get
a number "2".

In Evolution, if you open the message composer and start typing in the
Subject line, AltGr won't work if you move the mouse to be over the To
line, for example.

The bug happens in bonobo_plug_forward_key_press().  It takes the keysym
from event->keyval and converts it to a keycode for the synthesized X
event using XKeysymToKeycode().  What happens in the case above,
however, is that we are converting the keysym for an "@" sign into the
same keycode of the "2" key.  So the socket side thinks that the "2" key
was pressed plainly, and you get a "2" instead of an "@" sign.

There is not a one-to-one mapping of keysyms to keycodes, and we do need
the original keycode/state values from the XKeyEvent to forward them to
the socket window.  However, by the time the BonoboPlug keyboard
handlers get run, we have lost that information --- keycode/state values
get converted in gdk_event_translate(), which happens outside the key
event propagation cycle.

The patch does the following.  To preserve the original keycode/state
information, we install an event filter on the plug window itself and
all of its inferiors.  When we get a KeyPress/KeyRelease event in the
filter, we store its data in a table that maps timestamp -> (keycode,
state).  Later, when BonoboPlug is required to synthesize an event for
the socket window, it looks up the original information in the table
using the timestamp of the event.  So, by synthesizing the event using
the original data, everything works fine and you get "@" signs when you
should.

In addition, the event filter adds SubstructureNotifyMask to the event
mask of all the plug inferiors so that we can monitor CreateNotify
events --- if a new window is created somewhere inside the plug, we also
need to install the event filter for it.

We have to walk the window hierarchy to install the event filter for all
subwindows because GTK+ 1.2.x does not have a way to install a global
event filter that will get run for all windows.  This is fixed in GTK+
1.3.x, but is irrelevant there because the plug/socket mechanism has
changed completely and event forwarding as BonoboPlug's does not apply
there.

Please review this patch and tell me if it is OK to commit.  This is a
rather critical bug for Evolution users with non-English keyboards.

To give credit where it is due, Owen Taylor suggested taking this
approach.  Owen is a wise man indeed.

  Federico

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/bonobo/ChangeLog,v
retrieving revision 1.1139
diff -u -r1.1139 ChangeLog
--- ChangeLog	2001/10/31 11:19:28	1.1139
+++ ChangeLog	2001/11/06 20:19:17
@@ -1,3 +1,24 @@
+2001-11-06  Federico Mena Quintero  <federico ximian com>
+
+	Fix Ximian bug #6150.
+
+	* bonobo/bonobo-plug.c (bonobo_plug_key_release_event): Handle key
+	release events as well; the important part here is the forwarding
+	bit.
+	(event_filter_cb): New function; gets set up both as default event
+	filter and as a filter for all windows that are inferiors of the
+	plug window.  Handles key presses/releases by keeping a table with
+	the original (keyval, state) pairs from X events before they are
+	mangled by gdk_event_translate(); we need the original values so
+	that we can synthesize events for the socket window properly.
+	Also handles CreateNotify events by installing the event filter on
+	the new window.
+	(bonobo_plug_forward_key_event): Renamed from
+	bonobo_plug_forward_key_press().  Tries to get the original event
+	information from the key event table.
+	(bonobo_plug_map): Recursively add event filters for the children
+	of the plug window.
+
 2001-11-01  Michael Meeks  <michael ximian com>
 
 	* Version 1.0.14
Index: bonobo/bonobo-plug.c
===================================================================
RCS file: /cvs/gnome/bonobo/bonobo/bonobo-plug.c,v
retrieving revision 1.12
diff -u -r1.12 bonobo-plug.c
--- bonobo/bonobo-plug.c	2001/05/07 19:20:17	1.12
+++ bonobo/bonobo-plug.c	2001/11/06 20:19:23
@@ -52,8 +52,39 @@
 /* From Tk */
 #define EMBEDDED_APP_WANTS_FOCUS NotifyNormal+20
 
+/* Sometimes we have to forward keyboard events to our parent socket because
+ * they get received on the plug side when the plug does not have the focus, or
+ * because no widget within the plug handled the event and we must propagate it
+ * upwards.  However, since we are using XSendEvent(), we have to use the
+ * original keycode and state values for the synthetic event.  GDK translates
+ * these and we cannot recover them from a GTK+ widget keyboard handler.  So we
+ * install an event filter for all keyboard events, ignore those that are not
+ * for plug windows, and store the remaining ones in this table, which is a
+ * circular buffer.  The table is keyed by the timestamps of the events.  When
+ * we have to forward a key event, we fetch the original values from this table.
+ */
+#define KEY_EVENT_TABLE_SIZE 20
+
+struct KeyEvent {
+	guint32 time;
+	guint keycode;
+	guint state;
+};
+
+struct KeyEvent key_event_table[KEY_EVENT_TABLE_SIZE];
+
+/* The table is a circular buffer.  Events get inserted at the tail, get removed
+ * from the head.
+ */
+static int key_event_table_head;
+static int key_event_table_tail;
+static int key_event_table_nelements;
+
+
 static GtkWindowClass *parent_class = NULL;
 
+static void install_filter (GdkWindow *window);
+
 
 
 /**
@@ -145,7 +176,138 @@
 	}
 }
 
+/* Checks whether a window is a descendant of a plug window. */
+static gboolean
+is_descendant_window_of_plug (GdkWindow *window)
+{
+	while (window) {
+		GtkWidget *widget;
+
+		widget = window->user_data;
+		if (widget && BONOBO_IS_PLUG (widget))
+			return TRUE;
+
+		window = gdk_window_get_parent (window);
+	}
+
+	return FALSE;
+}
+
+/* Filter function for all events.  See the comments for key_event_table above.
+ * We filter key events and log them in the key_event_table, and we take care
+ * of child window creations.
+ */
+static GdkFilterReturn
+event_filter_cb (GdkXEvent *xevent, GdkEvent *event, gpointer data)
+{
+	XEvent *xev;
+	struct KeyEvent *ke;
+	GdkWindow *w;
+
+	xev = (XEvent *) xevent;
+
+	/* Ignore non-keyboard events */
+	if (!(xev->type == KeyPress || xev->type == KeyRelease || xev->type == CreateNotify))
+		return GDK_FILTER_CONTINUE;
+
+	if (!is_descendant_window_of_plug (gdk_window_lookup (xev->xany.window)))
+		return GDK_FILTER_CONTINUE;
+
+	switch (xev->type) {
+	case KeyPress:
+	case KeyRelease:
+		ke = key_event_table + key_event_table_tail;
+
+		ke->time = xev->xkey.time;
+		ke->keycode = xev->xkey.keycode;
+		ke->state = xev->xkey.state;
+
+		/* We just overwrite the oldest entries if the table becomes full */
+
+		if (key_event_table_tail == key_event_table_head) {
+			if (key_event_table_nelements == 0)
+				key_event_table_nelements = 1;
+			else {
+				g_assert (key_event_table_nelements == KEY_EVENT_TABLE_SIZE);
+
+				key_event_table_head++;
+				if (key_event_table_head == KEY_EVENT_TABLE_SIZE)
+					key_event_table_head = 0;
+			}
+
+			key_event_table_tail++;
+			if (key_event_table_tail == KEY_EVENT_TABLE_SIZE)
+				key_event_table_tail = 0;
+		} else {
+			key_event_table_tail++;
+			if (key_event_table_tail == KEY_EVENT_TABLE_SIZE)
+				key_event_table_tail = 0;
+
+			key_event_table_nelements++;
+			g_assert (key_event_table_nelements <= KEY_EVENT_TABLE_SIZE);
+		}
+
+		return GDK_FILTER_CONTINUE;
+
+	case CreateNotify:
+		w = gdk_window_lookup (xev->xcreatewindow.window);
+		if (w)
+			install_filter (w);
+
+		return GDK_FILTER_CONTINUE;
+
+	default:
+		g_assert_not_reached ();
+		return GDK_FILTER_CONTINUE;
+	}
+}
+
+/* Recursively sets up the key event filter for the specified window and all its
+ * children.
+ */
+static void
+install_filter (GdkWindow *window)
+{
+	GList *children;
+	GList *l;
+	Window xwindow;
+	XWindowAttributes attr;
+
+	gdk_window_add_filter (window, event_filter_cb, NULL);
+
+	/* Add SubstructureNotifyMask so that we can monitor for child window creations */
+
+	xwindow = GDK_WINDOW_XWINDOW (window);
+	if (XGetWindowAttributes (GDK_DISPLAY (), xwindow, &attr) == Success)
+		XSelectInput (GDK_DISPLAY (), xwindow, attr.your_event_mask | SubstructureNotifyMask);
+
+	/* Add filters for all the children */
+
+	children = gdk_window_get_children (window);
+
+	for (l = children; l; l = l->next) {
+		GdkWindow *w;
+
+		w = l->data;
+		install_filter (w);
+	}
+
+	g_list_free (children);
+}
+
+/* map handler for the plug widget.  We install the key event filter for all of
+ * our child windows here, ugh.
+ */
 static void
+bonobo_plug_map (GtkWidget *widget)
+{
+	if (GTK_WIDGET_CLASS (parent_class)->map)
+		(* GTK_WIDGET_CLASS (parent_class)->map) (widget);
+
+	install_filter (widget->window);
+}
+
+static void
 bonobo_plug_realize (GtkWidget *widget)
 {
 	BonoboPlug *plug;
@@ -209,29 +371,74 @@
 	gtk_style_set_background (widget->style, widget->window, GTK_STATE_NORMAL);
 }
 
+/* Looks up a KeyEvent structure in the key event table based on the timestamp
+ * of the specified event.  If it is not found, returns NULL.
+ */
+static struct KeyEvent *
+lookup_key_event (GdkEventKey *event)
+{
+	int i;
+
+	if (key_event_table_nelements == 0)
+		return NULL;
+
+	i = key_event_table_head;
+
+	while (key_event_table_nelements > 0) {
+		key_event_table_head++;
+		if (key_event_table_head == KEY_EVENT_TABLE_SIZE)
+			key_event_table_head = 0;
+
+		key_event_table_nelements--;
+
+		if (key_event_table[i].time == event->time)
+			return key_event_table + i;
+	}
+
+	return NULL;
+}
+
+/* Extracts a key event from the key event table and forwards it to our parent
+ * socket.
+ */
 static void
-bonobo_plug_forward_key_press (BonoboPlug *plug, GdkEventKey *event)
+bonobo_plug_forward_key_event (BonoboPlug *plug, GdkEventKey *event)
 {
 	BonoboPlugPrivate *priv;
 	XEvent xevent;
+	struct KeyEvent *ke;
 
 	priv = plug->priv;
 
-	xevent.xkey.type = KeyPress;
+	if (event->type == GDK_KEY_PRESS)
+		xevent.xkey.type = KeyPress;
+	else if (event->type == GDK_KEY_RELEASE)
+		xevent.xkey.type = KeyRelease;
+	else
+		g_assert_not_reached ();
+
 	xevent.xkey.display = GDK_WINDOW_XDISPLAY (GTK_WIDGET(plug)->window);
 	xevent.xkey.window = GDK_WINDOW_XWINDOW (priv->socket_window);
 	xevent.xkey.root = GDK_ROOT_WINDOW (); /* FIXME */
 	xevent.xkey.time = event->time;
-	/* FIXME, the following might cause big problems for non-GTK apps */
 	xevent.xkey.x = 0;
 	xevent.xkey.y = 0;
 	xevent.xkey.x_root = 0;
 	xevent.xkey.y_root = 0;
-	xevent.xkey.state = event->state;
-	xevent.xkey.keycode =  XKeysymToKeycode(GDK_DISPLAY(),
-						event->keyval);
 	xevent.xkey.same_screen = TRUE; /* FIXME ? */
 
+	ke = lookup_key_event (event);
+	if (ke) {
+		xevent.xkey.keycode = ke->keycode;
+		xevent.xkey.state = ke->state;
+	} else {
+		/* Do the best we can do at this point, even if we may lose
+		 * information from the original event.
+		 */
+		xevent.xkey.state = event->state;
+		xevent.xkey.keycode = XKeysymToKeycode(GDK_DISPLAY(), event->keyval);
+	}
+
 	gdk_error_trap_push ();
 	XSendEvent (gdk_display,
 		    GDK_WINDOW_XWINDOW (priv->socket_window),
@@ -240,7 +447,7 @@
 	gdk_error_trap_pop ();
 }
 
-/* Key_press_event handle for the plug widget */
+/* Key_press_event handler for the plug widget */
 static gint
 bonobo_plug_key_press_event (GtkWidget *widget, GdkEventKey *event)
 {
@@ -262,7 +469,7 @@
 	window = GTK_WINDOW (widget);
 
 	if (!GTK_WIDGET_HAS_FOCUS (widget)) {
-		bonobo_plug_forward_key_press (plug, event);
+		bonobo_plug_forward_key_event (plug, event);
 		return FALSE;
 	}
 
@@ -343,7 +550,7 @@
 				gdk_flush ();
 				gdk_error_trap_pop ();
 
-				bonobo_plug_forward_key_press (plug, event);
+				bonobo_plug_forward_key_event (plug, event);
 			}
 
 			return_val = TRUE;
@@ -355,11 +562,48 @@
 	 * keybinding or something interesting.
 	 */
 	if (!return_val)
-		bonobo_plug_forward_key_press (plug, event);
+		bonobo_plug_forward_key_event (plug, event);
 
 	return return_val;
 }
 
+/* Key_release_event handler for the plug widget */
+static gint
+bonobo_plug_key_release_event (GtkWidget *widget, GdkEventKey *event)
+{
+	BonoboPlug *plug;
+	GtkWindow *window;
+	gint return_val;
+
+	g_return_val_if_fail (widget != NULL, FALSE);
+	g_return_val_if_fail (BONOBO_IS_PLUG (widget), FALSE);
+	g_return_val_if_fail (event != NULL, FALSE);
+
+	plug = BONOBO_PLUG (widget);
+
+	if (!GTK_WIDGET_HAS_FOCUS (widget)) {
+		bonobo_plug_forward_key_event (plug, event);
+		return FALSE;
+	}
+
+	window = GTK_WINDOW (widget);
+
+	return_val = FALSE;
+	if (window->focus_widget
+	    && window->focus_widget != widget
+	    && GTK_WIDGET_IS_SENSITIVE (window->focus_widget))
+		return_val = gtk_widget_event (window->focus_widget, (GdkEvent*) event);
+
+	/*
+	 * If we havn't handled it pass it on to our socket, since it might be a
+	 * keybinding or something interesting.
+	 */
+	if (!return_val)
+		bonobo_plug_forward_key_event (plug, event);
+
+	return return_val;
+}
+
 /* Focus_in_event handler for the plug widget */
 static gint
 bonobo_plug_focus_in_event (GtkWidget *widget, GdkEventFocus *event)
@@ -484,7 +728,7 @@
 	priv = plug->priv;
 
 	(* GTK_WINDOW_CLASS (parent_class)->set_focus) (window, focus);
-  
+
 	if (focus && GTK_WIDGET_CAN_FOCUS (focus) && !GTK_WIDGET_HAS_FOCUS (window)) {
 		XEvent xevent;
 
@@ -521,7 +765,19 @@
 	priv->has_focus = FALSE;
 }
 
+/* Sets up the key event filter and key event table; see the comments for
+ * key_event_table above.
+ */
 static void
+setup_event_filter (void)
+{
+	key_event_table_head = key_event_table_tail = key_event_table_nelements = 0;
+
+	/* This is a default event filter, so we pass NULL for the window field */
+	gdk_window_add_filter (NULL, event_filter_cb, NULL);
+}
+
+static void
 bonobo_plug_class_init (BonoboPlugClass *class)
 {
 	GtkObjectClass *object_class;
@@ -538,15 +794,19 @@
 
 	object_class->destroy = bonobo_plug_destroy;
 
+	widget_class->map = bonobo_plug_map;
 	widget_class->realize = bonobo_plug_realize;
 	widget_class->unrealize = bonobo_plug_unrealize;
 	widget_class->key_press_event = bonobo_plug_key_press_event;
+	widget_class->key_release_event = bonobo_plug_key_release_event;
 	widget_class->focus_in_event = bonobo_plug_focus_in_event;
 	widget_class->focus_out_event = bonobo_plug_focus_out_event;
 
 	container_class->focus = bonobo_plug_focus;
 
 	window_class->set_focus = bonobo_plug_set_focus;
+
+	setup_event_filter ();
 }
 
 guint
@@ -577,7 +837,7 @@
  * bonobo_plug_set_control:
  * @plug: A plug.
  * @control: Control that wraps the plug widget.
- * 
+ *
  * Sets the #BonoboControl that the plug will use to proxy requests to the
  * parent container.
  **/
@@ -601,7 +861,7 @@
 /**
  * bonobo_plug_clear_focus_chain:
  * @plug: A plug.
- * 
+ *
  * Clears the focus children from the container hierarchy inside a plug.  This
  * should be used only by the #BonoboControl implementation.
  **/
Index: tests/test-focus.c
===================================================================
RCS file: /cvs/gnome/bonobo/tests/test-focus.c,v
retrieving revision 1.2
diff -u -r1.2 test-focus.c
--- tests/test-focus.c	2001/04/25 01:36:15	1.2
+++ tests/test-focus.c	2001/11/06 20:19:27
@@ -57,6 +57,9 @@
 	tmp = gtk_button_new_with_label ("In Container B");
 	gtk_box_pack_start_defaults (GTK_BOX (vbox), tmp);
 
+	tmp = bonobo_widget_new_control ("OAFIID:Bonobo_Sample_Entry", NULL);
+	gtk_box_pack_start_defaults (GTK_BOX (vbox), tmp);
+
 	gtk_widget_show_all (window);
 
 	gtk_main ();
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]