From 184a40c57dca8aa0c673f70061ba24c9354a3392 Mon Sep 17 00:00:00 2001 From: sudacode Date: Fri, 15 May 2026 01:59:38 -0700 Subject: [PATCH] fix: remove vendored Yomitan settings assumptions --- changes/tray-modal-lifecycle.md | 4 +- src/core/services/yomitan-settings.test.ts | 88 +++++++--------------- src/core/services/yomitan-settings.ts | 37 +++++++-- 3 files changed, 60 insertions(+), 69 deletions(-) diff --git a/changes/tray-modal-lifecycle.md b/changes/tray-modal-lifecycle.md index 4ae89d06..6a3261b1 100644 --- a/changes/tray-modal-lifecycle.md +++ b/changes/tray-modal-lifecycle.md @@ -3,9 +3,7 @@ area: tray - Kept the tray app running when closing tray-launched Yomitan settings. - Kept tray-launched Yomitan settings loading from blocking other tray actions. -- Removed the default native app menu from Yomitan settings so File > Quit cannot put the tray app into a stuck quit state. +- Replaced the default native Yomitan settings menu with a close-only menu so closing settings does not quit the tray app. - Disabled Yomitan's embedded popup preview in the tray-launched settings window to avoid renderer hangs during normal sidebar navigation. -- Skipped heavy Yomitan settings startup preview, storage, dictionary, and Anki controllers when launched from SubMiner to avoid renderer hangs with large dictionary databases. -- Cached Yomitan settings dictionary metadata after explicit loads to avoid repeated large IndexedDB reads. - Serialized copied Yomitan extension refreshes so startup cannot race itself and leave extension loading in an error state. - Fixed tray-launched session help focus handling so the modal can close without mpv running. diff --git a/src/core/services/yomitan-settings.test.ts b/src/core/services/yomitan-settings.test.ts index 7fd9a608..18cbc1cc 100644 --- a/src/core/services/yomitan-settings.test.ts +++ b/src/core/services/yomitan-settings.test.ts @@ -1,87 +1,55 @@ import assert from 'node:assert/strict'; -import { readFileSync } from 'node:fs'; import test from 'node:test'; import { + buildYomitanSettingsWindowMenuTemplate, buildYomitanSettingsUrl, configureYomitanSettingsWindowChrome, destroyYomitanSettingsWindow, showYomitanSettingsWindow, } from './yomitan-settings'; -function assertGuardedBySubminerSettingsSafe(source: string, call: string): void { - const callIndex = source.indexOf(call); - assert.notEqual(callIndex, -1, `missing call: ${call}`); - - const beforeCall = source.slice(0, callIndex); - const guardIndex = beforeCall.lastIndexOf('if (!subminerSettingsSafe) {'); - const blockCloseIndex = beforeCall.lastIndexOf('\n }'); - assert.ok( - guardIndex > blockCloseIndex, - `${call} must be inside its own !subminerSettingsSafe startup guard`, - ); -} - -test('yomitan settings window removes default app menu quit action', () => { +test('yomitan settings window uses a close-only menu without app quit', () => { const calls: string[] = []; configureYomitanSettingsWindowChrome({ + isDestroyed: () => false, + close: () => calls.push('close'), setAutoHideMenuBar: (hide: boolean) => calls.push(`auto-hide:${hide}`), setMenu: (menu: unknown) => calls.push(`menu:${menu === null ? 'null' : 'custom'}`), - } as never); + } as never, (template) => { + calls.push(`menu-label:${template[0]?.label ?? ''}`); + const submenu = template[0]?.submenu; + assert.ok(Array.isArray(submenu)); + const closeItem = submenu[0]; + assert.equal(closeItem?.label, 'Close'); + assert.notEqual(closeItem?.role, 'quit'); + closeItem?.click?.({} as never, {} as never, {} as never); + return { id: 'settings-menu' } as never; + }); - assert.deepEqual(calls, ['auto-hide:true', 'menu:null']); + assert.deepEqual(calls, ['auto-hide:false', 'menu-label:File', 'close', 'menu:custom']); +}); + +test('yomitan settings close menu skips destroyed windows', () => { + const calls: string[] = []; + const template = buildYomitanSettingsWindowMenuTemplate({ + isDestroyed: () => true, + close: () => calls.push('close'), + } as never); + const submenu = template[0]?.submenu; + assert.ok(Array.isArray(submenu)); + submenu[0]?.click?.({} as never, {} as never, {} as never); + assert.deepEqual(calls, []); }); test('yomitan settings URL disables the embedded popup preview', () => { assert.equal( buildYomitanSettingsUrl('abc123'), - 'chrome-extension://abc123/settings.html?popup-preview=false&subminer-settings-safe=true', + 'chrome-extension://abc123/settings.html?popup-preview=false', ); }); -test('vendored Yomitan settings safe mode skips heavy startup controllers', () => { - const source = readFileSync( - 'vendor/subminer-yomitan/ext/js/pages/settings/settings-main.js', - 'utf8', - ); - - assert.match(source, /subminer-settings-safe/); - assertGuardedBySubminerSettingsSafe(source, 'popupPreviewController.prepare()'); - assertGuardedBySubminerSettingsSafe(source, 'persistentStorageController.prepare()'); - assertGuardedBySubminerSettingsSafe(source, 'storageController.prepare()'); - assertGuardedBySubminerSettingsSafe(source, 'dictionaryController.prepare()'); - assertGuardedBySubminerSettingsSafe(source, 'ankiController.prepare()'); - assert.match(source, /if \(!subminerSettingsSafe\)[\s\S]*new AnkiDeckGeneratorController/); - assert.match(source, /if \(!subminerSettingsSafe\)[\s\S]*new SecondarySearchDictionaryController/); - assert.match(source, /if \(!subminerSettingsSafe\)[\s\S]*new SortFrequencyDictionaryController/); -}); - -test('vendored Yomitan settings caches dictionary metadata requests', () => { - const source = readFileSync( - 'vendor/subminer-yomitan/ext/js/pages/settings/settings-controller.js', - 'utf8', - ); - - assert.match(source, /_dictionaryInfoPromise/); - assert.match(source, /_dictionaryInfoCache/); - assert.match(source, /databaseUpdated/); - assert.match( - source, - /this\._dictionaryInfoPromise = this\._application\.api\.getDictionaryInfo\(\)/, - ); -}); - -test('vendored Yomitan Anki settings reuses SettingsController dictionary metadata cache', () => { - const source = readFileSync( - 'vendor/subminer-yomitan/ext/js/pages/settings/anki-controller.js', - 'utf8', - ); - - assert.match(source, /this\._settingsController\.getDictionaryInfo\(\)/); - assert.doesNotMatch(source, /this\._application\.api\.getDictionaryInfo\(\)/); -}); - test('showYomitanSettingsWindow restores, repaints, shows, and focuses an existing window', () => { const calls: string[] = []; diff --git a/src/core/services/yomitan-settings.ts b/src/core/services/yomitan-settings.ts index 6e67c8a0..2d85ead7 100644 --- a/src/core/services/yomitan-settings.ts +++ b/src/core/services/yomitan-settings.ts @@ -1,8 +1,8 @@ import electron from 'electron'; -import type { BrowserWindow, Extension, Session } from 'electron'; +import type { BrowserWindow, Extension, Menu, MenuItemConstructorOptions, Session } from 'electron'; import { createLogger } from '../../logger'; -const { BrowserWindow: ElectronBrowserWindow, session } = electron; +const { BrowserWindow: ElectronBrowserWindow, Menu: ElectronMenu, session } = electron; const logger = createLogger('main:yomitan-settings'); export interface OpenYomitanSettingsWindowOptions { @@ -13,15 +13,40 @@ export interface OpenYomitanSettingsWindowOptions { onWindowClosed?: () => void; } +type YomitanSettingsWindowMenuOwner = Pick; + +export function buildYomitanSettingsWindowMenuTemplate( + settingsWindow: YomitanSettingsWindowMenuOwner, +): MenuItemConstructorOptions[] { + return [ + { + label: 'File', + submenu: [ + { + label: 'Close', + accelerator: process.platform === 'darwin' ? 'Command+W' : 'Ctrl+W', + click: () => { + if (!settingsWindow.isDestroyed()) { + settingsWindow.close(); + } + }, + }, + ], + }, + ]; +} + export function configureYomitanSettingsWindowChrome( - settingsWindow: Pick, + settingsWindow: Pick, + buildMenu: (template: MenuItemConstructorOptions[]) => Menu = (template) => + ElectronMenu.buildFromTemplate(template), ): void { - settingsWindow.setAutoHideMenuBar(true); - settingsWindow.setMenu(null); + settingsWindow.setAutoHideMenuBar(false); + settingsWindow.setMenu(buildMenu(buildYomitanSettingsWindowMenuTemplate(settingsWindow))); } export function buildYomitanSettingsUrl(extensionId: string): string { - return `chrome-extension://${extensionId}/settings.html?popup-preview=false&subminer-settings-safe=true`; + return `chrome-extension://${extensionId}/settings.html?popup-preview=false`; } export function showYomitanSettingsWindow(settingsWindow: BrowserWindow): void {