fix(images): build Viewer.js only on a loaded viewer image

Destroying an unbuilt inline Viewer.js at once (429b3a8) is not safe in
one state. Viewer.js's own cancel removes its image listeners only while
`image.complete` is false, so an image that had finished downloading
with its load event still queued kept them. Closing the viewer then
released the src, the queued load built Viewer.js anyway, and its first
view found no listed image: "can't access property querySelector, c is
undefined" (workspace-history.spec.mjs:222 on Firefox in CI).

The viewer now constructs Viewer.js from its own load listener, which the
media scope removes when the image is left. Before the load no Viewer.js
exists, so leaving the image only releases its src; after it Viewer.js
sees a complete image and only its zero-delay build is pending, which
destroy() cancels. Transforms report Viewer.js as unavailable only when
it did not load at all.

tests/browser/workspace-images.spec.mjs

@@ -483,6 +483,30 @@ test('collection viewer leaving an image Viewer.js has not built yet releases it
     await page.evaluate(()=>{location.hash='/overview';});await expect(page.locator('.viewer-container,#viewer-dialog')).toHaveCount(0);
 });
 
+test('collection viewer closed while its image finishes loading never builds Viewer.js afterwards',async({page})=>{
+    await page.goto(workspace);await expect(cards(page)).toHaveCount(2);
+    let held;const waiting=new Promise(resolve=>{held=resolve;});
+    // The image is held for the whole test: its late load is delivered below, at the moment Viewer.js cannot cancel.
+    await page.route('**/images/serve/shared/same.png?*',async route=>{
+        const url=new URL(route.request().url());
+        if(url.searchParams.get('root')==='0'&&url.searchParams.has('_pm_viewer')) {held();return;}
+        await route.continue().catch(()=>{});
+    });
+    await cards(page).first().getByRole('button',{name:'Open same.png',exact:true}).click();await waiting;
+    const preview=page.locator('dialog#viewer-dialog');
+    // Viewer.js's own cancel removes its image listeners only while `image.complete` is false. An image that has
+    // finished downloading but whose load event is still queued is already complete: close in that state, then deliver it.
+    await page.evaluate(()=>{window.finishingImage=document.querySelector('#viewer-stage > img');Object.defineProperty(window.finishingImage,'complete',{get:()=>true});});
+    await page.keyboard.press('Escape');await expect(preview).toHaveCount(0);
+    const after=await page.evaluate(async()=>{
+        const image=window.finishingImage;image.dispatchEvent(new Event('load'));
+        // Viewer.js builds an inline viewer in a zero-delay timeout after its image loads; this runs after it.
+        await new Promise(resolve=>setTimeout(resolve));
+        return {src:image.hasAttribute('src'),built:!!image.parentNode?.querySelector('.viewer-container')};
+    });
+    expect(after).toEqual({src:false,built:false});
+});
+
 test('collection viewer describes video transforms and releases native playback on navigation',async({page,request})=>{
     await expect(await request.post('/__test__/reset',{data:{include_video:true}})).toBeOK();await page.goto(workspace);
     await page.locator('article[data-image-key="0:clip.webm"] .image-open').click();

tests/js/workspace-media-viewer.test.js

@@ -124,23 +124,32 @@ test('real inspection navigation preserves slideshow across two advances and def
     } finally {view.dispose();controller.dispose();parentScope.dispose();globalThis.setTimeout=oldSetTimeout;globalThis.clearTimeout=oldClearTimeout;f.restore();}
 });
 
