Revised key event support to remove some invalid casts, fix bugs relating

to key event filtration, and enable "global" key listeners based on
key sets.


git-svn-id: http://svn.gnome.org/svn/at-spi/trunk@187 e2bd861d-eb25-0410-b326-f6ed22b6b98c

18 files changed, 570 insertions, 311 deletions

diff --git a/ChangeLog b/ChangeLog
index 1239c791..2f3fd68e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,69 @@
+2001-12-15  Bill Haneman <bill.haneman@sun.com>
+
+	* idl/Accessibility_Registry.idl:
+	Removed Accessibility_KeyStroke structure in favor of generic
+	Accessibility_DeviceEvent structure.
+	Changed Accessibility_ControllerEventMask from a struct to an
+	unsigned long.
+
+	* at-bridge/bridge.c:
+	Changed APIs to use DeviceEvent structure as above, and removed
+	bogus casting between these event structures.
+
+	* cspi/spi-listener.h:
+	Added keystring member of AccessibleKeystroke structure, to enable
+	matching on event "names" rather than only hardware codes and
+	keysyms.
+
+	* cspi/spi.h:
+	Added keystrings member of AccessibleKeySet struct, to allow
+	matching on event names (as above).
+	Added declarations for SPI_createAccessibleKeySet and
+	SPI_freeAccessibleKeySet.  Due to changes in libspi, we now pass
+	event modmasks directly as unsigned ints rather than structs with
+	refcounts, in the DeviceEventController methods.
+
+	* cspi/spi_registry.c:
+	Add SPI_createAccessibleKeySet and SPI_freeAccessibleKeySet methods.
+
+	* cspi/spi-roletypes.h:
+	Added documentation of newly added SPI Roles.
+
+	* cspi/bonobo/cspi-bonobo-listener.c:
+	Changes in support of API changes above.
+	
+	* libspi/accessible.c:
+	Converted APIs to use DeviceEvent structure (see IDL changes
+	above).	
+
+	* registryd/deviceeventcontroller.c:
+	Added DEControllerGrabMask structure to track keygrabs not only by
+	modmask but by keyset as well; this allows us to do "global"
+	(i.e. X) keygrabs on a per-key or keyset basis rather than always
+	grabbing on AnyKey and then filtering after-the-fact.
+	Bugfixes for event filtration mean that we don't get false matches
+	on SPI_KEY_RELEASED, when only SPI_KEY_PRESSED was requested.
+
+	* registryd/deviceeventcontroller.c:
+	Namespaced a number of static methods to use spi_ prefix.  Major
+	revision to internals of global (i.e. X) key grabs.
+
+	* registryd/deviceeventcontroller.h:
+	Removed keymask_list and added keygrabs_list to
+	SpiDeviceEventController struct.
+
+	* test/simple-at.c:
+	Added use of SPI_createAccessibleKeySet API when creating a
+	listener for only one key.  Attach a listener to "shift spacebar
+	SPI_KEY_RELEASE" as a demonstration.  Changed (incorrect) usage of
+	X key event names to SPI key event names, so that listeners are
+	registered for (SPI_KEY_PRESSED | SPI_KEY_RELEASED), for instance.
+	
+	* test/keysynth-demo.c:
+        Changed (incorrect) use of X key event names (KeyPressed,
+	KeyReleased) to SPI enums SPI_KEY_PRESSED and SPI_KEY_RELEASED.
+	
+
 2001-12-12  Bill Haneman <bill.haneman@sun.com>
 
 	* libspi/accessible.c:
diff --git a/TODO b/TODO
index 99e631a0..9224bc07 100644
--- a/TODO
+++ b/TODO
@@ -2,20 +2,17 @@ TODO:
 
 idl: 
 	+ audit IDL for conformance with bonobo/doc/FAQ's [Java]
-	naming practice [Michael]
+	  naming practice [Michael]
 
 cspi: 
 	+ ensure spi-listener-impl's list notification methods don't
 	  have a re-enterancy hazard. [Michael]
 	+ Put LGPL headers everywhere.
-	+ Fix the conversion of Accessibility_Role CORBA enum to
-	  AccessibleRole enums. [Bill]
 
 registry:
 	+ move code into here from libspi [Michael]
 	+ fire an event on dead application & re-factor the listen for
 	  broken code.
-        + consider use of GHashTable instead of GList for listeners [Bill]
 
 bridge:
 	+ move code into here from libspi
@@ -24,17 +21,11 @@ libspi:
 	+ remove redundant casts throughout
 	+ determine if there are other headers we don't need to install
 	+ consider moving the non-impl. parts into registryd/ bridge/ etc.
-	+ merge the DeviceEvent and KeyEvent structs to avoid the
-	bogus casting of these back and forth.
 	+ complete implementation of support for RelationSet and
-	StateSet, and export to cspi.
-	+ complete and test the pre-emptive key API so that we don't
-	always do AnyKey grabs. [Bill]
-	+ complete the conversion from AtkRoles to Accessibility_Role
-	enums. [Bill]
+	StateSet, and export to cspi. [Marc]
+	+ complete conversion of static methods to "spi_" namespace. [Bill]
 
 test:
 	+ update test-simple to do complete API tests
 	+ Expand tests for AtkTable
-	+ remove comment from test_value; ensure that it works.
 
diff --git a/at-bridge/bridge.c b/at-bridge/bridge.c
index e32dfaf0..e58440cc 100644
--- a/at-bridge/bridge.c
+++ b/at-bridge/bridge.c
@@ -142,8 +142,14 @@ register_atk_event_listeners (void)
   atk_add_global_event_listener (bridge_signal_listener, "Gtk:AtkTable:column-reordered");
   atk_add_global_event_listener (bridge_signal_listener, "Gtk:AtkTable:column-deleted");
   atk_add_global_event_listener (bridge_signal_listener, "Gtk:AtkTable:model-changed");
+/*
+ * May add the following listeners to implement preemptive key listening for GTK+
+ *
+ * atk_add_global_event_listener (bridge_widgetkey_listener, "Gtk:GtkWidget:key-press-event");
+ * atk_add_global_event_listener (bridge_widgetkey_listener, "Gtk:GtkWidget:key-release-event");
+ */
   atk_add_key_event_listener    (bridge_key_listener, NULL);
-
+  
   g_object_unref (G_OBJECT (bo));
   g_object_unref (ao);
 }
@@ -325,8 +331,8 @@ bridge_state_event_listener (GSignalInvocationHint *signal_hint,
 }
 
 static void
-accessibility_init_keystroke_from_atk_key_event (Accessibility_KeyStroke *keystroke,
-						 AtkKeyEventStruct       *event)
+accessibility_init_keystroke_from_atk_key_event (Accessibility_DeviceEvent  *keystroke,
+						 AtkKeyEventStruct          *event)
 {
 #ifdef SPI_DEBUG
   if (event)
@@ -340,8 +346,8 @@ accessibility_init_keystroke_from_atk_key_event (Accessibility_KeyStroke *keystr
       g_print ("WARNING: NULL key event!");
     }
   
-  keystroke->keyID     = (CORBA_long) event->keyval;
-  keystroke->keycode   = (CORBA_short) event->keycode;
+  keystroke->id        = (CORBA_long) event->keyval;
+  keystroke->hw_code   = (CORBA_short) event->keycode;
   keystroke->timestamp = (CORBA_unsigned_long) event->timestamp;
   keystroke->modifiers = (CORBA_unsigned_short) (event->state & 0xFFFF);
 
@@ -362,8 +368,8 @@ accessibility_init_keystroke_from_atk_key_event (Accessibility_KeyStroke *keystr
 static gint
 bridge_key_listener (AtkKeyEventStruct *event, gpointer data)
 {
-  CORBA_boolean           result;
-  Accessibility_KeyStroke key_event;
+  CORBA_boolean             result;
+  Accessibility_DeviceEvent key_event;
   Accessibility_DeviceEventController controller =
     Accessibility_Registry_getDeviceEventController (registry, &ev);
 
@@ -377,9 +383,8 @@ bridge_key_listener (AtkKeyEventStruct *event, gpointer data)
 
       accessibility_init_keystroke_from_atk_key_event (&key_event, event);
 
-  /* FIXME: this casting is just totaly bogus */
       result = Accessibility_DeviceEventController_notifyListenersSync (
-        controller, (Accessibility_DeviceEvent *) &key_event, &ev);
+        controller, &key_event, &ev);
 
       CORBA_exception_free (&ev);
     }
@@ -417,3 +422,10 @@ bridge_signal_listener (GSignalInvocationHint *signal_hint,
 
   return TRUE;
 }
+
+
+
+
+
+
+
diff --git a/atk-bridge/bridge.c b/atk-bridge/bridge.c
index e32dfaf0..e58440cc 100644
--- a/atk-bridge/bridge.c
+++ b/atk-bridge/bridge.c
@@ -142,8 +142,14 @@ register_atk_event_listeners (void)
   atk_add_global_event_listener (bridge_signal_listener, "Gtk:AtkTable:column-reordered");
   atk_add_global_event_listener (bridge_signal_listener, "Gtk:AtkTable:column-deleted");
   atk_add_global_event_listener (bridge_signal_listener, "Gtk:AtkTable:model-changed");
+/*
+ * May add the following listeners to implement preemptive key listening for GTK+
+ *
+ * atk_add_global_event_listener (bridge_widgetkey_listener, "Gtk:GtkWidget:key-press-event");
+ * atk_add_global_event_listener (bridge_widgetkey_listener, "Gtk:GtkWidget:key-release-event");
+ */
   atk_add_key_event_listener    (bridge_key_listener, NULL);
-
+  
   g_object_unref (G_OBJECT (bo));
   g_object_unref (ao);
 }
