gdkkeys-win32: Fix capslock handling
authorPhilip Zander <philip.zander@gmail.com>
Wed, 12 Jan 2022 20:17:28 +0000 (21:17 +0100)
committerLuca Bacci <luca.bacci982@gmail.com>
Wed, 12 Jan 2022 20:43:53 +0000 (21:43 +0100)
Previously, we treated CapsLock and KanaLock as part of the global
keyboard state, much like NumLock and ScrollLock, rather than using
the supplied modifier mask. This was because GDK does not have a
modifier mask for KanaLock, only for CapsLock, so it would not have been
possible to properly support it.

However, this approach ended up causing problems, with certain keyboard
shortcuts not registering when capslock was active. This was first
observed in Inkscape [0] and appears to affect shortcuts consisting of a
single key (like 'a') with no additional modifiers (wheareas shortcuts
like 'ctrl+a' work).

So now we are using the supplied GDK_LOCK_MASK instead, and dropped
support for KanaLock, which we probably don't need anyway (since regular
text input should be handled by the IME input module -- the keymap is
mainly for shortcuts and keybindings, where you don't really want
KanaLock).

[0] https://gitlab.com/inkscape/inkscape/-/issues/3082

gdk/win32/gdkkeys-win32-impl.c
gdk/win32/gdkkeys-win32.c
gdk/win32/gdkkeys-win32.h

