From 505c16a7247b4191f488271a97715903fc4aac68 Mon Sep 17 00:00:00 2001 From: Vito Date: Thu, 29 Oct 2026 04:36:30 +0000 Subject: [PATCH 1/3] test(collections): order the review M1 reads by events, not the 300 ms timer (#459) (#463) The M1 test let the rename answer at once and changed the hash only after the toast, so on a loaded host the rename's 300 ms count read could start before Fresh's page read; the page then waited on that one read and the poll for two held reads failed. Hold the rename's answer until Fresh's read is held. --- tests/browser/workspace-collections.spec.mjs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/browser/workspace-collections.spec.mjs b/tests/browser/workspace-collections.spec.mjs index a77564ed..9decc16d 100644 --- a/tests/browser/workspace-collections.spec.mjs +++ b/tests/browser/workspace-collections.spec.mjs @@ -363,14 +363,26 @@ test('a list read superseded by another read still opens a collection made since const fresh = (await added.json()).collection.id; await expect(rows(page)).toHaveCount(2); const hold = await holdListReads(page); - // A rename schedules the 300 ms count re-read; opening "Fresh" first reads the list once more. The timer's read - // supersedes that one while both are held. + // A confirmed rename schedules the 300 ms count re-read; opening "Fresh" first reads the list once more. The + // timer's read supersedes that one while both are held. The rename's answer is held until "Fresh"'s read is, so + // the timer starts only then: with the answer let through at once, a slow host could fire the timer before the + // hash change, and the page would then wait on the timer's read alone (#459). + let releaseRename; + const renameHeld = new Promise(resolve => { releaseRename = resolve; }); + await page.route(`**${API}/${manifest.collections.Landscapes.id}`, async route => { + if (route.request().method() !== 'PATCH') return route.fallback(); + await renameHeld; + return route.fallback(); + }); await editButton(page).click(); await nameField(page).fill('Landscape grids'); await editDialog(page).getByRole('button', { name: 'Save', exact: true }).click(); - await expect(toast(page)).toHaveText('Collection renamed.'); await page.evaluate(hash => { window.location.hash = hash; }, `#/collection/${fresh}`); - await expect.poll(() => hold.held.length).toBeGreaterThanOrEqual(2); + await expect.poll(() => hold.held.length).toBe(1); + releaseRename(); + // The rename landed (the sidebar row has the new name), so the count read is now due. + await expect(row(page, 'Landscape grids')).toHaveCount(1); + await expect.poll(() => hold.held.length).toBe(2); await expect(main(page)).not.toContainText('Collections could not be loaded.'); await hold.release(); await expect(heading(page)).toHaveText('Fresh'); From 2941a9b748fed0e564851279584572cf386bbcc0 Mon Sep 17 00:00:00 2001 From: Vito Date: Thu, 29 Oct 2026 05:19:04 +0000 Subject: [PATCH 2/3] feat(inspect): choosing a file opens the viewer with its details (#448) (#458) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(inspector): give image tags their own labelled section (#449) The inspector drew a file's tags as a bare chip row right under the negative prompt, so they read as part of it. The side panel, the viewer details and History's current file now draw them in labelled sections in the prompt sections' frame: "Image tags · N" for the file's own tags, then "Prompt tags · N" or "Recipe tags · N" for the linked tags. Each count is true: repeats are removed within a source only. Recorded as D154. * feat(inspect): choosing a file opens the viewer with its details (#448) On Inspect a file, choosing or dropping a file now opens the shared viewer on it at once, details showing, instead of previewing it and waiting for "Open file metadata". Details show on this page even when "Collapse viewer details by default" is on. An unreadable file or an AVI/WMV note stays on the page; the heading action reopens any selection. A new selection that starts while the viewer shows the previous file closes it first, so its blob URL is never revoked under an open element. Recorded as D155. * fix(inspect): open a video's viewer only once its preview settles (#448) Replacing a video while both the inline preview and the auto-opened viewer's video were still starting deadlocked WebKit (D62). The viewer now opens for a video once its inline preview reports metadata or an error (or after 15 s). * test(inspect): cover the AVI note and an open dialog holding the auto-open back (#448) Frontend coverage for inspect.mjs fell below its floor (branches 91.04 < 92, functions 92.85 < 93): no spec reached the AVI toast or the open-dialog guard, and the video wait had a page-end callback no spec reached. The wait now ends through its own callback at page end, and two specs cover the AVI note and a selection landing under another dialog. * test(comfyui): the Inspect sender spec takes the viewer choosing a file opens (#448) The real-host spec chose a file and then pressed "Open file metadata"; with D155 the viewer is already open, so the press waited behind the modal until the test timed out in the merge queue (classic-start and nodes2-start). --- TESTING.md | 40 ++++++- tests/browser/inspect-concept.spec.mjs | 31 +++-- tests/browser/workspace-inspect.spec.mjs | 145 +++++++++++++++++++++-- tests/comfyui/inspect-sender.spec.mjs | 4 +- web/js/workspace/inspect.mjs | 53 +++++++-- web/js/workspace/media-viewer.mjs | 13 +- 6 files changed, 243 insertions(+), 43 deletions(-) diff --git a/TESTING.md b/TESTING.md index 4d8daa7d..d7c45ef7 100644 --- a/TESTING.md +++ b/TESTING.md @@ -2811,10 +2811,12 @@ mapped differences"): - **D60** (heading action, LI-3): "Open file metadata" (`ca-primary`) replaces the concept's "Open sample metadata". It is `aria-disabled="true"` and `aria-describedby="local-file-note"` (the drop note) until a classified selection is - ready, then opens the shared viewer on that file. + ready, then opens the shared viewer on that file. Amended by D155: choosing the file + already opens it; the action reopens it. - **D61** (copy, LI-3): the surface `p` reads "Read embedded metadata in this browser. The file is not uploaded or added to your library."; the drop note "Your file stays in - this browser. Its metadata opens with “Open file metadata”."; the chooser label "Choose + this browser. Its metadata opens with “Open file metadata”." (D155: "Your file stays in + this browser. Choosing it opens its metadata."); the chooser label "Choose an image or video for a local preview"; the toast "Local preview only. No file uploaded; metadata is available above.". The h2 "Inspect without importing" is the concept's, verbatim. @@ -2868,6 +2870,7 @@ mapped differences"): by the next selection attempt or after the concept's toast interval (6.5 s). The toast "Local preview only…; metadata is available above." shows only for a selection whose status is empty or the AVI/WMV note, never beside an unreadable or failed preview. + Amended by D155: only the AVI/WMV note keeps the toast; an empty status opens the viewer. - **D64** (drop, LI-6): `dragover`/`drop` on `.file-drop` accept exactly one File, read only `dataTransfer.files`/`items` (never its strings) and change nothing visually. While the page is mounted, `window` `dragover`/`drop` call `preventDefault`, so a missed @@ -2880,6 +2883,36 @@ mapped differences"): Modified with no Path row and no Show in folder, and ends with "Local file; no saved library association was checked." and Download original. There is no Prompt history action and no category or tag chips. +- **D155** (#448, maintainer choice 2026-10-28, beyond the concept): choosing or dropping + a file opens the shared viewer on it at once, with `#viewer-details` showing, in one + step; the concept previews it and waits for its heading action. The viewer opens once + per selection, after the session has selected the record (a microtask after `ready`), + only while that selection is current and no other dialog is open. A new selection + that starts while the viewer shows the previous file closes it first, so no element + holds the old `blob:` URL when the session revokes it (`URLs and media end with their + owning session`); the new file's viewer opens once it is ready. A video opens the + viewer only once its inline preview has settled (metadata, an error, or D62's 15 s): + replacing a video while both the inline and the viewer's video were still starting + deadlocked WebKit (`a video replaced while still starting is reset once it settles` + hung 2 of 2 runs before, passes 5 of 5 after). It opens for a selection with no status and for one + without a preview URL ("The preview could not be displayed. Metadata and the original + remain available in the viewer."); an unreadable file stays on the page with its + alert, and AVI/WMV keeps its note and the toast (the viewer could not play it and + reads no metadata); "Open file metadata" opens either. An image or video that fails to + decode after the viewer opened shows the viewer's own media error; the page's alert + is there on close. The toast no longer shows for an opened file (the viewer is the + confirmation), and the drop note reads "Your file stays in this browser. Choosing it + opens its metadata.". On this page the viewer, by choice or by the heading action, + opens with the details showing even when "Collapse viewer details by default" is on + (D97); the toggle is still drawn then and starts as "Hide details" (`aria-expanded` + true). Closing returns focus to the chooser (the heading action when it opened the + viewer), with the preview, the chooser naming the file and the heading action in + place; the next file chosen or dropped replaces it and opens again. A drop is still + ignored while the viewer is open (D64). Nothing else changes: the file is read in this + browser only. Browser tests: "D155: choosing a file opens the viewer with its details + in one click", "D155: Inspect shows the details even when the viewer collapses them by + default (amends D97)", "D155: an unreadable file stays on the page with its alert" and + "D155: at 390px the opened details are reachable below the media". - **D117** (Copy settings summary, LI-4): the legacy metadata page's COPY ALL, consolidated into the shared full inspector (the viewer's `#viewer-details`, so Images, the recorded history's current file and Inspect all have @@ -3558,7 +3591,8 @@ the per-section hand-off panel, was retired by S12 (#329): every section is nati card's title and file line, Info and Tag stay at the end of the row), "Collapse viewer details by default" (`sidebarCollapsedByDefault`; the viewer opens with `#viewer-details` hidden and a "Show details"/"Hide details" toggle with - `aria-expanded`, the stage taking the whole width), and the operation "Reset display + `aria-expanded`, the stage taking the whole width; amended by D155: on Inspect a file + the viewer opens with the details showing and the toggle reading "Hide details"), and the operation "Reset display defaults" ("Restore this browser’s display defaults, then save. Other settings are kept.", button "Reset"), which drafts the gallery's defaults for the browser rows only. The node search timeout shows the server's real value, not the sample 30. CSS under D97. diff --git a/tests/browser/inspect-concept.spec.mjs b/tests/browser/inspect-concept.spec.mjs index 48be585d..99298fc5 100644 --- a/tests/browser/inspect-concept.spec.mjs +++ b/tests/browser/inspect-concept.spec.mjs @@ -9,7 +9,7 @@ import { parityPng } from './local-inspect-files.mjs'; // `#metadata` (docs/concepts/workspace/app.js renderMetadata 740–748, selection 1715–1729, style.css 709–722, // 1180–1194, 1543–1545, 1559–1564), at 1600x1100 and 390x844 in both themes, in three states: initial, selected (the // same synthetic PNG set into both `#local-file` inputs) and viewer-open. The concept is never edited: the recorded -// differences D60–D66 (TESTING.md) are applied to its measurement, and, where they change copy, to its page before it +// differences D60–D66 and D155 (TESTING.md) are applied to its measurement, and, where they change copy, to its page before it // is measured, so the browser lays the concept out with production's words. After that the diff must be empty. // Paired captures are artifacts only; there are no PNG baselines. test.use({ locale: 'en-US', timezoneId: 'UTC', reducedMotion: 'reduce' }); @@ -22,7 +22,8 @@ const COPY = { 'Read embedded metadata in this browser. The file is not uploaded or added to your library.'], chooser: ['Choose an image for a local preview', 'Choose an image or video for a local preview'], note: ['This mock previews your file locally. Sample metadata is available with “Open sample metadata”.', - 'Your file stays in this browser. Its metadata opens with “Open file metadata”.'], + // D155 (#448): choosing the file opens its metadata. + 'Your file stays in this browser. Choosing it opens its metadata.'], toast: ['Local preview only. No file uploaded; sample metadata is available above.', 'Local preview only. No file uploaded; metadata is available above.'], }; @@ -212,23 +213,29 @@ async function productionStates(page, theme, testInfo, tag) { await settle(page); const initial = await page.evaluate(measureInspect, '#workspace-main'); await captureCanonicalState(page, testInfo, `production-initial-${tag}`); + // D155 (#448): choosing the file opens the viewer with its details at once (the concept waits for its heading + // action), so the viewer state is measured first; the selected page is measured after Escape, with the focus the + // close returned to the chooser removed (the concept's page has no focused control there). await page.locator('#workspace-main #local-file').setInputFiles(parityPng(FILE)); - await expect(page.locator('#workspace-main #local-preview img.local-file-preview')).toBeVisible(); - await page.locator('#workspace-main #local-preview img').evaluate(image => image.decode().catch(() => {})); - await expect(page.locator('#workspace-toast')).toHaveText(COPY.toast[1]); - await expect(action).toHaveAttribute('aria-disabled', 'false'); - // D63: a readable PNG shows no status line. - await expect(page.locator('#workspace-main #local-preview p.small')).toHaveCount(0); - await settle(page); - const selected = await page.evaluate(measureInspect, '#workspace-main'); - await captureCanonicalState(page, testInfo, `production-selected-${tag}`); - await action.click(); await expect(page.locator('dialog#viewer-dialog[open] #viewer-details .panel-heading')).toBeVisible(); await expect(page.locator('dialog#viewer-dialog[open] #viewer-details .meta-grid').first()).toBeVisible(); await page.mouse.move(0, 0); await settle(page); const viewer = await page.evaluate(measureViewer); await captureCanonicalState(page, testInfo, `production-viewer-${tag}`); + await page.keyboard.press('Escape'); + await expect(page.locator('dialog#viewer-dialog')).toHaveCount(0); + await page.evaluate(() => document.activeElement?.blur()); + await expect(page.locator('#workspace-main #local-preview img.local-file-preview')).toBeVisible(); + await page.locator('#workspace-main #local-preview img').evaluate(image => image.decode().catch(() => {})); + // D155: no toast; the opened viewer was the confirmation. + await expect(page.locator('#workspace-toast')).toHaveText(''); + await expect(action).toHaveAttribute('aria-disabled', 'false'); + // D63: a readable PNG shows no status line. + await expect(page.locator('#workspace-main #local-preview p.small')).toHaveCount(0); + await settle(page); + const selected = await page.evaluate(measureInspect, '#workspace-main'); + await captureCanonicalState(page, testInfo, `production-selected-${tag}`); return { initial, selected, viewer }; } diff --git a/tests/browser/workspace-inspect.spec.mjs b/tests/browser/workspace-inspect.spec.mjs index 2640cff4..3d77f112 100644 --- a/tests/browser/workspace-inspect.spec.mjs +++ b/tests/browser/workspace-inspect.spec.mjs @@ -8,10 +8,11 @@ import { parityPng, localFixture, tinyWebm, sendablePng, corruptPng, disguised, const workspace = '/prompt_manager/workspace'; const TOAST = 'Local preview only. No file uploaded; metadata is available above.'; +const NO_TOAST = ''; const ACTION = 'Open file metadata'; const COPY = { intro: 'Read embedded metadata in this browser. The file is not uploaded or added to your library.', - note: 'Your file stays in this browser. Its metadata opens with “Open file metadata”.', + note: 'Your file stays in this browser. Choosing it opens its metadata.', chooser: 'Choose an image or video for a local preview', multiple: 'Drop one file at a time.', directory: 'Folders cannot be inspected. Choose one image or video file.', @@ -53,13 +54,26 @@ async function choose(page, file) { await chooser(page).setInputFiles(file); } +/** D155: choosing a file opens the viewer on it with its details; waits for that. */ async function openViewer(page) { - await expect(action(page)).toHaveAttribute('aria-disabled', 'false'); - await action(page).click(); await expect(viewer(page)).toBeVisible(); await expect(rail(page).locator('.panel-heading')).toBeVisible(); } +/** After the viewer was closed, the heading action (D60) opens it again on the current selection. */ +async function reopenViewer(page) { + await expect(action(page)).toHaveAttribute('aria-disabled', 'false'); + await action(page).click(); + await openViewer(page); +} + +/** Waits for the viewer a selection opened (D155) and closes it, back on the page. */ +async function closeOpened(page) { + await openViewer(page); + await page.keyboard.press('Escape'); + await expect(viewer(page)).toHaveCount(0); +} + /** Dispatches dragover + drop on `selector` with Files built in the page (and optional string items). */ function drop(page, selector, { files = [], strings = [] } = {}) { return page.evaluate(async ({ selector, files, strings }) => { @@ -152,7 +166,8 @@ test('chooser opens the shared full inspector without a second read', async ({ p const box = await image.evaluate(node => ({ height: node.getBoundingClientRect().height, fit: getComputedStyle(node).objectFit, margin: getComputedStyle(node).marginTop })); expect(box.height).toBeLessThanOrEqual(400); expect(box).toMatchObject({ fit: 'contain', margin: '20px' }); - await expect(page.locator('#workspace-toast')).toHaveText(TOAST); + // D155: the open viewer is the confirmation, so no toast. + await expect(page.locator('#workspace-toast')).toHaveText(NO_TOAST); await expect(action(page)).toHaveAttribute('aria-disabled', 'false'); await expect(action(page)).not.toHaveAttribute('aria-describedby', /.*/); @@ -195,18 +210,120 @@ test('chooser opens the shared full inspector without a second read', async ({ p await page.keyboard.press('Escape'); await expect(viewer(page)).toHaveCount(0); - await expect(action(page)).toBeFocused(); + // D155: focus returns to the chooser that opened it. + await expect(chooser(page)).toBeFocused(); // Opening the viewer again shows the same record without reading the File a second time. await page.evaluate(() => { window.__slices = 0; const slice = Blob.prototype.slice; Blob.prototype.slice = function (...args) { if (this.name === 'f02.png') window.__slices++; return slice.apply(this, args); }; }); - await openViewer(page); + await reopenViewer(page); expect(await railContent(rail(page))).toEqual(served); expect(await page.evaluate(() => window.__slices)).toBe(0); }); +// #448 (D155): choosing or dropping a file opens the shared viewer with its details showing, in one step. Closing it +// returns to the page with the file still chosen and focus on the chooser, ready for the next file. +test('D155: choosing a file opens the viewer with its details in one click', async ({ page }) => { + await openInspect(page); + await choose(page, parityPng('f02.png')); + await expect(viewer(page)).toBeVisible(); + await expect(viewer(page).locator('#viewer-title')).toHaveText('f02.png'); + await expect(rail(page)).toBeVisible(); + await expect(rail(page).locator('.prompt-section.positive p.prompt-text')).toHaveText('a red fox in snow, detailed fur'); + await expect(viewer(page).getByRole('button', { name: /^(Show|Hide) details$/ })).toHaveCount(0); + // The open viewer is the confirmation: no toast says the metadata is "available above". + await expect(page.locator('#workspace-toast')).toHaveText(''); + await page.keyboard.press('Escape'); + await expect(viewer(page)).toHaveCount(0); + await expect(chooser(page)).toBeFocused(); + await expect(preview(page).locator('img.local-file-preview')).toBeVisible(); + await expect(action(page)).toHaveAttribute('aria-disabled', 'false'); + // A dropped second file replaces the first and opens at once. + await drop(page, '.file-drop', { files: [sendablePng()] }); + await expect(viewer(page)).toBeVisible(); + await expect(viewer(page).locator('#viewer-title')).toHaveText('send-me.png'); + await page.keyboard.press('Escape'); + await expect(viewer(page)).toHaveCount(0); + await expect(chooser(page)).toBeFocused(); + // The same file again is a new selection and opens again; the heading action reopens the current one. + expect(await chooser(page).evaluate(node => { node.dispatchEvent(new MouseEvent('click')); return node.value; })).toBe(''); + await choose(page, parityPng('f02.png')); + await expect(viewer(page).locator('#viewer-title')).toHaveText('f02.png'); + await page.keyboard.press('Escape'); + await action(page).click(); + await expect(viewer(page).locator('#viewer-title')).toHaveText('f02.png'); + await expect(rail(page)).toBeVisible(); + await page.keyboard.press('Escape'); + await expect(action(page)).toBeFocused(); +}); + +test('D155: Inspect shows the details even when the viewer collapses them by default (amends D97)', async ({ page }) => { + await page.addInitScript(() => localStorage.setItem('gallerySettings', JSON.stringify({ sidebarCollapsedByDefault: true }))); + await openInspect(page); + await choose(page, parityPng('f02.png')); + await expect(viewer(page)).toBeVisible(); + await expect(rail(page)).toBeVisible(); + const toggle = viewer(page).getByRole('button', { name: 'Hide details', exact: true }); + await expect(toggle).toHaveAttribute('aria-expanded', 'true'); + await toggle.click(); + await expect(rail(page)).toBeHidden(); + await page.keyboard.press('Escape'); + await action(page).click(); + await expect(rail(page)).toBeVisible(); + await expect(viewer(page).getByRole('button', { name: 'Hide details', exact: true })).toHaveAttribute('aria-expanded', 'true'); +}); + +test('D155: an unreadable file stays on the page with its alert', async ({ page }) => { + await page.addInitScript(() => { + const slice = Blob.prototype.slice; + Blob.prototype.slice = function (...args) { + if (this.name === 'gone.png') return { arrayBuffer: () => Promise.reject(new DOMException('The file is gone', 'NotReadableError')) }; + return slice.apply(this, args); + }; + }); + await openInspect(page); + await choose(page, parityPng('f02.png', 'gone.png')); + await expect(status(page)).toHaveText('Embedded metadata could not be read.'); + await expect(action(page)).toHaveAttribute('aria-disabled', 'false'); + await expect(viewer(page)).toHaveCount(0); +}); + +test('D155: an AVI keeps its note and toast and does not open the viewer', async ({ page }) => { + await openInspect(page); + // AVI and WMV metadata is never read and the browser cannot play them, so the page keeps the selection's note. + const avi = { name: 'clip.avi', mimeType: 'video/x-msvideo', buffer: Buffer.concat([Buffer.from('RIFF'), Buffer.alloc(4), Buffer.from('AVI LIST'), Buffer.alloc(64)]) }; + await choose(page, avi); + await expect(page.locator('#workspace-toast')).toHaveText(TOAST); + await expect(action(page)).toHaveAttribute('aria-disabled', 'false'); + await expect(viewer(page)).toHaveCount(0); + await reopenViewer(page); + await expect(viewer(page).locator('#viewer-title')).toHaveText('clip.avi'); +}); + +test('D155: a selection that lands while another dialog is open leaves the viewer closed', async ({ page }) => { + await openInspect(page); + await page.evaluate(() => { const other = document.createElement('dialog'); other.id = 'other-dialog'; other.textContent = 'Other'; document.body.append(other); other.showModal(); }); + await choose(page, parityPng('f02.png')); + await expect(action(page)).toHaveAttribute('aria-disabled', 'false'); + await expect(viewer(page)).toHaveCount(0); + await page.evaluate(() => document.querySelector('#other-dialog').close()); + await reopenViewer(page); + await expect(viewer(page).locator('#viewer-title')).toHaveText('f02.png'); +}); + +test('D155: at 390px the opened details are reachable below the media', async ({ page }) => { + await page.setViewportSize({ width: 390, height: 844 }); + await openInspect(page); + await choose(page, parityPng('f02.png')); + await expect(viewer(page)).toBeVisible(); + const facts = rail(page).locator('.meta-grid').first(); + await facts.scrollIntoViewIfNeeded(); + await expect(facts).toBeInViewport(); + expect(await viewer(page).evaluate(node => node.scrollWidth <= node.clientWidth + 1)).toBe(true); +}); + // PR L3 Task 10: local roles are the server inspector's (local-resolver.mjs), so for every parity PNG the rail shows the // prompts, their status lines and the facts Images shows for the same bytes in the output folder. const PARITY_PNGS = ['f01b', 'f02', 'f03', 'f06a', 'f06b', 'f06c', 'f06d', 'f06e', 'f07a', 'f07b', 'f08', 'f09', 'f10', 'f11', 'f12', @@ -260,7 +377,9 @@ test('drop accepts exactly one File and never navigates', async ({ page }) => { await expect(preview(page)).toBeEmpty(); expect(await drop(page, '.file-drop', { files: [parityPng('f02.png')] })).toEqual({ over: true, drop: true }); await expect(preview(page).locator('img.local-file-preview')).toBeVisible(); - await expect(page.locator('#workspace-toast')).toHaveText(TOAST); + // D155: the drop opened the viewer; drops are ignored while it is open (D64), so close it first. + await closeOpened(page); + await expect(page.locator('#workspace-toast')).toHaveText(NO_TOAST); // The chooser names the dropped file, never the one it replaced and never "No file chosen". expect(await chooser(page).evaluate(node => [...node.files].map(file => file.name))).toEqual(['f02.png']); const first = await preview(page).locator('img').getAttribute('src'); @@ -275,7 +394,7 @@ test('drop accepts exactly one File and never navigates', async ({ page }) => { await expect(preview(page).locator('img')).toHaveAttribute('src', first); } await expect(action(page)).toHaveAttribute('aria-disabled', 'false'); - await openViewer(page); + await reopenViewer(page); await expect(viewer(page).locator('#viewer-title')).toHaveText('f02.png'); await expect(page).toHaveURL(/#\/inspect$/); }); @@ -285,6 +404,7 @@ test('cancel and same-file reselect', async ({ page }) => { await openInspect(page); await choose(page, parityPng('f02.png')); await expect(preview(page).locator('img.local-file-preview')).toBeVisible(); + await closeOpened(page); const first = await preview(page).locator('img').getAttribute('src'); // The chooser still names the chosen file, as in the concept; a cancel (no files) keeps the page as it is. expect(await chooser(page).evaluate(node => node.files[0]?.name)).toBe('f02.png'); @@ -308,6 +428,7 @@ test('cancel and same-file reselect', async ({ page }) => { const state = await urls(page); expect(state.created).toHaveLength(2); expect(state.revoked).toEqual([first]); + // The new selection opened the viewer again (D155). await openViewer(page); await expect(viewer(page).locator('.media-position')).toHaveText('1 of 1 loaded: f02.png'); }); @@ -337,6 +458,7 @@ test('preview does not imply metadata support', async ({ page }) => { // Text, markup, SVG and a playlist under media names are unsupported and never get a URL. const before = (await urls(page)).created.length; for (const file of disguised()) { + // Unsupported: nothing opens (D155). await choose(page, file); await expect(status(page)).toHaveText(COPY.unsupported); await expect(status(page)).toHaveAttribute('role', 'alert'); @@ -360,6 +482,7 @@ test('local video plays inline and in the viewer with real tags', async ({ page, await expect(rail(page).locator('.meta-grid').first().locator('dd').first()).toHaveText('7'); await expect(viewer(page).locator('#viewer-stage video')).toHaveCount(1); await page.keyboard.press('Escape'); + await expect(viewer(page)).toHaveCount(0); await choose(page, tinyWebm()); await expect(preview(page).locator('video.local-file-preview')).toBeVisible(); await openViewer(page); @@ -386,6 +509,9 @@ test('the toast and a rejected drop never contradict the status line', async ({ // concept's toast interval. await choose(page, undecodableMp4()); const failed = status(page).filter({ hasText: COPY.previewFailed }); + // D155: the selection opened the viewer; its media failed there too, and the page's alert waits under it. + await expect(failed).toHaveCount(1); + await closeOpened(page); await expect(failed).toHaveCount(1); await drop(page, '.file-drop', { files: [parityPng('f02.png'), parityPng('f03.png')] }); const rejected = status(page).filter({ hasText: COPY.multiple }); @@ -442,7 +568,8 @@ test('URLs and media end with their owning session', async ({ page }) => { let state = await urls(page); expect(state.revoked).toEqual([state.created[0]]); expect(await page.evaluate(() => window.__loads)).toBeGreaterThanOrEqual(1); - // Route change. + // Route change, from the page (D155: the selection opened the viewer, which makes the sidebar inert). + await closeOpened(page); await page.locator('aside.sidebar nav').getByRole('link', { name: 'Overview', exact: true }).click(); await expect(main(page).getByRole('heading', { level: 1, name: 'Overview', exact: true })).toBeVisible(); state = await urls(page); diff --git a/tests/comfyui/inspect-sender.spec.mjs b/tests/comfyui/inspect-sender.spec.mjs index 2fcb8f10..4fede99c 100644 --- a/tests/comfyui/inspect-sender.spec.mjs +++ b/tests/comfyui/inspect-sender.spec.mjs @@ -36,11 +36,11 @@ async function watchHost(page,id,reset=true) { } const restore=page=>page.evaluate(async()=>{const {app}=await import('/scripts/app.js');Object.assign(app,window.inspectOriginals);}); -/** Chooses `file` on the Inspect page and opens the shared viewer on its local record. */ +/** Chooses `file` on the Inspect page, which opens the shared viewer on its local record (D155, #448). */ async function inspect(workspace,{file,positive,shown}) { await workspace.locator('#local-file').setInputFiles(file()); const open=workspace.locator('#workspace-main .heading-actions').getByRole('button',{name:'Open file metadata',exact:true}); - await expect(open).toHaveAttribute('aria-disabled','false');await open.click(); + await expect(open).toHaveAttribute('aria-disabled','false'); const rail=workspace.locator('dialog#viewer-dialog[open] #viewer-details'); await expect(rail.locator('.prompt-section.positive p.prompt-text')).toHaveText(positive); await expect(rail.locator('.prompt-section.negative p.prompt-text')).toHaveText(shown); diff --git a/web/js/workspace/inspect.mjs b/web/js/workspace/inspect.mjs index c514d02b..ee67d7ec 100644 --- a/web/js/workspace/inspect.mjs +++ b/web/js/workspace/inspect.mjs @@ -1,6 +1,6 @@ // The concept's "Inspect a file" page (docs/concepts/workspace/app.js renderMetadata 740–748, selection 1715–1729): -// one local file, chosen or dropped, previews inline; the heading's one action opens the shared viewer with the full -// inspector on a record read entirely in this browser. Nothing is uploaded, imported or stored. The selection, its +// one local file, chosen or dropped, previews inline and (D155, #448) opens the shared viewer with its details showing +// at once; the heading's one action opens it again. The record is read entirely in this browser. Nothing is uploaded, imported or stored. The selection, its // blob: URL and its record belong to one local session (local-session.mjs) that ends with this page. import { createButton, createScope } from '../shared/toolkit/core.js'; import { pageHeading } from './heading.mjs'; @@ -17,7 +17,8 @@ export const INSPECT_COPY = Object.freeze({ heading: 'Inspect without importing', intro: 'Read embedded metadata in this browser. The file is not uploaded or added to your library.', chooser: 'Choose an image or video for a local preview', - note: 'Your file stays in this browser. Its metadata opens with “Open file metadata”.', + // D155 (#448): choosing a file opens its metadata (D61's note named the heading action). + note: 'Your file stays in this browser. Choosing it opens its metadata.', toast: 'Local preview only. No file uploaded; metadata is available above.', }); // D62: the concept's image list plus the videos the local readers support (AVI/WMV preview only). @@ -38,8 +39,11 @@ function element(tag, className, text) { return node; } -// The status shown for the selection; AVI/WMV's note and an empty status keep the toast's "metadata is available". -const TOASTABLE = new Set(['', LOCAL_SESSION_MESSAGES.containerOnly]); +// D155 (#448): a selection with no status, or one without a preview URL (its copy points at the viewer), opens the viewer +// at once. AVI/WMV's note keeps the toast instead (the viewer could not play it and reads no metadata), and an +// unreadable file stays on the page with its alert; the heading action opens either. +const AUTO_OPEN = new Set(['', LOCAL_SESSION_MESSAGES.previewFailed]); +const TOASTABLE = new Set([LOCAL_SESSION_MESSAGES.containerOnly]); // How long a replaced video still starting may stay in place before it is released anyway. const STARTING_VIDEO_MS = 15000; @@ -131,9 +135,10 @@ export function mountInspect(main, { parentScope, api, sender }) { }; const session = createLocalSession({ parentScope: sessionScope, pageTarget: null, revokeObjectURL }); const viewer = createMediaViewer({ api, container: main, parentScope: scope, inspectionController: session.controller, sender, - adapter: localFileAdapter, onRestoreFocus: () => { if (open.isConnected) open.focus({ preventScroll: true }); } }); + adapter: localFileAdapter, onRestoreFocus: () => { if (open.isConnected) open.focus({ preventScroll: true }); }, + onClose: () => { viewerKey = null; } }); - let media = null, mediaUrl = null, statusLine = null, rejectionLine = null, rejectionTimer = 0, announced = null, chosen = null; + let viewerKey = null, media = null, mediaUrl = null, statusLine = null, rejectionLine = null, rejectionTimer = 0, announced = null, chosen = null; function statusElement(message) { // /* D63 */ one status line for the states the mock never shows. const line = element('p', 'small spaced-16', message); @@ -202,14 +207,25 @@ export function mountInspect(main, { parentScope, api, sender }) { else preview.prepend(node); } scope.cleanup(session.subscribe(state => { + // D155: a new selection can start while the viewer shows the previous one; it closes first, so no element holds + // the old URL when the session revokes it (the new file's viewer opens once it is ready). + if (viewerKey !== null && (state.phase !== 'ready' || session.entry?.key !== viewerKey)) { viewer.close(); viewerKey = null; } showMedia(state); showStatus(state.message); const entry = session.entry; setAvailable(state.phase === 'ready' && !!entry); if (state.phase === 'ready' && entry && announced !== entry.key) { announced = entry.key; + // The session selects the record right after it publishes `ready`, so the viewer opens after that, and only + // while this selection is still the current one and no other dialog is open (the viewer is replaced). D62: + // a video opens it once the inline preview has settled, so a replacement while it starts never tears down a + // second starting video in the viewer (that deadlocked WebKit). + if (AUTO_OPEN.has(state.message)) whenSettled(media, () => { + if (scope.signal.aborted || session.entry !== entry || session.state.phase !== 'ready' || document.querySelector('dialog[open]:not(#viewer-dialog)')) return; + openViewer(input); + }); // An unreadable or failed preview says so in its alert; the toast never claims metadata is available. - if (TOASTABLE.has(state.message)) showWorkspaceToast(INSPECT_COPY.toast); + else if (TOASTABLE.has(state.message)) showWorkspaceToast(INSPECT_COPY.toast); } })); // A replacement or page leave revokes the URL after this subscriber dropped the media; a dispose closes the @@ -261,10 +277,25 @@ export function mountInspect(main, { parentScope, api, sender }) { }); // A drop that misses the area never navigates away from the page while it is mounted. for (const type of ['dragover', 'drop']) scope.listen(window, type, event => event.preventDefault()); - scope.listen(open, 'click', () => { + /** Runs `run` after this task, or once `preview`'s video stops starting (metadata, an error, or the D62 limit). */ + function whenSettled(preview, run) { + if (!preview || !isStarting(preview.node)) { queueMicrotask(run); return; } + // `run` checks that the page and the selection are still current, so the page's end may call `done` too. + const settled = new AbortController(); + const done = () => { settled.abort(); clearTimeout(timer); run(); }; + const timer = setTimeout(done, STARTING_VIDEO_MS); + for (const type of ['loadedmetadata', 'error']) preview.node.addEventListener(type, done, { once: true, signal: settled.signal }); + scope.cleanup(done); + } + /** The viewer on the current selection, details showing (D155 amends D97); focus returns to `trigger` on close. */ + function openViewer(trigger) { const entry = session.entry; - if (open.getAttribute('aria-disabled') === 'true' || !entry || session.state.phase !== 'ready') return; - viewer.open({ items: [entry], selectedKey: entry.key, trigger: open }); + if (!entry || session.state.phase !== 'ready') return; + viewer.open({ items: [entry], selectedKey: entry.key, trigger, details: 'shown' }); + viewerKey = entry.key; + } + scope.listen(open, 'click', () => { + if (open.getAttribute('aria-disabled') !== 'true') openViewer(open); }); /** Settles when no video on the page is still starting; `null` when none is. */ function leaveGuard() { diff --git a/web/js/workspace/media-viewer.mjs b/web/js/workspace/media-viewer.mjs index 10645720..da424337 100644 --- a/web/js/workspace/media-viewer.mjs +++ b/web/js/workspace/media-viewer.mjs @@ -39,7 +39,7 @@ export function createMediaViewer({api,container,parentScope,inspectionControlle function open(options) { if (scope.signal.aborted) return; active?.close(false); - const {items,selectedKey,trigger = document.activeElement} = options; + const {items,selectedKey,trigger = document.activeElement,details:detailsMode} = options; let mounted = false, initial, entry, currentKey, viewer = null, withoutViewer = false, mediaScope = null, closed = false; let originalDownload = null, inspectionState = inspectionController.state, collectionChanges = null; const session = createViewerSession({adapter,items,initialKey:selectedKey,onChange:(state,selected) => { @@ -79,11 +79,12 @@ export function createMediaViewer({api,container,parentScope,inspectionControlle local.listen(download,'click',event => {session.stop();if (download.getAttribute('aria-disabled') === 'true') event.preventDefault();}); controls.append(download); const next = button('Next media','Next →',() => session.navigate(1)); - // D97: "Collapse viewer details by default" opens the viewer with its details hidden behind one toggle. - const collapsed = displayPreferences()?.sidebarCollapsedByDefault === true; + // D97: "Collapse viewer details by default" opens the viewer with its details hidden behind one toggle. D155 + // (#448): `details: 'shown'` (Inspect a file) keeps the toggle but opens with the details showing. + const collapsible = displayPreferences()?.sidebarCollapsedByDefault === true, collapsed = collapsible && detailsMode !== 'shown'; let detailsToggle = null; - if (collapsed) { - detailsToggle = createButton('Show details');detailsToggle.setAttribute('aria-expanded','false'); + if (collapsible) { + detailsToggle = createButton(collapsed ? 'Show details' : 'Hide details');detailsToggle.setAttribute('aria-expanded',String(!collapsed)); detailsToggle.setAttribute('aria-controls','viewer-details');controls.append(detailsToggle); } const position = element('p','media-position muted');position.id = `media-viewer-position-${++viewerId}`; @@ -93,7 +94,7 @@ export function createMediaViewer({api,container,parentScope,inspectionControlle controls.append(position,reason,slideshowReason);left.append(stage,controls); const details = element('aside');details.id = 'viewer-details';details.setAttribute('aria-label','Image metadata'); if (detailsToggle) { - details.hidden = true; + details.hidden = collapsed; local.listen(detailsToggle,'click',() => { details.hidden = !details.hidden; detailsToggle.textContent = details.hidden ? 'Show details' : 'Hide details'; From 1ce25777335d0850cf7571af4f159327a892886d Mon Sep 17 00:00:00 2001 From: Vito Date: Thu, 29 Oct 2026 05:38:39 +0000 Subject: [PATCH 3/3] fix(library): show a loaded page of prompts without waiting for an animation frame (#464) (#466) The shelf laid out each loaded page only in its next animation frame. A browser that holds frames (a background tab, headless WebKit under CPU load, seen holding them for more than 5 s) left the grid empty while the caption already showed the count. A loaded batch is now laid out at once when the grid is in the document; measurements still flush per frame. The regression test holds every animation frame and fails before the fix on all four projects. --- tests/browser/workspace-library.spec.mjs | 20 ++++++++++++++++++++ web/js/workspace/library.mjs | 5 ++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/browser/workspace-library.spec.mjs b/tests/browser/workspace-library.spec.mjs index 9c18949c..b91a73df 100644 --- a/tests/browser/workspace-library.spec.mjs +++ b/tests/browser/workspace-library.spec.mjs @@ -57,6 +57,26 @@ test('library groups reusable negatives under one positive without exposing nega for(const item of rows.items) await expect(page.locator(`article[data-prompt-id="${item.id}"] .card-meta > span`).nth(1)).toHaveText(`${item.run_count} run${item.run_count===1?'':'s'}`); }); +// #464: a starved renderer (headless WebKit under CPU load, a background tab) can hold animation frames for seconds; +// a loaded page of prompts is shown without waiting for one. +test('a held animation frame does not hide a loaded page of prompts, the first or a later one',async({page,request})=>{ + await expect(await request.post('/__test__/reset',{data:{native_library:true,extra_prompts:45}})).toBeOK(); + const {total}=await(await request.get('/prompt_manager/library?limit=100')).json(); + await page.addInitScript(()=>{ + const request=window.requestAnimationFrame.bind(window),cancel=window.cancelAnimationFrame.bind(window); + const held=new Map(),real=new Map();let holding=true,next=0; + window.requestAnimationFrame=callback=>{const id=--next;if(holding)held.set(id,callback);else real.set(id,request(callback));return id;}; + window.cancelAnimationFrame=id=>{held.delete(id);if(real.has(id))cancel(real.get(id));real.delete(id);}; + window.__releaseAnimationFrames=()=>{holding=false;for(const [id,callback] of held)real.set(id,request(callback));held.clear();}; + }); + await page.goto(route); + const more=page.getByRole('button',{name:'Load more prompts',exact:true}); + await expect(more).toHaveAttribute('aria-disabled','false');await expect(cards(page)).toHaveCount(40); + await more.evaluate(node=>node.click());await expect(page.locator('.library-status')).toHaveText('You’re all caught up.'); + await expect(cards(page)).toHaveCount(total); + await page.evaluate(()=>window.__releaseAnimationFrames());await nextFrame(page); + await expect(cards(page)).toHaveCount(total);await expect(page.locator('.library-row-spacer')).toHaveCount(0); +}); test('history displays the selected full negative beneath its positive and filters one shared physical gallery',async({page,request})=>{ const rows=await(await request.get('/prompt_manager/library')).json();const cat=rows.items.find(item=>item.text==='Cat portrait'); const data=await(await request.get(`/prompt_manager/library/${cat.id}`)).json(); diff --git a/web/js/workspace/library.mjs b/web/js/workspace/library.mjs index 97de6b37..fe187f9e 100644 --- a/web/js/workspace/library.mjs +++ b/web/js/workspace/library.mjs @@ -394,7 +394,10 @@ export function mountLibrary(main,api,parentScope,{positiveId,savedState=null,on const hidden=hiddenSave!==null && !next.loading && !next.error && !next.hasMore && !next.items.some(item=>item.id===hiddenSave)?' The saved prompt is hidden by the current filters.':''; message.textContent=(next.error?'Prompt library could not be loaded. Retry.':next.loading?'Loading prompt library…':!next.items.length?'No matching prompts. Try changing the filters.':!next.hasMore?'You’re all caught up.':'')+hidden; message.setAttribute('role',next.error?'alert':'status');retry.hidden=!next.error;more.hidden=!next.hasMore || !!next.error;more.setAttribute('aria-disabled',String(next.loading)); - schedule();scheduleLoad(); + // #464: a loaded page is laid out now, not on the next frame: a starved renderer (a background tab, headless + // WebKit under load) can hold frames for seconds, which left a loaded shelf empty. Measurements still flush per frame. + if(batch && grid.isConnected) {cancelAnimationFrame(frame);flush();} else schedule(); + scheduleLoad(); } const stream=createLibraryStream({api,onChange:render,restore:restorable?{filters,state:restorable.state}:undefined}); function maybeLoad() {