fix(overlay): keep keyboard mode from swallowing modal and text-field keys (#291)

changes/keyboard-mode-modal-input.md

@@ -0,0 +1,4 @@
+type: fixed
+area: overlay
+
+- Keyboard-only mode no longer swallows `h`, `l`, and the left/right arrows while a modal is open or a text field has focus, so typing into fields like the Jimaku or TsukiHime title works and no longer seeks mpv. Pressing `Ctrl/Cmd+Shift+Y` (or the controller toggle) inside a modal no longer turns on a hidden keyboard-only mode in the modal window.

src/renderer/handlers/keyboard.test.ts

@@ -315,8 +315,9 @@ function installKeyboardTestGlobals() {
     shiftKey?: boolean;
     repeat?: boolean;
     target?: unknown;
-  }): void {
+  }): boolean {
     const listeners = documentListeners.get('keydown') ?? [];
+    let prevented = false;
     const keyboardEvent = {
       key: event.key,
       code: event.code,
@@ -325,12 +326,15 @@ function installKeyboardTestGlobals() {
       altKey: event.altKey ?? false,
       shiftKey: event.shiftKey ?? false,
       repeat: event.repeat ?? false,
-      preventDefault: () => {},
+      preventDefault: () => {
+        prevented = true;
+      },
       target: event.target ?? null,
     };
     for (const listener of listeners) {
       listener(keyboardEvent);
     }
+    return prevented;
   }
 
   function dispatchMousedown(event: {
@@ -472,6 +476,12 @@ function installKeyboardTestGlobals() {
       target.closest = (selector: string) => (selector.includes('.modal') ? target : null);
       return target;
     },
+    createTextInputTarget: () => {
+      const target = new TestElement();
+      target.tagName = 'INPUT';
+      target.closest = (selector: string) => (selector.includes('input') ? target : null);
+      return target;
+    },
     setGetSessionBindings: (value: () => Promise<CompiledSessionBinding[]>) => {
       getSessionBindingsImpl = value;
     },
@@ -501,6 +511,7 @@ function createKeyboardHandlerHarness() {
   let openControllerDebugCount = 0;
   let playlistBrowserKeydownCount = 0;
   let changelogKeydownCount = 0;
+  let jimakuKeydownCount = 0;
 
   const createWordNode = (left: number) => ({
     classList: createClassList(),
@@ -536,7 +547,10 @@ function createKeyboardHandlerHarness() {
     handleCharacterDictionaryKeydown: () => false,
     handleSubsyncKeydown: () => false,
     handleKikuKeydown: () => false,
-    handleJimakuKeydown: () => false,
+    handleJimakuKeydown: () => {
+      jimakuKeydownCount += 1;
+      return false;
+    },
     handleTsukihimeKeydown: () => false,
     handleMediaTimingReviewKeydown: () => {
       mediaTimingReviewKeydownCount += 1;
@@ -577,6 +591,7 @@ function createKeyboardHandlerHarness() {
     openControllerDebugCount: () => openControllerDebugCount,
     playlistBrowserKeydownCount: () => playlistBrowserKeydownCount,
     changelogKeydownCount: () => changelogKeydownCount,
+    jimakuKeydownCount: () => jimakuKeydownCount,
     setWordCount: (count: number) => {
       wordNodes = Array.from({ length: count }, (_, index) => createWordNode(10 + index * 70));
     },
@@ -1417,6 +1432,59 @@ test('keyboard mode: changelog modal handles h/l fold keys before lookup control
   }
 });
 
+const KEYBOARD_MODE_LOOKUP_KEYS = [
+  { key: 'h', code: 'KeyH' },
+  { key: 'l', code: 'KeyL' },
+  { key: 'ArrowLeft', code: 'ArrowLeft' },
+  { key: 'ArrowRight', code: 'ArrowRight' },
+];
+
+test('keyboard mode: an open modal receives h/l/arrows instead of lookup controls', async () => {
+  const { ctx, testGlobals, handlers, jimakuKeydownCount } = createKeyboardHandlerHarness();
+
+  try {
+    await handlers.setupMpvInputForwarding();
+    handlers.handleKeyboardModeToggleRequested();
+    ctx.state.jimakuModalOpen = true;
+    ctx.state.keyboardSelectedWordIndex = 1;
+    const titleInput = testGlobals.createTextInputTarget();
+
+    for (const key of KEYBOARD_MODE_LOOKUP_KEYS) {
+      const prevented = testGlobals.dispatchKeydown({ ...key, target: titleInput });
+      assert.equal(prevented, false, `${key.code} must reach the focused modal input`);
+    }
+    await wait(0);
+
+    assert.equal(jimakuKeydownCount(), KEYBOARD_MODE_LOOKUP_KEYS.length);
+    assert.equal(ctx.state.keyboardSelectedWordIndex, 1);
+    assert.deepEqual(testGlobals.mpvCommands, []);
+  } finally {
+    testGlobals.restore();
+  }
+});
+
+test('keyboard mode: h/l/arrows typed into a text field are not consumed by lookup controls', async () => {
+  const { ctx, testGlobals, handlers, setWordCount } = createKeyboardHandlerHarness();
+
+  try {
+    await handlers.setupMpvInputForwarding();
+    handlers.handleKeyboardModeToggleRequested();
+    // No words means lookup controls would seek mpv to the adjacent subtitle.
+    setWordCount(0);
+    const input = testGlobals.createTextInputTarget();
+
+    for (const key of KEYBOARD_MODE_LOOKUP_KEYS) {
+      const prevented = testGlobals.dispatchKeydown({ ...key, target: input });
+      assert.equal(prevented, false, `${key.code} must reach the text field`);
+    }
+    await wait(0);
+
+    assert.deepEqual(testGlobals.mpvCommands, []);
+  } finally {
+    testGlobals.restore();
+  }
+});
+
 test('keyboard mode: configured stats toggle works even while popup is open', async () => {
   const { handlers, testGlobals } = createKeyboardHandlerHarness();
 
@@ -1683,7 +1751,7 @@ test('keyboard mode: visible-layer Ctrl+Shift+Y should not be toggled by rendere
   }
 });
 
-test('keyboard mode: modal-layer Ctrl+Shift+Y still toggles via renderer keydown', async () => {
+test('keyboard mode: modal layer never enables keyboard mode', async () => {
   const { ctx, handlers, testGlobals } = createKeyboardHandlerHarness();
 
   try {
@@ -1691,9 +1759,10 @@ test('keyboard mode: modal-layer Ctrl+Shift+Y still toggles via renderer keydown
     ctx.platform.isModalLayer = true;
 
     testGlobals.dispatchKeydown({ key: 'Y', code: 'KeyY', ctrlKey: true, shiftKey: true });
-    assert.equal(ctx.state.keyboardDrivenModeEnabled, true);
+    assert.equal(ctx.state.keyboardDrivenModeEnabled, false);
 
-    testGlobals.dispatchKeydown({ key: 'Y', code: 'KeyY', ctrlKey: true, shiftKey: true });
+    // Controller toggle path.
+    handlers.handleKeyboardModeToggleRequested();
     assert.equal(ctx.state.keyboardDrivenModeEnabled, false);
   } finally {
     ctx.state.keyboardDrivenModeEnabled = false;

src/renderer/handlers/keyboard.ts

@@ -403,11 +403,6 @@ export function createKeyboardHandlers(
     return e.ctrlKey || e.metaKey;
   }
 
-  function isKeyboardDrivenModeToggle(e: KeyboardEvent): boolean {
-    const isYKey = e.code === 'KeyY' || e.key.toLowerCase() === 'y';
-    return isPrimaryModifierPressed(e) && !e.altKey && e.shiftKey && isYKey && !e.repeat;
-  }
-
   function isLookupWindowToggle(e: KeyboardEvent): boolean {
     const isYKey = e.code === 'KeyY' || e.key.toLowerCase() === 'y';
     return isPrimaryModifierPressed(e) && !e.altKey && !e.shiftKey && isYKey && !e.repeat;
@@ -723,7 +718,10 @@ export function createKeyboardHandlers(
     return true;
   }
 
+  // Keyboard mode drives the visible overlay's subtitle words. The modal layer hides
+  // subtitles, so enabling it there only lets arrows/H/L seek mpv from behind a modal.
   function handleKeyboardModeToggleRequested(): void {
+    if (ctx.platform.isModalLayer) return;
     toggleKeyboardDrivenMode();
   }
 
@@ -1144,12 +1142,6 @@ export function createKeyboardHandlers(
         return;
       }
 
-      if (isKeyboardDrivenModeToggle(e) && ctx.platform.isModalLayer) {
-        e.preventDefault();
-        handleKeyboardModeToggleRequested();
-        return;
-      }
-
       if (isLookupWindowToggle(e)) {
         e.preventDefault();
         handleLookupWindowToggleRequested();
@@ -1162,19 +1154,11 @@ export function createKeyboardHandlers(
         }
       }
 
-      // Ahead of the keyboard-driven lookup controls: the changelog modal binds
-      // arrows/H/L for folding, and those would otherwise move the subtitle word
-      // selection (and seek mpv) behind the open modal.
       if (ctx.state.changelogModalOpen) {
         options.handleChangelogKeydown(e);
         return;
       }
 
-      if (handleKeyboardDrivenModeLookupControls(e)) {
-        e.preventDefault();
-        return;
-      }
-
       if (ctx.state.runtimeOptionsModalOpen) {
         options.handleRuntimeOptionsKeydown(e);
         return;
@@ -1222,6 +1206,13 @@ export function createKeyboardHandlers(
         return;
       }
 
+      // After open modals and text fields, which own arrows/H/L; before Yomitan popup
+      // keybinds so word navigation keeps working while the popup is open.
+      if (handleKeyboardDrivenModeLookupControls(e)) {
+        e.preventDefault();
+        return;
+      }
+
       if (handlePendingNumericSelection(e)) {
         return;
       }