fix: send Korean IME toggle keys as stateless pulses

src/frontend/GuacamoleDisplay.tsx

@@ -58,7 +58,7 @@ import { clampGuacamoleZoom, stepGuacamoleZoom } from "./guacamole-zoom.ts";
 import { createRdpReadyGate } from "./guacamole-ready.ts";
 import {
   remoteKeysym,
-  shouldSendKey,
+  isToggleKey,
   watchRightAlt,
 } from "./guacamole-meta-key.ts";
 
@@ -455,15 +455,14 @@ export const GuacamoleDisplay = forwardRef<
         document.activeElement === activeDisplay;
 
       if (!hasKeyboardFocusRef.current && !stillFocused) return;
-      if (!shouldSendKey(keysym, true)) return;
       clientRef.current.sendKeyEvent(
         1,
         remoteKeysym(sessionProtocolRef.current, keysym, true),
       );
+      if (isToggleKey(keysym)) clientRef.current.sendKeyEvent(0, keysym);
     };
 
     keyboard.onkeyup = (keysym: number) => {
-      if (!shouldSendKey(keysym, false)) return;
       // Focus loss must still release keys already sent to the remote host.
       clientRef.current?.sendKeyEvent(
         0,
@@ -679,7 +678,9 @@ export const GuacamoleDisplay = forwardRef<
     );
 
     unwatchAltRef.current?.();
-    unwatchAltRef.current = watchRightAlt(displayElement);
+    unwatchAltRef.current = watchRightAlt(displayElement, (keysym) => {
+      keyboardRef.current?.onkeydown?.(keysym);
+    });
     const keyboard = new Guacamole.Keyboard(displayElement);
     keyboardRef.current = keyboard;
 

src/frontend/guacamole-meta-key.ts

@@ -138,56 +138,43 @@ const TOGGLE_KEYS: Record<string, number> = {
   HanjaMode: 0xff34,
 };
 const toggleKeysyms = new Set(Object.values(TOGGLE_KEYS));
-const togglesHeld = new Set<number>();
-const togglesSent = new Set<number>();
 
 /** Tracks whether the browser sees right Alt as Alt or as AltGraph. */
-export function watchRightAlt(target: HTMLElement): () => void {
+export function watchRightAlt(
+  target: HTMLElement,
+  onToggle?: (keysym: number) => void,
+): () => void {
   const onKey = (event: KeyboardEvent) => {
     if (event.code === "AltRight") rightAltIsPlain = event.key === "Alt";
     if (event.type !== "keyup") lastKeyCode = event.code;
     const toggle = TOGGLE_KEYS[event.key];
     if (toggle === undefined) return;
-    if (event.type === "keyup") {
-      togglesHeld.delete(toggle);
-    } else {
-      togglesHeld.add(toggle);
-      // A real press always goes through, even if a release got lost on blur.
-      if (!event.repeat) togglesSent.delete(toggle);
-    }
+    // Some Korean drivers never send a matching keyup. Keep these keys out
+    // of Guacamole's pressed table and software repeat timer entirely.
+    if (!onToggle) return;
+    event.preventDefault();
+    event.stopImmediatePropagation();
+    if (event.type === "keydown" && !event.repeat) onToggle(toggle);
   };
-  const onBlur = () => togglesHeld.clear();
   target.addEventListener("keydown", onKey, true);
   target.addEventListener("keyup", onKey, true);
   // A Mac browser reports what Option types on keypress, not keydown.
   target.addEventListener("keypress", onKey, true);
-  target.addEventListener("blur", onBlur);
   return () => {
     target.removeEventListener("keydown", onKey, true);
     target.removeEventListener("keyup", onKey, true);
     target.removeEventListener("keypress", onKey, true);
-    target.removeEventListener("blur", onBlur);
     rightAltIsPlain = false;
     rightAltSentAs = null;
-    togglesHeld.clear();
-    togglesSent.clear();
     optionsDown.clear();
     optionKeysSent.clear();
     lastKeyCode = "";
   };
 }
 
-/** False for a repeat of a toggle key: one press sends one down and one up. */
-export function shouldSendKey(keysym: number, down: boolean): boolean {
-  if (!toggleKeysyms.has(keysym)) return true;
-  if (down) {
-    if (togglesSent.has(keysym)) return false;
-    togglesSent.add(keysym);
-    return true;
-  }
-  if (togglesHeld.has(keysym)) return false;
-  togglesSent.delete(keysym);
-  return true;
+/** Toggle keys are sent as a single down/up pulse, never held remotely. */
+export function isToggleKey(keysym: number): boolean {
+  return toggleKeysyms.has(keysym);
 }
 
 /**

tests/frontend/guacamole-key-release.test.ts

@@ -5,7 +5,8 @@ import { afterEach, describe, expect, it, vi } from "vitest";
 import displaySource from "../../src/frontend/GuacamoleDisplay.tsx?raw";
 import {
   remoteKeysym,
-  shouldSendKey,
+  isToggleKey,
+  watchRightAlt,
 } from "../../src/frontend/guacamole-meta-key.js";
 
 // Exercise the display's actual focus callback with Guacamole's real keyboard.
@@ -33,7 +34,7 @@ if (!callback) throw new Error("Missing refreshKeyboardHandlers callback");
 const runRefresh = new Function(
   "state",
   ts.transpileModule(
-    `const { keyboardRef, clientRef, displayElementRef, isVisible, windowFocusedRef, hasKeyboardFocusRef, sessionProtocolRef, remoteKeysym, shouldSendKey } = state;
+    `const { keyboardRef, clientRef, displayElementRef, isVisible, windowFocusedRef, hasKeyboardFocusRef, sessionProtocolRef, remoteKeysym, isToggleKey } = state;
     (${callback})();`,
     { compilerOptions: { target: ts.ScriptTarget.ES2022 } },
   ).outputText,
@@ -68,7 +69,7 @@ describe("remote keyboard focus loss", () => {
         hasKeyboardFocusRef: { current: true },
         sessionProtocolRef: { current: "vnc" },
         remoteKeysym,
-        shouldSendKey,
+        isToggleKey,
       };
       const refresh = () => runRefresh(state);
       refresh();
@@ -137,7 +138,7 @@ describe("remote keyboard on a Mac", () => {
         hasKeyboardFocusRef: { current: true },
         sessionProtocolRef: { current: protocol },
         remoteKeysym,
-        shouldSendKey,
+        isToggleKey,
       });
 
       keyboard.press(0xffe7); // the browser's Meta key
@@ -154,3 +155,73 @@ describe("remote keyboard on a Mac", () => {
     }
   });
 });
+
+describe("Korean toggle keys with the real keyboard", () => {
+  it.each([
+    ["HangulMode", 0xff31],
+    ["HanjaMode", 0xff34],
+  ] as const)(
+    "%s sends pulses without requiring keyup and ignores repeats",
+    (key, keysym) => {
+      vi.useFakeTimers();
+      const display = document.createElement("div");
+      display.tabIndex = 0;
+      document.body.append(display);
+      display.focus();
+      let keyboard: Guacamole.Keyboard;
+      const stop = watchRightAlt(display, (toggle) => {
+        keyboard.onkeydown?.(toggle);
+      });
+      keyboard = new Guacamole.Keyboard(display);
+      const sendKeyEvent = vi.fn();
+      const state = {
+        keyboardRef: { current: keyboard },
+        clientRef: { current: { sendKeyEvent } },
+        displayElementRef: { current: display },
+        isVisible: true,
+        windowFocusedRef: { current: true },
+        hasKeyboardFocusRef: { current: true },
+        sessionProtocolRef: { current: "rdp" },
+        remoteKeysym,
+        isToggleKey,
+      };
+      runRefresh(state);
+      const dispatch = (type: string, repeat = false) =>
+        display.dispatchEvent(
+          new KeyboardEvent(type, {
+            key,
+            repeat,
+            bubbles: true,
+            cancelable: true,
+          }),
+        );
+      try {
+        dispatch("keydown");
+        vi.advanceTimersByTime(2000);
+        dispatch("keydown", true);
+        expect(sendKeyEvent.mock.calls).toEqual([
+          [1, keysym],
+          [0, keysym],
+        ]);
+        // Latching drivers can deliver another physical down without any up.
+        dispatch("keydown");
+        dispatch("keyup");
+        dispatch("keypress");
+        expect(sendKeyEvent.mock.calls).toEqual([
+          [1, keysym],
+          [0, keysym],
+          [1, keysym],
+          [0, keysym],
+        ]);
+        state.isVisible = false;
+        runRefresh(state);
+        dispatch("keydown");
+        expect(sendKeyEvent).toHaveBeenCalledTimes(4);
+      } finally {
+        stop();
+        keyboard.reset();
+        vi.useRealTimers();
+      }
+    },
+  );
+});

tests/frontend/guacamole-meta-key.test.ts

@@ -3,7 +3,6 @@ import {
   detectMetaKeyFamily,
   metaKeyLabels,
   remoteKeysym,
-  shouldSendKey,
   watchRightAlt,
 } from "../../src/frontend/guacamole-meta-key.js";
 
@@ -110,44 +109,6 @@ describe("right Alt", () => {
   });
 });
 
-describe("toggle key repeat", () => {
-  const HANGUL = 0xff31;
-  const press = (target: HTMLElement, type: string, repeat = false) =>
-    target.dispatchEvent(
-      new KeyboardEvent(type, { key: "HangulMode", repeat, bubbles: true }),
-    );
-
-  it("sends one down and one up however long the key is held", () => {
-    const target = document.createElement("div");
-    const stop = watchRightAlt(target);
-    press(target, "keydown");
-    expect(shouldSendKey(HANGUL, true)).toBe(true);
-    // The library's own repeat while the key is still down.
-    expect(shouldSendKey(HANGUL, false)).toBe(false);
-    expect(shouldSendKey(HANGUL, true)).toBe(false);
-    press(target, "keydown", true);
-    expect(shouldSendKey(HANGUL, true)).toBe(false);
-    press(target, "keyup");
-    expect(shouldSendKey(HANGUL, false)).toBe(true);
-    // The next tap goes through again.
-    press(target, "keydown");
-    expect(shouldSendKey(HANGUL, true)).toBe(true);
-    stop();
-  });
-
-  it("releases on focus loss and never filters other keys", () => {
-    const target = document.createElement("div");
-    const stop = watchRightAlt(target);
-    press(target, "keydown");
-    shouldSendKey(HANGUL, true);
-    target.dispatchEvent(new FocusEvent("blur"));
-    expect(shouldSendKey(HANGUL, false)).toBe(true);
-    expect(shouldSendKey(0x61, true)).toBe(true);
-    expect(shouldSendKey(0x61, true)).toBe(true);
-    stop();
-  });
-});
-
 describe("Option on an ARD host", () => {
   const ALT_L = 0xffe9;
   const ALT_R = 0xffea;