fix(modals): keep restored settings and theme windows open (#6924)

* fix(modals): keep restored settings and theme windows open

* fix(theme): cancel closing on a rapid toggle reopen

* test: isolate photo routing from external MCP registry state

* fix(cache): preserve the upstream editor worker key format

static/app.js

@@ -1319,8 +1319,7 @@ function initializeEventListeners() {
   const toolThemeBtn = el('tool-theme-btn');
   if (toolThemeBtn) {
     toolThemeBtn.addEventListener('click', () => {
-      const tm = document.getElementById('theme-modal');
-      if (tm) tm.classList.remove('hidden');
+      themeModule.openPopup();
     });
   }
 

static/js/modalVisibility.js

@@ -0,0 +1,42 @@
+// Own both completion paths of an animated close so neither can outlive it.
+const _pendingCloses = new WeakMap();
+
+function cancelClose(modal) {
+  const pending = _pendingCloses.get(modal);
+  if (!pending) return;
+  clearTimeout(pending.timer);
+  pending.content.removeEventListener('animationend', pending.onEnd);
+  pending.content.classList.remove('modal-closing');
+  _pendingCloses.delete(modal);
+}
+
+export function showModal(modal) {
+  if (!modal) return;
+  cancelClose(modal);
+  modal.classList.remove('hidden');
+}
+
+export function hideModal(modal) {
+  if (!modal) return;
+  cancelClose(modal);
+  const content = modal.querySelector('.modal-content, .settings-modal-content');
+  // A minimized window has no exit animation. Registering a listener here
+  // would let its next entrance animation close the newly opened window.
+  if (!content || modal.classList.contains('hidden')) {
+    modal.classList.add('hidden');
+    return;
+  }
+  const pending = { content, timer: null, onEnd: null };
+  const finish = () => {
+    if (_pendingCloses.get(modal) !== pending) return;
+    cancelClose(modal);
+    modal.classList.add('hidden');
+  };
+  pending.onEnd = event => {
+    if (event.target === content) finish();
+  };
+  _pendingCloses.set(modal, pending);
+  content.classList.add('modal-closing');
+  content.addEventListener('animationend', pending.onEnd);
+  pending.timer = setTimeout(finish, 250);
+}

static/js/settings/lifecycle.js

@@ -6,6 +6,8 @@
 
 import { makeWindowDraggable } from '../windowDrag.js';
 import { clearDockSide } from '../modalSnap.js';
+import { toggle as restoreMinimizedModal } from '../modalManager.js';
+import { showModal, hideModal } from '../modalVisibility.js';
 
 const _dragBound = new WeakSet();
 const _closeBound = new WeakSet();
@@ -147,30 +149,16 @@ export function bindSettingsClose(modalEl, options = {}) {
 
 export function showSettingsModal(modalEl) {
   if (!modalEl) return;
+  if (restoreMinimizedModal(modalEl.id)) {
+    showModal(modalEl);
+    return;
+  }
   if (modalEl.classList.contains('hidden')) {
     resetSettingsWindowPlacement(modalEl);
   }
-  modalEl.classList.remove('hidden');
+  showModal(modalEl);
 }
 
 export function hideSettingsModal(modalEl) {
-  if (!modalEl) return;
-
-  const content = modalEl.querySelector('.modal-content, .settings-modal-content');
-  if (content && !content.classList.contains('modal-closing')) {
-    content.classList.add('modal-closing');
-    content.addEventListener('animationend', () => {
-      modalEl.classList.add('hidden');
-      content.classList.remove('modal-closing');
-    }, { once: true });
-    setTimeout(() => {
-      if (!modalEl.classList.contains('hidden')) {
-        modalEl.classList.add('hidden');
-        content.classList.remove('modal-closing');
-      }
-    }, 250);
-    return;
-  }
-
-  modalEl.classList.add('hidden');
+  hideModal(modalEl);
 }

static/js/theme.js

@@ -7,6 +7,8 @@ import { initColorPickers, attachColorPicker } from './colorPicker.js?v=20260910
 import { hexToRgb } from './color/hex.js';
 import { makeWindowDraggable } from './windowDrag.js';
 import { snapModalToZone } from './tileManager.js?v=20260910responsivebounds1';
+import { toggle as restoreMinimizedModal } from './modalManager.js';
+import { showModal, hideModal } from './modalVisibility.js';
 
 export const THEMES = {
   dark:       { bg:'#282c34', fg:'#9cdef2', panel:'#111111', border:'#355a66', red:'#e06c75' },
@@ -1570,31 +1572,29 @@ export function makeDraggable(el, handle) {
 }
 
 // Toggle the popup
+export function openPopup() {
+  const modal = document.getElementById('theme-modal');
+  if (!modal) return;
+  restoreMinimizedModal(modal.id);
+  showModal(modal);
+}
+
 export function togglePopup() {
   const modal = document.getElementById('theme-modal');
   if (!modal) return;
-  const visible = !modal.classList.contains('hidden');
+  const visible = !modal.classList.contains('hidden')
+    && !modal.classList.contains('modal-minimized')
+    && !modal.querySelector('.modal-closing');
   if (visible) {
-    modal.classList.add('hidden');
+    closePopup();
   } else {
-    modal.classList.remove('hidden');
+    openPopup();
   }
 }
 
 export function closePopup() {
   const modal = document.getElementById('theme-modal');
-  if (!modal) return;
-  const content = modal.querySelector('.modal-content');
-  if (content && !content.classList.contains('modal-closing')) {
-    content.classList.add('modal-closing');
-    content.addEventListener('animationend', () => {
-      modal.classList.add('hidden');
-      content.classList.remove('modal-closing');
-    }, { once: true });
-    setTimeout(() => { if (!modal.classList.contains('hidden')) { modal.classList.add('hidden'); content.classList.remove('modal-closing'); } }, 250);
-  } else {
-    modal.classList.add('hidden');
-  }
+  hideModal(modal);
 }
 
 // Expose for app.js wiring + AI ui_control
@@ -2278,7 +2278,7 @@ function _initEmbers() {
   draw();
 }
 
-const themeModule = { initThemeUI, togglePopup, closePopup, makeDraggable,
+const themeModule = { initThemeUI, openPopup, togglePopup, closePopup, makeDraggable,
                        THEMES, applyColors, applyFontDensity, applyBgPattern,
                        applyBgEffectColor, applyBgEffectIntensity, applyBgEffectSize, applyBgEffectSpeed,
                        applyFrostedGlass,

static/sw.js

@@ -38,6 +38,7 @@ const KATEX_FONTS = [
 // Both are fetched at install time, in the background. Entries must match the
 // exact URL the browser requests, query string included.
 const PRECACHE = [
+  '/static/js/modalVisibility.js',
   '/static/js/compare/sse.js',
   '/',
   '/static/css/00-tokens.css?v=20261008confirmwrap1',

tests/helpers/test_settings_shell.js

@@ -348,6 +348,8 @@ function moduleSource(relativePath) {
     },
     makeWindowDraggable: (...args) => dragCalls.push(args),
     clearDockSide: (...args) => dockCalls.push(args),
+    restoreMinimizedModal: () => false,
+    clearTimeout() {},
     setTimeout: callback => { callback(); return 1; },
     WeakSet,
   };
@@ -356,6 +358,7 @@ function moduleSource(relativePath) {
   vm.runInContext(moduleSource('search.js'), context, { filename: 'search.js' });
   vm.runInContext(moduleSource('sidebar.js'), context, { filename: 'sidebar.js' });
   vm.runInContext(moduleSource('navigation.js'), context, { filename: 'navigation.js' });
+  vm.runInContext(moduleSource('../modalVisibility.js'), context, { filename: 'modalVisibility.js' });
   vm.runInContext(moduleSource('lifecycle.js'), context, { filename: 'lifecycle.js' });
   vm.runInContext(moduleSource('dom.js'), context, { filename: 'dom.js' });
 

tests/helpers/test_settings_shell_coordinator.mjs

@@ -20,6 +20,7 @@ const REAL_MODULES = new Set([
   path.join(JS, 'settings/sidebar.js'),
   path.join(JS, 'settings/navigation.js'),
   path.join(JS, 'settings/lifecycle.js'),
+  path.join(JS, 'modalVisibility.js'),
   path.join(JS, 'settings/api.js'),
   path.join(JS, 'settings/speech.js'),
   path.join(JS, 'settings/writingStyle.js'),
@@ -869,6 +870,7 @@ const STUBS = new Map([
       clearDockSide() {},
     },
   ],
+  [path.join(JS, 'modalManager.js'), { toggle() { return false; } }],
 ]);
 
 

tests/test_modal_reopen_lifecycle.py

@@ -0,0 +1,126 @@
+"""Real ESM/DOM regressions for Settings and Theme close/restore races."""
+import json
+import subprocess
+
+import pytest
+
+from tests.helpers.stylesheets import stylesheet_link_tags
+
+
+@pytest.mark.parametrize('modal_id', ['settings-modal', 'theme-modal'])
+@pytest.mark.parametrize('width', [1440, 390])
+def test_modal_reopen_after_dock_close_and_pending_animation(modal_id, width):
+    script = r"""
+      import { chromium } from 'playwright';
+      const browser = await chromium.launch({headless:true});
+      try {
+        const page = await browser.newPage({viewport:{width:WIDTH,height:1000}});
+        await page.route('**/api/**', route => route.fulfill({json:{value:null}}));
+        await page.goto(process.env.ODYSSEUS_TEST_STATIC_ORIGIN+'/static/js/storage.js');
+        await page.setContent('__STYLES__<div id="sidebar"></div>');
+        const result = await page.evaluate(async id => {
+          const html = await (await fetch('/static/index.html')).text();
+          const source = new DOMParser().parseFromString(html,'text/html');
+          document.body.append(source.getElementById(id));
+          const modal = document.getElementById(id);
+          const content = modal.querySelector('.modal-content');
+          const manager = await import('/static/js/modalManager.js');
+          let open, close;
+          if(id === 'settings-modal') {
+            const lifecycle = await import('/static/js/settings/lifecycle.js');
+            open = () => lifecycle.showSettingsModal(modal);
+            close = () => lifecycle.hideSettingsModal(modal);
+          } else {
+            const theme = await import('/static/js/theme.js');
+            // The old sidebar simply removed hidden; the public opener now
+            // also cancels a pending close and restores manager state.
+            open = theme.openPopup || (() => modal.classList.remove('hidden'));
+            close = () => theme.closePopup();
+          }
+          const visible = () => getComputedStyle(modal).display !== 'none';
+          const wait = () => new Promise(resolve=>setTimeout(resolve,350));
+          const states = {};
+          if (id === 'theme-modal') {
+            const theme = await import('/static/js/theme.js');
+            theme.togglePopup();
+            theme.togglePopup();
+            theme.togglePopup();
+            await wait();
+            states.fastToggleReopens = visible();
+            close();
+            await wait();
+          }
+          open();
+          manager.register(id,{closeFn:close,restoreFn:()=>{}});
+          manager.minimize(id);
+          manager.close(id);
+          await wait();
+          open();
+          await wait();
+          states.afterDockClose = visible();
+          // An opening/descendant animation must not reach an orphan closer.
+          content.dispatchEvent(new AnimationEvent('animationend',{bubbles:true,animationName:'modal-enter'}));
+          states.afterOpeningAnimation = visible();
+          // Begin a visible close, then reopen before its fallback expires.
+          close();
+          open();
+          await wait();
+          states.afterFastReopen = visible();
+          manager.register(id,{closeFn:close,restoreFn:()=>{}});
+          manager.minimize(id);
+          open();
+          states.directOpenRestoresMinimized = visible() && !manager.isMinimized(id);
+          // A child animation must not terminate a normal parent close.
+          close();
+          content.querySelector('button').dispatchEvent(new AnimationEvent('animationend',{bubbles:true}));
+          states.childAnimationDoesNotClose = !modal.classList.contains('hidden');
+          content.dispatchEvent(new AnimationEvent('animationend',{bubbles:true,animationName:'modal-exit'}));
+          states.normalCloseCompletes = modal.classList.contains('hidden') && !content.classList.contains('modal-closing');
+          // Completing by animation must also remove the timer.
+          open();
+          await wait();
+          states.reopenAfterCompletedAnimation = visible();
+          close();
+          await wait();
+          states.fallbackCompletes = modal.classList.contains('hidden') && !content.classList.contains('modal-closing');
+          return states;
+        }, MODAL_ID);
+        console.log(JSON.stringify(result));
+      } finally { await browser.close(); }
+    """
+    script = script.replace('WIDTH', str(width)).replace('MODAL_ID', json.dumps(modal_id))
+    script = script.replace('__STYLES__', stylesheet_link_tags())
+    result = subprocess.run(['node','--input-type=module','-e',script], capture_output=True, text=True, timeout=60)
+    assert result.returncode == 0, result.stdout + result.stderr
+    states = json.loads(result.stdout.strip())
+    assert all(states.values()), states
+
+
+def test_modal_visibility_is_available_offline_before_first_open():
+    script = r"""
+      import assert from 'node:assert/strict';
+      import { chromium } from 'playwright';
+      const browser = await chromium.launch({headless:true});
+      try {
+        const context = await browser.newContext({serviceWorkers:'allow'});
+        const page = await context.newPage();
+        const origin = process.env.ODYSSEUS_TEST_STATIC_ORIGIN;
+        await page.goto(origin+'/static/js/storage.js');
+        await page.evaluate(async () => {
+          await navigator.serviceWorker.register('/static/sw.js');
+          await navigator.serviceWorker.ready;
+          if(!navigator.serviceWorker.controller) await new Promise(resolve =>
+            navigator.serviceWorker.addEventListener('controllerchange',resolve,{once:true}));
+          const response = await caches.match('/static/js/modalVisibility.js');
+          if(!response?.ok) throw new Error('Modal helper is missing from install-time precache');
+        });
+        await context.setOffline(true);
+        const offline = await context.newPage();
+        await offline.goto(origin+'/static/js/storage.js');
+        const exports = await offline.evaluate(async () =>
+          Object.keys(await import('/static/js/modalVisibility.js')).sort());
+        assert.deepEqual(exports,['hideModal','showModal']);
+      } finally { await browser.close(); }
+    """
+    result = subprocess.run(['node','--input-type=module','-e',script], capture_output=True, text=True, timeout=90)
+    assert result.returncode == 0, result.stdout + result.stderr