[PATCH 3/3] XWayland: Use FocusIn events for keyboard enter

[Daniel Stone]

Hi,

On 28 November 2014 at 06:09, Peter Hutterer <peter.hutterer at who-t.net>
wrote:

> On Tue, Nov 25, 2014 at 09:30:59PM +0000, Daniel Stone wrote:
> > On 25 November 2014 at 02:38, Keith Packard <keithp at keithp.com> wrote:
> > > Yeah, I'm fine with figuring out how to make it perfect; if the above
> > > patch will suffice to get modifiers handled correctly for now, I'd like
> > > to go with that for this release and pend the perfect solution until
> > > after 1.17; just too many changes outside of hw/xwayland to make me
> > > happy at this point in the release cycle.
> >
> > I think they're all pretty harmless as they're confined to new codepaths
> > which were not previously triggerable, and are still not triggerable
> > outside of Xwayland.
>
> that's the theory :) if you forgot to add one check somewhere, you may now
> get weird events. I can't seem to find a 1.17 release schedule - Keith,
> where're we at?


Sure, but the same is true of the suggested global state. Forget to check
it and you've got a bug.


> > > In particular, I can imagine passing a new boolean in, or setting some
> > > device state, that muted event generation and avoided having to check
> > > for ET_KeyFocusIn in a bunch of places now and in the future. That
> would
> > > also encourage us to actually deal with this case in Xephyr and Xnest.
> >
> > How is that better? You've shifted some local state (what's the event
> > type?) into global state (is this thing set?). The penalties for missing
> a
> > check are exactly the same for both, except the intention is much more
> > obvious with an explicit event. The event type also has the benefit of
> > being able to be posted through the queue, rather than OsBlockSignals ->
> > ProcessInputEvents -> mangle magic global flag -> QueueKeyboardEvents ->
> > ProcessKeyboardEvents -> OsUnblockSignals. I'd much rather take the hit
> of
> > tiny extra complexity in the core (really, it's not much at all) rather
> > than having all the users get it wrong, or simply not bother because it's
> > too hard. Much like they do today.
>
> You could add that state to the internal event, add a fake_event field to
> the DeviceEvent and check that where necessary, otherwise let it pass
> through as normal event. That way the penalty for forgetting a check only
> matters where you actually care about it.
>
> That should also reduce the risk of getting odd events.
>

So to be clear, it would be ET_KeyPress, but DeviceEvent grows an extra
field that tells you that really it should be suppressed?

I don't think that makes too much difference from above (again, if you miss
a check, you're sending something out incorrectly; it also makes it less
easy to tell at a glance what's going on), but if that's what you'd prefer,
then fair enough.

Cheers,
Daniel


> Cheers,
>    Peter
>
> > If you're implacably opposed, then I'll ack the minimal change (once we
> can
> > be certain that the focus is indeed NULL whilst posting those events) for
> > 1.17, but I'll also request a revert immediately after master reopens,
> and
> > resubmit this patchset. Your patch does just happen to work, but it's an
> > extra whack of magic bonghits right where we least need them.
> >
> > Cheers,
> > Daniel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20141128/88268b09/attachment.html>