Fix GlobalKeyMap matching for keyboards with layout-dependent charactersIgnoringModifiers

ModernTests/iTermKeystrokeBindingTests.swift

@@ -0,0 +1,114 @@
+//
+//  iTermKeystrokeBindingTests.swift
+//  iTerm2
+//
+//  Test for key binding matching when charactersIgnoringModifiers differs
+//  from the recorded binding due to keyboard layout / input method state.
+//
+
+import XCTest
+@testable import iTerm2SharedARC
+
+final class iTermKeystrokeBindingTests: XCTestCase {
+
+    /// kVK_ANSI_Grave = 0x32 (50), the physical backtick/tilde key on US keyboards.
+    /// On Chinese/Pinyin layouts the same physical key reports a different
+    /// charactersIgnoringModifiers value (0xb7 = middle dot ·) in some input
+    /// modes and 0x60 (backtick `) in others.
+    private let graveKeyCode: Int32 = 0x32
+    private let cmdModifier: Int64 = 0x100000  // NSEventModifierFlagCommand
+
+    // MARK: - Binding created with Chinese-layout character (0xb7)
+
+    func testBindingRecordedWithChineseCharMatchesUSCharPress() {
+        // Simulate binding created when keyboard reports 0xb7 for the backtick key.
+        let bindingKey = "0xb7-0x100000-0x32"
+        let bindingValue: NSDictionary = [
+            "Version": 2,
+            "Apply Mode": 0,
+            "Action": 30,
+            "Text": "",
+            "Escaping": 0
+        ]
+        let dict: NSDictionary = [bindingKey: bindingValue]
+
+        // Simulate the same key pressed later when charactersIgnoringModifiers
+        // returns 0x60 (standard US backtick) — e.g. after text input changed
+        // the input method state.
+        let keystroke = iTermKeystroke(
+            virtualKeyCode: Int(graveKeyCode),
+            hasKeyCode: true,
+            modifierFlags: UInt(cmdModifier),
+            character: 0x60,         // US backtick character
+            modifiedCharacter: 0x60
+        )
+
+        let foundKey = keystroke.keyInBindingDictionary(dict as! [String: NSDictionary])
+
+        // Before the fix this returned nil because the serialized key
+        // 0x60-0x100000-0x32 did not match the stored 0xb7-0x100000-0x32.
+        // After the fix the portableSerialized fallback (*-0x100000-0x32)
+        // matches regardless of the character field.
+        XCTAssertNotNil(foundKey,
+            "Keystroke (char=0x60) should match binding (char=0xb7) " +
+            "via portableSerialized fallback. If this fails, the " +
+            "language-agnostic fallback in keyInBindingDictionary: is broken.")
+        XCTAssertEqual(foundKey, bindingKey)
+    }
+
+    // MARK: - Exact match still works (regression guard)
+
+    func testExactMatchStillWorks() {
+        let bindingKey = "0xb7-0x100000-0x32"
+        let dict: NSDictionary = [bindingKey: ["Action": 30]]
+
+        let keystroke = iTermKeystroke(
+            virtualKeyCode: Int(graveKeyCode),
+            hasKeyCode: true,
+            modifierFlags: UInt(cmdModifier),
+            character: 0xb7,
+            modifiedCharacter: 0xb7
+        )
+
+        XCTAssertEqual(keystroke.keyInBindingDictionary(dict as! [String: NSDictionary]),
+                       bindingKey)
+    }
+
+    // MARK: - Different virtual key code should NOT match
+
+    func testDifferentKeyCodeDoesNotMatch() {
+        let bindingKey = "0xb7-0x100000-0x32"
+        let dict: NSDictionary = [bindingKey: ["Action": 30]]
+
+        // Same character and modifiers, but different physical key (kVK_ANSI_1 = 0x12)
+        let keystroke = iTermKeystroke(
+            virtualKeyCode: 0x12,
+            hasKeyCode: true,
+            modifierFlags: UInt(cmdModifier),
+            character: 0x60,
+            modifiedCharacter: 0x60
+        )
+
+        XCTAssertNil(keystroke.keyInBindingDictionary(dict as! [String: NSDictionary]),
+                     "Different virtual key code should never match")
+    }
+
+    // MARK: - Legacy binding (no virtual key code) still works
+
+    func testLegacyBindingWithoutKeyCodeStillWorks() {
+        // Binding stored without virtual key code component
+        let bindingKey = "0xb7-0x100000"
+        let dict: NSDictionary = [bindingKey: ["Action": 30]]
+
+        let keystroke = iTermKeystroke(
+            virtualKeyCode: Int(graveKeyCode),
+            hasKeyCode: true,
+            modifierFlags: UInt(cmdModifier),
+            character: 0xb7,
+            modifiedCharacter: 0xb7
+        )
+
+        XCTAssertEqual(keystroke.keyInBindingDictionary(dict as! [String: NSDictionary]),
+                       bindingKey)
+    }
+}

