[PATCH] xwayland-input: Always set the xkb group index on modifiers events

[Daniel Stone]

Hi,

On 15 July 2014 14:57, Rui Matos <tiagomatos at gmail.com> wrote:

> While we have keyboard focus, the server's xkb code is already locking
> and latching modifiers appropriately while processing keyboard
> events.
>
> Since there is no guaranteed order between wl_keyboard key and
> modifiers events, if we got the modifiers event with a locked or
> latched modifier and then process the key press event for that
> modifier we would wrongly unlock/unlatch. To prevent this, we ignore
> locked and latched modifiers while any of our surfaces has keyboard
> focus.
>

Wow, I really wish I'd encoded that properly in the wl_keyboard spec.

Sending modifiers before key is _seriously_ broken, and no-one should ever
do it. There are corner cases it completely and horrifically breaks.

X11 actually got this right for once: the state sent with the key event is
the state that applied _immediately before the press_, i.e. unaffected by
the press itself. Where it slipped up was not updating you with the state
immediately after the press as well, but you cannot interpret the press
with the state the press itself created.

I'm very much tempted to merge Jonny's repeat rate patches which take us to
a new wl_keyboard version, and including in that version a requirement that
this ordering be observed.

If Mutter sends modifiers before key, _please_ reverse that.


> But we always need to set the xkb group index since this might be
> triggered programatically by the wayland compositor at any time.
> ---
>
> Note that xwayland's xkb state handling needs quite a bit of work to
> make it reliable. Basically, if the wl_keyboard.modifiers event comes
> in after the wl_keyboard.enter event we won't update the locked and
> latched modifiers properly. Right now this just happens to work with
> mutter.
>
> I'm working on a proper fix for this which requires API changes to the
> core xkb code in order for us to be able to tell it to ignore modifier
> state processing on key events and instead always set that state from
> wl_keyboard.modifiers events like any other wayland client.
>
> Extra API will also be needed to tell the xkb request processing code
> to return errors and/or silently drop X client requests that try to
> change any xkb state and keymaps.
>
> Anyway, that wouldn't be acceptable for the upcoming 1.16 release so
> I'm just proposing this targetted fix for the group index to be
> honored which I'd like to have working on the next GNOME release.
>

Agreed to all of the above, but, wouldn't we be better off waiting for a
modifiers event straight after enter? Although that's another thing which
really needs encoding into the protocol ... oh well.

Reviewed-by: Daniel Stone <daniels at collabora.com>

Cheers,
Daniel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20140718/5a1ca907/attachment.html>