index f792c4e24b026f3fe7415edaa9f5598f391f3fd3..4baf9201b362be624baf3295e7a01c267ec21ff4 100644 (file)
@@ -213,7 +213,7 @@ modbits_to_level (GdkWin32KeymapLayoutInfo *info,
 /*
  * vk_to_char_fuzzy:
  *
- * For a given key and keystate, return the best-fit character and the
+ * For a given key and modifier state, return the best-fit character and the
  * modifiers used to produce it. Note that not all modifiers need to be used,
  * because some modifier combination aren't actually mapped in the keyboard
  * layout (for example the Ctrl key typically has no effect, unless used in
@@ -221,26 +221,32 @@ modbits_to_level (GdkWin32KeymapLayoutInfo *info,
  *
  * 'Best-fit' means 'consume as many modifiers as possibe'.
  *
- * For example (assuming a neutral keystate):
+ * For example (assuming a neutral lock state):
  *
+ * - a                -> 'a', consumed_mod_bits: []
  * - Shift + a        -> 'A', consumed_mod_bits: [Shift]
  * - Ctrl + a         -> 'a', consumed_mod_bits: []
  * - Ctrl + Shift + a -> 'A', consumed_mod_bits: [Shift]
  *
  * If capslock is active, the result could be:
  *
- * - Shift + a        -> 'a', consumed_mod_bits: [Shift]
+ * - a                -> 'A', consumed_mod_bits: [Shift]
+ * - Shift + a        -> 'a', consumed_mod_bits: []
+ * - Ctrl + a         -> 'a', consumed_mod_bits: []
+ * - Ctrl + Shift + a -> 'A', consumed_mod_bits: [Shift]
  *
- * The caller can supply additional modifiers to be added to the
- * keystate in `extra_mod_bits`.
+ * The held down modifiers are supplied in `mod_bits` as a bitmask of KBDSHIFT,
+ * KBDCTRL, KBDALT etc.
+ *
+ * The toggled modifiers are supplied in `lock_state` as a bitmask of CAPLOK and KANALOK.
  *
  * If the key combination results in a dead key, `is_dead` will be set to TRUE,
  * otherwise it will be set to FALSE.
  */
 static WCHAR
 vk_to_char_fuzzy (GdkWin32KeymapLayoutInfo *info,
-                  const BYTE                keystate[256],
-                  BYTE                      extra_mod_bits,
+                  BYTE                      mod_bits,
+                  BYTE                      lock_bits,
                   BYTE                     *consumed_mod_bits,
                   gboolean                 *is_dead,
                   BYTE                      vk)
@@ -282,18 +288,12 @@ vk_to_char_fuzzy (GdkWin32KeymapLayoutInfo *info,
 
   if (entry->VirtualKey == vk)
     {
-      BYTE     modbits;
       WCHAR    best_char      = WCH_NONE;
       BYTE     best_modifiers = 0;
       int      best_score     = -1;
       gboolean best_is_dead   = FALSE;
       int      level;
 
-      /* Add modbits of currently pressed keys. */
-      modbits  = keystate_to_modbits (info, keystate);
-      /* Add modbits supplied by caller. */
-      modbits |= extra_mod_bits;
-
       /* Take toggled keys into account. For example, capslock normally inverts the
        * state of KBDSHIFT (with some exceptions). */
 
@@ -302,27 +302,27 @@ vk_to_char_fuzzy (GdkWin32KeymapLayoutInfo *info,
           /* Ignore capslock if any modifiers other than shift are pressed.
            * E.g. on the German layout, CapsLock + AltGr + q is the same as
            * AltGr + q ('@'), but NOT the same as Shift + AltGr + q (not mapped). */
-          !(modbits & ~KBDSHIFT) &&
-         (keystate[VK_CAPITAL] & 0x01))
-        modbits ^= KBDSHIFT;
+          !(mod_bits & ~KBDSHIFT) &&
+          (lock_bits & CAPLOK))
+        mod_bits ^= KBDSHIFT;
 
       /* Key supporting combination of capslock + altgr */
       if ((entry->Attributes & CAPLOKALTGR) &&
-          (modbits & KBDALTGR) &&
-          (keystate[VK_CAPITAL] & 0x01))
-        modbits ^= KBDSHIFT;
+          (mod_bits & KBDALTGR) &&
+          (lock_bits & CAPLOK))
+        mod_bits ^= KBDSHIFT;
 
       /* In the Swiss German layout, CapsLock + key is different from Shift + key
        * for some keys. For such keys, Capslock toggles the KBDCTRL bit. */
       if ((entry->Attributes & SGCAPS) &&
-          (keystate[VK_CAPITAL] & 0x01))
-        modbits ^= KBDCTRL;
+          (lock_bits & CAPLOK))
+        mod_bits ^= KBDCTRL;
 
       /* I'm not totally sure how kanalok behaves, for now I assume that there
        * aren't any special cases. */
       if ((entry->Attributes & KANALOK) &&
-          (keystate[VK_KANA] & 0x01))
-        modbits ^= KBDKANA;
+          (lock_bits & KANALOK))
+        mod_bits ^= KBDKANA;
 
       /* We try to find the entry with the most matching modifiers */
       for (level = 0; level < n_levels; ++level)
