[PATCH weston] input: don't send to clients key events eaten by bindings

[Daniel Stone]

Hi,

On Thursday, November 13, 2014, Daniel Stone <daniel at fooishbar.org> wrote:
>
> On 13 November 2014 07:35, Pekka Paalanen <ppaalanen at gmail.com
> <javascript:_e(%7B%7D,'cvml','ppaalanen at gmail.com');>> wrote:
>
>> On Wed, 12 Nov 2014 12:06:16 -0800
>> Bill Spitzak <spitzak at gmail.com
>> <javascript:_e(%7B%7D,'cvml','spitzak at gmail.com');>> wrote:
>> > The important point is that in both your and my case the client that has
>> > just received focus sees the 'd' as being held down. The 'd' is on in
>> > the key-down array attached to the focus-in event!!!!
>>
>> Correct. Apparently X apps interpret a "key already down" the same way
>> as a "key goes down" event. That is not a Wayland issue at all.
>
>
> No, X _apps_ don't - XWayland does.
>

To be clear: X11 has this as well. FocusIn + KeymapNotify events are
equivalent to wl_keyboard::enter with a fully-populated key list.


> > IMHO they should not be different, therefore this patch is wrong. There
>> > are two possible solutions:
>> >
>> > 1. Revert it. The actual bug is that a client that already has focus
>> > before a "compositor shortcut" will not get any changes to the keymap.
>> > Add something to fix this instead. I recommend a redundant focus-in
>> event.
>> >
>> > 2. Always send a blank key-down array in the focus-in event, since it is
>> > possible that any key held down was the "shortcut" that caused the
>> > focus-in event.
>>
>> No, we are not going to break the Wayland core protocol. Also, I do not
>> see a problem with the current behaviour.
>>
>> We are not going to screw up Wayland just so that X apps in Xwayland
>> would work "better" for some cornercases.
>>
>> Instead, we fixed Weston to act consistently, which corrected the
>> original problem.
>>
>
> I think the fix makes Weston act _less_ consistently, especially when you
> consider how it behaves when it's nested. If Weston nested under Wayland
> behaves differently to DRM, that's proof that our model is broken.
>

And the consistency argument is backed up by different consumers.
wl_keyboard::enter gives you the list of currently-pressed keys, to be used
for bookkeeping and _not_ for actions.

If we enter from a VT switch, we'll send Ctrl+Alt+Fx to the clients in
the enter event when they get the focus. If it was a compositor key
binding, with this patch, we won't send the key that immediately triggered
the binding, but we will send the other keys.

I'm struggling to see a case where this makes sense, especially if you take
it as a given that the current XWayland behaviour (which this patch tries
to work around) as broken and in need of fixing.

wl_keyboard::enter sends the list of currently-depressed keys to the
client. If we make it a partial list, depending on context, we make it
useless. We'll send releases for keys the client never knew were down -
which is _exactly_ the problem this event is trying to prevent.

If we're making it an arbitrary list depending on whether it was a
compositor hotkey, or depending on how you're nested, the event becomes
useless as a guarantee and we might as well just drop it completely.

Cheers,
Daniel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20141113/3cc38da2/attachment-0001.html>