From c9678245f884e6090d8faf1b3cde6de9455266ee Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Mon, 19 Oct 2026 19:47:49 -0700 Subject: [PATCH] fix(viewer): keep the image src until a closed Viewer is released Viewer.js rebuilds its item list from images that still have a src. On close, the src was removed before the viewer cleanup ran, so a pending view found no items and threw "c is undefined" (WebKit and Firefox). With Viewer.js, the src is now removed after the viewer is destroyed, once its pending view settles. The recorded-run history test passed 40 of 40 runs in WebKit and in Firefox. --- tests/js/workspace-media-viewer.test.js | 8 ++++++-- web/js/workspace/media-viewer.mjs | 15 +++++++++++---- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/js/workspace-media-viewer.test.js b/tests/js/workspace-media-viewer.test.js index 1dd08662..d4e99947 100644 --- a/tests/js/workspace-media-viewer.test.js +++ b/tests/js/workspace-media-viewer.test.js @@ -164,18 +164,22 @@ test('an inline Viewer still loading its image is destroyed only after it has vi 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); } + f.win.Viewer = class { constructor(node,options) { this.node = node; 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; + const [loading] = instances, image = loading.node, removed = []; + image.removeAttribute = key => removed.push(key); 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'); + // Viewer.js lists only images that still have a src; the pending view needs it. + assert.deepEqual(removed, [], 'the image keeps its src while the view is pending'); loading.viewed = true; loading.options.viewed.call(loading); assert.equal(loading.destroyed, 1, 'destroyed once its pending view settles'); + assert.deepEqual(removed, ['src'], 'the src is released after the viewer'); loading.options.viewed.call(loading); assert.equal(loading.destroyed, 1, 'never destroyed twice'); diff --git a/web/js/workspace/media-viewer.mjs b/web/js/workspace/media-viewer.mjs index db43be31..5f10bf02 100644 --- a/web/js/workspace/media-viewer.mjs +++ b/web/js/workspace/media-viewer.mjs @@ -129,13 +129,19 @@ export function createMediaViewer({container,parentScope,inspectionController,se } else { node.alt = selected.filename;node.decoding = 'async'; node.hidden = !!window.Viewer; - if (!window.Viewer) mediaLocal.listen(node,'load',() => {if (current()) session.ready(key);}); - mediaLocal.cleanup(() => node.removeAttribute('src')); + // With Viewer.js the src is released after the viewer (see release below). + if (!window.Viewer) { + mediaLocal.listen(node,'load',() => {if (current()) session.ready(key);}); + mediaLocal.cleanup(() => node.removeAttribute('src')); + } } node.src = url.href;media.append(node); if (!selected.is_video && window.Viewer) { let fitFrame = 0, fitWidth, fitHeight, viewSettled = false, released = false, fallback = 0; - const release = () => { if (released) return; released = true; clearTimeout(fallback); instance.destroy(); }; + const release = () => { + if (released) return; + released = true;clearTimeout(fallback);instance.destroy();node.removeAttribute('src'); + }; const scheduleFit = () => { if (!current() || fitFrame) return; fitFrame = requestAnimationFrame(() => { @@ -157,7 +163,8 @@ export function createMediaViewer({container,parentScope,inspectionController,se 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. + // rebuilds its list from images that still have a src; keep both until the view + // settles, fails or times out. if (viewSettled) { release(); return; } node.addEventListener('error',release,{once:true}); fallback = setTimeout(release,PENDING_VIEW_DESTROY_MS);