Re: considering modifier keys after gaining focus

[Jon TURNEY <jon dot turney at dronecode dot org dot uk>]

On 09/01/2012 18:11, Oliver Schmidt wrote:
> On 09.01.2012 15:06, Jon TURNEY wrote:
>> The code would seem to end up simpler (which is an important consideration) if
>> we were to modify winKeybdReleaseKeys() not to release modifier keys.  Some
>> archaeology is probably required to determine if releasing the modifier keys
>> in winKeybdReleaseKeys() is necessary to avoid some other undesirable behaviour?
> 
> I don't know anything about the cygwin X server history, I can only
> guess why the current code is as it is: Perhaps the modifier keys are
> released afer loosing a window's focus because if another Non-cygwin
> window gains the focus, no more modifier change events will arrive to
> the cygwin x server.

Then you know as much as I do. I didn't write this code.

Fortunately, the history of the code is preserved both in the VCS, and in
discussions on this mailing list.

I think it is useful to consider this history when reviewing a patch, as there
are a couple of dangers this avoids:

Are we going in circles?  Are we fixing one bug just to re-introduce another
bug which has already been fixed?

Are we doing in the wrong direction? Adding special case on top of special
case, increasing the complexity of the code, is often a sign that something is
wrong with the approach taken.

Anyhow, in a brief look at some mailing list discussions from 2002 or so, it
seems that:

i) We must release modifier keys when focus leaves the X server, as modifier
keys may be part of a Windows shell shortcut which moves the focus elsewhere,
e.g. alt-tab) so the key-release isn't received by the X server.

ii) We must release non-modifier keys when focus leaves the X server, or they
continue to auto-repeat in the X server (specifically a problem when a
key-press closes a window (such as typing ctrl-d or exit into an Xterm), as
the key-release goes to the next window to receive focus, which may not be an
X window)

After your change we will have:

iii) We must press held modifier keys when focus enters the X server (toggling
latching ones as appropriate), so that the future key-presses have the correct
modifiers.

So I guess we should also consider:

iv) What should we do about held non-modifier keys when focus enters the X server?

It looks like these should be pressed as well for strict correctness.  If we
hold down a non-modifier key so it auto-repeats, and move the focus between X
and native windows, the native windows receive repeats, but the the X windows
do not.  I doubt many people care about this behaviour, though :-)

Staring at the code some more, this means that winInitializeModeKeyStates() is
possibly wrong:  It should do the same work as winRestoreModeKeyStates().  If
it's possible to launch the X server with a key held down, that key won't be
noticed until it's pressed and released :-)

>> This maps VK_CONTROL to KEY_LCtrl.  Why not use VK_LCONTROL and VK_RCONTROL,
>> so the generated key-press is for the correct key?
>> Ditto for VK_LSHIFT and VK_RSHIFT
> 
> Perhaps this might improve the patch, however the internal modifier
> state of the xserver has only ShiftMask, LockMask, ControlMask,
> Mod1Mask, Mod2Mask, Mod3Mask, Mod4Mask, and Mod5Mask. So the internal
> state does not distinguish betweem left or right shift key.

Hm... on looking at this again, isn't that code you are adding checking the
internal state of non-latching modifiers bogus?  If we release all keys on
WM_KILLFOCUS, then the non-latching modifiers will always be clear when the
WM_SETFOCUS occurs, so we will always generate the keypress for the modifier.

(This is different to the existing code below for the latching modifiers,
where we generate a keypress and release if necessary to toggle the state of
the latching modifier in the X server to match the actual state)

-- 
Jon TURNEY
Volunteer Cygwin/X X Server maintainer

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://x.cygwin.com/docs/
FAQ:                   http://x.cygwin.com/docs/faq/