public inbox for cygwin-xfree@sourceware.org
help / color / mirror / Atom feed
* considering modifier keys after gaining focus
@ 2011-08-16 15:31 Oliver Schmidt
  2011-08-21  8:44 ` Corinna Vinschen
  2012-01-08 15:23 ` Oliver Schmidt
  0 siblings, 2 replies; 11+ messages in thread
From: Oliver Schmidt @ 2011-08-16 15:31 UTC (permalink / raw)
  To: cygwin-xfree

[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]

Hi,

I had the problem, that the state of the modifier keys was lost when a 
window is created (or raised).

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.

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.

This patch works fine for me.

However one problem is unsolved: if the key combination for opening 
window B (in the above example) is an AltGr key combination, the 
GetAsyncKeyState will also report, that the Ctrl key is pressed, which 
is not true, since this is the well known Windows fake Ctrl_L :-(

Any suggestions how to solve this?

Best regards,
Oliver

[-- Attachment #2: diff03.txt --]
[-- Type: text/plain, Size: 1675 bytes --]

diff --git a/cygwin/hw/xwin/winkeybd.c b/cygwin/hw/xwin/winkeybd.c
index 9e5a9b0..e807fc5 100644
--- a/cygwin/hw/xwin/winkeybd.c
+++ b/cygwin/hw/xwin/winkeybd.c
@@ -255,6 +255,7 @@ void
 winRestoreModeKeyStates (void)
 {
   DWORD			dwKeyState;
+  BOOL			modifierPressed;
   BOOL			processEvents = TRUE;
   unsigned short	internalKeyStates;
 
@@ -282,6 +283,34 @@ winRestoreModeKeyStates (void)
    * have a logical XOR operator, so we use a macro instead.
    */
 
+  modifierPressed = (GetAsyncKeyState (VK_CONTROL) < 0);
+  if (WIN_XOR (internalKeyStates & ControlMask, modifierPressed))
+    {
+      if (modifierPressed) winSendKeyEvent (KEY_LCtrl, TRUE);
+      else                 winSendKeyEvent (KEY_LCtrl, FALSE);
+    }
+
+  modifierPressed = (GetAsyncKeyState (VK_SHIFT) < 0);
+  if (WIN_XOR (internalKeyStates & ShiftMask, modifierPressed))
+    {
+      if (modifierPressed) winSendKeyEvent (KEY_ShiftL, TRUE);
+      else                 winSendKeyEvent (KEY_ShiftL, FALSE);
+    }
+
+  modifierPressed = (GetAsyncKeyState (VK_LMENU) < 0);
+  if (WIN_XOR (internalKeyStates & Mod1Mask, modifierPressed))
+    {
+      if (modifierPressed) winSendKeyEvent (KEY_Alt, TRUE);
+      else                 winSendKeyEvent (KEY_Alt, FALSE);
+    }
+
+  modifierPressed = (GetAsyncKeyState (VK_RMENU) < 0);
+  if (WIN_XOR (internalKeyStates & Mod5Mask, modifierPressed))
+    {
+      if (modifierPressed) winSendKeyEvent (KEY_AltLang, TRUE);
+      else                 winSendKeyEvent (KEY_AltLang, FALSE);
+    }
+
   /* Has the key state changed? */
   dwKeyState = GetKeyState (VK_NUMLOCK) & 0x0001;
   if (WIN_XOR (internalKeyStates & NumLockMask, dwKeyState))


[-- Attachment #3: Type: text/plain, Size: 223 bytes --]

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: considering modifier keys after gaining focus
  2011-08-16 15:31 considering modifier keys after gaining focus Oliver Schmidt
@ 2011-08-21  8:44 ` Corinna Vinschen
  2011-08-21 10:04   ` Oliver Schmidt
  2012-01-08 15:23 ` Oliver Schmidt
  1 sibling, 1 reply; 11+ messages in thread
From: Corinna Vinschen @ 2011-08-21  8:44 UTC (permalink / raw)
  To: cygwin-xfree

On Aug 16 17:31, Oliver Schmidt wrote:
> Hi,
> 
> I had the problem, that the state of the modifier keys was lost when
> a window is created (or raised).
> 
> 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.
> 
> 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.
> 
> This patch works fine for me.
> 
> However one problem is unsolved: if the key combination for opening
> window B (in the above example) is an AltGr key combination, the
> GetAsyncKeyState will also report, that the Ctrl key is pressed,
> which is not true, since this is the well known Windows fake Ctrl_L
> :-(
> 
> Any suggestions how to solve this?

At that time, doesn't GetAsyncKeyState (VK_RMENU) also return < 0?
So, shouldn't something along these lines do the trick:

  BOOL ctrl = (GetAsyncKeyState (VK_CONTROL) < 0);
  BOOL shift = (GetAsyncKeyState (VK_CONTROL) < 0);
  BOOL alt = (GetAsyncKeyState (VK_CONTROL) < 0);
  BOOL altlang = (GetAsyncKeyState (VK_CONTROL) < 0);
  if (ctrl & altlang)
    ctrl = FALSE;
  if (WIN_XOR (internalKeyStates & ControlMask, ctrl)
    winSendKeyEvent (KEY_LCtrl, ctrl);
  if (WIN_XOR (internalKeyStates & ShiftMask, shift))
    winSendKeyEvent (KEY_ShiftL, shift);
  if (WIN_XOR (internalKeyStates & Mod1Mask, alt))
    winSendKeyEvent (KEY_Alt, alt);
  if (WIN_XOR (internalKeyStates & Mod5Mask, altlang))
    winSendKeyEvent (KEY_AltLang, altlang);


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: considering modifier keys after gaining focus
  2011-08-21  8:44 ` Corinna Vinschen
@ 2011-08-21 10:04   ` Oliver Schmidt
  0 siblings, 0 replies; 11+ messages in thread
From: Oliver Schmidt @ 2011-08-21 10:04 UTC (permalink / raw)
  To: cygwin-xfree

Hi Corinna,

On 8/21/2011 10:43 AM, Corinna Vinschen wrote:
>> However one problem is unsolved: if the key combination for opening
>> window B (in the above example) is an AltGr key combination, the
>> GetAsyncKeyState will also report, that the Ctrl key is pressed,
>> which is not true, since this is the well known Windows fake Ctrl_L
> 
> So, shouldn't something along these lines do the trick:
>   if (ctrl && altlang)
>     ctrl = FALSE;

thanks! I tried your suggestion and now it is nearly perfect ;-)

Only remaining drawback is now, that Ctrl+AltGr key kombinations still
don't work when invoking/raising top level windows, but everything else
is now working flawless: Ctrl, Shift+Ctrl, Alt, AltGr, Shift+Alt,
Shift+AltGr etc. ;-) I think one has to live with the restriction that
Ctrl+AltrGr doesn't work under Windows (BTW it works under Linux
xserver, wheres Alt+AltGr works neither under Linux nor Windows).

Best regards,
Oliver

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: considering modifier keys after gaining focus
  2011-08-16 15:31 considering modifier keys after gaining focus Oliver Schmidt
  2011-08-21  8:44 ` Corinna Vinschen
@ 2012-01-08 15:23 ` Oliver Schmidt
  2012-01-09 14:06   ` Jon TURNEY
  1 sibling, 1 reply; 11+ messages in thread
From: Oliver Schmidt @ 2012-01-08 15:23 UTC (permalink / raw)
  To: cygwin-xfree

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

Hi,

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.

Best regards,
Oliver


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1399 bytes --]

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);
+
+    if (ctrl && altgr) ctrl = FALSE;
+    
+    if (WIN_XOR (internalKeyStates & ControlMask, ctrl))
+      winSendKeyEvent (KEY_LCtrl, ctrl);
+  
+    if (WIN_XOR (internalKeyStates & ShiftMask, shift))
+      winSendKeyEvent (KEY_ShiftL, shift);
+  
+    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;


[-- Attachment #3: Type: text/plain, Size: 223 bytes --]

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: considering modifier keys after gaining focus
  2012-01-08 15:23 ` Oliver Schmidt
@ 2012-01-09 14:06   ` Jon TURNEY
  2012-01-09 18:11     ` Oliver Schmidt
  0 siblings, 1 reply; 11+ messages in thread
From: Jon TURNEY @ 2012-01-09 14:06 UTC (permalink / raw)
  To: cygwin-xfree; +Cc: oschmidt-mailinglists

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/


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: considering modifier keys after gaining focus
  2012-01-09 14:06   ` Jon TURNEY
@ 2012-01-09 18:11     ` Oliver Schmidt
  2012-01-10  9:47       ` Oliver Schmidt
  2012-01-11 17:16       ` Jon TURNEY
  0 siblings, 2 replies; 11+ messages in thread
From: Oliver Schmidt @ 2012-01-09 18:11 UTC (permalink / raw)
  To: cygwin-xfree

Hi Jon,

On 09.01.2012 15:06, Jon TURNEY wrote:
> 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?

because current cygwin x-server is not able to raise windows, you could
only test the case, that a new window is created. Any X11 program that
can create windows and has keyboard shortcuts will do it, e.g. take
NEdit, press Ctrl+N for a new window, hold the Ctrl key and press S
(i.e. Ctrl+S) for saving.

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

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

My code simply updates the xserver's internal state about the modifier
keys after gaining the window focus. Other keys behave correctly,
because the xserver doesn't have an internal state for them.

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

I will try using GetKeyState tomorrow. I just wanted to be sure to get
the current key state when the window gets the focus.

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

Best regards,
Oliver

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: considering modifier keys after gaining focus
  2012-01-09 18:11     ` Oliver Schmidt
@ 2012-01-10  9:47       ` Oliver Schmidt
  2012-01-10 10:15         ` Oliver Schmidt
  2012-01-11 17:16       ` Jon TURNEY
  1 sibling, 1 reply; 11+ messages in thread
From: Oliver Schmidt @ 2012-01-10  9:47 UTC (permalink / raw)
  To: cygwin-xfree

Hi,

On 1/9/2012 7:11 PM, Oliver Schmidt wrote:
> NEdit, press Ctrl+N for a new window, hold the Ctrl key and press S
> (i.e. Ctrl+S) for saving.

For the above NEdit example you have to disable the entry "Preferences /
Default Settings / Tabbed Editing / Open File In New Tab" from NEdit's
menu bar. Otherwise Ctrl+N will not open a new window.

Of course every keyboard driven program that opens new windows with
keyboard shortcuts will have problems with lost modifier key state.
Moreover, if using my patch for "programmatically raising top level
windows" ( http://cygwin.com/ml/cygwin-xfree/2011-08/msg00034.html ) the
problem also occurs if a application raises windows by keyboard shortcuts.

> I will try using GetKeyState tomorrow. I just wanted to be sure to get
> the current key state when the window gets the focus.

Ok, I tried using "GetKeyState" instead of "GetAsyncKeyState" and it
also works. So it seems to be a good idea to switch to "GetKeyState".

Best regards,
Oliver

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: considering modifier keys after gaining focus
  2012-01-10  9:47       ` Oliver Schmidt
@ 2012-01-10 10:15         ` Oliver Schmidt
  0 siblings, 0 replies; 11+ messages in thread
From: Oliver Schmidt @ 2012-01-10 10:15 UTC (permalink / raw)
  To: cygwin-xfree

It's me again ;-)

On 1/10/2012 10:47 AM, Oliver Schmidt wrote:
> Ok, I tried using "GetKeyState" instead of "GetAsyncKeyState" and it
> also works. So it seems to be a good idea to switch to "GetKeyState".

My answer was too fast: unfortunately I got problems using "GetKeyState"
instead of "GetAsyncKeyState": I had hanging Alt-Keys if switching into
a x11 window using alt+tab when the mouse was already in the window
before the alt+tab keypress. So I switched back to using
"GetAsyncKeyState" and everything now works again as it should.

So after my experience we should use "GetAsyncKeyState": this works for
me without any problems since August 2011 and I'm using this 8 hours
every day, doing a lot of keyboard stuff and opening, raising and
closing X11 windows frequently using the keyboard.

Best regards,
Oliver

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: considering modifier keys after gaining focus
  2012-01-09 18:11     ` Oliver Schmidt
  2012-01-10  9:47       ` Oliver Schmidt
@ 2012-01-11 17:16       ` Jon TURNEY
  2012-01-12 12:19         ` Oliver Schmidt
  1 sibling, 1 reply; 11+ messages in thread
From: Jon TURNEY @ 2012-01-11 17:16 UTC (permalink / raw)
  To: cygwin-xfree; +Cc: oschmidt-mailinglists

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/


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: considering modifier keys after gaining focus
  2012-01-11 17:16       ` Jon TURNEY
@ 2012-01-12 12:19         ` Oliver Schmidt
  2012-01-12 12:38           ` Oliver Schmidt
  0 siblings, 1 reply; 11+ messages in thread
From: Oliver Schmidt @ 2012-01-12 12:19 UTC (permalink / raw)
  To: cygwin-xfree

Hi Jon,

On 1/11/2012 6:16 PM, Jon TURNEY wrote:
> I think it is useful to consider this history when reviewing a patch, 
> Are we going in circles?  Are we doing in the wrong direction? 

I appreciate your carefulness und thoroughness. It's of course always
better to understand what is going on, especially in a large code base
with a long history.


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

Ah ok, so my guess was in the right direction ;-)


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

Interesting, I didn't know this. Thank you for figuring this out from
the malinglists archives.


> 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 :-)

Yes, you are right: I can reproduce this phenomenon by holding down
Ctrl+N for opening windows and the key is not autorepeated (so only one
window is opened, whereas under Linux xserver many windows are opened).

In my daily usage I didn't discover this phenomenon. My patch only
addresses the problem, that the modifier keys are not right after
keyboard driven focus change, disrupting my workflow. So I agree that
there might not many people caring about this behaviour.


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

Yes, my patch also generates key release events for modifiers despite
the fact, that all modifier have been released after the xserver looses
the window focus. When writing the patch, I wasn't sure if this is
always the case, so I made the code a little bit more "robust" in the
sense that it tries to correct the modifier keys in any case (so it will
always work, even if something goes wrong in other code locations).

Best regards,
Oliver

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: considering modifier keys after gaining focus
  2012-01-12 12:19         ` Oliver Schmidt
@ 2012-01-12 12:38           ` Oliver Schmidt
  0 siblings, 0 replies; 11+ messages in thread
From: Oliver Schmidt @ 2012-01-12 12:38 UTC (permalink / raw)
  To: cygwin-xfree


On 1/12/2012 1:19 PM, Oliver Schmidt wrote:
> On 1/11/2012 6:16 PM, Jon TURNEY wrote:
>> 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.
> 
> Yes, my patch also generates key release events for modifiers despite
> the fact, that all modifier have been released after the xserver looses
> the window focus. When writing the patch, I wasn't sure if this is
> always the case, so I made the code a little bit more "robust" in the
> sense that it tries to correct the modifier keys in any case (so it will
> always work, even if something goes wrong in other code locations).

My answer is perhaps a little bit unexact. To be more precise:

+    BOOL ctrl   = (GetAsyncKeyState (VK_CONTROL) < 0);
+    if (WIN_XOR (internalKeyStates & ControlMask, ctrl))
+      winSendKeyEvent (KEY_LCtrl, ctrl);

The above code fragment will send a key press event for the ctrl key, if
the current "real" ctrl modifier state is pressed and differs from the
xserver's internal key state for ControlMask.

It will send a key release event for the ctrl key, if the current "real"
ctrl modifier state is "not pressed" and differs from the xserver's
internal key state for ControlMask.

Best regards,
Oliver

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2012-01-12 12:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-16 15:31 considering modifier keys after gaining focus Oliver Schmidt
2011-08-21  8:44 ` Corinna Vinschen
2011-08-21 10:04   ` Oliver Schmidt
2012-01-08 15:23 ` Oliver Schmidt
2012-01-09 14:06   ` Jon TURNEY
2012-01-09 18:11     ` Oliver Schmidt
2012-01-10  9:47       ` Oliver Schmidt
2012-01-10 10:15         ` Oliver Schmidt
2012-01-11 17:16       ` Jon TURNEY
2012-01-12 12:19         ` Oliver Schmidt
2012-01-12 12:38           ` Oliver Schmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).