@@ -325,8 +331,8 @@ bridge_state_event_listener (GSignalInvocationHint *signal_hint,
 }
 
 static void
-accessibility_init_keystroke_from_atk_key_event (Accessibility_KeyStroke *keystroke,
-						 AtkKeyEventStruct       *event)
+accessibility_init_keystroke_from_atk_key_event (Accessibility_DeviceEvent  *keystroke,
+						 AtkKeyEventStruct          *event)
 {
 #ifdef SPI_DEBUG
   if (event)
@@ -340,8 +346,8 @@ accessibility_init_keystroke_from_atk_key_event (Accessibility_KeyStroke *keystr
       g_print ("WARNING: NULL key event!");
     }
   
-  keystroke->keyID     = (CORBA_long) event->keyval;
-  keystroke->keycode   = (CORBA_short) event->keycode;
+  keystroke->id        = (CORBA_long) event->keyval;
+  keystroke->hw_code   = (CORBA_short) event->keycode;
   keystroke->timestamp = (CORBA_unsigned_long) event->timestamp;
   keystroke->modifiers = (CORBA_unsigned_short) (event->state & 0xFFFF);
 
@@ -362,8 +368,8 @@ accessibility_init_keystroke_from_atk_key_event (Accessibility_KeyStroke *keystr
 static gint
 bridge_key_listener (AtkKeyEventStruct *event, gpointer data)
 {
-  CORBA_boolean           result;
-  Accessibility_KeyStroke key_event;
+  CORBA_boolean             result;
+  Accessibility_DeviceEvent key_event;
   Accessibility_DeviceEventController controller =
     Accessibility_Registry_getDeviceEventController (registry, &ev);
 
@@ -377,9 +383,8 @@ bridge_key_listener (AtkKeyEventStruct *event, gpointer data)
 
       accessibility_init_keystroke_from_atk_key_event (&key_event, event);
 
-  /* FIXME: this casting is just totaly bogus */
       result = Accessibility_DeviceEventController_notifyListenersSync (
-        controller, (Accessibility_DeviceEvent *) &key_event, &ev);
+        controller, &key_event, &ev);
 
       CORBA_exception_free (&ev);
     }
@@ -417,3 +422,10 @@ bridge_signal_listener (GSignalInvocationHint *signal_hint,
 
   return TRUE;
 }
+
+
+
+
+
+
+
diff --git a/cspi/bonobo/cspi-bonobo-listener.c b/cspi/bonobo/cspi-bonobo-listener.c
index bc401002..fdd759dd 100644
--- a/cspi/bonobo/cspi-bonobo-listener.c
+++ b/cspi/bonobo/cspi-bonobo-listener.c
@@ -159,8 +159,8 @@ cspi_event_listener_remove_cb (AccessibleEventListener  *al,
  * Key event dispatcher
  */
 static gboolean
-cspi_key_event (SpiKeystrokeListener          *listener,
-		const Accessibility_KeyStroke *keystroke)
+cspi_key_event (SpiKeystrokeListener            *listener,
+		const Accessibility_DeviceEvent *keystroke)
 {
   GList *l;
   CSpiKeystrokeListener *clistener = (CSpiKeystrokeListener *) listener;
@@ -192,8 +192,8 @@ cspi_key_event (SpiKeystrokeListener          *listener,
 	akeystroke.type = 0;
 	break;
     }
-  akeystroke.keyID     = keystroke->keyID;
-  akeystroke.keycode   = keystroke->keycode;
+  akeystroke.keyID     = keystroke->id;
+  akeystroke.keycode   = keystroke->hw_code;
   akeystroke.timestamp = keystroke->timestamp;
   akeystroke.modifiers = keystroke->modifiers;
 
diff --git a/cspi/spi-listener.h b/cspi/spi-listener.h
index e37390e3..427473a2 100644
--- a/cspi/spi-listener.h
+++ b/cspi/spi-listener.h
@@ -26,6 +26,7 @@ typedef enum {
 typedef struct {
   long                   keyID;
   short                  keycode;
+  char *                 keystring;
   long                   timestamp;
   AccessibleKeyEventType type;
   unsigned short         modifiers;
diff --git a/cspi/spi-roletypes.h b/cspi/spi-roletypes.h
index c3fd1e76..1bd76644 100644
--- a/cspi/spi-roletypes.h
+++ b/cspi/spi-roletypes.h
@@ -8,16 +8,25 @@ extern "C" {
 /**
  * AccessibleRole:
  * @SPI_ROLE_INVALID: role is not legal, something is wrong with this object
+ * @SPI_ROLE_ACCELERATOR_LABEL: object is a label indicating keyboard
+ *                              accelerators for the parent
  * @SPI_ROLE_ALERT: Object is used to alert the user about something
+ * @SPI_ROLE_ANIMATION: Object contains a dynamic or moving image
+ * @SPI_ROLE_ARROW: Object is a 2d directional indicator
+ * @SPI_ROLE_CALENDAR: Object contains one or more dates, usually arranged into
+ *                     a 2d list
  * @SPI_ROLE_CANVAS: Object that can be drawn into and is used to trap events
  * @SPI_ROLE_CHECK_BOX: Object representd a choice that can be checked or unchecked
  *                    and provides a separate indicator for the current state.
+ * @SPI_ROLE_CHECK_MENU_ITEM: Object is a menu item that behaves like a CHECK_BOX
  * @SPI_ROLE_COLOR_CHOOSER: A specialized dialog that lets the user choose a color.
  * @SPI_ROLE_COLUMN_HEADER: The header for a column of data 
  * @SPI_ROLE_COMBO_BOX: A list of choices the user can select from
- * @SPI_ROLE_SPI_DESKTOP_ICON: An inconifed internal frame within a SPI_DESKTOP_PANE
- * @SPI_ROLE_SPI_DESKTOP_FRAME: A pane that supports internal frames and
+ * @SPI_ROLE_DATE_EDITOR: Allows entry of a date
+ * @SPI_ROLE_DESKTOP_ICON: An inconifed internal frame within a DESKTOP_PANE
+ * @SPI_ROLE_DESKTOP_FRAME: A pane that supports internal frames and
  *              iconified versions of those internal frames.
+ * @SPI_ROLE_DIAL: A rotating/rotatable valuator
  * @SPI_ROLE_DIALOG: A top level window with title bar and a border
  * @SPI_ROLE_DIRECTORY_PANE: A pane that allows the user to navigate
  *              through and select the contents of a directory.
@@ -26,12 +35,14 @@ extern "C" {
  *	 	 directory, or specify a filename.
  * @SPI_ROLE_FILLER: A object that fills up space in a user interface
  * @SPI_ROLE_FOCUS_TRAVERSABLE: XXX Not sure about this.
+ * @SPI_ROLE_FONT_CHOOSER: Allows selection of a display font
  * @SPI_ROLE_FRAME: A top level window with a title bar, border, menubar, etc.
  * @SPI_ROLE_GLASS_PANE: A pane that is guaranteed to be painted on top of all
  *               panes beneath it
  * @SPI_ROLE_HTML_CONTAINER: A document container for HTML, whose children
  *               represent the document content.
  * @SPI_ROLE_ICON: A small fixed size picture, typically used to decorate components.
+ * @SPI_ROLE_IMAGE: An image, typically static
  * @SPI_ROLE_INTERNAL_FRAME: A frame-like object that is clipped by a desktop pane.
  * @SPI_ROLE_LABEL: An object used to present an icon or short string in an interface
  * @SPI_ROLE_LAYERED_PANE: A specialized pane that allows its children to be drawn in layers
@@ -69,23 +80,32 @@ extern "C" {
  * @SPI_ROLE_SEPARATOR: An object usually contained in a menu to provide a visible and
  *        logical separation of the contents in a menu.
  * @SPI_ROLE_SLIDER: An object that allows the user to select from a bounded range.
+ * @SPI_ROLE_SPIN_BUTTON: An object which allows selection from a set of choices and
+ *                        displays the current choice.
  * @SPI_ROLE_SPLIT_PANE: A specialized panel that presents two other panels at the same time.
+ * @SPI_ROLE_STATUS_BAR: An object the display qualitative status information. c.f. PROGRESS_BAR
  * @SPI_ROLE_TABLE: An object used to rpesent information in terms of rows and columns.
  * @SPI_ROLE_TABLE_CELL: An object which is a descendant of a table,
  *        with a row/column location.  A cell may span multiple rows and columns.
  * @SPI_ROLE_TABLE_COLUMN_HEADER: An object which serves to describe a column in a table.
  * @SPI_ROLE_TABLE_ROW_HEADER: An object which serves to label or describe a row in a table.
+ * @SPI_ROLE_TEAROFF_MENU_ITEM: A menu item which allows the menu to be removed from
+ *                              a menubar and placed in its own window
+ * @SPI_ROLE_TERMINAL: An object that emulates a teletype or terminal
  * @SPI_ROLE_TEXT: An object that presents text to the user
  * @SPI_ROLE_TOGGLE_BUTTON: A specialized push button that can be checked or unchecked,
  *	  but does not provide a separate indicator for the current state.
  * @SPI_ROLE_TOOL_BAR: A bar or palette usually composed of push buttons or toggle buttons
  * @SPI_ROLE_TOOL_TIP: An object that provides information about another object
- * @SPI_ROLE_TREE: An object used to repsent hierarchical information to the user.
+ * @SPI_ROLE_TREE: An object used to represent hierarchical information to the user
+ * @SPI_ROLE_TREE_TABLE: An object which represents both hierarchical and tabular information
  * @SPI_ROLE_UNKNOWN: The object contains some SpiAccessible information, but its role is
  *                    not known.
  * @SPI_ROLE_VIEWPORT: An object usually used in a scroll pane, which restricts the visual
  *                    area into which its contents are presented.
  * @SPI_ROLE_WINDOW: A top level window with no title or border.
+ * @SPI_ROLE_EXTENDED: This object's role is not included in the standard role list and
+ *                     should be queried by name.
  **/
 typedef enum
 {
diff --git a/cspi/spi.h b/cspi/spi.h
index 82131bdd..b07f9d75 100644
--- a/cspi/spi.h
+++ b/cspi/spi.h
@@ -111,9 +111,10 @@ typedef enum {
  **/
 typedef struct _AccessibleKeySet
 {
-	unsigned long *keysyms;
-	unsigned short *keycodes;
-	short len;
+  unsigned long  *keysyms;
+  unsigned short *keycodes;
+  char          **keystrings;
+  short           len;
 } AccessibleKeySet;
 
 /**
@@ -137,6 +138,13 @@ int              SPI_exit         (void);
 
 /* Event Listener creation and support.  */
 
+void                      SPI_freeAccessibleKeySet (
+	                                           AccessibleKeySet         *keyset);
+AccessibleKeySet        * SPI_createAccessibleKeySet (
+                                         	   int                       len,
+						   const char               *keysyms,
+						   short                    *keycodes,
+						   const char              **keystrings);
 AccessibleEventListener * SPI_createAccessibleEventListener (
 	                                           AccessibleEventListenerCB callback,
 						   void                     *user_data);
diff --git a/cspi/spi_event.c b/cspi/spi_event.c
index 6fc82c6c..8550a9a0 100644
--- a/cspi/spi_event.c
+++ b/cspi/spi_event.c
@@ -22,6 +22,83 @@
 
 #include <cspi/spi-private.h>
 
+
+
+/**
+ * SPI_freeAccessibleKeySet:
+ * @keyset: An AccessibleKeyset to free.
+ *
+ * Release the memory used by an AccessibleKeySet.
+ *
+ **/
+void
+SPI_freeAccessibleKeySet (AccessibleKeySet *keyset)
+{
+  int i = 0;	
+  g_free (keyset->keysyms);
+  g_free (keyset->keycodes);
+  while (keyset->keystrings [i])
+    {
+      g_free (keyset->keystrings [i++]);
+    }
+  g_free (keyset->keystrings);
+  g_free (keyset);
+}
+
+/**
+ * SPI_createAccessibleKeySet:
+ * @len: the number of key values in the key set.
+ * @keysyms: a UTF-8 string containing symbolic key values to be matched, or NULL if
+ *           matching is performed against other key values instead.
+ * @keycodes: an array of unsigned short values which are the hardware keycodes
+ *           to be matched, or NULL if the keyset is specified solely by keysyms
+ *           and/or keystrings.
+ * @keystrings: an array of null-terminated character strings which specify key
+ *             name values to match, or NULL if the keyset is specified solely by
+ *             keycodes and/or keysyms.
+ *
+ * Create a new #AccessibleKeySet of a specified length.
+ * A KeySet is used typically to match key event values, and a matches are made
+ * using the following criteria: a match exists with a key event if all non-null
+ * i-th members of the keyset match the key event.
+ * If both keystring and keysym values are NULL, a keycode value match is
+ * forced, thus the match for keysym=0, keycode=0, keystring=NULL is
+ * keycode 0.
+ *
+ * Returns: a pointer to a newly-created #AccessibleKeySet.
+ *
+ **/
+AccessibleKeySet *
+SPI_createAccessibleKeySet (int len, const char *keysyms, short *keycodes, const char **keystrings)
+{
+  AccessibleKeySet *keyset = g_new0 (AccessibleKeySet, 1);
+  int i, keysym_len = 0;
+  const char *keysym_ptr = keysyms;
+  keyset->len = len;
+  keyset->keysyms = g_new0 (unsigned long, len);
+  keyset->keycodes = g_new0 (unsigned short, len);
+  keyset->keystrings = g_new0 (char *, len);
+  if (keysyms)
+    {
+      keysym_len = g_utf8_strlen (keysyms, -1);
+    }
+  for (i = 0; i < len; ++i)
+    {
+      if (i < keysym_len)
+        {
+	  keyset->keysyms [i] = (unsigned long) g_utf8_get_char (keysym_ptr);
+	  keysym_ptr = g_utf8_find_next_char (keysym_ptr, NULL);
+        }
+      else
+        {
+          keyset->keysyms [i] = 0;
+        }
+      if (keycodes) keyset->keycodes [i] = keycodes [i];
+      if (keystrings) keyset->keystrings [i] = keystrings [i];
+    }
+  return keyset;	
+}
+
 /**
  * SPI_createAccessibleEventListener:
  * @callback : an #AccessibleEventListenerCB callback function, or NULL.
diff --git a/cspi/spi_registry.c b/cspi/spi_registry.c
index aae9358f..f99afc54 100644
--- a/cspi/spi_registry.c
+++ b/cspi/spi_registry.c
@@ -275,7 +275,7 @@ SPI_registerAccessibleKeystrokeListener (AccessibleKeystrokeListener *listener,
         {
           /* we overload the keyset long w/keycodes, the - bit acts as a flag */
           key_set._buffer[i] = (keys->keysyms[i]) ? keys->keysyms[i] :
-	                                         -keys->keycodes[i];
+		                                  - keys->keycodes[i];
 	  /* fprintf (stderr, "key-set %d = %d\n", i, (int) key_set->_buffer[i]); */
         }
     }
@@ -310,14 +310,13 @@ SPI_registerAccessibleKeystrokeListener (AccessibleKeystrokeListener *listener,
     }
   key_events._length = i;
   
-  controller_event_mask.value = (CORBA_unsigned_long) modmask;
-  controller_event_mask.refcount = (CORBA_unsigned_short) 1;
+  controller_event_mask = (CORBA_unsigned_long) modmask;
 
   Accessibility_DeviceEventController_registerKeystrokeListener (
     device_event_controller,
     cspi_event_listener_get_corba (listener),
     &key_set,
-    &controller_event_mask,
+    controller_event_mask,
     &key_events,
     (CORBA_boolean) ((sync_type & SPI_KEYLISTENER_ALL_WINDOWS)!=0),
     cspi_ev ());
@@ -358,8 +357,7 @@ SPI_deregisterAccessibleKeystrokeListener (AccessibleKeystrokeListener *listener
 
   cspi_return_val_if_ev ("getting keystroke listener", FALSE);
 
-  controller_event_mask.value = (CORBA_unsigned_long) modmask;
-  controller_event_mask.refcount = (CORBA_unsigned_short) 1;
+  controller_event_mask = (CORBA_unsigned_long) modmask;
 
   key_events._buffer = NULL;
   key_events._length = 0;
@@ -371,7 +369,7 @@ SPI_deregisterAccessibleKeystrokeListener (AccessibleKeystrokeListener *listener
     device_event_controller,
     cspi_event_listener_get_corba (listener),
     &key_set,
-    &controller_event_mask,
+    controller_event_mask,
     &key_events,
     (CORBA_boolean) TRUE,
     cspi_ev ());
diff --git a/idl/Accessibility_Registry.idl b/idl/Accessibility_Registry.idl
index 10e45dd1..765979d3 100644
--- a/idl/Accessibility_Registry.idl
+++ b/idl/Accessibility_Registry.idl
@@ -174,34 +174,20 @@ module Accessibility {
     MODIFIER_NUMLOCK
   };
 
-  struct ControllerEventMask {
-    unsigned long value;
-    unsigned short refcount;
-  };
-
-  struct KeyStroke {
-    long keyID;
-    short keycode;
-    unsigned long timestamp;
-    KeyEventType type; 
-    unsigned short modifiers;
-  };
+  typedef unsigned long ControllerEventMask;
 
   struct DeviceEvent {
-    long eventID;
-    short hw_code;
-    unsigned long timestamp;
     EventType type; 
+    long  id;
+    short hw_code;
     unsigned short modifiers;
+    unsigned long timestamp;
+    string event_string;
   };
 
   typedef sequence< long > KeySet;
   typedef sequence< KeyEventType > KeyEventTypeSeq;
 
-  interface KeystrokeListener : Bonobo::Unknown {
-        boolean keyEvent (in KeyStroke key);
-  };
-
   interface DeviceEventListener : Bonobo::Unknown {
         boolean notifyEvent (in DeviceEvent event);
   };
@@ -210,7 +196,7 @@ module Accessibility {
 
   	/**
          * registerKeystrokeListener:
-         * @listener: a @KeystrokeListener which will intercept key events.
+         * @listener: a @DeviceEventListener which will intercept key events.
          * @keys:     a @KeySet indicating which keys to intercept, or KEYSET_ALL_KEYS.
          * @mask:     a @ControllerEventMask filtering the intercepted key events.
 	 * @type:     an @EventType mask that may created by ORing event types together.
@@ -224,7 +210,7 @@ module Accessibility {
          * consume them. 
          *
 	 **/
-        void registerKeystrokeListener (in KeystrokeListener listener,
+        void registerKeystrokeListener (in DeviceEventListener listener,
 					in KeySet keys,
 					in ControllerEventMask mask,
 					in KeyEventTypeSeq type,
@@ -232,7 +218,7 @@ module Accessibility {
     
   	/**
          * deregisterKeystrokeListener:
-         * @listener: a @KeystrokeListener which will intercept key events.
+         * @listener: a @DeviceEventListener which will intercept key events.
          * @keys:     a @KeySet indicating which keys to intercept, or KEYSET_ALL_KEYS.
          * @mask:     a @ControllerEventMask filtering the intercepted key events.
 	 * @type:     an @EventType mask that may created by ORing event types together.
@@ -245,11 +231,11 @@ module Accessibility {
          * De-register a previously registered keyboard eventlistener.
          *
 	 **/
-        void deregisterKeystrokeListener (in KeystrokeListener listener,
-					in KeySet keys,
-					in ControllerEventMask mask,
-					in KeyEventTypeSeq type,
-					in boolean is_synchronous);
+        void deregisterKeystrokeListener (in DeviceEventListener listener,
+					  in KeySet keys,
+					  in ControllerEventMask mask,
+					  in KeyEventTypeSeq type,
+					  in boolean is_synchronous);
     
         boolean notifyListenersSync (in DeviceEvent event);
 
diff --git a/libspi/keystrokelistener.c b/libspi/keystrokelistener.c
index 43f2cf0c..5fec6b19 100644
--- a/libspi/keystrokelistener.c
+++ b/libspi/keystrokelistener.c
@@ -42,9 +42,9 @@ static guint signals [LAST_SIGNAL];
  * CORBA Accessibility::KeystrokeListener::keyEvent method implementation
  */
 static CORBA_boolean
-impl_key_event (PortableServer_Servant         servant,
-		const Accessibility_KeyStroke *key,
-		CORBA_Environment             *ev)
+impl_key_event (PortableServer_Servant           servant,
+		const Accessibility_DeviceEvent *key,
+		CORBA_Environment               *ev)
 {
   gboolean was_consumed = FALSE;
   SpiKeystrokeListener *listener = SPI_KEYSTROKE_LISTENER (
@@ -112,7 +112,7 @@ marshal_BOOLEAN__POINTER (GClosure     *closure,
 static void
 spi_keystroke_listener_class_init (SpiKeystrokeListenerClass *klass)
 {
-  POA_Accessibility_KeystrokeListener__epv *epv = &klass->epv;
+  POA_Accessibility_DeviceEventListener__epv *epv = &klass->epv;
   
   signals [KEY_EVENT] = g_signal_new (
     "key_event",
@@ -123,7 +123,7 @@ spi_keystroke_listener_class_init (SpiKeystrokeListenerClass *klass)
     marshal_BOOLEAN__POINTER,
     G_TYPE_BOOLEAN, 1, G_TYPE_POINTER);
   
-  epv->keyEvent = impl_key_event;
+  epv->notifyEvent = impl_key_event;
 }
 
 static void
@@ -132,7 +132,7 @@ spi_keystroke_listener_init (SpiKeystrokeListener *keystroke_listener)
 }
 
 BONOBO_TYPE_FUNC_FULL (SpiKeystrokeListener,
-		       Accessibility_KeystrokeListener,
+		       Accessibility_DeviceEventListener,
 		       BONOBO_TYPE_OBJECT,
 		       spi_keystroke_listener);
 
diff --git a/libspi/keystrokelistener.h b/libspi/keystrokelistener.h
index 5bd61ebf..08f91fed 100644
--- a/libspi/keystrokelistener.h
+++ b/libspi/keystrokelistener.h
@@ -44,10 +44,10 @@ struct _SpiKeystrokeListener {
 
 typedef struct {
         BonoboObjectClass parent_class;
-        POA_Accessibility_KeystrokeListener__epv epv;
+        POA_Accessibility_DeviceEventListener__epv epv;
 
 	gboolean (*key_event) (SpiKeystrokeListener *listener,
-			       const Accessibility_KeyStroke *key);
+			       const Accessibility_DeviceEvent *key);
 } SpiKeystrokeListenerClass;
 
 GType                  spi_keystroke_listener_get_type        (void);
diff --git a/registryd/deviceeventcontroller.c b/registryd/deviceeventcontroller.c
index 964fb908..25af77e6 100644
--- a/registryd/deviceeventcontroller.c
+++ b/registryd/deviceeventcontroller.c
@@ -24,6 +24,8 @@
 
 #include <config.h>
 
+#define SPI_DEBUG
+
 #ifdef SPI_DEBUG
 #  include <stdio.h>
 #endif
@@ -48,41 +50,57 @@ static Display *display;
 static Window root_window;
 
 typedef enum {
-	SPI_DEVICE_TYPE_KBD,
-	SPI_DEVICE_TYPE_MOUSE,
-	SPI_DEVICE_TYPE_LAST_DEFINED
+  SPI_DEVICE_TYPE_KBD,
+  SPI_DEVICE_TYPE_MOUSE,
+  SPI_DEVICE_TYPE_LAST_DEFINED
 } SpiDeviceTypeCategory;
 
+struct _DEControllerGrabMask {
+  Accessibility_ControllerEventMask modmask;
+  CORBA_unsigned_long               keyval;
+  unsigned int                      refcount;
+};
+
+typedef struct _DEControllerGrabMask DEControllerGrabMask;
+
 struct _DEControllerListener {
-	CORBA_Object          object;
-	SpiDeviceTypeCategory type;
+  CORBA_Object          object;
+  SpiDeviceTypeCategory type;
 };
 
 typedef struct _DEControllerListener DEControllerListener;
 
 struct _DEControllerKeyListener {
-	DEControllerListener listener;
-	Accessibility_KeySet *keys;
-	Accessibility_ControllerEventMask *mask;
-	Accessibility_KeyEventTypeSeq *typeseq;
-	gboolean is_system_global;	
+  DEControllerListener listener;
+  Accessibility_KeySet *keys;
+  Accessibility_ControllerEventMask mask;
+  Accessibility_KeyEventTypeSeq *typeseq;
+  gboolean is_system_global;	
 };
 
 typedef struct _DEControllerKeyListener DEControllerKeyListener;
 
-static gboolean _controller_register_with_devices (SpiDeviceEventController *controller);
-static gboolean _controller_grab_keyboard (SpiDeviceEventController *controller);
+static gboolean spi_controller_register_with_devices (SpiDeviceEventController *controller);
+static gboolean spi_controller_grab_keyboard (SpiDeviceEventController *controller);
 
-static void controller_register_device_listener (SpiDeviceEventController *controller,
-						 DEControllerListener *l,
-						 CORBA_Environment *ev);
+static void spi_controller_register_device_listener (SpiDeviceEventController *controller,
+						     DEControllerListener *l,
+						     CORBA_Environment *ev);
 
 /*
  * Private methods
  */
 
+static DEControllerGrabMask * 
+spi_grabmask_clone (DEControllerGrabMask *grabmask)
+{
+  DEControllerGrabMask *clone = g_new0 (DEControllerGrabMask, 1);
+  memcpy (clone, grabmask, sizeof (DEControllerGrabMask));
+  return clone;
+}
+
 static gint
-_compare_corba_objects (gconstpointer p1, gconstpointer p2)
+spi_compare_corba_objects (gconstpointer p1, gconstpointer p2)
 {
   CORBA_Environment ev;
   gint retval;
@@ -97,44 +115,49 @@ _compare_corba_objects (gconstpointer p1, gconstpointer p2)
 }
 
 static gint
-_compare_listeners (gconstpointer p1, gconstpointer p2)
+spi_compare_listeners (gconstpointer p1, gconstpointer p2)
 {
   DEControllerListener *l1 = (DEControllerListener *) p1;	
   DEControllerListener *l2 = (DEControllerListener *) p2;	
-  return _compare_corba_objects (l1->object, l2->object);
+  return spi_compare_corba_objects (l1->object, l2->object);
 }
 
 static gint
-_eventmask_compare_value (gconstpointer p1, gconstpointer p2)
+spi_grabmask_compare_values (gconstpointer p1, gconstpointer p2)
 {
-    long d;
-    if (!p1 || !p2)
-	return (gint) (p1?1:(p2?-1:0));
-    else
-	d = ((long)((Accessibility_ControllerEventMask*)p2)->value) -
-		((long)((Accessibility_ControllerEventMask*)p1)->value);
-    return (gint) d;
+  DEControllerGrabMask *l1;
+  DEControllerGrabMask *l2;
+  if (p1 == p2)
+    {
+      return 0;
+    }
+  else
+    { 
+      l1 = (DEControllerGrabMask *) p1;	
+      l2 = (DEControllerGrabMask *) p2;
+      return ((l1->modmask != l2->modmask) || (l1->keyval != l2->keyval));
+    }
 }
 
 static DEControllerKeyListener *
-dec_key_listener_new (CORBA_Object l,
-		      const Accessibility_KeySet *keys,
-		      const Accessibility_ControllerEventMask *mask,
-		      const Accessibility_KeyEventTypeSeq *typeseq,
-		      const CORBA_boolean is_system_global,
-		      CORBA_Environment *ev)
+spi_dec_key_listener_new (CORBA_Object l,
+			  const Accessibility_KeySet *keys,
+			  const Accessibility_ControllerEventMask mask,
+			  const Accessibility_KeyEventTypeSeq *typeseq,
+			  const CORBA_boolean is_system_global,
+			  CORBA_Environment *ev)
 {
   DEControllerKeyListener *key_listener = g_new0 (DEControllerKeyListener, 1);
   key_listener->listener.object = bonobo_object_dup_ref (l, ev);
   key_listener->listener.type = SPI_DEVICE_TYPE_KBD;
   key_listener->keys = ORBit_copy_value (keys, TC_Accessibility_KeySet);
-  key_listener->mask = ORBit_copy_value (mask, TC_Accessibility_ControllerEventMask);
+  key_listener->mask = mask;
   key_listener->typeseq = ORBit_copy_value (typeseq, TC_Accessibility_KeyEventTypeSeq);
   key_listener->is_system_global = is_system_global;
 
 #ifdef SPI_DEBUG
   g_print ("new listener, with mask %x, is_global %d, keys %p\n",
-	   (unsigned int) key_listener->mask->value,
+	   (unsigned int) key_listener->mask,
            (int) key_listener->is_system_global,
 	   (void *) key_listener->keys);
 #endif
@@ -142,21 +165,86 @@ dec_key_listener_new (CORBA_Object l,
 }
 
 static void
-dec_key_listener_free (DEControllerKeyListener *key_listener, CORBA_Environment *ev)
+spi_dec_key_listener_free (DEControllerKeyListener *key_listener, CORBA_Environment *ev)
 {
   bonobo_object_release_unref (key_listener->listener.object, ev);
   CORBA_free (key_listener->typeseq);
-  CORBA_free (key_listener->mask);
   CORBA_free (key_listener->keys);
   g_free (key_listener);
 }
 
 static void
-controller_register_device_listener (SpiDeviceEventController *controller,
-				     DEControllerListener *listener,
-				     CORBA_Environment *ev)
+spi_controller_deregister_global_keygrabs (SpiDeviceEventController *controller,
+					   DEControllerKeyListener *key_listener)
+{
+  GList *list_ptr;
+  DEControllerGrabMask *mask_ptr;
+  /* TODO: implement this! Also remember to release any keygrabs still held */
+  ;
+}
+
+static void
+spi_controller_register_global_keygrabs (SpiDeviceEventController *controller,
+					 DEControllerKeyListener *key_listener)
+{
+  DEControllerGrabMask grabmask, *grabmask_ptr;
+  GList *list_ptr;
+  gint i;
+  /* TODO: deregistration version of this function */
+  
+  grabmask.modmask = key_listener->mask;
+  if (key_listener->keys->_length == 0) /* special case means AnyKey/AllKeys */
+    {
+      grabmask.keyval = AnyKey;
+      list_ptr = g_list_find_custom (controller->keygrabs_list, &grabmask,
+				     spi_grabmask_compare_values);
+      if (list_ptr)
+        {
+          grabmask_ptr = (DEControllerGrabMask *) list_ptr->data;
+	  grabmask_ptr->refcount++;
+        }
+      else
+        {
+	  controller->keygrabs_list =
+		  g_list_prepend (controller->keygrabs_list,
+				  spi_grabmask_clone (&grabmask));
+        }
+    }
+  else
+    {
+      for (i = 0; i < key_listener->keys->_length; ++i)
+        {
+	  long int keyval = key_listener->keys->_buffer[i];
+	  /* X Grabs require keycodes, not keysyms */
+	  if (keyval >= 0)
+	    {
+	      keyval = XKeysymToKeycode(display, (KeySym) keyval);		    
+	    }
+	  grabmask.keyval = keyval;
+          list_ptr = g_list_find_custom (controller->keygrabs_list, &grabmask,
+					     spi_grabmask_compare_values);
+          if (list_ptr)
+            {
+	      grabmask_ptr = (DEControllerGrabMask *) list_ptr->data;
+              grabmask_ptr->refcount++;
+            }
+          else
+            {
+	      controller->keygrabs_list =
+		  g_list_prepend (controller->keygrabs_list,
+				  spi_grabmask_clone (&grabmask));
+	      fprintf (stderr, "appending mask with val=%lu\n",
+		       (unsigned long) grabmask.modmask);
+            }
+        }
+    }
+}
+
+static void
+spi_controller_register_device_listener (SpiDeviceEventController *controller,
+					 DEControllerListener *listener,
+					 CORBA_Environment *ev)
 {
-  Accessibility_ControllerEventMask *mask_ptr = NULL;
   DEControllerKeyListener *key_listener;
   
   switch (listener->type) {
@@ -165,90 +253,52 @@ controller_register_device_listener (SpiDeviceEventController *controller,
       controller->key_listeners = g_list_prepend (controller->key_listeners, key_listener);
       if (key_listener->is_system_global)
         {
-          mask_ptr = (Accessibility_ControllerEventMask *)
-	      g_list_find_custom (controller->keymask_list, (gpointer) key_listener->mask,
-				  _eventmask_compare_value);
-          if (mask_ptr)
-	      ++(mask_ptr->refcount);
-          else
-           {
-	      if (key_listener->mask->refcount != (CORBA_unsigned_short) 1)
-		      fprintf (stderr, "mask initial refcount is not 1!\n");
-	      if (key_listener->mask->value > (CORBA_unsigned_long) 2048)
-		      fprintf (stderr, "mask value looks invalid (%lu)\n",
-			       (unsigned long) key_listener->mask->value);
-	      else
-		      fprintf (stderr, "appending mask with val=%lu\n",
-			       (unsigned long) key_listener->mask->value);
-	      mask_ptr = Accessibility_ControllerEventMask__alloc();
-	      mask_ptr->value = key_listener->mask->value;
-	      mask_ptr->refcount = (CORBA_unsigned_short) 1;
-	      controller->keymask_list = g_list_append (controller->keymask_list,
-							(gpointer) mask_ptr);
-          }
+	  spi_controller_register_global_keygrabs (controller, key_listener);	
 	}
       break;
   case SPI_DEVICE_TYPE_MOUSE:
 /*    controller->mouse_listeners = g_list_append (controller->mouse_listeners,
                                                    CORBA_Object_duplicate (l, ev));*/
 
-/* possibly this interface should NOT be used for mouse events ? */
+/* this interface should only be used for mouse motion events, not mouse clicks events */
       break;
   }
 }
 
 static void
-controller_deregister_device_listener (SpiDeviceEventController *controller,
-				       DEControllerListener *listener,
-				       CORBA_Environment *ev)
+spi_controller_deregister_device_listener (SpiDeviceEventController *controller,
+					   DEControllerListener *listener,
+					   CORBA_Environment *ev)
 {
   Accessibility_ControllerEventMask *mask_ptr;
-  DEControllerKeyListener *key_listener;
   DEControllerListener *dec_listener;
   GList *list_ptr;
-  switch (listener->type) {
-  case SPI_DEVICE_TYPE_KBD:
-      key_listener = (DEControllerKeyListener *) listener;
-      /* first, deref matching event mask, if any */
-      list_ptr = (GList *)
-	          g_list_find_custom (controller->keymask_list, (gpointer) key_listener->mask,
-				     _eventmask_compare_value);
-      if (list_ptr)
-        {
-	  mask_ptr = (Accessibility_ControllerEventMask *) list_ptr->data;
-          if (mask_ptr)
-	      --mask_ptr->refcount;
-          if (!mask_ptr->refcount)
-            {
-	      controller->keymask_list =
-		      g_list_remove_link (controller->keymask_list, list_ptr);
-	      ;  /* TODO: release any key grabs that are in place for this key mask */
-	    }
-	}
-      /* now, remove this listener from the keylistener list */
-      list_ptr = g_list_find_custom (controller->key_listeners, listener, _compare_listeners);
-      if (list_ptr)
-        {
-	  dec_listener = (DEControllerListener *) list_ptr->data;
+  switch (listener->type)
+    {
+      case SPI_DEVICE_TYPE_KBD:
+        spi_controller_deregister_global_keygrabs (controller,
+						   (DEControllerKeyListener *) listener);
+
+        /* now, remove this listener from the keylistener list */
+        list_ptr = g_list_find_custom (controller->key_listeners, listener, spi_compare_listeners);
+        if (list_ptr)
+          {
+	    dec_listener = (DEControllerListener *) list_ptr->data;
 #ifdef SPI_DEBUG	  
-	  g_print ("removing keylistener %p\n", dec_listener->object);
+	    g_print ("removing keylistener %p\n", dec_listener->object);
 #endif
-	  controller->key_listeners = g_list_remove_link (controller->key_listeners,
+	    controller->key_listeners = g_list_remove_link (controller->key_listeners,
 							  list_ptr);
-	  dec_key_listener_free ((DEControllerKeyListener *) dec_listener, ev);
-	}
-      break;
-  case SPI_DEVICE_TYPE_MOUSE:
-/*    controller->mouse_listeners = g_list_append (controller->mouse_listeners,
-                                                   CORBA_Object_duplicate (l, ev));*/
-
-/* possibly this interface should NOT be used for mouse events ? */
-      break;
-  }
+	    spi_dec_key_listener_free ((DEControllerKeyListener *) dec_listener, ev);
+	  }
+        break;
+      case SPI_DEVICE_TYPE_MOUSE: /* TODO: implement */
+        break;
+    }
 }
 
 static gboolean
-_controller_register_with_devices (SpiDeviceEventController *controller)
+spi_controller_register_with_devices (SpiDeviceEventController *controller)
 {
   gboolean retval = FALSE;
 
@@ -271,7 +321,7 @@ _controller_register_with_devices (SpiDeviceEventController *controller)
 }
 
 static gboolean
-key_set_contains_key (Accessibility_KeySet *key_set, Accessibility_KeyStroke *key_event)
+spi_key_set_contains_key (Accessibility_KeySet *key_set, const Accessibility_DeviceEvent *key_event)
 {
   gint i;
   gint len;
@@ -283,6 +333,7 @@ key_set_contains_key (Accessibility_KeySet *key_set, Accessibility_KeyStroke *ke
   
   if (len == 0) /* special case, means "all keys/any key" */
     {
+      g_print ("anykey\n");	    
       return TRUE;
     }
 
@@ -292,19 +343,19 @@ key_set_contains_key (Accessibility_KeySet *key_set, Accessibility_KeyStroke *ke
       g_print ("key_set[%d] = %d; key_event %d, code %d\n",
 	        i,
 	       (int) key_set->_buffer[i],
-	       (int) key_event->keyID,
-	       (int) key_event->keycode); 
+	       (int) key_event->id,
+	       (int) key_event->hw_code); 
 #endif
-      if (key_set->_buffer[i] == (CORBA_long) key_event->keyID) return TRUE;
-      if (key_set->_buffer[i] == (CORBA_long) -key_event->keycode) return TRUE;
+      if (key_set->_buffer[i] == (CORBA_long) key_event->id) return TRUE;
+      if (key_set->_buffer[i] == (CORBA_long) -key_event->hw_code) return TRUE;
     }
   
   return FALSE;
 }
 
 static gboolean
-key_eventtype_seq_contains_event (Accessibility_KeyEventTypeSeq *type_seq,
-				  Accessibility_KeyStroke *key_event)
+spi_key_eventtype_seq_contains_event (Accessibility_KeyEventTypeSeq *type_seq,
+				  const Accessibility_DeviceEvent *key_event)
 {
   gint i;
   gint len;
@@ -321,21 +372,24 @@ key_eventtype_seq_contains_event (Accessibility_KeyEventTypeSeq *type_seq,
 
   for (i=0; i<len; ++i)
     {
-/*      g_print ("type_seq[%d] = %d\n", i, (int) type_seq->_buffer[i]); */
+#ifdef SPI_DEBUG	    
+      g_print ("type_seq[%d] = %d; key event type = %d\n", i, (int) type_seq->_buffer[i],
+	       (int) key_event->type);
+#endif      
       if (type_seq->_buffer[i] == (CORBA_long) key_event->type) return TRUE;	    
     }
   
-  return TRUE;
+  return FALSE;
 }
 
 static gboolean
-key_event_matches_listener (Accessibility_KeyStroke *key_event,
+spi_key_event_matches_listener (const Accessibility_DeviceEvent *key_event,
 			    DEControllerKeyListener *listener,
 			    CORBA_boolean is_system_global)
 {
-  if ((key_event->modifiers == (CORBA_unsigned_short) (listener->mask->value & 0xFFFF)) &&
-       key_set_contains_key (listener->keys, key_event) &&
-       key_eventtype_seq_contains_event (listener->typeseq, key_event) && 
+  if ((key_event->modifiers == (CORBA_unsigned_short) (listener->mask & 0xFFFF)) &&
+       spi_key_set_contains_key (listener->keys, key_event) &&
+       spi_key_eventtype_seq_contains_event (listener->typeseq, key_event) && 
       (is_system_global == listener->is_system_global))
     {
       return TRUE;
@@ -345,25 +399,25 @@ key_event_matches_listener (Accessibility_KeyStroke *key_event,
 }
 
 static gboolean
-notify_keylisteners (GList *key_listeners,
-		     Accessibility_KeyStroke *key_event,
-		     CORBA_boolean is_system_global,
-		     CORBA_Environment *ev)
+spi_notify_keylisteners (GList *key_listeners,
+			 const Accessibility_DeviceEvent *key_event,
+			 CORBA_boolean is_system_global,
+			 CORBA_Environment *ev)
 {
   int i, n_listeners = g_list_length (key_listeners);
   gboolean is_consumed = FALSE;
 
   for (i=0; i<n_listeners && !is_consumed; ++i)
     {
-      Accessibility_KeystrokeListener ls;
+      Accessibility_DeviceEventListener ls;
       DEControllerKeyListener *key_listener = (DEControllerKeyListener *)
 	    g_list_nth_data (key_listeners, i);
-      ls = (Accessibility_KeystrokeListener) key_listener->listener.object;
-      if (key_event_matches_listener (key_event, key_listener, is_system_global))
+      ls = (Accessibility_DeviceEventListener) key_listener->listener.object;
+      if (spi_key_event_matches_listener (key_event, key_listener, is_system_global))
         {
           if (!CORBA_Object_is_nil(ls, ev))
             {
-	      is_consumed = Accessibility_KeystrokeListener_keyEvent (ls, key_event, ev);
+	      is_consumed = Accessibility_DeviceEventListener_notifyEvent (ls, key_event, ev);
             }		
         }
       else
@@ -377,15 +431,49 @@ notify_keylisteners (GList *key_listeners,
   return is_consumed;
 }
 
+static Accessibility_DeviceEvent
+spi_keystroke_from_x_key_event (XKeyEvent *x_key_event)
+{
+  Accessibility_DeviceEvent key_event;
+  KeySym keysym;
+  
+  keysym = XLookupKeysym (x_key_event, 0);
+  key_event.id = (CORBA_long)(keysym);
+  key_event.hw_code = (CORBA_short) x_key_event->keycode;
+  if (((XEvent *) x_key_event)->type == KeyPress)
+    {
+      key_event.type = Accessibility_KEY_PRESSED;
+    }
+  else
+    {
+      key_event.type = Accessibility_KEY_RELEASED;
+    } 
+  key_event.modifiers = (CORBA_unsigned_short)(x_key_event->state);
+#ifdef SPI_KEYEVENT_DEBUG
+  fprintf (stderr,
+     "Key %lu pressed (%c), modifiers %d\n",
+     (unsigned long) keysym,
+     keysym ? (int) keysym : '*',
+     (int) x_key_event->state);
+#endif
+#ifdef SPI_DEBUG
+  fprintf (stderr, "%s%c",
+     (x_key_event->state & Mod1Mask)?"Alt-":"",
+     ((x_key_event->state & ShiftMask)^(x_key_event->state & LockMask))?
+     g_ascii_toupper (keysym) : g_ascii_tolower (keysym));
+#endif /* SPI_DEBUG */
+  return key_event;	
+}
+
+
 static gboolean
-_check_key_event (SpiDeviceEventController *controller)
+spi_check_key_event (SpiDeviceEventController *controller)
 {
 	static gboolean initialized = FALSE;
 	XEvent *x_event = g_new0 (XEvent, 1);
 	XKeyEvent *x_key_event;
-	KeySym keysym;
 	gboolean is_consumed = FALSE;
-	Accessibility_KeyStroke key_event;
+	Accessibility_DeviceEvent key_event;
 	static CORBA_Environment ev;
 
 	if (!initialized)
@@ -398,57 +486,41 @@ _check_key_event (SpiDeviceEventController *controller)
 	  {
 	    XNextEvent (display, x_event);
 	    if (XFilterEvent (x_event, None)) continue;	  
-	    if (x_event->type == KeyPress)
+	    if (x_event->type == KeyPress || x_event->type == KeyRelease)
 	      {
-	        x_key_event = (XKeyEvent *)x_event;
-		keysym = XLookupKeysym (x_key_event, 0);
-		key_event.keyID = (CORBA_long)(keysym);
-		key_event.keycode = (CORBA_short) x_key_event->keycode;
-		key_event.type = Accessibility_KEY_PRESSED;
-		key_event.modifiers = (CORBA_unsigned_short)(x_key_event->state);
-#ifdef SPI_KEYEVENT_DEBUG
-	    fprintf (stderr,
-		     "Key %lu pressed (%c), modifiers %d\n",
-		     (unsigned long) keysym,
-		     keysym ? (int) keysym : '*',
-		     (int) x_key_event->state);
-#endif
-#ifdef SPI_DEBUG
-	    fprintf (stderr, "%s%c",
-		     (x_key_event->state & Mod1Mask)?"Alt-":"",
-		     ((x_key_event->state & ShiftMask)^(x_key_event->state & LockMask))?
-		     g_ascii_toupper (keysym) : g_ascii_tolower (keysym));
-#endif /* SPI_DEBUG */
+	        fprintf (stderr, "x event type=%d\n", x_event->type);      
+	        key_event = spi_keystroke_from_x_key_event ((XKeyEvent *) x_event);
+	        /* relay to listeners, and decide whether to consume it or not */
+	        is_consumed = spi_notify_keylisteners (controller->key_listeners, &key_event, CORBA_TRUE, &ev);
 	      }
 	    else
-	    {
+	      {
 #ifdef SPI_KEYEVENT_DEBUG
-		    fprintf (stderr, "other event, type %d\n", (int) x_event->type);
+	        fprintf (stderr, "other event, type %d\n", (int) x_event->type);
 #endif
-	    }
-	    /* relay to listeners, and decide whether to consume it or not */
-	    is_consumed = notify_keylisteners (controller->key_listeners, &key_event, CORBA_TRUE, &ev);
+	      }
 
 	    if (is_consumed)
-	    {
-	      XAllowEvents (display, AsyncKeyboard, CurrentTime);
-	    }
+	      {
+	        XAllowEvents (display, AsyncKeyboard, CurrentTime);
+	      }
 	    else
-	    {
-	      XAllowEvents (display, ReplayKeyboard, CurrentTime);
-	    }
+	      {
+	        XAllowEvents (display, ReplayKeyboard, CurrentTime);
+	      }
 	  }
 	XUngrabKey (display, AnyKey, AnyModifier, root_window);
-	return _controller_grab_keyboard (controller);
+
+	return spi_controller_grab_keyboard (controller);
 }
 
 static gboolean
-_controller_grab_keyboard (SpiDeviceEventController *controller)
+spi_controller_grab_keyboard (SpiDeviceEventController *controller)
 {
-	GList *maskList = controller->keymask_list;
-	int i;
-	int last_mask;
-	last_mask = g_list_length (maskList);
+  GList *maskList = controller->keygrabs_list;
+  int i;
+  int last_mask;
+  last_mask = g_list_length (maskList);
 
 /*
  * masks known to work with default RH 7.1: 
@@ -460,30 +532,37 @@ _controller_grab_keyboard (SpiDeviceEventController *controller)
  * ControlMask grabs are broken, must be in use already
  */
 	
-	for (i=0; i < last_mask; ++i)
-	{
-		Accessibility_ControllerEventMask *mask
-			= (Accessibility_ControllerEventMask *)g_list_nth_data (maskList, i);
-		unsigned long maskVal = 0xFFFFFFFF;
-		if (mask) maskVal = (unsigned long) mask->value;
-#ifdef SPI_KEYEVENT_DEBUG
-		fprintf (stderr, "mask=%lx\n", maskVal);
+  for (i=0; i < last_mask; ++i)
+    {
+      DEControllerGrabMask * grab_mask
+		= (DEControllerGrabMask *) g_list_nth_data (maskList, i);
+      unsigned long maskVal = 0xFFFFFFFF;
+      int           keyVal = AnyKey;
+      if (grab_mask)
+        {
+	  maskVal = (unsigned long) grab_mask->modmask;
+	  keyVal =  grab_mask->keyval;
+        }
+#ifdef SPI_DEBUG
+      fprintf (stderr, "mask=%lx\n", maskVal);
 #endif
-		if (!(maskVal & ControlMask))
-		{
-			XGrabKey (display,
-				  AnyKey,
-				  maskVal,
-				  root_window,
-				  True,
-				  GrabModeAsync,
-				  GrabModeAsync);
-			/* TODO: check call for errors and return FALSE if error occurs */
-		} else {
-			return FALSE; /* can't do control key yet */
-		}
+      if (!(maskVal & ControlMask))
+  	{
+	  XGrabKey (display,
+		    keyVal, 
+		    maskVal,
+		    root_window,
+		    True,
+		    GrabModeAsync,
+		    GrabModeAsync);
+	  /* TODO: check call for errors and return FALSE if error occurs */
 	}
-	return TRUE;
+      else
+        {
+	  return FALSE; /* can't do control key yet */
+        }
+    }
+  return TRUE;
 }
 
 /*
@@ -507,9 +586,9 @@ spi_device_event_controller_object_finalize (GObject *object)
  */
 static void
 impl_register_keystroke_listener (PortableServer_Servant     servant,
-				  const Accessibility_KeystrokeListener l,
+				  const Accessibility_DeviceEventListener l,
 				  const Accessibility_KeySet *keys,
-				  const Accessibility_ControllerEventMask *mask,
+				  const Accessibility_ControllerEventMask mask,
 				  const Accessibility_KeyEventTypeSeq *type,
 				  const CORBA_boolean is_system_global,
 				  CORBA_Environment         *ev)
@@ -519,10 +598,10 @@ impl_register_keystroke_listener (PortableServer_Servant     servant,
 	DEControllerKeyListener *dec_listener;
 #ifdef SPI_DEBUG
 	fprintf (stderr, "registering keystroke listener %p with maskVal %lu\n",
-		 (void *) l, (unsigned long) mask->value);
+		 (void *) l, (unsigned long) mask);
 #endif
-	dec_listener = dec_key_listener_new (l, keys, mask, type, is_system_global, ev);
-	controller_register_device_listener (controller, (DEControllerListener *) dec_listener, ev);
+	dec_listener = spi_dec_key_listener_new (l, keys, mask, type, is_system_global, ev);
+	spi_controller_register_device_listener (controller, (DEControllerListener *) dec_listener, ev);
 }
 
 /*
@@ -531,29 +610,29 @@ impl_register_keystroke_listener (PortableServer_Servant     servant,
  */
 static void
 impl_deregister_keystroke_listener (PortableServer_Servant     servant,
-				    const Accessibility_KeystrokeListener l,
+				    const Accessibility_DeviceEventListener l,
 				    const Accessibility_KeySet *keys,
-				    const Accessibility_ControllerEventMask *mask,
+				    const Accessibility_ControllerEventMask mask,
 				    const Accessibility_KeyEventTypeSeq *type,
 				    const CORBA_boolean is_system_global,
 				    CORBA_Environment         *ev)
 {
 	SpiDeviceEventController *controller = SPI_DEVICE_EVENT_CONTROLLER (
 		bonobo_object_from_servant (servant));
-	DEControllerKeyListener *key_listener = dec_key_listener_new (l,
-								      keys,
-								      mask,
-								      type,
-								      is_system_global,
-								      ev);
+	DEControllerKeyListener *key_listener = spi_dec_key_listener_new (l,
+									  keys,
+									  mask,
+									  type,
+									  is_system_global,
+									  ev);
 #ifdef SPI_DEREGISTER_DEBUG
 	fprintf (stderr, "deregistering keystroke listener %p with maskVal %lu\n",
 		 (void *) l, (unsigned long) mask->value);
 #endif
-	controller_deregister_device_listener(controller,
+	spi_controller_deregister_device_listener(controller,
 					      (DEControllerListener *) key_listener,
 					      ev);
-	dec_key_listener_free (key_listener, ev);
+	spi_dec_key_listener_free (key_listener, ev);
 }
 
 /*
@@ -571,7 +650,7 @@ impl_register_mouse_listener (PortableServer_Servant     servant,
 #ifdef SPI_DEBUG
 	fprintf (stderr, "registering mouse listener %p\n", l);
 #endif
-	controller_register_device_listener(controller, DEVICE_TYPE_MOUSE, l, keys, mask, ev);
+	spi_controller_register_device_listener(controller, DEVICE_TYPE_MOUSE, l, keys, mask, ev);
 }
 */
 
@@ -643,12 +722,11 @@ impl_notify_listeners_sync(PortableServer_Servant     servant,
 {
   SpiDeviceEventController *controller = SPI_DEVICE_EVENT_CONTROLLER (
                                          bonobo_object_from_servant (servant));
-  Accessibility_KeyStroke *key_event = (Accessibility_KeyStroke *) event;	
 #ifdef SPI_DEBUG
-  g_print ("notifying listeners synchronously: controller %x, event id %d\n",
-	   (void *) controller, (int) event->eventID);
+  g_print ("notifylistening listeners synchronously: controller %x, event id %d\n",
+	   (void *) controller, (int) event->id);
 #endif
-  return (notify_keylisteners (controller->key_listeners, key_event, CORBA_FALSE, ev) ?
+  return (spi_notify_keylisteners (controller->key_listeners, event, CORBA_FALSE, ev) ?
 	  CORBA_TRUE : CORBA_FALSE); 
 }
 
@@ -660,11 +738,10 @@ impl_notify_listeners_async (PortableServer_Servant     servant,
 {
   SpiDeviceEventController *controller = SPI_DEVICE_EVENT_CONTROLLER(
 	                                 bonobo_object_from_servant (servant));
-  Accessibility_KeyStroke *key_event = (Accessibility_KeyStroke *) event;	
 #ifdef SPI_DEBUG
   fprintf (stderr, "notifying listeners asynchronously\n");
 #endif
-  notify_keylisteners (controller->key_listeners, key_event, CORBA_FALSE, ev); 
+  spi_notify_keylisteners (controller->key_listeners, event, CORBA_FALSE, ev); 
 }
 
 static void
@@ -683,7 +760,7 @@ spi_device_event_controller_class_init (SpiDeviceEventControllerClass *klass)
         epv->generateMouseEvent = impl_generate_mouse_event;
 	epv->notifyListenersSync = impl_notify_listeners_sync;
 	epv->notifyListenersAsync = impl_notify_listeners_async;
-	klass->check_key_event = _check_key_event;
+	klass->check_key_event = spi_check_key_event;
 }
 
 static void
@@ -691,17 +768,17 @@ spi_device_event_controller_init (SpiDeviceEventController *device_event_control
 {
   device_event_controller->key_listeners = NULL;
   device_event_controller->mouse_listeners = NULL;
-  device_event_controller->keymask_list = NULL;
-  kbd_registered = _controller_register_with_devices (device_event_controller);
+  device_event_controller->keygrabs_list = NULL;
+  kbd_registered = spi_controller_register_with_devices (device_event_controller);
 }
 
 gboolean
 spi_device_event_controller_check_key_event (SpiDeviceEventController *controller)
 {
-	SpiDeviceEventControllerClass *klass = SPI_DEVICE_EVENT_CONTROLLER_GET_CLASS (controller);
-	if (klass->check_key_event)
-		return (klass->check_key_event) (controller);
-	return FALSE;
+  SpiDeviceEventControllerClass *klass = SPI_DEVICE_EVENT_CONTROLLER_GET_CLASS (controller);
+  if (klass->check_key_event)
+	return (klass->check_key_event) (controller);
+  return FALSE;
 }
 
 SpiDeviceEventController *
diff --git a/registryd/deviceeventcontroller.h b/registryd/deviceeventcontroller.h
index 700c84a7..a9430631 100644
--- a/registryd/deviceeventcontroller.h
+++ b/registryd/deviceeventcontroller.h
@@ -37,17 +37,17 @@ G_BEGIN_DECLS
 #define SPI_DEVICE_EVENT_CONTROLLER_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), SPI_DEVICE_EVENT_CONTROLLER_TYPE, SpiDeviceEventControllerClass))
 
 typedef struct {
-        BonoboObject parent;
-	void  *registry;
-        GList *key_listeners;
-        GList *mouse_listeners;
-	GList *keymask_list;
+  BonoboObject parent;
+  void  *registry;
+  GList *key_listeners;
+  GList *mouse_listeners;
+  GList *keygrabs_list;
 } SpiDeviceEventController;
 
 typedef struct {
-        BonoboObjectClass parent_class;
-        POA_Accessibility_DeviceEventController__epv epv;
-	gboolean (*check_key_event) (SpiDeviceEventController *controller);
+  BonoboObjectClass parent_class;
+  POA_Accessibility_DeviceEventController__epv epv;
+  gboolean (*check_key_event) (SpiDeviceEventController *controller);
 } SpiDeviceEventControllerClass;
 
 GType                     spi_device_event_controller_get_type        (void);
diff --git a/test/keysynth-demo.c b/test/keysynth-demo.c
index c6cb1ee1..ebf00c15 100644
--- a/test/keysynth-demo.c
+++ b/test/keysynth-demo.c
@@ -498,7 +498,7 @@ main (int argc, char **argv)
   SPI_registerAccessibleKeystrokeListener (switch_listener,
 					   &switch_set,
 					   SPI_KEYMASK_UNMODIFIED,
-					   (unsigned long) ( KeyPress | KeyRelease),
+					   (unsigned long) ( SPI_KEY_PRESSED | SPI_KEY_RELEASED ),
 					   SPI_KEYLISTENER_CANCONSUME);
   
   SPI_event_main ();
diff --git a/test/simple-at.c b/test/simple-at.c
index 1901e68a..b3fb76f1 100644
--- a/test/simple-at.c
+++ b/test/simple-at.c
@@ -50,6 +50,7 @@ static AccessibleEventListener *generic_listener;
 static AccessibleEventListener *button_listener;
 static AccessibleKeystrokeListener *command_key_listener;
 static AccessibleKeystrokeListener *ordinary_key_listener;
+static AccessibleKeySet *spacebar_key_set;
 
 int
 main (int argc, char **argv)
@@ -119,22 +120,32 @@ main (int argc, char **argv)
   SPI_registerAccessibleKeystrokeListener(command_key_listener,
 					  (AccessibleKeySet *) SPI_KEYSET_ALL_KEYS,
 					  SPI_KEYMASK_ALT,
-					  (unsigned long) ( KeyPress ),
+					  (unsigned long) ( SPI_KEY_PRESSED ),
 					  SPI_KEYLISTENER_ALL_WINDOWS);
   
   /* will listen only to unshifted key events, both press and release */
   SPI_registerAccessibleKeystrokeListener(ordinary_key_listener,
 					  (AccessibleKeySet *) SPI_KEYSET_ALL_KEYS,
 					  SPI_KEYMASK_UNMODIFIED,
-					  (unsigned long) ( KeyPress | KeyRelease),
+					  (unsigned long) ( SPI_KEY_PRESSED | SPI_KEY_RELEASED ),
 					  SPI_KEYLISTENER_NOSYNC);
 				      
   /* will listen only to shifted key events, both press and release */
   SPI_registerAccessibleKeystrokeListener(ordinary_key_listener,
 					  (AccessibleKeySet *) SPI_KEYSET_ALL_KEYS,
 					  SPI_KEYMASK_SHIFT,
-					  (unsigned long) ( KeyPress | KeyRelease),
+					  (unsigned long) ( SPI_KEY_PRESSED | SPI_KEY_RELEASED ),
 					  SPI_KEYLISTENER_NOSYNC);
+
+  spacebar_key_set = SPI_createAccessibleKeySet (1, " ", NULL, NULL);
+  
+  /* will listen only to shift-spacebar events, on release, globally */
+  SPI_registerAccessibleKeystrokeListener(command_key_listener,
+					  spacebar_key_set,
+					  SPI_KEYMASK_SHIFT,
+					  (unsigned long) ( SPI_KEY_RELEASED ),
+					  SPI_KEYLISTENER_ALL_WINDOWS);
+
   
   get_environment_vars ();
 
@@ -324,6 +335,8 @@ simple_at_exit ()
   SPI_deregisterAccessibleKeystrokeListener (ordinary_key_listener, SPI_KEYMASK_UNMODIFIED);
   SPI_deregisterAccessibleKeystrokeListener (ordinary_key_listener, SPI_KEYMASK_SHIFT);
   AccessibleKeystrokeListener_unref         (ordinary_key_listener);
+
+  SPI_freeAccessibleKeySet (spacebar_key_set);
   
   SPI_event_quit ();
 }
diff --git a/test/test-simple.c b/test/test-simple.c
index ea5a799e..47cad074 100644
--- a/test/test-simple.c
+++ b/test/test-simple.c
@@ -308,8 +308,6 @@ test_value (AccessibleValue *value)
 {
 	float original_value;
 	
-        /* Note: test_value assertions are known not to work as of Dec 09 */
-	
 	fprintf (stderr, "Testing value ...\n");
 
 	original_value = AccessibleValue_getCurrentValue (value);