GNOME Bugzilla – Bug 599907
[PATCH] Gail implementation of atk_add_key_event_listener return a wrong even listener id
Last modified: 2011-02-17 03:58:28 UTC
Created attachment 146423 [details] Little gtk application to reproduce the bug http://library.gnome.org/devel/atk/stable/AtkUtil.html#atk-add-key-event-listener http://library.gnome.org/devel/atk/stable/AtkUtil.html#atk-remove-global-event-listener STEPS TO REPRODUCE 1. Uncompress and compile the application attached (Makefile included) 1.1 This program adds two event_key listeners 1.2 They basically print the key pressed 1.3 It tries to remove the last one using the listener id returned by add_key_event_listener 2. Start to press some keys EXPECTED OUTCOME The second key event listener should had been removed correctly, so only the first callback print messages on screen ACTUAL OUTCOME Both listeners are executed, so the second listener was not removed correctly. EXTRA COMMENTS Problem found and patch available, I will attach it in the next comment.
Created attachment 146424 [details] [review] Return the correct listener id added The problem was in this gail_util_add_key_event_listener code: g_hash_table_insert (key_listener_list, GUINT_TO_POINTER (key++), (gpointer) listener); /* XXX: we don't check to see if n_listeners > MAXUINT */ return key; key is static variable in this function, used to count the key_listeners added, and used as the key in the hash table. The problem is that the value of the key is updated just after insert the table, so the function is returning the *next* key to be added, not the key really added on the hash table. One obvious solution is just return key-1 (patch).
Review of attachment 146424 [details] [review]: The patch is OK to me.
Created attachment 146500 [details] [review] First value correct A litte update of the patch. With this patch, the first listener properly registered returns a value of 0. But reading the documentation: http://library.gnome.org/devel/atk/stable/AtkUtil.html#atk-add-key-event-listener 0 is a reserved value for failure. So this patch changes the initial value of key to 1, like listener_idx (used for the global events). Sorry for not realize that before.