<div dir="ltr">Hi,<div>Sorry for the long absence from this thread.<br><div class="gmail_extra"><br><div class="gmail_quote">On 12 November 2014 20:06, Bill Spitzak <span dir="ltr"><<a href="mailto:spitzak@gmail.com" target="_blank">spitzak@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On 11/12/2014 03:15 AM, Pekka Paalanen wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I don't get this behaviour. The xterm gets Ctrl+d, interprets it as<br>
EOF, and causes it eventually exit.<br>
</blockquote>
<br></span>
That does not happen to me. My xterm seems to ignore the fact that 'd' is held down until Ctrl is released, at which point it starts producing the 'd'.<br>
<br>
However it does not matter.<br>
<br>
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!!!!<br>
<br>
Apparently if Ctrl+D was a "compositor shortcut" to close a window, then this patch will cause the 'd' to not be there. But since Ctrl+D is handled by a client to close the window, the client with the new focus *does* see it. WHY IS THIS DIFFERENT??????<br></blockquote><div><br></div><div>In a fairly shock twist of events, I actually agree with Bill here. Part of the problem is wl_keyboard's, er, rather sparse documentation, for which my many apologies. It's sitting on my TODO for after I clear out my patch review backlog.</div><div><br></div><div>wl_keyboard::key is fairly obvious: a key was pressed, it was directed at you, so please take any action required by this (printing keys, running shortcuts). So, any key event received here will update the internal state, and trigger actions.</div><div><br></div><div>wl_keyboard::enter, OTOH, is strictly advisory: you've got the focus now, oh and by the way here's the state of the keyboard which changed whilst you were away. Any key event received here will update internal state, but _not_ trigger actions.</div><div><br></div><div>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.</div><div><br></div><div>This also matches exactly the new semantics of notify_keyboard_focus_in(), in Pekka's last mail above. Why should the compositor be different from any other client, especially when the compositor _is_ a client itself?</div><div><br></div><div>If you think about it, the notify_keyboard_focus_in() semantics are the only ones which makes sense. Say you trigger Exposay or something on a raw Ctrl+Alt press, and switch VTs on Ctrl+Alt+Fx. The user presses Ctrl+Alt+F2 to focus the compositor on VT 2, and that binding is triggered on press. We'll land on that compositor, which will see Ctrl+Alt+F2 down on its focus event. Running keybindings would make it switch VTs over to itself in a tight loop, so obviously we don't want to do that. But if we release F2 and press F3, we want the Ctrl+Alt+F3 binding (switch to VT 3) to fire: so again, we're using the focus events to update state, nothing else.</div><div><br></div><div>Looking at what we have now:</div><div>  - notify_keyboard_focus_in runs bindings - as above, this is a bug</div><div>  - compositor-drm does the right thing: in evdev_notify_keyboard_focus, it gets the current state of keys down from evdev, and calls notify_keyboard_focus_in, and not notify_key - this is the right thing to do, modulo notify_keyboard_focus_in running bindings</div><div>  - compositor-x11 also does the right thing: in the XCB_FOCUS_IN/XCB_KEYMAP_NOTIFY handler, it pulls the list of currently-pressed keys, and calls notify_keyboard_focus_in with them - same caveat about notifykeyboard_focus_in running bindings</div><div>  - compositor-wayland does the right thing: in the keyboard_enter handler, it calls notify_keyboard_focus_in with the list of keys down - same caveat</div><div>  - XWayland does the wrong thing: in the keyboard_enter handler, it queues KeyPress events for every key listed as down</div><div><br></div><div>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.</div><div><br></div><div>The problem with fixing XWayland is that we don't have a particularly good mechanism for these kinds of focus notifications, since it's really just not designed to be nested. I've attached a couple of putative patches which would do the right thing in XWayland; unfortunately, XWayland isn't playing nice for me at the moment, so these are wildly untested.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
IMHO they should not be different, therefore this patch is wrong. There are two possible solutions:<br>
<br>
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.<br>
<br>
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.</blockquote><div><br></div><div>I'd like to see this reverted and XWayland fixed.</div><div><br></div><div>Sorry for the drive-by IRC review that may have gave the impression I actually supported this overall change, rather than just commented on one isolated question about whether or not focus_in should run bindings. I've been meaning to get to this thread for a little while, but sometimes these threads make me question my will to live.</div><div><br></div><div>Cheers,</div><div>Daniel </div></div></div></div></div>