<div dir="ltr">Hi,<div class="gmail_extra"><br><div class="gmail_quote">On 28 November 2014 at 06:09, Peter Hutterer <span dir="ltr"><<a href="mailto:peter.hutterer@who-t.net" target="_blank">peter.hutterer@who-t.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Nov 25, 2014 at 09:30:59PM +0000, Daniel Stone wrote:<br>> On 25 November 2014 at 02:38, Keith Packard <<a href="mailto:keithp@keithp.com">keithp@keithp.com</a>> wrote:<br>> > Yeah, I'm fine with figuring out how to make it perfect; if the above<br>
> > patch will suffice to get modifiers handled correctly for now, I'd like<br>
> > to go with that for this release and pend the perfect solution until<br>
> > after 1.17; just too many changes outside of hw/xwayland to make me<br>
> > happy at this point in the release cycle.<br>><br>
> I think they're all pretty harmless as they're confined to new codepaths<br>
> which were not previously triggerable, and are still not triggerable<br>
> outside of Xwayland.<br>
<br>
</div></div>that's the theory :) if you forgot to add one check somewhere, you may now<br>
get weird events. I can't seem to find a 1.17 release schedule - Keith,<br>
where're we at?</blockquote><div><br></div><div>Sure, but the same is true of the suggested global state. Forget to check it and you've got a bug.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> > In particular, I can imagine passing a new boolean in, or setting some<br>
> > device state, that muted event generation and avoided having to check<br>
> > for ET_KeyFocusIn in a bunch of places now and in the future. That would<br>
> > also encourage us to actually deal with this case in Xephyr and Xnest.<br>
><br>
> How is that better? You've shifted some local state (what's the event<br>
> type?) into global state (is this thing set?). The penalties for missing a<br>
> check are exactly the same for both, except the intention is much more<br>
> obvious with an explicit event. The event type also has the benefit of<br>
> being able to be posted through the queue, rather than OsBlockSignals -><br>
> ProcessInputEvents -> mangle magic global flag -> QueueKeyboardEvents -><br>
> ProcessKeyboardEvents -> OsUnblockSignals. I'd much rather take the hit of<br>
> tiny extra complexity in the core (really, it's not much at all) rather<br>
> than having all the users get it wrong, or simply not bother because it's<br>
> too hard. Much like they do today.<br>
<br>
</span>You could add that state to the internal event, add a fake_event field to<br>
the DeviceEvent and check that where necessary, otherwise let it pass<br>
through as normal event. That way the penalty for forgetting a check only<br>
matters where you actually care about it.<br>
<br>
That should also reduce the risk of getting odd events.<br></blockquote><div><br></div><div>So to be clear, it would be ET_KeyPress, but DeviceEvent grows an extra field that tells you that really it should be suppressed?</div><div><br></div><div>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.</div><div><br></div><div>Cheers,</div><div>Daniel</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Cheers,<br>
   Peter<br>
<div class="HOEnZb"><div class="h5"><br>
> If you're implacably opposed, then I'll ack the minimal change (once we can<br>
> be certain that the focus is indeed NULL whilst posting those events) for<br>
> 1.17, but I'll also request a revert immediately after master reopens, and<br>
> resubmit this patchset. Your patch does just happen to work, but it's an<br>
> extra whack of magic bonghits right where we least need them.<br>
><br>
> Cheers,<br>
> Daniel<br>
<br>
</div></div></blockquote></div><br></div></div>