@@ -332,7 +332,7 @@ vk_to_char_fuzzy (GdkWin32KeymapLayoutInfo *info,
           WCHAR    c;
           int      score;
 
-          if (candidate_modbits & ~modbits)
+          if (candidate_modbits & ~mod_bits)
               continue;
 
           c = entry->wch[level];
@@ -349,7 +349,7 @@ vk_to_char_fuzzy (GdkWin32KeymapLayoutInfo *info,
           if (c == WCH_DEAD || c == WCH_LGTR || c == WCH_NONE)
             continue;
 
-          score = POPCOUNT (candidate_modbits & modbits);
+          score = POPCOUNT (candidate_modbits & mod_bits);
           if (score > best_score)
             {
               best_score = score;
index a8f976e2ab1c3159714209a7f137e1c2f2418abd..1530b8754839016f4f881f5530d67f0c3da9b08a 100644 (file)
@@ -158,13 +158,13 @@ modbits_to_level (GdkWin32Keymap           *keymap,
 static WCHAR
 vk_to_char_fuzzy (GdkWin32Keymap           *keymap,
                   GdkWin32KeymapLayoutInfo *info,
-                  const BYTE                keystate[256],
-                  BYTE                      extra_mod_bits,
+                  BYTE                      mod_bits,
+                  BYTE                      lock_bits,
                   BYTE                     *consumed_mod_bits,
                   gboolean                 *is_dead,
                   BYTE                      vk)
 {
-  return keymap->gdkwin32_keymap_impl->vk_to_char_fuzzy (info, keystate, extra_mod_bits,
+  return keymap->gdkwin32_keymap_impl->vk_to_char_fuzzy (info, mod_bits, lock_bits,
                                                          consumed_mod_bits, is_dead, vk);
 }
 
@@ -349,8 +349,8 @@ static guint
 vk_and_mod_bits_to_gdk_keysym (GdkWin32Keymap     *keymap,
                                GdkWin32KeymapLayoutInfo *info,
                                guint               vk,
-                               const BYTE          keystate[256],
                                BYTE                mod_bits,
+                               BYTE                lock_bits,
                                BYTE               *consumed_mod_bits)
 
 {
@@ -386,7 +386,7 @@ vk_and_mod_bits_to_gdk_keysym (GdkWin32Keymap     *keymap,
     }
 
   /* Handle regular keys (including dead keys) */
-  c = vk_to_char_fuzzy (keymap, info, keystate, mod_bits,
+  c = vk_to_char_fuzzy (keymap, info, mod_bits, lock_bits,
                         consumed_mod_bits, &is_dead, vk);
 
   if (c == WCH_NONE)
@@ -479,26 +479,6 @@ gdk_mod_mask_to_mod_bits (GdkModifierType mod_mask)
   return result;
 }
 
-static void
-get_lock_state (BYTE lock_state[256])
-{
-  static const guint mode_keys[] =
-    {
-      VK_CAPITAL,
-      VK_KANA, VK_HANGUL, VK_JUNJA, VK_FINAL, VK_HANJA, VK_KANJI, /* Is this correct? */
-      VK_NUMLOCK, VK_SCROLL
-    };
-
-  BYTE keystate[256] = {0};
-  guint i;
-
-  GetKeyboardState (keystate);
-
-  /* Copy over some keystates like numlock and capslock */
-  for (i = 0; i < G_N_ELEMENTS(mode_keys); ++i)
-    lock_state[mode_keys[i]] = keystate[mode_keys[i]] & 0x1;
-}
-
 
 /* keypad decimal mark depends on active keyboard layout
  * return current decimal mark as unicode character
@@ -745,7 +725,6 @@ gdk_win32_keymap_get_entries_for_keyval (GdkKeymap     *gdk_keymap,
                                          GArray        *retval)
 {
   GdkWin32Keymap *keymap;
-  BYTE            keystate[256] = {0};
   int             group;
   guint           len = retval->len;
 
@@ -798,8 +777,7 @@ gdk_win32_keymap_get_entries_for_keyval (GdkKeymap     *gdk_keymap,
               /* Check if the additional modifiers change the semantics.
                * If they do not, add them. */
               sym = vk_and_mod_bits_to_gdk_keysym (keymap, info, entry->vk,
-                                                   keystate, modbits,
-                                                   NULL);
+                                                   modbits, 0, NULL);
               if (sym == keyval || sym == GDK_KEY_VoidSymbol)
                 {
                   gdk_key.keycode = entry->vk;
@@ -827,7 +805,6 @@ gdk_win32_keymap_get_entries_for_keycode (GdkKeymap     *gdk_keymap,
   GArray         *key_array;
   GArray         *keyval_array;
   int             group;
-  BYTE            keystate[256] = {0};
   BYTE            vk;
 
   g_return_val_if_fail (GDK_IS_KEYMAP (gdk_keymap), FALSE);
@@ -864,7 +841,7 @@ gdk_win32_keymap_get_entries_for_keycode (GdkKeymap     *gdk_keymap,
           GdkKeymapKey key              = {0};
           guint        keyval;
 
-          keyval = vk_and_mod_bits_to_gdk_keysym (keymap, info, vk, keystate, modbits, &consumed_modbits);
+          keyval = vk_and_mod_bits_to_gdk_keysym (keymap, info, vk, modbits, 0, &consumed_modbits);
 
           if (keyval == GDK_KEY_VoidSymbol || consumed_modbits != modbits)
             continue;
@@ -899,7 +876,6 @@ gdk_win32_keymap_lookup_key (GdkKeymap          *gdk_keymap,
   GdkWin32Keymap           *keymap;
   GdkWin32KeymapLayoutInfo *info;
 
-  BYTE                      keystate[256] = {0};
   BYTE                      modbits;
   guint                     sym;
 
@@ -915,9 +891,9 @@ gdk_win32_keymap_lookup_key (GdkKeymap          *gdk_keymap,
     return 0;
   if (key->level < 0 || key->level > info->max_level)
     return 0;
-  
+
   modbits = info->level_to_modbits[key->level];
-  sym = vk_and_mod_bits_to_gdk_keysym (keymap, info, key->keycode, keystate, modbits, NULL);
+  sym = vk_and_mod_bits_to_gdk_keysym (keymap, info, key->keycode, modbits, 0, NULL);
 
   if (sym == GDK_KEY_VoidSymbol)
     return 0;
@@ -944,7 +920,7 @@ gdk_win32_keymap_translate_keyboard_state (GdkKeymap       *gdk_keymap,
   GdkWin32KeymapLayoutInfo *layout_info;
   guint                     vk;
   BYTE                      mod_bits;
-  BYTE                      keystate[256] = {0};
+  BYTE                      lock_bits = 0;
 
   g_return_val_if_fail (GDK_IS_KEYMAP (gdk_keymap), FALSE);
 
@@ -968,11 +944,25 @@ gdk_win32_keymap_translate_keyboard_state (GdkKeymap       *gdk_keymap,
   if (vk == VK_RMENU)
     mod_bits &= ~KBDALTGR;
 
-  /* We need to query the existing keyboard state for NumLock, CapsLock etc. */
-  get_lock_state (keystate);
+  /* Translate lock state
+   *
+   * Right now the only locking modifier is CAPSLOCK. We don't handle KANALOK
+   * because GDK doesn't have an equivalent modifier mask to my knowledge (On
+   * X11, I believe the same effect is achieved by shifting to a different
+   * group. It's just a different concept, that doesn't translate to Windows).
+   * But since KANALOK is only used on far-eastern keyboards, which require IME
+   * anyway, this is probably fine. The IME input module has actually been the
+   * default for all languages (not just far-eastern) for a while now, which
+   * means that the keymap is now only used for things like accelerators and
+   * keybindings, where you probably don't even want KANALOK to affect the
+   * translation.
+   */
+
+  if (state & GDK_LOCK_MASK)
+    lock_bits |= CAPLOK;
 
-  tmp_keyval = vk_and_mod_bits_to_gdk_keysym (keymap, layout_info, vk, keystate,
-                                              mod_bits, &consumed_mod_bits);
+  tmp_keyval = vk_and_mod_bits_to_gdk_keysym (keymap, layout_info, vk, mod_bits,
+                                              lock_bits, &consumed_mod_bits);
   tmp_effective_group = group;
   tmp_level = modbits_to_level (keymap, layout_info, consumed_mod_bits);
 
index 17b7d7125c6944723eda2629b2abf2f21e7d0df1..f128996de1b4c969e94c074f6136674f1ce46c45 100644 (file)
@@ -160,8 +160,8 @@ typedef struct
   BYTE      (*modbits_to_level)     (GdkWin32KeymapLayoutInfo *info,
                                      BYTE                      modbits);
   WCHAR     (*vk_to_char_fuzzy)     (GdkWin32KeymapLayoutInfo *info,
-                                     const BYTE                keystate[256],
-                                     BYTE                      extra_mod_bits,
+                                     BYTE                      mod_bits,
+                                     BYTE                      lock_bits,
                                      BYTE                     *consumed_mod_bits,
                                      gboolean                 *is_dead,
                                      BYTE                      vk);