From de9b8877984a91f6e8ba4242a0cec979eecae20d Mon Sep 17 00:00:00 2001 From: Kyle Date: Tue, 7 Apr 2026 21:45:12 -0700 Subject: [PATCH] Fix nested Yomitan popup focus loss --- changes/fix-yomitan-nested-popup-focus.md | 4 + src/core/services/overlay-visibility.test.ts | 132 +++++++++++++++++++ src/core/services/overlay-visibility.ts | 18 ++- src/renderer/error-recovery.test.ts | 46 ++++++- src/renderer/handlers/keyboard.ts | 4 + src/renderer/handlers/mouse.test.ts | 132 ++++++++++++++++++- src/renderer/handlers/mouse.ts | 31 ++++- src/renderer/style.css | 6 +- src/renderer/yomitan-popup.ts | 47 +++++-- vendor/subminer-yomitan | 2 +- 10 files changed, 397 insertions(+), 25 deletions(-) create mode 100644 changes/fix-yomitan-nested-popup-focus.md diff --git a/changes/fix-yomitan-nested-popup-focus.md b/changes/fix-yomitan-nested-popup-focus.md new file mode 100644 index 00000000..1a343cae --- /dev/null +++ b/changes/fix-yomitan-nested-popup-focus.md @@ -0,0 +1,4 @@ +type: fixed +area: overlay + +- Fixed Windows Yomitan popup focus loss after closing nested lookups so the original popup stays interactive instead of falling through to mpv. diff --git a/src/core/services/overlay-visibility.test.ts b/src/core/services/overlay-visibility.test.ts index b43d7d79..f405b3cf 100644 --- a/src/core/services/overlay-visibility.test.ts +++ b/src/core/services/overlay-visibility.test.ts @@ -10,12 +10,16 @@ type WindowTrackerStub = { function createMainWindowRecorder() { const calls: string[] = []; + let visible = false; const window = { isDestroyed: () => false, + isVisible: () => visible, hide: () => { + visible = false; calls.push('hide'); }, show: () => { + visible = true; calls.push('show'); }, focus: () => { @@ -200,6 +204,134 @@ test('Windows visible overlay stays click-through and does not steal focus while assert.ok(!calls.includes('focus')); }); +test('tracked Windows overlay refresh preserves renderer-managed mouse interaction when already visible', () => { + const { window, calls } = createMainWindowRecorder(); + const tracker: WindowTrackerStub = { + isTracking: () => true, + getGeometry: () => ({ x: 0, y: 0, width: 1280, height: 720 }), + }; + + updateVisibleOverlayVisibility({ + visibleOverlayVisible: true, + mainWindow: window as never, + windowTracker: tracker as never, + trackerNotReadyWarningShown: false, + setTrackerNotReadyWarningShown: () => {}, + updateVisibleOverlayBounds: () => { + calls.push('update-bounds'); + }, + ensureOverlayWindowLevel: () => { + calls.push('ensure-level'); + }, + syncPrimaryOverlayWindowLayer: () => { + calls.push('sync-layer'); + }, + enforceOverlayLayerOrder: () => { + calls.push('enforce-order'); + }, + syncOverlayShortcuts: () => { + calls.push('sync-shortcuts'); + }, + isMacOSPlatform: false, + isWindowsPlatform: true, + } as never); + + calls.length = 0; + + updateVisibleOverlayVisibility({ + visibleOverlayVisible: true, + mainWindow: window as never, + windowTracker: tracker as never, + trackerNotReadyWarningShown: false, + setTrackerNotReadyWarningShown: () => {}, + updateVisibleOverlayBounds: () => { + calls.push('update-bounds'); + }, + ensureOverlayWindowLevel: () => { + calls.push('ensure-level'); + }, + syncPrimaryOverlayWindowLayer: () => { + calls.push('sync-layer'); + }, + enforceOverlayLayerOrder: () => { + calls.push('enforce-order'); + }, + syncOverlayShortcuts: () => { + calls.push('sync-shortcuts'); + }, + isMacOSPlatform: false, + isWindowsPlatform: true, + } as never); + + assert.ok(!calls.includes('mouse-ignore:true:forward')); + assert.ok(!calls.includes('show')); + assert.ok(calls.includes('ensure-level')); + assert.ok(calls.includes('sync-shortcuts')); +}); + +test('forced passthrough still reapplies while visible on Windows', () => { + const { window, calls } = createMainWindowRecorder(); + const tracker: WindowTrackerStub = { + isTracking: () => true, + getGeometry: () => ({ x: 0, y: 0, width: 1280, height: 720 }), + }; + + updateVisibleOverlayVisibility({ + visibleOverlayVisible: true, + mainWindow: window as never, + windowTracker: tracker as never, + trackerNotReadyWarningShown: false, + setTrackerNotReadyWarningShown: () => {}, + updateVisibleOverlayBounds: () => { + calls.push('update-bounds'); + }, + ensureOverlayWindowLevel: () => { + calls.push('ensure-level'); + }, + syncPrimaryOverlayWindowLayer: () => { + calls.push('sync-layer'); + }, + enforceOverlayLayerOrder: () => { + calls.push('enforce-order'); + }, + syncOverlayShortcuts: () => { + calls.push('sync-shortcuts'); + }, + isMacOSPlatform: false, + isWindowsPlatform: true, + } as never); + + calls.length = 0; + + updateVisibleOverlayVisibility({ + visibleOverlayVisible: true, + mainWindow: window as never, + windowTracker: tracker as never, + trackerNotReadyWarningShown: false, + setTrackerNotReadyWarningShown: () => {}, + updateVisibleOverlayBounds: () => { + calls.push('update-bounds'); + }, + ensureOverlayWindowLevel: () => { + calls.push('ensure-level'); + }, + syncPrimaryOverlayWindowLayer: () => { + calls.push('sync-layer'); + }, + enforceOverlayLayerOrder: () => { + calls.push('enforce-order'); + }, + syncOverlayShortcuts: () => { + calls.push('sync-shortcuts'); + }, + isMacOSPlatform: false, + isWindowsPlatform: true, + forceMousePassthrough: true, + } as never); + + assert.ok(calls.includes('mouse-ignore:true:forward')); +}); + test('visible overlay stays hidden while a modal window is active', () => { const { window, calls } = createMainWindowRecorder(); const tracker: WindowTrackerStub = { diff --git a/src/core/services/overlay-visibility.ts b/src/core/services/overlay-visibility.ts index c74e6bbc..9cb17ead 100644 --- a/src/core/services/overlay-visibility.ts +++ b/src/core/services/overlay-visibility.ts @@ -37,13 +37,21 @@ export function updateVisibleOverlayVisibility(args: { const showPassiveVisibleOverlay = (): void => { const forceMousePassthrough = args.forceMousePassthrough === true; - if (args.isMacOSPlatform || args.isWindowsPlatform || forceMousePassthrough) { - mainWindow.setIgnoreMouseEvents(true, { forward: true }); - } else { - mainWindow.setIgnoreMouseEvents(false); + const shouldDefaultToPassthrough = + args.isMacOSPlatform || args.isWindowsPlatform || forceMousePassthrough; + const wasVisible = mainWindow.isVisible(); + + if (!wasVisible || forceMousePassthrough) { + if (shouldDefaultToPassthrough) { + mainWindow.setIgnoreMouseEvents(true, { forward: true }); + } else { + mainWindow.setIgnoreMouseEvents(false); + } } args.ensureOverlayWindowLevel(mainWindow); - mainWindow.show(); + if (!wasVisible) { + mainWindow.show(); + } if (!args.isWindowsPlatform && !args.isMacOSPlatform && !forceMousePassthrough) { mainWindow.focus(); } diff --git a/src/renderer/error-recovery.test.ts b/src/renderer/error-recovery.test.ts index d282b66d..eea72bdf 100644 --- a/src/renderer/error-recovery.test.ts +++ b/src/renderer/error-recovery.test.ts @@ -3,7 +3,9 @@ import assert from 'node:assert/strict'; import { createRendererRecoveryController } from './error-recovery.js'; import { + YOMITAN_POPUP_HOST_SELECTOR, YOMITAN_POPUP_IFRAME_SELECTOR, + YOMITAN_POPUP_VISIBLE_HOST_SELECTOR, hasYomitanPopupIframe, isYomitanPopupIframe, isYomitanPopupVisible, @@ -284,9 +286,25 @@ test('hasYomitanPopupIframe queries for modern + legacy selector', () => { assert.equal(selector, YOMITAN_POPUP_IFRAME_SELECTOR); }); +test('hasYomitanPopupIframe falls back to popup host selector for shadow-hosted popups', () => { + const selectors: string[] = []; + const root = { + querySelector: (value: string) => { + selectors.push(value); + if (value === YOMITAN_POPUP_HOST_SELECTOR) { + return {}; + } + return null; + }, + } as unknown as ParentNode; + + assert.equal(hasYomitanPopupIframe(root), true); + assert.deepEqual(selectors, [YOMITAN_POPUP_IFRAME_SELECTOR, YOMITAN_POPUP_HOST_SELECTOR]); +}); + test('isYomitanPopupVisible requires visible iframe geometry', () => { const previousWindow = (globalThis as { window?: unknown }).window; - let selector = ''; + const selectors: string[] = []; const visibleFrame = { getBoundingClientRect: () => ({ width: 320, height: 180 }), } as unknown as HTMLIFrameElement; @@ -309,18 +327,40 @@ test('isYomitanPopupVisible requires visible iframe geometry', () => { try { const root = { querySelectorAll: (value: string) => { - selector = value; + selectors.push(value); + if (value === YOMITAN_POPUP_VISIBLE_HOST_SELECTOR || value === YOMITAN_POPUP_HOST_SELECTOR) { + return []; + } return [hiddenFrame, visibleFrame]; }, } as unknown as ParentNode; assert.equal(isYomitanPopupVisible(root), true); - assert.equal(selector, YOMITAN_POPUP_IFRAME_SELECTOR); + assert.deepEqual(selectors, [ + YOMITAN_POPUP_VISIBLE_HOST_SELECTOR, + YOMITAN_POPUP_IFRAME_SELECTOR, + ]); } finally { Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow }); } }); +test('isYomitanPopupVisible detects visible shadow-hosted popup marker without iframe access', () => { + let selector = ''; + const root = { + querySelectorAll: (value: string) => { + selector = value; + if (value === YOMITAN_POPUP_VISIBLE_HOST_SELECTOR) { + return [{ getAttribute: () => 'true' }]; + } + return []; + }, + } as unknown as ParentNode; + + assert.equal(isYomitanPopupVisible(root), true); + assert.equal(selector, YOMITAN_POPUP_VISIBLE_HOST_SELECTOR); +}); + test('scrollActiveRuntimeOptionIntoView scrolls active runtime option with nearest block', () => { const calls: Array<{ block?: ScrollLogicalPosition }> = []; const activeItem = { diff --git a/src/renderer/handlers/keyboard.ts b/src/renderer/handlers/keyboard.ts index 8d97a3a9..67f6d02e 100644 --- a/src/renderer/handlers/keyboard.ts +++ b/src/renderer/handlers/keyboard.ts @@ -2,6 +2,7 @@ import { SPECIAL_COMMANDS } from '../../config/definitions'; import type { Keybinding, ShortcutsConfig } from '../../types'; import type { RendererContext } from '../context'; import { + YOMITAN_POPUP_HOST_SELECTOR, YOMITAN_POPUP_HIDDEN_EVENT, YOMITAN_POPUP_SHOWN_EVENT, YOMITAN_POPUP_COMMAND_EVENT, @@ -61,6 +62,9 @@ export function createKeyboardHandlers( if (target.closest('.modal')) return true; if (ctx.dom.subtitleContainer.contains(target)) return true; if (isYomitanPopupIframe(target)) return true; + if (target.closest && target.closest(YOMITAN_POPUP_HOST_SELECTOR)) { + return true; + } if (target.closest && target.closest('iframe.yomitan-popup, iframe[id^="yomitan-popup"]')) return true; return false; diff --git a/src/renderer/handlers/mouse.test.ts b/src/renderer/handlers/mouse.test.ts index 6018c885..a841c80f 100644 --- a/src/renderer/handlers/mouse.test.ts +++ b/src/renderer/handlers/mouse.test.ts @@ -3,7 +3,12 @@ import test from 'node:test'; import type { SubtitleSidebarConfig } from '../../types'; import { createMouseHandlers } from './mouse.js'; -import { YOMITAN_POPUP_HIDDEN_EVENT, YOMITAN_POPUP_SHOWN_EVENT } from '../yomitan-popup.js'; +import { + YOMITAN_POPUP_HIDDEN_EVENT, + YOMITAN_POPUP_HOST_SELECTOR, + YOMITAN_POPUP_SHOWN_EVENT, + YOMITAN_POPUP_VISIBLE_HOST_SELECTOR, +} from '../yomitan-popup.js'; function createClassList() { const classes = new Set(); @@ -78,11 +83,13 @@ function createMouseTestContext() { }, platform: { shouldToggleMouseIgnore: false, + isLinuxPlatform: false, isMacOSPlatform: false, }, state: { isOverSubtitle: false, isOverSubtitleSidebar: false, + yomitanPopupVisible: false, subtitleSidebarModalOpen: false, subtitleSidebarConfig: null as SubtitleSidebarConfig | null, isDragging: false, @@ -712,6 +719,129 @@ test('popup open pauses and popup close resumes when yomitan popup auto-pause is } }); +test('nested popup close reasserts interactive state and focus when another popup remains visible on Windows', async () => { + const ctx = createMouseTestContext(); + const previousWindow = (globalThis as { window?: unknown }).window; + const previousDocument = (globalThis as { document?: unknown }).document; + const previousMutationObserver = (globalThis as { MutationObserver?: unknown }).MutationObserver; + const previousNode = (globalThis as { Node?: unknown }).Node; + const windowListeners = new Map void>>(); + const ignoreCalls: Array<{ ignore: boolean; forward?: boolean }> = []; + let focusMainWindowCalls = 0; + let windowFocusCalls = 0; + let overlayFocusCalls = 0; + + ctx.platform.shouldToggleMouseIgnore = true; + (ctx.dom.overlay as { focus?: (options?: { preventScroll?: boolean }) => void }).focus = () => { + overlayFocusCalls += 1; + }; + + const visiblePopupHost = { + tagName: 'DIV', + getAttribute: (name: string) => + name === 'data-subminer-yomitan-popup-visible' ? 'true' : null, + }; + + Object.defineProperty(globalThis, 'window', { + configurable: true, + value: { + addEventListener: (type: string, listener: () => void) => { + const bucket = windowListeners.get(type) ?? []; + bucket.push(listener); + windowListeners.set(type, bucket); + }, + electronAPI: { + setIgnoreMouseEvents: (ignore: boolean, options?: { forward?: boolean }) => { + ignoreCalls.push({ ignore, forward: options?.forward }); + }, + focusMainWindow: () => { + focusMainWindowCalls += 1; + }, + }, + focus: () => { + windowFocusCalls += 1; + }, + getComputedStyle: () => ({ + visibility: 'visible', + display: 'block', + opacity: '1', + }), + innerHeight: 1000, + getSelection: () => null, + setTimeout, + clearTimeout, + }, + }); + Object.defineProperty(globalThis, 'document', { + configurable: true, + value: { + querySelector: () => null, + querySelectorAll: (selector: string) => { + if ( + selector === YOMITAN_POPUP_VISIBLE_HOST_SELECTOR || + selector === YOMITAN_POPUP_HOST_SELECTOR + ) { + return [visiblePopupHost]; + } + return []; + }, + body: {}, + elementFromPoint: () => null, + addEventListener: () => {}, + }, + }); + Object.defineProperty(globalThis, 'MutationObserver', { + configurable: true, + value: class { + observe() {} + }, + }); + Object.defineProperty(globalThis, 'Node', { + configurable: true, + value: { + ELEMENT_NODE: 1, + }, + }); + + try { + const handlers = createMouseHandlers(ctx as never, { + modalStateReader: { + isAnySettingsModalOpen: () => false, + isAnyModalOpen: () => false, + }, + applyYPercent: () => {}, + getCurrentYPercent: () => 10, + persistSubtitlePositionPatch: () => {}, + getSubtitleHoverAutoPauseEnabled: () => false, + getYomitanPopupAutoPauseEnabled: () => false, + getPlaybackPaused: async () => false, + sendMpvCommand: () => {}, + }); + + handlers.setupYomitanObserver(); + ignoreCalls.length = 0; + + for (const listener of windowListeners.get(YOMITAN_POPUP_HIDDEN_EVENT) ?? []) { + listener(); + } + + assert.equal(ctx.state.yomitanPopupVisible, true); + assert.equal(ctx.dom.overlay.classList.contains('interactive'), true); + assert.deepEqual(ignoreCalls, [{ ignore: false, forward: undefined }]); + assert.equal(focusMainWindowCalls, 1); + assert.equal(windowFocusCalls, 1); + assert.equal(overlayFocusCalls, 1); + } finally { + Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow }); + Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument }); + Object.defineProperty(globalThis, 'MutationObserver', { + configurable: true, + value: previousMutationObserver, + }); + Object.defineProperty(globalThis, 'Node', { configurable: true, value: previousNode }); + } +}); + test('restorePointerInteractionState re-enables subtitle hover when pointer is already over subtitles', () => { const ctx = createMouseTestContext(); const originalWindow = globalThis.window; diff --git a/src/renderer/handlers/mouse.ts b/src/renderer/handlers/mouse.ts index 5c7e6e63..8765edd6 100644 --- a/src/renderer/handlers/mouse.ts +++ b/src/renderer/handlers/mouse.ts @@ -34,6 +34,29 @@ export function createMouseHandlers( let lastPointerPosition: { clientX: number; clientY: number } | null = null; let pendingPointerResync = false; + function reclaimOverlayWindowFocusForPopup(): void { + if (!ctx.platform.shouldToggleMouseIgnore) { + return; + } + if (ctx.platform.isMacOSPlatform || ctx.platform.isLinuxPlatform) { + return; + } + + if (typeof window.electronAPI.focusMainWindow === 'function') { + void window.electronAPI.focusMainWindow(); + } + window.focus(); + if (typeof ctx.dom.overlay.focus === 'function') { + ctx.dom.overlay.focus({ preventScroll: true }); + } + } + + function sustainPopupInteraction(): void { + yomitanPopupVisible = true; + ctx.state.yomitanPopupVisible = true; + syncOverlayMouseIgnoreState(ctx); + } + function isElementWithinContainer(element: Element | null, container: HTMLElement): boolean { if (!element) { return false; @@ -205,9 +228,7 @@ export function createMouseHandlers( } function enablePopupInteraction(): void { - yomitanPopupVisible = true; - ctx.state.yomitanPopupVisible = true; - syncOverlayMouseIgnoreState(ctx); + sustainPopupInteraction(); if (ctx.platform.isMacOSPlatform) { window.focus(); } @@ -215,8 +236,8 @@ export function createMouseHandlers( function disablePopupInteractionIfIdle(): void { if (typeof document !== 'undefined' && isYomitanPopupVisible(document)) { - yomitanPopupVisible = true; - ctx.state.yomitanPopupVisible = true; + sustainPopupInteraction(); + reclaimOverlayWindowFocusForPopup(); return; } diff --git a/src/renderer/style.css b/src/renderer/style.css index 5ca4db41..53c4ce7d 100644 --- a/src/renderer/style.css +++ b/src/renderer/style.css @@ -684,7 +684,8 @@ body.settings-modal-open #subtitleContainer { } body.settings-modal-open iframe.yomitan-popup, -body.settings-modal-open iframe[id^='yomitan-popup'] { +body.settings-modal-open iframe[id^='yomitan-popup'], +body.settings-modal-open [data-subminer-yomitan-popup-host='true'] { display: none !important; pointer-events: none !important; } @@ -1151,7 +1152,8 @@ body.subtitle-sidebar-embedded-open #secondarySubContainer.secondary-sub-hover { } iframe.yomitan-popup, -iframe[id^='yomitan-popup'] { +iframe[id^='yomitan-popup'], +[data-subminer-yomitan-popup-host='true'] { pointer-events: auto !important; z-index: 2147483647 !important; } diff --git a/src/renderer/yomitan-popup.ts b/src/renderer/yomitan-popup.ts index 28aa62f6..82c2f2ff 100644 --- a/src/renderer/yomitan-popup.ts +++ b/src/renderer/yomitan-popup.ts @@ -1,4 +1,8 @@ export const YOMITAN_POPUP_IFRAME_SELECTOR = 'iframe.yomitan-popup, iframe[id^="yomitan-popup"]'; +export const YOMITAN_POPUP_HOST_SELECTOR = '[data-subminer-yomitan-popup-host="true"]'; +export const YOMITAN_POPUP_VISIBLE_HOST_SELECTOR = + '[data-subminer-yomitan-popup-host="true"][data-subminer-yomitan-popup-visible="true"]'; +const YOMITAN_POPUP_VISIBLE_ATTRIBUTE = 'data-subminer-yomitan-popup-visible'; export const YOMITAN_POPUP_SHOWN_EVENT = 'yomitan-popup-shown'; export const YOMITAN_POPUP_HIDDEN_EVENT = 'yomitan-popup-hidden'; export const YOMITAN_POPUP_MOUSE_ENTER_EVENT = 'yomitan-popup-mouse-enter'; @@ -29,21 +33,48 @@ export function isYomitanPopupIframe(element: Element | null): boolean { } export function hasYomitanPopupIframe(root: ParentNode = document): boolean { - return root.querySelector(YOMITAN_POPUP_IFRAME_SELECTOR) !== null; + return ( + root.querySelector(YOMITAN_POPUP_IFRAME_SELECTOR) !== null || + root.querySelector(YOMITAN_POPUP_HOST_SELECTOR) !== null + ); +} + +function isVisiblePopupElement(element: Element): boolean { + const rect = element.getBoundingClientRect(); + if (rect.width <= 0 || rect.height <= 0) { + return false; + } + + const styles = window.getComputedStyle(element); + if (styles.visibility === 'hidden' || styles.display === 'none' || styles.opacity === '0') { + return false; + } + + return true; +} + +function isMarkedVisiblePopupHost(element: Element): boolean { + return element.getAttribute(YOMITAN_POPUP_VISIBLE_ATTRIBUTE) === 'true'; } export function isYomitanPopupVisible(root: ParentNode = document): boolean { + const visiblePopupHosts = root.querySelectorAll(YOMITAN_POPUP_VISIBLE_HOST_SELECTOR); + if (visiblePopupHosts.length > 0) { + return true; + } + const popupIframes = root.querySelectorAll(YOMITAN_POPUP_IFRAME_SELECTOR); for (const iframe of popupIframes) { - const rect = iframe.getBoundingClientRect(); - if (rect.width <= 0 || rect.height <= 0) { - continue; + if (isVisiblePopupElement(iframe)) { + return true; } - const styles = window.getComputedStyle(iframe); - if (styles.visibility === 'hidden' || styles.display === 'none' || styles.opacity === '0') { - continue; + } + + const popupHosts = root.querySelectorAll(YOMITAN_POPUP_HOST_SELECTOR); + for (const host of popupHosts) { + if (isMarkedVisiblePopupHost(host)) { + return true; } - return true; } return false; } diff --git a/vendor/subminer-yomitan b/vendor/subminer-yomitan index 69620abc..5e46b8ba 160000 --- a/vendor/subminer-yomitan +++ b/vendor/subminer-yomitan @@ -1 +1 @@ -Subproject commit 69620abcbc126edd2dcbe637f0fac582e9b555c5 +Subproject commit 5e46b8bac661001b857c17de344fd7ee0a72629a