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

[Daniel Stone]

Hi,

On Thursday, November 13, 2014, Giulio Camuffo <giuliocamuffo at gmail.com>
wrote:

> 2014-11-13 11:18 GMT+02:00 Daniel Stone <daniel at fooishbar.org
> <javascript:;>>:
> > Think about the case where pressing Alt on its own will focus the system
> > menu, but pressing Alt-F will raise the file menu directly. Let's say the
> > compositor has an Alt-F12 hotkey which triggers some action and then
> > immediately returns focus. We'll come back to the client with Alt held
> down,
> > and nothing else. When we come back, we do _not_ want the client to focus
> > the menu as if Alt was pressed normally. However, we _do_ want the
> client to
> > raise the file menu if F is then pressed (so, Alt+F). This exactly
> matches
> > the previous paragraph: update your internal state, but do not
> immediately
> > trigger any actions.
>
> But this should not be affected by this patch. The key binding just
> eats the F12, so the Alt is still being sent to the client.


So we've introduced an inconsistency. It happens to mostly kind of work for
modifiers, but that's luck of the draw.

> We can see exactly one outlier here, doing the exactly wrong thing, which
> is
> > XWayland. So it stands to reason that XWayland should be fixed: changing
> the
> > others would introduce massive inconsistencies depending on whether your
> > compositor is native or nested, which makes ~no sense.
>
> I perfectly agree XWayland is broken here. But i don't see how this
> patch breaks weston.


By introducing an inconsistency for, as far as I can see, no reason. That
it mostly happens to work is great, but that's luck rather than design.


> > 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.
>
> But consider this case: There is a Alt+X compositor binding which
> triggers something, without changing the focus. Then the client has a
> Alt+X+Y binding. When pressing Alt+X the X is eaten by the compositor,
> so trying to press also Y to trigger the client binding will have no
> effect, because the client will actually see just Alt+Y. Without this
> patch, if the Alt+X compositor binding was to instead switch the focus
> to the client, the Alt+X+Y binding would work, because the client sees
> the X too. Now it doesn't, as it doesn't without switching the focus.
> This is where it is more consistent.
>

But this is just a client issue, and nothing in sending the full keys array
precludes this working.

If Alt+X is a modifier (i.e. any time Alt+X is held, pressing Y triggers
the shortcut), then the client can use the keys array to notice this, and
ensure the shortcut is fired.

If Alt+(X+Y) is a cumulative shortcut, then the client knows from seeing X
in the enter array but not a key event, that it must wait for a release on
X before it arms the shortcut for Y.

In both cases, no problem. If we suppress X in the key-down array, we break
the first case, and in the second case, we send the client a release for a
key it's never seen. IOW, we introduce enormous inconsistencies in the key
book-keeping. As there's ~nothing more infuriating than broken key/button
state handling, and it's already relatively complicated code, I don't see
how any good can come out of making it easier to screw up for pretty much
no gain.

Given the above, and the possibility of making XWayland do the right thing,
I just can't see how this patch improves the status quo (pre-this-patch),
given the enormous downsides.

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