Re: considering modifier keys after gaining focus

[Jon TURNEY <jon.turney@dronecode.org.uk>]

On 08/01/2012 15:23, Oliver Schmidt wrote:
> On 8/16/2011 5:31 PM, Oliver Schmidt wrote:
>> I had the problem, that the state of the modifier keys was lost when a
>> window is created (or raised).
>> I send a patch to fix this problem with this email: I just extended the
> 
> I just merged the current official source from xorg-server-1.11.3-1 into
> my local development source and discovered, that my patch for the
> problem "lost modifier key after a new window is created" has not been
> applied.
> 
> Is there a chance, that this patch could be applied to a future version
> or that another solution could be provided to fix this problem? I'm also
> attaching a newer version of the patch with this email.

Thanks very much for the updated patch, and for following up on this, and
apologies for overlooking it.

I have a few questions and comments below:

> Example: in window A Ctrl + some key opens a window B, then in window B
> Ctrl + some other key triggers the next action. However after the opening
> of window B the Ctrl key has to be released and pressed again. If the user
> keeps the Ctrl key holding when the window B is opened, the next key press
> X will be interpreted as X and not as Ctrl+X.

Can you give an example of an application where this causes a problem, so I
can test your patch?

> I send a patch to fix this problem with this email: I just extended the
> function winRestoreModeKeyStates in winkeybd.c to consider not only the
> mode switch key but also the modifiers Ctrl, Shift, Alt/AltGr by using the
> Windows function GetAsyncKeyState.

After your patch, the X server is releasing all keys on WM_KILLFOCUS, then
pressing again some subset of them on WM_SETFOCUS.

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?

This also begs the question why is it only necessary to press some some subset
of the down keys on WM_SETFOCUS?  Does the X server behave correctly if a
non-modifier key is held down while focus moves from one X server window to
another, or from one X server window to a  native window an back?

> diff --git a/cygwin/hw/xwin/winkeybd.c b/cygwin/hw/xwin/winkeybd.c
> index 278342f..a2ac4d0 100644
> --- a/cygwin/hw/xwin/winkeybd.c
> +++ b/cygwin/hw/xwin/winkeybd.c
> @@ -283,6 +283,29 @@ winRestoreModeKeyStates (void)
>     * have a logical XOR operator, so we use a macro instead.
>     */
>  
> +  {
> +    /* consider modifer keys */
> +    
> +    BOOL ctrl   = (GetAsyncKeyState (VK_CONTROL) < 0);
> +    BOOL shift  = (GetAsyncKeyState (VK_SHIFT)   < 0);
> +    BOOL alt    = (GetAsyncKeyState (VK_LMENU)   < 0);
> +    BOOL altgr  = (GetAsyncKeyState (VK_RMENU)   < 0);

Why is is correct to use GetAsyncKeyState() here and not GetKeyState()?  If we
use GetAsyncKeyState() there may be a message pending (See the remarks on
GetKeyState() in MSDN) to change to the key state, so we might conceivably
double the key press?

> +
> +    if (ctrl && altgr) ctrl = FALSE;
> +    
> +    if (WIN_XOR (internalKeyStates & ControlMask, ctrl))
> +      winSendKeyEvent (KEY_LCtrl, ctrl);

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?

> +  
> +    if (WIN_XOR (internalKeyStates & ShiftMask, shift))
> +      winSendKeyEvent (KEY_ShiftL, shift);

Ditto for VK_LSHIFT and VK_RSHIFT

> +  
> +    if (WIN_XOR (internalKeyStates & Mod1Mask, alt))
> +      winSendKeyEvent (KEY_Alt, alt);
> +  
> +    if (WIN_XOR (internalKeyStates & Mod5Mask, altgr))
> +      winSendKeyEvent (KEY_AltLang, altgr);
> +  }
> +
>    /* Has the key state changed? */
>    dwKeyState = GetKeyState (VK_NUMLOCK) & 0x0001;
>    if (WIN_XOR (internalKeyStates & NumLockMask, dwKeyState))
> @@ -328,7 +351,7 @@ winIsFakeCtrl_L (UINT message, WPARAM wParam, LPARAM lParam)
>    MSG		msgNext;
>    LONG		lTime;
>    Bool		fReturn;
> -
> +  
>    static Bool   lastWasControlL = FALSE;
>    static UINT   lastMessage;
>    static LONG   lastTime;

--
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/