Patchwork [4/8] vnc: fix numlock+capslock tracking

login
register
mail settings
Submitter Gerd Hoffmann
Date Jan. 24, 2011, 4:30 p.m.
Message ID <1295886655-32312-5-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/80209/
State New
Headers show

Comments

Gerd Hoffmann - Jan. 24, 2011, 4:30 p.m.
This patch makes the numlock+capslock tracking logic only look at
keydown events.  Without this patch the vnc server will insert
bogous capslock keypress in case it sees the following key sequence:

  shift down --- 'A' down --- shift up  --- 'A' up
                                         ^ here

It doesn't hurt with a PS/2 keyboard, but it disturbs the USB Keyboard.
And with the key event queue just added to the usb keyboard the guest
will actually notice.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Paolo Bonzini - Jan. 28, 2011, 1:36 p.m.
On 01/24/2011 05:30 PM, Gerd Hoffmann wrote:
> This patch makes the numlock+capslock tracking logic only look at
> keydown events.  Without this patch the vnc server will insert
> bogous capslock keypress in case it sees the following key sequence:
>
>    shift down --- 'A' down --- shift up  --- 'A' up
>                                           ^ here
>
> It doesn't hurt with a PS/2 keyboard, but it disturbs the USB Keyboard.
> And with the key event queue just added to the usb keyboard the guest
> will actually notice.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
>   ui/vnc.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 495d6d6..0820d99 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -1504,7 +1504,7 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
>           break;
>       }
>
> -    if (vs->vd->lock_key_sync&&
> +    if (down&&  vs->vd->lock_key_sync&&
>           keycode_is_keypad(vs->vd->kbd_layout, keycode)) {
>           /* If the numlock state needs to change then simulate an additional
>              keypress before sending this one.  This will happen if the user
> @@ -1523,7 +1523,7 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
>           }
>       }
>
> -    if (vs->vd->lock_key_sync&&
> +    if (down&&  vs->vd->lock_key_sync&&
>           ((sym>= 'A'&&  sym<= 'Z') || (sym>= 'a'&&  sym<= 'z'))) {
>           /* If the capslock state needs to change then simulate an additional
>              keypress before sending this one.  This will happen if the user

This should be 1/8 or 2/8 in the series.

Also, perhaps these four could go in 0.14?  The USB device are really 
much more usable (especially the keyboard) with them.

Paolo
Gerd Hoffmann - Jan. 28, 2011, 7:58 p.m.
Hi,

> This should be 1/8 or 2/8 in the series.
>
> Also, perhaps these four could go in 0.14? The USB device are really
> much more usable (especially the keyboard) with them.

IMHO the whole series should go to 0.14, otherwise the remote wakeup 
support added recently will simply not play together with migration.

cheers,
   Gerd
Anthony Liguori - Jan. 28, 2011, 8:11 p.m.
On 01/28/2011 01:58 PM, Gerd Hoffmann wrote:
>   Hi,
>
>> This should be 1/8 or 2/8 in the series.
>>
>> Also, perhaps these four could go in 0.14? The USB device are really
>> much more usable (especially the keyboard) with them.
>
> IMHO the whole series should go to 0.14, otherwise the remote wakeup 
> support added recently will simply not play together with migration.

Yeah, and for something as core as "add migration" support, I'm inclined 
to agree.  I'm amazed this hasn't been a bigger problem to date.

Regards,

Anthony Liguori

> cheers,
>   Gerd
>

Patch

diff --git a/ui/vnc.c b/ui/vnc.c
index 495d6d6..0820d99 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1504,7 +1504,7 @@  static void do_key_event(VncState *vs, int down, int keycode, int sym)
         break;
     }
 
-    if (vs->vd->lock_key_sync &&
+    if (down && vs->vd->lock_key_sync &&
         keycode_is_keypad(vs->vd->kbd_layout, keycode)) {
         /* If the numlock state needs to change then simulate an additional
            keypress before sending this one.  This will happen if the user
@@ -1523,7 +1523,7 @@  static void do_key_event(VncState *vs, int down, int keycode, int sym)
         }
     }
 
-    if (vs->vd->lock_key_sync &&
+    if (down && vs->vd->lock_key_sync &&
         ((sym >= 'A' && sym <= 'Z') || (sym >= 'a' && sym <= 'z'))) {
         /* If the capslock state needs to change then simulate an additional
            keypress before sending this one.  This will happen if the user