sources/Keyboard/iTermKeystroke.m

@@ -369,6 +369,21 @@ - (NSString *)keyInBindingDictionary:(NSDictionary<NSString *, NSDictionary *> *
             }
         }
     }
+    // Language-agnostic fallback: match on virtual key code + modifiers only,
+    // ignoring the character. On some keyboard layouts (e.g., Chinese/Pinyin)
+    // charactersIgnoringModifiers can vary depending on input method state,
+    // causing the key to be unrecognized after text input.
+    if (self.hasVirtualKeyCode) {
+        @autoreleasepool {
+            NSString *myPortable = self.portableSerialized;
+            for (NSString *key in dict) {
+                iTermKeystroke *candidate = [[iTermKeystroke alloc] initWithString:key];
+                if ([candidate.portableSerialized isEqualToString:myPortable]) {
+                    return key;
+                }
+            }
+        }
+    }
     return nil;
 }
 

tests/reproduce_keystroke_bug.py

@@ -0,0 +1,145 @@
+"""
+1:1 reproduction of iTermKeystroke.keyInBindingDictionary: matching logic.
+
+This script mirrors the exact serialization and matching algorithm from
+iTermKeystroke.m (lines 286-372), demonstrating the bug and the fix.
+"""
+
+# --- Serialization (from iTermKeystroke.m) ---
+
+def serialized(char, mods, keycode):
+    """0x<char>-0x<mods>-0x<keycode> — the primary key format."""
+    return f"0x{char:x}-0x{mods:x}-0x{keycode:x}"
+
+def legacy_serialized(char, mods):
+    """0x<char>-0x<mods> — without virtual keycode."""
+    return f"0x{char:x}-0x{mods:x}"
+
+def portable_serialized(mods, keycode):
+    """*-0x<mods>-0x<keycode> — language-agnostic, keycode only."""
+    return f"*-0x{mods:x}-0x{keycode:x}"
+
+
+# --- Matching BEFORE fix (from iTermKeystroke.m lines 351-372) ---
+
+def key_in_binding_dict_before(char, mods, keycode, binding_dict):
+    """Simulates the original matching logic without portable fallback."""
+    s = serialized(char, mods, keycode)
+    ls = legacy_serialized(char, mods)
+
+    # 1. Exact match
+    if s in binding_dict:
+        return s
+
+    # 2. Legacy match (no keycode)
+    if ls in binding_dict:
+        return ls
+
+    # 3. Slow fallback: iterate keys looking for legacy match
+    for key in binding_dict:
+        # Try to parse key as char-mods-keycode and compare legacy form
+        parts = key.split("-")
+        if len(parts) == 3:
+            cand_char = int(parts[0], 16)
+            cand_mods = int(parts[1], 16)
+            if legacy_serialized(cand_char, cand_mods) == ls:
+                return key
+        elif len(parts) == 2:
+            if key == ls:
+                return key
+
+    return None
+
+
+# --- Matching AFTER fix (with portable fallback) ---
+
+def key_in_binding_dict_after(char, mods, keycode, binding_dict):
+    """Simulates the fixed matching logic WITH portable fallback."""
+    s = serialized(char, mods, keycode)
+    ls = legacy_serialized(char, mods)
+
+    # 1. Exact match
+    if s in binding_dict:
+        return s
+
+    # 2. Legacy match (no keycode)
+    if ls in binding_dict:
+        return ls
+
+    # 3. Slow fallback: iterate keys looking for legacy match
+    for key in binding_dict:
+        parts = key.split("-")
+        if len(parts) == 3:
+            cand_char = int(parts[0], 16)
+            cand_mods = int(parts[1], 16)
+            if legacy_serialized(cand_char, cand_mods) == ls:
+                return key
+        elif len(parts) == 2:
+            if key == ls:
+                return key
+
+    # 4. NEW: Language-agnostic fallback on virtual key code
+    if True:  # hasVirtualKeyCode
+        my_portable = portable_serialized(mods, keycode)
+        for key in binding_dict:
+            parts = key.split("-")
+            if len(parts) == 3:
+                cand_mods = int(parts[1], 16)
+                cand_keycode = int(parts[2], 16)
+                if portable_serialized(cand_mods, cand_keycode) == my_portable:
+                    return key
+
+    return None
+
+
+# --- Test scenario ---
+
+CMD = 0x100000          # NSEventModifierFlagCommand
+GRAVE_KEY = 0x32        # kVK_ANSI_Grave (physical backtick key)
+CHINESE_CHAR = 0xb7     # charactersIgnoringModifiers on Chinese layout (·)
+US_CHAR = 0x60          # charactersIgnoringModifiers on US layout (`)
+
+# The binding stored in GlobalKeyMap — created when keyboard reported 0xb7
+binding_key = serialized(CHINESE_CHAR, CMD, GRAVE_KEY)
+binding_dict = {binding_key: {"Action": 30, "Version": 2}}
+
+print("=" * 60)
+print("Binding stored in plist:", binding_key)
+print("=" * 60)
+
+# Simulate pressing Cmd+` when charactersIgnoringModifiers returns 0x60
+# (e.g. after typing text changed input method state)
+print(f"\nPressing Cmd+` — charactersIgnoringModifiers = 0x{US_CHAR:x}")
+print(f"  serialized:           {serialized(US_CHAR, CMD, GRAVE_KEY)}")
+print(f"  legacy_serialized:    {legacy_serialized(US_CHAR, CMD)}")
+print(f"  portable_serialized:  {portable_serialized(CMD, GRAVE_KEY)}")
+
+result_before = key_in_binding_dict_before(US_CHAR, CMD, GRAVE_KEY, binding_dict)
+result_after = key_in_binding_dict_after(US_CHAR, CMD, GRAVE_KEY, binding_dict)
+
+print(f"\n  BEFORE fix: {'MATCH' if result_before else 'NO MATCH — BUG REPRODUCED'}")
+print(f"  AFTER  fix: {'MATCH' if result_after else 'NO MATCH'}")
+
+# Also test: same character as stored (should always work)
+print(f"\nPressing Cmd+` — charactersIgnoringModifiers = 0x{CHINESE_CHAR:x} (same as stored)")
+result_same_before = key_in_binding_dict_before(CHINESE_CHAR, CMD, GRAVE_KEY, binding_dict)
+result_same_after = key_in_binding_dict_after(CHINESE_CHAR, CMD, GRAVE_KEY, binding_dict)
+print(f"  BEFORE fix: {'MATCH' if result_same_before else 'NO MATCH'}")
+print(f"  AFTER  fix: {'MATCH' if result_same_after else 'NO MATCH'}")
+
+# Test: different key code should NOT match
+print(f"\nPressing Cmd+1 (keycode=0x12) — should NEVER match")
+result_wrong_key = key_in_binding_dict_after(0x31, CMD, 0x12, binding_dict)
+print(f"  AFTER  fix: {'MATCH (BUG!)' if result_wrong_key else 'NO MATCH (correct)'}")
+
+# Summary
+print("\n" + "=" * 60)
+if result_before is None and result_after is not None:
+    print("BUG CONFIRMED & FIX VERIFIED")
+    print("Without fix: key press with different character not recognized.")
+    print("With fix:    portableSerialized fallback matches on keycode + modifiers.")
+elif result_before is not None:
+    print("UNEXPECTED: bug not reproduced, check assumptions.")
+else:
+    print("UNEXPECTED: fix did not resolve the issue.")
+print("=" * 60)