GNOME Bugzilla – Bug 649559
AtkKeyEventStruct has several issues to solve
Last modified: 2021-06-10 11:24:46 UTC
There are several issues on AtkKeyEventStruct [1] that requires to be solved. A field by field analysis: * gint type; An AtkKeyEventType, generally one of ATK_KEY_EVENT_PRESS or ATK_KEY_EVENT_RELEASE No problem here. * guint state; A bitmask representing the state of the modifier keys immediately after the event takes place. The meaning of the bits is currently defined to match the bitmask used by GDK in GdkEventType.state, see http://developer.gnome.org/doc/API/2.0/gdk/gdk-Event-Structures.htmlGdkEventKey This definition is not toolkit agnostic. I think that it would worth to define a AtkModifierType or something like that. So if any toolkit has a different representation of the state, it would be a translation from this representation to ATK insted of GDK. Example1: right now on Unity (nux) I need to translate NUX_STATE_SHIFT to GDK_SHIFT_MASK. This is not a problem because Unity has already a gtk dependency (but it would be cleaner, IMHO, to translate this NUX_STATE_SHIFT to something like ATK_SHIFT_MASK). Example2: in the case of clutter this works because the value of the modifier state on his key events are the same. But it would be a problem if not, as clutter doesn't have a gtk dependency. * guint keyval; A guint representing a keysym value corresponding to those used by GDK and X11: see /usr/X11/include/keysymdef.h. Also not toolkit agnostic. Although we can rely on the X11 definition for the moment, eventually we should also discard it (see bug 649556) * gchar *string; A string containing one of the following: either a string approximating the text that would result from this keypress, if the key is a control or graphic character, or a symbolic name for this keypress. Alphanumeric and printable keys will have the symbolic key name in this string member, for instance "A". "0", "semicolon", "aacute". Keypad keys have the prefix "KP". IMHO, this field should be removed. In general it is just a keyval derived field, and not all the AT tools requires this field. I think that getting this string should be done at the AT level, and not at the toolkit one. In the practice, it is hard to get this string at the toolkit level. Ie: clutter doesn't provides any way to get this name, and it would be too complex to implement on cally. So cally emits the key event with no string and Orca fill this field properly. * gint length; The length of member string. It should be removed. See string. * guint16 keycode; The raw hardware code that generated the key event. This field is rarely useful. This field is vaguely defined, and not sure about this "rarely useful". I guess that was added "just in case", so not sure if we should keep it, but I also don't have too many arguments to remove it. * guint32 timestamp; A timestamp in milliseconds indicating when the event occurred. These timestamps are relative to a starting point which should be considered arbitrary, and only used to compare the dispatch times of events to one another. No problems here. PS: note that most of the examples are GTK and Clutter based, as I'm more experienced here. But AFAIK, there is some issues with this key event struct and Java, so some of the workarounds implemented on Orca due those clutter issues were also used for JAVA. [1] http://developer.gnome.org/atk/stable/AtkUtil.html#AtkKeyEventStruct
ATK/AT-SPI2 Hackfest 2011 conclusions (sorry for the delay): > * guint state; Agreed to define an specific ATK state definition. > * guint keyval; A guint representing a keysym value corresponding to those > used by GDK and X11: see /usr/X11/include/keysymdef.h. Agreed to keep it relying on the X11 definition, as it is not clear how to approach Wayland (GDK reference should be removed). Anyway, not clear if it should include the modifiers applied or not. At first it seems to make sense to not have them applied. But probably GDK made that for any reason. TODO: investigate why GDK apply the modifiers here. > * gchar *string; > * gint length; The length of member string. Agreed to remove them (probably deprecating them first). Additional note: Frederik and Mike said that they have on some projects a translation table. So probably it would be good to have it in just one place, probably libatspi. > * guint16 keycode; The raw hardware code that generated the key event. This > field is rarely useful. No reasons to remove it, no harm maintaining it. So final conclusion was maintain it. > * guint32 timestamp; No problem here. In general people agreed to simplify this struct, although it is still pending some investigation. Final note: it was also discussed that the key event propagation shouldn't be in general a application responsibility, for performance and other things. It should be done by at-spi2 itself, as the mouse events. In fact this was done on the past, but this is not possible right now because X module <Idontremember> is not properly maintained. That would be the ideal solution. Anyway, this struct would be required to be defined, at least on at-spi2 level.
(In reply to comment #1) > Final note: it was also discussed that the key event propagation shouldn't be > in general a application responsibility, for performance and other things. It > should be done by at-spi2 itself, as the mouse events. In fact this was done on > the past, but this is not possible right now because X module <Idontremember> > is not properly maintained. That would be the ideal solution. > > Anyway, this struct would be required to be defined, at least on at-spi2 level. This X module is called xevie: http://linux.die.net/man/3/xevie On the hackfest Li mentioned that at-spi was using that module, but then it needed to stop to use them because it was not maintained. But this was a lot of time ago. It would be good to check again now if this has changed, or if there are other alternatives. If we found that alternative, it would be also good to move this feature also from at-spi, to avoid that dbus message (something like an atspi API to interact with those X events, so no dbus message would be required for those key events).
BTW, an additional advantage of get rid of this code from ATK is that we would remove any X dependency on ATK, making it more abstract and general use case (so wayland eventual move would be less scary)
[Mass-reassigning open atk bug reports for better trackability as requested in https://bugzilla.gnome.org/show_bug.cgi?id=653179 . PLEASE NOTE: If you have watched the previous assignee of this bug report as a workaround for actually getting notified of changes in atk bugs, you yourself will now have to add atk-maint@gnome.bugs to your watchlist at the bottom of https://bugzilla.gnome.org/userprefs.cgi?tab=email to keep watching atk bug reports in GNOME Bugzilla. Sorry for the noise: Feel free to filter for this comment in order to mass-delete the triggered bugmail.]
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version of atk, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a ticket at https://gitlab.gnome.org/GNOME/atk/-/issues/ Thank you for your understanding and your help.