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.
This commit is contained in:
Vito Sansevero
2026-10-19 19:23:35 -07:00
parent 02e339ca1f
commit 5f21894196
2 changed files with 41 additions and 2 deletions
+27
View File
@@ -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); }
});
+14 -2
View File
@@ -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);
}