keymap: fix keyup mappings

It is possible that the modifier state on keyup is different from the
modifier state on keydown.  In that case the keycode lookup can end up
with different keys in case multiple keysym -> keycode mappings exist,
because it picks the mapping depending on modifier state.

To fix that change the lookup logic for keyup events.  Instead of
looking at the modifier state check the key state and prefer a keycodes
where the key is in "down" state right now.

Fixes: abb4f2c965 keymap: consider modifier state when picking a mapping
Buglink: https://bugs.launchpad.net/bugs/1738283
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1658676
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20190122092814.14919-9-kraxel@redhat.com

diff --git a/ui/curses.c b/ui/curses.c
index 54687589d4076574130b58e277b9bdf786caf5f6..6e0091c3b2865ecf05549083ca604c9b910fa2da 100644 (file)
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -273,7 +273,7 @@ static void curses_refresh(DisplayChangeListener *dcl)
             }
 
             keycode = keysym2scancode(kbd_layout, keysym & KEYSYM_MASK,
-                                      NULL);
+                                      NULL, false);
             if (keycode == 0)
                 continue;
 

diff --git a/ui/keymaps.c b/ui/keymaps.c
index c8b21353405344f364bb773b6094d3f8c2450ae4..544b55c27b16e330c296816fb0be12218143be38 100644 (file)
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -28,6 +28,7 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
+#include "ui/input.h"
 
 struct keysym2code {
     uint32_t count;
@@ -188,7 +189,7 @@ kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
 
 
 int keysym2scancode(kbd_layout_t *k, int keysym,
-                    QKbdState *kbd)
+                    QKbdState *kbd, bool down)
 {
     static const uint32_t mask =
         SCANCODE_SHIFT | SCANCODE_ALTGR | SCANCODE_CTRL;
@@ -212,27 +213,39 @@ int keysym2scancode(kbd_layout_t *k, int keysym,
         return keysym2code->keycodes[0];
     }
 
-    /*
-     * We have multiple keysym -> keycode mappings.
-     *
-     * Check whenever we find one mapping where the modifier state of
-     * the mapping matches the current user interface modifier state.
-     * If so, prefer that one.
-     */
-    mods = 0;
-    if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_SHIFT)) {
-        mods |= SCANCODE_SHIFT;
-    }
-    if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_ALTGR)) {
-        mods |= SCANCODE_ALTGR;
-    }
-    if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_CTRL)) {
-        mods |= SCANCODE_CTRL;
-    }
+    /* We have multiple keysym -> keycode mappings. */
+    if (down) {
+        /*
+         * On keydown: Check whenever we find one mapping where the
+         * modifier state of the mapping matches the current user
+         * interface modifier state.  If so, prefer that one.
+         */
+        mods = 0;
+        if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_SHIFT)) {
+            mods |= SCANCODE_SHIFT;
+        }
+        if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_ALTGR)) {
+            mods |= SCANCODE_ALTGR;
+        }
+        if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_CTRL)) {
+            mods |= SCANCODE_CTRL;
+        }
 
-    for (i = 0; i < keysym2code->count; i++) {
-        if ((keysym2code->keycodes[i] & mask) == mods) {
-            return keysym2code->keycodes[i];
+        for (i = 0; i < keysym2code->count; i++) {
+            if ((keysym2code->keycodes[i] & mask) == mods) {
+                return keysym2code->keycodes[i];
+            }
+        }
+    } else {
+        /*
+         * On keyup: Try find a key which is actually down.
+         */
+        for (i = 0; i < keysym2code->count; i++) {
+            QKeyCode qcode = qemu_input_key_number_to_qcode
+                (keysym2code->keycodes[i]);
+            if (kbd && qkbd_state_key_get(kbd, qcode)) {
+                return keysym2code->keycodes[i];
+            }
         }
     }
     return keysym2code->keycodes[0];

diff --git a/ui/keymaps.h b/ui/keymaps.h
index d8652b8a5a005c4548539be9d1ea9b8daa347fde..b6d48aac403d79e0c3f35bdafaab4fc69baaf61b 100644 (file)
--- a/ui/keymaps.h
+++ b/ui/keymaps.h
@@ -56,7 +56,7 @@ typedef struct kbd_layout_t kbd_layout_t;
 kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
                                    const char *language, Error **errp);
 int keysym2scancode(kbd_layout_t *k, int keysym,
-                    QKbdState *kbd);
+                    QKbdState *kbd, bool down);
 int keycode_is_keypad(kbd_layout_t *k, int keycode);
 int keysym_is_numlock(kbd_layout_t *k, int keysym);
 

diff --git a/ui/vnc.c b/ui/vnc.c
index f66ed53094f3867e118291aa5d4e7d4a61d8413b..0fef646fc4898aecdc4eda26edc349392c9b6b94 100644 (file)
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1981,7 +1981,7 @@ static void key_event(VncState *vs, int down, uint32_t sym)
     }
 
     keycode = keysym2scancode(vs->vd->kbd_layout, lsym & 0xFFFF,
-                              vs->vd->kbd) & SCANCODE_KEYMASK;
+                              vs->vd->kbd, down) & SCANCODE_KEYMASK;
     trace_vnc_key_event_map(down, sym, keycode, code2name(keycode));
     do_key_event(vs, down, keycode, sym);
 }