From 5f218941969602dc2384df56fdecd9789309d06b Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Mon, 19 Oct 2026 19:23:35 -0700 Subject: [PATCH] fix(viewer): destroy an inline Viewer only after its pending view settles Viewer.js's inline destroy() only unbinds and leaves an in-flight image view running; when it finishes it reads the destroyed item list and throws "c is undefined". Closing the preview while an image was still loading hit this in Firefox about once in 15 runs of the recorded-run history test. A closed viewer is now destroyed once that view settles, its image fails, or a 10 second bound passes. The history test passed 60 of 60 runs in Firefox with the fix. --- tests/js/workspace-media-viewer.test.js | 27 +++++++++++++++++++++++++ web/js/workspace/media-viewer.mjs | 16 +++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/js/workspace-media-viewer.test.js b/tests/js/workspace-media-viewer.test.js index e69187e..1dd0866 100644 --- a/tests/js/workspace-media-viewer.test.js +++ b/tests/js/workspace-media-viewer.test.js @@ -158,3 +158,30 @@ test('real inspection navigation preserves slideshow across two advances and def assert.equal(f.container.children.length,0);assert.equal(controller.state.record.identity.occurrence.output_node_id,'3'); } finally {view.dispose();controller.dispose();parentScope.dispose();globalThis.setTimeout=oldSetTimeout;globalThis.clearTimeout=oldClearTimeout;f.restore();} }); + +test('an inline Viewer still loading its image is destroyed only after it has viewed it', async () => { + const {reportedLocationAdapter:adapter} = await import('../../web/js/workspace/history-media.mjs'); + const f = mountedDom(), parentScope = createScope(), controller = borrowedController(), items = [occurrenceMedia('1',1,'c')]; + const instances = []; + // Viewer.js's inline destroy() leaves an in-flight view running; its callback then needs a live instance. + f.win.Viewer = class { constructor(node,options) { this.options = options; this.destroyed = 0; this.viewed = false; instances.push(this); } + destroy() { this.destroyed++; } resize() {} }; + const globals = {ResizeObserver:globalThis.ResizeObserver, requestAnimationFrame:globalThis.requestAnimationFrame, cancelAnimationFrame:globalThis.cancelAnimationFrame}; + Object.assign(globalThis, {ResizeObserver:class { observe() {} disconnect() {} }, requestAnimationFrame:() => 1, cancelAnimationFrame() {}}); + const view = module.createMediaViewer({container:f.container,parentScope,inspectionController:controller,adapter,sender:{open(){}}}); + try { + view.open({items,selectedKey:'c'.repeat(64)}); + const [loading] = instances; + f.walk(f.container,n => n.attributes['aria-label'] === 'Close preview')[0].fire('click'); + assert.equal(loading.destroyed, 0, 'not destroyed while its image view is pending'); + loading.viewed = true; loading.options.viewed.call(loading); + assert.equal(loading.destroyed, 1, 'destroyed once its pending view settles'); + loading.options.viewed.call(loading); + assert.equal(loading.destroyed, 1, 'never destroyed twice'); + + view.open({items,selectedKey:'c'.repeat(64)}); + const shown = instances[1]; shown.viewed = true; shown.options.viewed.call(shown); + f.walk(f.container,n => n.attributes['aria-label'] === 'Close preview')[0].fire('click'); + assert.equal(shown.destroyed, 1, 'a viewed image is destroyed at once'); + } finally { view.dispose(); parentScope.dispose(); f.restore(); Object.assign(globalThis, globals); } +}); diff --git a/web/js/workspace/media-viewer.mjs b/web/js/workspace/media-viewer.mjs index a3437a4..db43be3 100644 --- a/web/js/workspace/media-viewer.mjs +++ b/web/js/workspace/media-viewer.mjs @@ -4,6 +4,8 @@ import {rootedFileAdapter} from './inspection-state.mjs'; import {mountFileInspector} from './file-inspector.mjs'; let viewerId = 0; +// Upper bound on waiting for a closed Viewer's in-flight image view before destroying it. +const PENDING_VIEW_DESTROY_MS = 10000; export function readViewerPreferences(getStorage = () => window.localStorage, normalize = globalThis.GallerySettings.normalizeGallerySettings) { let stored; try { stored = JSON.parse(getStorage().getItem('gallerySettings')); } catch { stored = null; } @@ -132,7 +134,8 @@ export function createMediaViewer({container,parentScope,inspectionController,se } node.src = url.href;media.append(node); if (!selected.is_video && window.Viewer) { - let fitFrame = 0, fitWidth, fitHeight; + let fitFrame = 0, fitWidth, fitHeight, viewSettled = false, released = false, fallback = 0; + const release = () => { if (released) return; released = true; clearTimeout(fallback); instance.destroy(); }; const scheduleFit = () => { if (!current() || fitFrame) return; fitFrame = requestAnimationFrame(() => { @@ -145,11 +148,20 @@ export function createMediaViewer({container,parentScope,inspectionController,se }; const instance = new window.Viewer(node,{inline:true,container:media,toolbar:false,navbar:false,title:false,button:false, keyboard:false,fullscreen:false,transition:false,viewed() { + viewSettled = true; + if (mediaLocal.signal.aborted) { release(); return; } if (!current()) return; session.ready(key);scheduleFit(); }}); const observer = new ResizeObserver(scheduleFit);observer.observe(stage); - viewer = instance;mediaLocal.cleanup(() => {observer.disconnect();cancelAnimationFrame(fitFrame);instance.destroy();}); + viewer = instance;mediaLocal.cleanup(() => { + observer.disconnect();cancelAnimationFrame(fitFrame); + // Inline Viewer.js keeps viewing a still-loading image after destroy(), and that view + // then reads the destroyed list; destroy once the view settles, fails or times out. + if (viewSettled) { release(); return; } + node.addEventListener('error',release,{once:true}); + fallback = setTimeout(release,PENDING_VIEW_DESTROY_MS); + }); } updateControls(session.state); }