-test('an inline Viewer not yet built is destroyed and its image released at once', async () => {
+test('Viewer.js is built only on a loaded image and an unbuilt viewer is released at once', async () => {
     const {reportedLocationAdapter:adapter} = await import('../../web/js/workspace/history-media.mjs');
     const f = mountedDom(), parentScope = createScope(), controller = borrowedController(), items = [occurrenceMedia('1',1,'c'),occurrenceMedia('2',0,'d')];
     const instances = [];
-    // Viewer.js builds an inline viewer only once its image loads; until then `ready` is false and destroy() cancels it.
+    // On a loaded image Viewer.js schedules its inline build; until it runs `ready` is false and destroy() cancels it.
     f.win.Viewer = class { constructor(node,options) { this.node = node; this.options = options; this.destroyed = 0; this.ready = false; 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 stageImage = () => f.walk(f.container,n => n.tagName === 'img')[0];
+        view.open({items,selectedKey:'d'.repeat(64)});
+        // Viewer.js's own cancel misses an image whose load event is still queued, so none exists before the load.
+        const unloaded = stageImage(), dropped = [];unloaded.removeAttribute = key => dropped.push(key);
+        assert.equal(instances.length, 0, 'no Viewer.js before the image loads');
+        f.walk(f.container,n => n.attributes['aria-label'] === 'Previous media')[0].fire('click');
+        assert.deepEqual(dropped, ['src'], 'an image left before it loads is released at once');
+        unloaded.fire('load');
+        assert.equal(instances.length, 0, 'its late load builds no Viewer.js');
+        stageImage().fire('load');
         const [loading] = instances, image = loading.node, removed = [];
         image.removeAttribute = key => removed.push(key);
-        // Next drops the still-loading image from the stage; a deferred destroy would let its late load build there.
+        // Next drops the image whose build is pending; a deferred destroy would let that build run there.
         f.walk(f.container,n => n.attributes['aria-label'] === 'Next media')[0].fire('click');
-        assert.equal(instances.length, 2, 'Next builds the next image');
+        stageImage().fire('load');assert.equal(instances.length, 2, 'the next image builds its own Viewer.js once loaded');
         assert.equal(loading.destroyed, 1, 'an unbuilt viewer is destroyed at once');
         assert.deepEqual(removed, ['src'], 'and its image is released at once');
         loading.options.viewed.call(loading);
@@ -159,7 +168,7 @@ test('an inline Viewer still loading its image is destroyed only after it has vi
     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)});
+        view.open({items,selectedKey:'c'.repeat(64)});f.walk(f.container,n => n.tagName === 'img')[0].fire('load');
         const [loading] = instances, image = loading.node, removed = [];
         image.removeAttribute = key => removed.push(key);
         f.walk(f.container,n => n.attributes['aria-label'] === 'Close image viewer')[0].fire('click');
@@ -172,7 +181,7 @@ test('an inline Viewer still loading its image is destroyed only after it has vi
         loading.options.viewed.call(loading);
         assert.equal(loading.destroyed, 1, 'never destroyed twice');
 
-        view.open({items,selectedKey:'c'.repeat(64)});
+        view.open({items,selectedKey:'c'.repeat(64)});f.walk(f.container,n => n.tagName === 'img')[0].fire('load');
         const shown = instances[1]; shown.viewed = true; shown.options.viewed.call(shown);
         f.walk(f.container,n => n.attributes['aria-label'] === 'Close image viewer')[0].fire('click');
         assert.equal(shown.destroyed, 1, 'a viewed image is destroyed at once');
@@ -260,7 +269,7 @@ test('viewer uses adapter.mediaUrl for image and video', async () => {
         const img = f.walk(f.container,n => n.tagName === 'img')[0];
         assert.equal(img.src, image.url, 'the blob URL is used exactly');
         assert.doesNotMatch(img.src, /\?/);
-        assert.equal(instances[0].node, img, 'Viewer.js shows the blob image');
+        img.fire('load');assert.equal(instances[0].node, img, 'Viewer.js shows the blob image');
         const download = f.walk(f.container,n => n.attributes['aria-label'] === 'Download original')[0];
         assert.equal(download.href, image.url);assert.equal(download.download, 'a.png');
         f.walk(f.container,n => n.attributes['aria-label'] === 'Next media')[0].fire('click');

web/js/workspace/media-viewer.mjs

@@ -151,6 +151,14 @@ export function createMediaViewer({api,container,parentScope,inspectionControlle
             }
             node.src = src;stage.append(node);
             if (!selected.is_video && window.Viewer) {
+                // Viewer.js is built on an image that has loaded. Its own cancel removes its image listeners only
+                // while `image.complete` is false, so an image left with its load event still queued would build a
+                // viewer into the released image after destroy() (#293). Until then no viewer exists to cancel.
+                let mountedViewer = false;
+                mediaLocal.listen(node,'load',() => { if (current()) { mountedViewer = true;mountViewer(); } },{once:true});
+                mediaLocal.cleanup(() => { if (!mountedViewer) node.removeAttribute('src'); });
+            }
+            function mountViewer() {
                 let fitFrame = 0, fitWidth, fitHeight, viewSettled = false, released = false, fallback = 0;
                 const release = () => {
                     if (released) return;
@@ -176,8 +184,8 @@ export function createMediaViewer({api,container,parentScope,inspectionControlle
                 const observer = new ResizeObserver(scheduleFit);observer.observe(stage);
                 viewer = instance;mediaLocal.cleanup(() => {
                     observer.disconnect();cancelAnimationFrame(fitFrame);
-                    // Before Viewer.js has built (its image still loading), destroy() cancels the pending build;
-                    // deferring it would let the late load build into the image the stage has just dropped.
+                    // Before Viewer.js has built (its zero-delay build is pending), destroy() cancels that build;
+                    // deferring it would let the build run into the image the stage has just dropped.
                     // Once built, inline Viewer.js keeps viewing a still-loading image after destroy(), and that
                     // view rebuilds its list from images that still have a src; keep both until the view settles,
                     // fails or times out.
@@ -190,7 +198,7 @@ export function createMediaViewer({api,container,parentScope,inspectionControlle
         }
         function updateControls(state) {
             const unavailable = inspectionState.refreshRequired ? inspectionState.message : state.phase === 'error' ? 'Media could not be loaded. Retry this file.' : entry.is_video ?
-                'Image transforms are unavailable for video.' : !viewer ?
+                'Image transforms are unavailable for video.' : !window.Viewer ?
                 'Image transforms are unavailable because Viewer.js could not load.' : !state.mediaReady ? 'Image transforms are available when the image is ready.' : '';
             reason.textContent = unavailable;reason.hidden = !unavailable;
             for (const node of transforms) {