From b007194865c025b009eceb72be45ec3ab677ec50 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Sep 2026 21:32:29 -0700 Subject: [PATCH 01/11] Fix dor tool issues found in innerdogfood QC --- docs/specs/dor-browser.md | 3 +- docs/specs/dor-tool.md | 4 +- docs/specs/dor-tool.rationale.md | 2 + docs/specs/layout.md | 5 + docs/specs/layout.rationale.md | 4 + docs/specs/security-local.md | 2 +- docs/specs/security-local.rationale.md | 2 + docs/testing/dor-tool-qc.md | 80 ++++++++ dor/package.json | 8 +- dor/scripts/build-pdf-viewer.mjs | 45 +++++ dor/src/file-viewer.ts | 18 +- dor/src/pdf-viewer-assets.ts | 30 +++ dor/test/file-viewer.test.mjs | 58 +++++- dor/test/pdf-viewer.test.mjs | 187 +++++++++++++++++ dor/viewer/controller.mjs | 190 ++++++++++++++++++ dor/viewer/viewer.css | 14 ++ dor/viewer/viewer.html | 31 +++ dor/viewer/viewer.mjs | 9 + lib/package.json | 3 +- .../wall/SurfacePaneHeader.test.tsx | 122 ++++++++++- lib/src/components/wall/SurfacePaneHeader.tsx | 144 ++++++++++--- lib/src/host/file-viewer-proxy.test.ts | 22 +- .../stories/BrowserChromeHeader.stories.tsx | 28 ++- pnpm-lock.yaml | 15 ++ pnpm-workspace.yaml | 4 + scripts/dor-tool-qc/server.mjs | 28 +++ scripts/spec-word-budgets.json | 6 +- website/scripts/generate-deps.js | 1 + website/src/data/dependencies-npm.json | 7 + 29 files changed, 1016 insertions(+), 56 deletions(-) create mode 100644 docs/testing/dor-tool-qc.md create mode 100644 dor/scripts/build-pdf-viewer.mjs create mode 100644 dor/src/pdf-viewer-assets.ts create mode 100644 dor/test/pdf-viewer.test.mjs create mode 100644 dor/viewer/controller.mjs create mode 100644 dor/viewer/viewer.css create mode 100644 dor/viewer/viewer.html create mode 100644 dor/viewer/viewer.mjs create mode 100644 scripts/dor-tool-qc/server.mjs diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index e32cadee6..fcbcafd71 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -116,8 +116,7 @@ Header contract: - **Must keep back/forward/reload enabled.** Agent-browser uses native commands; iframe uses parent history and re-resolves its proxy. - **Must show non-default managed `--key` as a badge, never a title prefix.** -- **Must hide split/zoom below `420px` and nav below `360px`;** minimize and kill - remain. +Header sizing and narrow-pane control placement follow `docs/specs/layout.md` → Pane header. Source of truth: `lib/src/components/wall/SurfacePaneHeader.tsx`, `lib/src/components/wall/agent-browser-screen.ts`, diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index a651e9249..04e4a1fd6 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -162,9 +162,11 @@ Source of truth: `toolCommand` in `dor/src/commands/tool.ts`; `dor/test/snapshot **Must run the built-in viewer as a Tool-owned `dor` process**, serving HTML, images, PDF/media, and escaped text/source previews. Markdown is source text; custom viewers may render it. Text previews and HTML/CSS dependency inspection are limited to 8 MiB per file. Text/source previews grant only their opened file and skip dependency inspection. (rationale) Oversized HTML and referenced CSS still stream without dependency inspection. The grant contains at most 256 files: the opened document and statically referenced relative HTML/CSS assets within its directory tree; exceeding that bound fails the open without serving a partial grant. Never expand the grant through root-relative, external, or dynamic references; requests can read only granted paths. +**Must render PDFs with the bundled PDF.js renderer inside the existing iframe sandbox**, with page navigation, fit/zoom, selectable page text, password input, and visible loading/error/cancellation states. Render one page at a time; cancel superseded work, release previous-page resources after rendering settles, and bound each canvas to 8,192 pixels per axis and 16 Mi pixels total. Renderer assets use an exact build inventory, separate from the document grant; no CDN or Node-side PDF renderer is used. (rationale) + **Must retain the viewer's opened file descriptors until the Tool exits.** Refresh reads those files again, but atomic replacements and changes to the dependency graph require restarting the viewer. Cold restore runs the saved file command with a fresh URL capability; Workspace movement keeps the live binding. The listener's authority is `docs/specs/security-local.md` → Local-file viewer. -Source of truth: `openCommand` in `dor/src/commands/open.ts`; `resolveOpenTool` in `lib/src/host/tool-open.ts`; `parseToolFile` in `lib/src/host/tool-registry.ts`; `surface.tool` in `lib/src/components/wall/use-dor-control.ts`; `fileViewerFormat` in `dor/src/file-viewer-format.ts`; `startFileViewer` / `runFileViewer` in `dor/src/file-viewer.ts`. Tests: `lib/src/host/tool-open.test.ts`, `dor/test/cli-output.test.mjs`, `lib/src/components/Wall.test.tsx`, `dor/test/file-viewer.test.mjs`. +Source of truth: `openCommand` in `dor/src/commands/open.ts`; `resolveOpenTool` in `lib/src/host/tool-open.ts`; `parseToolFile` in `lib/src/host/tool-registry.ts`; `surface.tool` in `lib/src/components/wall/use-dor-control.ts`; `fileViewerFormat` in `dor/src/file-viewer-format.ts`; `startFileViewer` / `runFileViewer` in `dor/src/file-viewer.ts`. Tests: `lib/src/host/tool-open.test.ts`, `dor/test/cli-output.test.mjs`, `lib/src/components/Wall.test.tsx`, `dor/test/file-viewer.test.mjs`, `dor/test/pdf-viewer.test.mjs`. ## Take-over diff --git a/docs/specs/dor-tool.rationale.md b/docs/specs/dor-tool.rationale.md index fbb687aaf..68d74e592 100644 --- a/docs/specs/dor-tool.rationale.md +++ b/docs/specs/dor-tool.rationale.md @@ -44,6 +44,8 @@ The September 2026 integration reuses Terminal Context for the Tool's primary te ## Opening local files +The native PDF plugin rendered a gray pane under the required iframe sandbox in the September 2026 browser harness, while the same 586-byte PDF rendered after removing that sandbox. A bundled PDF.js browser renderer preserves the framing boundary. Its assets ride the existing recursive CLI staging path, so the Node 18 host never imports a package whose Node renderer requires a newer runtime. The browser build excludes the optional native canvas backend and document-scripting sandbox. + A CSS source preview escapes its contents, so its URLs cannot load assets. Scanning those references adds unused authority and can reject a small source file at the asset limit. CSS loaded by HTML is active, so its dependencies still enter the bounded grant. Keeping the built-in viewer in the Tool's process tree reuses port discovery, kill, restart, and Workspace transfer. An OSC path carries the per-run URL capability without saving that secret in the restart command. Holding the selected file descriptors bounds what the server can read after launch; it trades automatic replacement-file refresh for a grant whose contents cannot widen through path replacement. diff --git a/docs/specs/layout.md b/docs/specs/layout.md index deef243fc..2240874dd 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -50,6 +50,11 @@ A 30px header doubling as a drag handle: **a `pointerdown` past a 5px threshold **Must use browser chrome for a serving Tool, with a Terminal Context disclosure for its serving terminal.** Tool composition belongs to `docs/specs/dor-tool.md` → Lifecycle. +**Must size browser chrome by available pane width, excluding Tool context.** Hide inline split/zoom below 420px and navigation below 360px. Below 180px, move browser controls into a keyboard-accessible, viewport-clamped popover; minimize/kill remain inline until 72px, then join the popover. A filled notepad glyph and note count identify saved notes on its trigger. Long keys and connection labels yield before controls. (rationale) + +Source of truth: `SurfacePaneHeader` in `lib/src/components/wall/SurfacePaneHeader.tsx`; tests: `lib/src/components/wall/SurfacePaneHeader.test.tsx`; stories: `lib/src/stories/BrowserChromeHeader.stories.tsx`. + + Elements left to right: derived label; alert bell; TODO pill (compact+); flexible gap; mouse-reporting override icon (compact+, only while the inside program requests mouse reporting); notepad icon (`docs/specs/notepad.md` → "Notepad UI"); split left/right, split top/bottom, zoom/unzoom (full only); minimize; kill (hover turns error-red). The label is the `DerivedHeader` from `deriveHeader(...)`; `docs/specs/terminal-state.md` owns the priority chain and disambiguator. Layout renders it: primary truncates with ellipsis, secondary muted beside it, a failed last command appends an error-colored glyph. Click renames/pins; right-click — or `>` in command mode — opens the header context menu. diff --git a/docs/specs/layout.rationale.md b/docs/specs/layout.rationale.md index 33e9b15da..102a9ccd7 100644 --- a/docs/specs/layout.rationale.md +++ b/docs/specs/layout.rationale.md @@ -2,6 +2,10 @@ > Informative companion to [layout.md](layout.md): the evidence, measurements, and dead-approach history behind its rules, keyed by that spec's headings (AGENTS.md → "What, not why"). Nothing here is normative. +## Pane header + +Viewport breakpoints keep every button visible when a wide window contains a narrow split. Tool headers have even less browser width because Terminal Context occupies its own button. Measuring the browser header and moving fixed controls together prevents long keys, note buttons, or renderer chips from pushing minimize/kill into a neighboring pane. + ## Pane body xterm.js paints only its own rendered surface, and integer row fitting leaves a sub-row remainder at the bottom of the pane: a host background differing from the terminal screen shows as a stripe under the last row, and an unclipped host squares off the rounded bottom corners. diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index a70e41dca..bf1936add 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -134,7 +134,7 @@ Source of truth: the shared rule and predicates — `isLoopbackHost`, `isOwnOrig **FAIL IF** `dor/src/file-viewer.ts` serves any request without the fresh 256-bit URL capability, its own case-insensitive loopback `Host`, an absent or same-listener `Origin`, and a GET/HEAD method. Compare capability prefixes by SHA-256 then `timingSafeEqual`, including malformed lengths. `allowsFileViewerRequest` in `dor/src/file-viewer-loopback-guard.ts` gates every route. Never grant CORS access to foreign origins, cache responses, or send the capability as a referrer. -**FAIL IF** the local-file viewer exposes directory listings, arbitrary path reads, writes, or a file outside its opened-document grant. Grant construction permits only regular files, rejects symlinks escaping the canonical document directory, bounds static dependency discovery, and retains descriptors so later path replacement cannot widen the grant. Viewer resource loads are restricted by CSP to its own origin plus inline scripts/styles and data images, including through the iframe proxy; escaped text previews execute no document markup. The viewer opts into the proxy's upstream-policy preservation (`docs/specs/dor-browser.md` → Iframe Renderer). +**FAIL IF** the local-file viewer exposes directory listings, arbitrary path reads, writes, user files outside its opened-document grant, or renderer assets outside the build inventory. Grant construction permits only regular files, rejects symlinks escaping the canonical document directory, bounds static dependency discovery, and retains descriptors so later path replacement cannot widen the grant. Viewer resource loads are restricted by CSP to its own origin plus inline scripts/styles and data images, including through the iframe proxy; escaped text previews execute no document markup. Only the PDF shell and its bundled worker permit local WebAssembly compilation; never enable general JavaScript eval, external renderer assets, or PDF document scripting. The viewer opts into the proxy's upstream-policy preservation (`docs/specs/dor-browser.md` → Iframe Renderer). **Must not describe the viewer CSP as confining active documents' navigation.** HTML/SVG scripts can navigate their frame to external URLs, including with granted contents; the resource policy is not a no-egress boundary. (rationale) diff --git a/docs/specs/security-local.rationale.md b/docs/specs/security-local.rationale.md index c8dca10f8..1d32d05c2 100644 --- a/docs/specs/security-local.rationale.md +++ b/docs/specs/security-local.rationale.md @@ -171,4 +171,6 @@ What the snapshot tests cover. `restrict_to_owner_leaves_one_owner_only_ace` is ## Local-file viewer +PDF.js uses local WebAssembly decoders for embedded images and ICC color conversion. Their compilation permission is confined to the generated PDF shell and its worker response; raw document resources keep the ordinary viewer policy. Both the worker and its assets share the capability URL and listener origin, avoiding blob/CDN worker permissions. The build inventory excludes PDF.js document-scripting assets. + The viewer allows inline and granted scripts for interactive local reports. CSP fetch directives constrain resource requests, but do not prevent a script assigning an external URL to its own frame. `form-action` constrains form submissions, not arbitrary navigation. Preserving the policy through the proxy repairs the resource-load boundary; it does not establish that active documents cannot send granted contents outside the machine. [CSP3 navigation checks](https://www.w3.org/TR/CSP3/) and its multiple-policy rules distinguish these mechanisms. diff --git a/docs/testing/dor-tool-qc.md b/docs/testing/dor-tool-qc.md new file mode 100644 index 000000000..9707c7d9e --- /dev/null +++ b/docs/testing/dor-tool-qc.md @@ -0,0 +1,80 @@ +# Dor Tool innerdogfood QC + +Branch: `dor-tool-qc`, based on the reviewed stack at `4c7f9012`. + +## Harness and isolation + +Run source-mutating root self-tests before starting the live harness. Run +`pnpm innerdogfood` in a visible `dor ensure` pane. Use its real sidecar, +PTYs, staged CLI, and browser UI through `dor ab`. Keep fixture files, captured +inner CLI credentials, and the separate XDG user config under this worktree's +ignored `standalone/src-tauri/target/dor-tool-qc/` directory. Never use the +installed application's configuration or trust records. Capture credentials +only to a mode-0600 local file; do not include them in reports. + +## Test plan + +| Area | Exercise | Expected result | Result | +| --- | --- | --- | --- | +| Boot and flag | Start harness; run Tool with flag off, then enable it | Clear disabled error; healthy terminal and CLI after enable | Pass: disabled rejection, then real PTY and Tool startup; disabling later preserves existing Tools. | +| Project trust | Invoke named Tool; repeat pending invocation; decline, then allow folder | No execution before approval; pending dedupes; decline records nothing; approved command serves | Pass: pending deduplication, decline/re-prompt, folder-only approval, failure without PTY, repair and Retry. | +| Inputs | argv with spaces, quotes, dollar signs; canonical target and symlink | Exact argument values; no shell expansion; canonical file identity | Pass: literal shell metacharacters survive; symlink resolves to canonical target and reuse key. | +| Identity | Keyed live reuse, concurrent invocations, `--fresh`, idle restart | One keyed live process; fresh splits; restart keeps Surface/ref | Pass: three concurrent invocations reuse one live Tool; fresh splits; idle and fast-command restarts retain ref. | +| Takeover | Type standalone `dor tool` at a plain prompt; compare compound line and `dor open` | Eligible Tool retains terminal/ref; other paths split | Pass: standalone Tool takes over; compound command and standalone open create separate Tools. | +| Serving | Automatic single port, multiple-port conflict, announced port/path | Only owned ports frame; conflict explains refusal; announcement resolves it | Pass: automatic single port; three-port conflict; OSC announcement resolves the conflict. | +| File dispatch | Ordered user rules, explicit handler, malformed config, project rule isolation | Correct user handler; useful errors; project config cannot intercept opens | Pass: first matching user rule, explicit override, malformed user errors; malformed/project associations do not intercept opens. | +| Built-in viewer | Text/Markdown, HTML+CSS+image, PDF/image/audio; awkward filename | Correct content, relative assets load, filename is one argument | Pass: text/Markdown source, HTML/CSS/image, SVG, audio and awkward names. PDF embedded font, image, three-page navigation, fit/zoom, password retry, cancel and malformed-file error pass with sandbox intact. | +| Rejection/bounds | URL, directory, missing/unsupported file, oversized text | Clear errors and no leaked Tool/process | Pass: URL/directory/missing/unsupported rejected; oversized text reports 8 MiB limit and exits. | +| UI and lifecycle | Narrow approval, Terminal Context, minimize/reveal, browser exit/refocus, renderer swap | Reachable controls; same Session; input reaches terminal after exit | Pass: approval at 249×203, same-session context, minimize/reveal state, exit/refocus, narrow popup Zoom/Unzoom/Display focus, and iframe↔screencast round trip. | +| Workspace/reload | Live page reload, move serving Tool, cross-Workspace identity | State/PTY survive live reload; scoped reuse and correct movement | Pass: live reload preserves IDs/kinds/URLs; cross-Workspace identity is scoped. Tool transfer was not exercised; native-window transfer is unavailable in this harness. | +| Cleanup/regression | Close owned Tools, verify listeners retire; rerun affected tests/builds | No orphan fixture servers; fixes covered and retested live | Pass: all final viewer PIDs exited on close; both owned harness runs stopped. Full suite/build and final affected-package suites pass. | + +## Findings and evidence + +### Narrow browser and Tool headers + +At a 103-pixel pane width, header controls extended into the neighboring pane; +Zoom could not be clicked. The original breakpoints measured the viewport. +The fix measures available header width and exposes overflow controls in a +keyboard-accessible popup. Real clicks also exposed premature popup dismissal +before the selected action ran; dismissal now waits until that action completes. +The clean-harness native-click retest passes: Zoom reaches 716×403 pixels, Unzoom returns to the compact header, Reload works, and Display retains modal focus. Header buttons remain inside every pane at the final 1200×800 viewport. Regression coverage lives in +`lib/src/components/wall/SurfacePaneHeader.test.tsx` and narrow header stories. + +### Sandboxed PDF rendering + +A valid one-page PDF showed Chromium's broken-document icon in the normal +sandboxed iframe. Temporarily removing the sandbox on that generated test +fixture confirmed the native PDF plugin was the incompatibility; the normal +sandbox was immediately restored. The fix bundles a browser-only PDF.js +renderer with capability-protected assets and retains the application sandbox. +Live retesting passes for an embedded font, JPEG image, three pages, navigation, fit/zoom, password rejection/retry, cancellation, and malformed PDF. The iframe retains its original sandbox attributes. Regression coverage includes descriptor-backed PDF +routes, renderer inventory, controller cancellation, and the real iframe proxy. + +### Evidence and limits + +Local screenshots and JSON observations are in the ignored +`standalone/src-tauri/target/dor-tool-qc/` directory. Useful before-fix images: +`html.png`, `pdf-zoom.png`; the diagnostic PDF image is `pdf-diagnostic.png`. After-fix PDF evidence is `pdf-fixed.png` and `pdf-image.png`. +`approval-error.png` captures the persisted error with no PTY. Reload snapshots +are `before-reload.json` and `after-reload.json`. Captured CLI credentials are +private runtime artifacts, not report material. + +Native Tauri/VS Code rendering, native-window transfer, and Windows shell +behavior are outside this browser harness's direct coverage. The tests use the +real standalone sidecar, staged CLI, PTYs, and iframe proxy. + +### Review and automated checks + +- Full `pnpm test` and `pnpm build` passed after the first integrated fixes. +- Focused header/Tool/iframe/Wall coverage: 139 tests pass. +- PDF review found two issues, both fixed and re-reviewed: build assets explicitly before lib's proxy tests, and release superseded PDF page resources. Final dor suite: 174 tests; final lib suite: 3,539 tests in 224 files. Typecheck, spec lint and diff checks pass. Navigation back to a released image page also passes live. +- An unexpected development-state reset occurred while the full build/test suite ran beside the harness: Tools appeared as terminals. Investigation confirmed `e2e-lint-selftest` temporarily mutates Vite inputs, including invalid root package JSON; the exact metadata-loss trigger was not captured. A clean harness restart restored normal operation. The final stable-build reload preserved every ID, kind, URL and Workspace (`final-before-reload.json` / `final-after-reload.json`). No claim of cold-restore or native-host coverage is made from the disturbed development reload. + +## Cleanup + +The four final viewer processes exited after their verified Tool Surfaces were +killed; `cleanup-result.json` records the check. Earlier closed fixture listeners +also exited, and both QC harness processes were stopped. Private connection +captures were deleted after shutdown. Screenshot and result artifacts remain +ignored locally for inspection. diff --git a/dor/package.json b/dor/package.json index 258245704..2df59c746 100644 --- a/dor/package.json +++ b/dor/package.json @@ -13,8 +13,9 @@ ], "scripts": { "prebuild": "pnpm --filter dor-lib-common build && node ../scripts/generate-dor-version.mjs && node ../scripts/generate-dor-skill.mjs", - "build": "tsc -p tsconfig.json && esbuild src/dor.ts --bundle --format=esm --platform=node --outfile=dist/dor.js --banner:js=\"import { createRequire } from 'module'; const require = createRequire(import.meta.url);\"", - "test": "pnpm run build && node --test test/*.test.mjs" + "build": "tsc -p tsconfig.json && esbuild src/dor.ts --bundle --format=esm --platform=node --outfile=dist/dor.js --banner:js=\"import { createRequire } from 'module'; const require = createRequire(import.meta.url);\" && pnpm build:pdf-viewer", + "test": "pnpm run build && node --test test/*.test.mjs", + "build:pdf-viewer": "node scripts/build-pdf-viewer.mjs" }, "devDependencies": { "@types/node": "^24.13.4", @@ -23,6 +24,7 @@ }, "dependencies": { "@stricli/core": "^1.2.7", - "dor-lib-common": "workspace:*" + "dor-lib-common": "workspace:*", + "pdfjs-dist": "6.3.289" } } diff --git a/dor/scripts/build-pdf-viewer.mjs b/dor/scripts/build-pdf-viewer.mjs new file mode 100644 index 000000000..cec897a79 --- /dev/null +++ b/dor/scripts/build-pdf-viewer.mjs @@ -0,0 +1,45 @@ +import { createRequire } from 'node:module'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { cp, mkdir, readdir, rm, writeFile } from 'node:fs/promises'; + +const require = createRequire(import.meta.url); +const pdfjs = dirname(require.resolve('pdfjs-dist/package.json')); +const dor = fileURLToPath(new URL('../', import.meta.url)); +export async function buildPdfViewer(output = join(dor, 'dist/pdf-viewer')) { + await rm(output, { recursive: true, force: true }); + await mkdir(output, { recursive: true }); + // Browser artifacts only: never import PDF.js (or its optional native canvas) + // into the Node CLI. Both hosts stage this directory along with dor.js. + for (const [source, target] of [ + ['legacy/build/pdf.min.mjs', 'pdf.mjs'], + ['legacy/build/pdf.worker.min.mjs', 'pdf.worker.mjs'], + ['web/pdf_viewer.css', 'pdf_viewer.css'], + ['web/images', 'images'], + ['cmaps', 'cmaps'], + ['standard_fonts', 'standard_fonts'], + ['iccs', 'iccs'], + ['LICENSE', 'LICENSE'], + ]) await cp(join(pdfjs, source), join(output, target), { recursive: true }); + await mkdir(join(output, 'wasm')); + for (const name of [ + 'openjpeg.wasm', 'openjpeg_nowasm_fallback.js', 'jbig2.wasm', 'jbig2_nowasm_fallback.js', 'qcms_bg.wasm', + 'LICENSE_JBIG2', 'LICENSE_OPENJPEG', 'LICENSE_PDFJS_JBIG2', 'LICENSE_PDFJS_OPENJPEG', 'LICENSE_PDFJS_QCMS', 'LICENSE_QCMS', + ]) await cp(join(pdfjs, 'wasm', name), join(output, 'wasm', name)); + for (const name of ['viewer.html', 'viewer.css', 'viewer.mjs', 'controller.mjs']) { + await cp(join(dor, 'viewer', name), join(output, name)); + } + const files = []; + async function inventory(directory, prefix = '') { + for (const entry of await readdir(directory, { withFileTypes: true })) { + const name = prefix + entry.name; + if (entry.isDirectory()) await inventory(join(directory, entry.name), name + '/'); + else if (entry.isFile()) files.push(name); + else throw new Error(`Unexpected PDF viewer asset: ${name}`); + } + } + await inventory(output); + await writeFile(join(output, 'manifest.json'), JSON.stringify(files.sort()) + '\n'); +} + +if (process.argv[1] && fileURLToPath(import.meta.url) === resolve(process.argv[1])) await buildPdfViewer(); diff --git a/dor/src/file-viewer.ts b/dor/src/file-viewer.ts index e99478169..1f6d17b72 100644 --- a/dor/src/file-viewer.ts +++ b/dor/src/file-viewer.ts @@ -65,6 +65,7 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa const target = await realpath(input); const format = fileViewerFormat(target); if (!format) throw new Error('unsupported file format; configure a user Tool association'); + const pdf = format.mime === 'application/pdf' ? await (await import('./pdf-viewer-assets.js')).pdfViewerAssets() : null; // A source preview escapes the document; none of its references load. const inspectDependencies = !format.text; const root = dirname(target); @@ -130,6 +131,21 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa try { route = decodeURIComponent(new URL(req.url!, 'http://localhost').pathname.slice(prefix.length)); } catch { finish(res, 400); return; } if (route.includes('\\') || route.split('/').some(part => part === '..' || part === '.')) { finish(res, 403); return; } + if (pdf && (route === 'view' || route.startsWith('pdfjs/'))) { + // Only our PDF renderer compiles local decoder WASM. Document routes + // retain the ordinary viewer policy; iframe sandboxing is unchanged. + // unsafe-inline is required by the proxy's injected navigation/focus shim. + if (route === 'view' || route === 'pdfjs/pdf.worker.mjs') res.setHeader('Content-Security-Policy', "default-src 'none'; script-src 'self' 'unsafe-inline' 'wasm-unsafe-eval'; worker-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; connect-src 'self'; object-src 'none'; base-uri 'self'; form-action 'none'"); + const asset = route === 'view' ? { + bytes: Buffer.from(pdf.html.replace(/__DOR_PDF_(TITLE|DOCUMENT)__/g, (_, part: string) => escapeHtml(part === 'TITLE' + ? basename(target) : `./file/${encodeURIComponent(basename(target))}`))), + mime: 'text/html; charset=utf-8', + } : await pdf.read(route.slice('pdfjs/'.length)); + if (!asset) { finish(res, 404); return; } + res.writeHead(200, { 'Content-Type': asset.mime, 'Content-Length': asset.bytes.length }); + res.end(req.method === 'HEAD' ? undefined : asset.bytes); + return; + } if (route === 'view' && format.text) { const text = await readText(main.file); const body = `${escapeHtml(basename(target))}
${escapeHtml(text)}
`; @@ -175,7 +191,7 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa await new Promise((yes, no) => { server.once('error', no); server.listen(0, '127.0.0.1', yes); }); port = (server.address() as { port: number }).port; let closing: Promise | undefined; - return { port, path: `${prefix}${format.text ? 'view' : `file/${encodeURIComponent(basename(target))}`}`, + return { port, path: `${prefix}${format.text || pdf ? 'view' : `file/${encodeURIComponent(basename(target))}`}`, close: () => closing ??= new Promise((yes, no) => { server.close(error => { void closeFiles().then(() => error ? no(error) : yes(), no); }); server.closeAllConnections(); diff --git a/dor/src/pdf-viewer-assets.ts b/dor/src/pdf-viewer-assets.ts new file mode 100644 index 000000000..43ba77db5 --- /dev/null +++ b/dor/src/pdf-viewer-assets.ts @@ -0,0 +1,30 @@ +import { readFile } from 'node:fs/promises'; + +// Resolves beside dist/dor.js in the staged CLI and beside dist/file-viewer.js +// in tests. Source imports from lib tests use the same built artifacts. +const root = new URL('../dist/pdf-viewer/', import.meta.url); +const MIME: Record = { + mjs: 'text/javascript; charset=utf-8', js: 'text/javascript; charset=utf-8', + css: 'text/css; charset=utf-8', svg: 'image/svg+xml', wasm: 'application/wasm', +}; + +/** A build-generated exact inventory, never a directory/file URL supplied by + * the document. These application assets do not enlarge its file grant. */ +export async function pdfViewerAssets() { + let manifest: unknown; + try { manifest = JSON.parse(await readFile(new URL('manifest.json', root), 'utf8')); } + catch { throw new Error('PDF renderer is missing; rebuild or reinstall Dormouse'); } + if (!Array.isArray(manifest) || !manifest.every(name => typeof name === 'string' + && /^[a-zA-Z0-9_.\/-]+$/.test(name) && !name.split('/').some(part => part === '..' || part === '.' || part === ''))) { + throw new Error('Invalid PDF renderer asset inventory'); + } + const allowed = new Set(manifest); + const html = await readFile(new URL('viewer.html', root), 'utf8'); + return { + html, + async read(name: string): Promise<{ bytes: Buffer; mime: string } | null> { + if (!allowed.has(name) || name === 'viewer.html') return null; + return { bytes: await readFile(new URL(name, root)), mime: MIME[name.split('.').pop()!] ?? 'application/octet-stream' }; + }, + }; +} diff --git a/dor/test/file-viewer.test.mjs b/dor/test/file-viewer.test.mjs index 478d45b96..7b86b7efb 100644 --- a/dor/test/file-viewer.test.mjs +++ b/dor/test/file-viewer.test.mjs @@ -5,10 +5,11 @@ import { join } from 'node:path'; import { request } from 'node:http'; import { spawn } from 'node:child_process'; import { once } from 'node:events'; -import { fileURLToPath } from 'node:url'; import { afterEach, beforeEach, test } from 'node:test'; import { startFileViewer } from '../dist/file-viewer.js'; import { fileViewerFormat } from '../dist/file-viewer-format.js'; +import { stageDorCli } from '../../scripts/stage-dor-cli.mjs'; +import { buildPdfViewer } from '../scripts/build-pdf-viewer.mjs'; let root; const viewers = []; @@ -100,8 +101,9 @@ test('rejects parent-directory references and symlinks escaping the document dir assert.notEqual((await get(viewer, asset(viewer, '../secret.txt'))).status, 200); }); -test('supports byte ranges and HEAD for native PDF/image presentation', async () => { - const viewer = await start('README.pdf', '%PDF-1.7 example bytes'); +test('keeps descriptor-backed PDF byte ranges and HEAD behind the rendered preview', async () => { + const opened = await start('README.pdf', '%PDF-1.7 example bytes'); + const viewer = { ...opened, path: opened.path.replace(/view$/, 'file/README.pdf') }; assert.equal((await get(viewer)).headers['content-type'], 'application/pdf'); const range = await get(viewer, viewer.path, { Range: 'bytes=0-3' }); assert.equal(range.status, 206); @@ -112,6 +114,38 @@ test('supports byte ranges and HEAD for native PDF/image presentation', async () assert.equal((await get(viewer, viewer.path, {}, 'HEAD')).body, ''); }); +test('serves an exact capability-gated PDF renderer inventory with narrowly scoped WASM permission', async () => { + const viewer = await start('report & notes.pdf', '%PDF-1.7 example bytes'); + const prefix = viewer.path.slice(0, -'view'.length); + const shell = await get(viewer); + assert.equal(shell.headers['content-type'], 'text/html; charset=utf-8'); + assert.match(shell.body, /report & notes.pdf/); + assert.match(shell.body, /data-document="\.\/file\/report%20%26%20notes.pdf"/); + assert.match(shell.body, /Page number/); + assert.match(shell.headers['content-security-policy'], /worker-src 'self'/); + assert.match(shell.headers['content-security-policy'], /'wasm-unsafe-eval'/); + assert.doesNotMatch(shell.headers['content-security-policy'], /(?:^| )'unsafe-eval'/); + for (const name of ['viewer.mjs', 'controller.mjs', 'pdf.mjs', 'pdf.worker.mjs', 'pdf_viewer.css', 'viewer.css', + 'cmaps/Adobe-Japan1-UCS2.bcmap', 'standard_fonts/LiberationSans-Regular.ttf', 'wasm/openjpeg.wasm', 'LICENSE']) { + const path = `${prefix}pdfjs/${name}`; + const response = await get(viewer, path); + assert.equal(response.status, 200, name); + assert.ok(response.body.length > 0, name); + assert.equal(response.headers['cache-control'], 'no-store'); + assert.equal(response.headers['referrer-policy'], 'no-referrer'); + assert.equal(response.headers['content-security-policy'].includes("'wasm-unsafe-eval'"), name === 'pdf.worker.mjs'); + assert.equal((await get(viewer, path, {}, 'HEAD')).body, ''); + assert.equal((await get(viewer, path, { Origin: 'https://evil.test' })).status, 403); + assert.equal((await get(viewer, path.replace(prefix, '/wrong/'))).status, 403); + } + for (const name of ['manifest.json', 'viewer.html', '../package.json', '%2e%2e/package.json', 'pdf.sandbox.mjs', 'wasm/quickjs-eval.wasm']) { + assert.notEqual((await get(viewer, `${prefix}pdfjs/${name}`)).status, 200, name); + } + assert.equal((await get(viewer, `${prefix}file/private.pdf`)).status, 404); + const text = await start('plain.txt', 'text'); + assert.equal((await get(text, text.path.replace(/view$/, 'pdfjs/pdf.mjs'))).status, 404); +}); + test('fails unsupported formats and oversized text before starting a viewer', async () => { await writeFile(join(root, 'unknown.bin'), 'binary'); await assert.rejects(startFileViewer(join(root, 'unknown.bin')), /unsupported/); @@ -167,10 +201,17 @@ test('bounds the asset graph and keeps a grant on the opened file after path rep await assert.rejects(startFileViewer(html), /256 referenced files/); }); -test('the bundled private entry announces its port and path, then exits on termination', { timeout: 10_000 }, async () => { - const file = join(root, 'cli.txt'); - await writeFile(file, 'cli preview'); - const child = spawn(process.execPath, [fileURLToPath(new URL('../dist/dor.js', import.meta.url)), '__view-file', file], { stdio: ['ignore', 'pipe', 'pipe'] }); +test('the staged private entry serves PDF assets without node_modules and exits on termination', { timeout: 10_000 }, async () => { + const file = join(root, 'cli.pdf'); + await writeFile(file, '%PDF-1.7 example bytes'); + const staged = join(root, 'staged'); + await stageDorCli(staged); + // The library pretest runs this asset-only prerequisite on a clean checkout. + // Rebuild into an empty staged directory, without relying on dor's artifacts. + const assets = join(staged, 'dist/pdf-viewer'); + await rm(assets, { recursive: true }); + await buildPdfViewer(assets); + const child = spawn(process.execPath, [join(staged, 'dist/dor.js'), '__view-file', file], { stdio: ['ignore', 'pipe', 'pipe'] }); try { let output = ''; const announce = await new Promise((resolve, reject) => { @@ -182,7 +223,8 @@ test('the bundled private entry announces its port and path, then exits on termi if (match) resolve(JSON.parse(match[1])); }); }); - assert.equal((await get(announce)).status, 200); + assert.match((await get(announce)).body, /PDF controls/); + assert.equal((await get(announce, announce.path.replace(/view$/, 'pdfjs/pdf.worker.mjs'))).status, 200); const exited = once(child, 'exit'); child.kill('SIGTERM'); await exited; diff --git a/dor/test/pdf-viewer.test.mjs b/dor/test/pdf-viewer.test.mjs new file mode 100644 index 000000000..318158d81 --- /dev/null +++ b/dor/test/pdf-viewer.test.mjs @@ -0,0 +1,187 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { canvasSize, startPdfViewer } from '../viewer/controller.mjs'; + +const tick = () => new Promise(resolve => setImmediate(resolve)); +class Element extends EventTarget { + disabled = false; + hidden = false; + value = ''; + textContent = ''; + clientWidth = 824; + attributes = new Map(); + style = { setProperty() {} }; + dataset = {}; + setAttribute(name, value) { this.attributes.set(name, value); } + getContext() { return {}; } + replaceChildren() {} + focus() { this.focused = true; } + click() { this.dispatchEvent(new Event('click')); } +} +function fixture({ delayTextCancellation = false } = {}) { + const elements = Object.fromEntries(['status', 'page', 'canvas', 'text', 'page-number', 'password-form', 'password', + 'previous', 'next', 'zoom-out', 'fit', 'zoom-in', 'cancel', 'pages', 'page-count'].map(id => [id, new Element()])); + const loadingGate = Promise.withResolvers(); + const renders = []; + const pages = new Map(); + const texts = []; + const options = []; + let destroyed = false; + const loading = { promise: loadingGate.promise, destroy: async () => { destroyed = true; } }; + const pdf = { + numPages: 3, + getPage: async number => { + if (!pages.has(number)) pages.set(number, { + cleanupCount: 0, + cleanup() { ++this.cleanupCount; }, + getViewport: ({ scale }) => ({ width: 600 * scale, height: 800 * scale, userUnit: 1 }), + streamTextContent: () => ({}), + render: args => { + const gate = Promise.withResolvers(); + const task = { number, args, promise: gate.promise, done: gate.resolve, + cancel() { this.cancelled = true; gate.reject(Object.assign(new Error('cancelled'), { name: 'RenderingCancelledException' })); } }; + renders.push(task); + return task; + }, + }); + return pages.get(number); + }, + }; + const pdfjs = { + GlobalWorkerOptions: {}, PasswordResponses: { INCORRECT_PASSWORD: 2 }, + getDocument: value => { options.push(value); return loading; }, + TextLayer: class { + gate = Promise.withResolvers(); + constructor() { texts.push(this); } + render() { return delayTextCancellation ? this.gate.promise : Promise.resolve(); } + cancel() { this.cancelled = true; } + settle() { this.gate.resolve(); } + }, + }; + const document = { getElementById: id => elements[id], body: { dataset: { document: './file/report.pdf' } } }; + const window = Object.assign(new EventTarget(), { location: { href: 'http://127.0.0.1:1234/cap/view' }, devicePixelRatio: 2, setTimeout, clearTimeout }); + const viewer = startPdfViewer(pdfjs, document, window); + return { elements, loading, loadingGate, pdf, pdfjs, options, renders, pages, texts, viewer, window, destroyed: () => destroyed }; +} + +test('bounds enormous page bitmaps at both per-axis and total-pixel limits', () => { + for (const [width, height, dpr] of [[600, 800, 2], [1e6, 1e6, 4], [1e7, 10, 2], [10, 1e7, 2]]) { + const size = canvasSize(width, height, dpr); + assert.ok(size.width <= 8192 && size.height <= 8192); + assert.ok(size.width * size.height <= 16 * 1024 * 1024); + } +}); + +test('loads only capability-relative PDF assets, cancels obsolete rendering and navigates/zooms', async () => { + const f = fixture(); + assert.equal(f.pdfjs.GlobalWorkerOptions.workerSrc, 'http://127.0.0.1:1234/cap/pdfjs/pdf.worker.mjs'); + assert.equal(f.options[0].url, 'http://127.0.0.1:1234/cap/file/report.pdf'); + assert.equal(f.options[0].enableXfa, false); + f.loadingGate.resolve(f.pdf); + await tick(); + assert.equal(f.renders[0].number, 1); + f.elements.next.click(); + await tick(); + assert.equal(f.renders[0].cancelled, true); + assert.equal(f.renders[1].number, 2); + f.renders[1].done(); + await tick(); + assert.equal(f.elements.status.textContent, 'Page 2 of 3'); + const originalWidth = f.elements.canvas.width; + f.elements['zoom-in'].click(); + await tick(); + assert.ok(f.elements.canvas.width > originalWidth); + f.renders.at(-1).done(); + await tick(); + f.elements.fit.click(); + await tick(); + assert.equal(f.elements.canvas.width, originalWidth); + f.renders.at(-1).done(); + await tick(); + f.elements['page-number'].value = '999'; + f.elements['page-number'].dispatchEvent(new Event('change')); + await tick(); + assert.equal(f.renders.at(-1).number, 2); + f.viewer.cancel(); + await f.viewer.ready; + assert.equal(f.destroyed(), true); +}); + +test('keeps loading failures visible and accepts password retries without keeping the entered password', async () => { + const f = fixture(); + let supplied; + f.loading.onPassword(password => { supplied = password; }, 2); + assert.equal(f.elements['password-form'].hidden, false); + assert.equal(f.elements.status.textContent, 'Incorrect password. Try again.'); + f.elements.password.value = 'secret'; + f.elements['password-form'].dispatchEvent(new Event('submit', { cancelable: true })); + assert.equal(supplied, 'secret'); + assert.equal(f.elements.password.value, ''); + f.loadingGate.reject(new Error('Invalid PDF structure')); + await f.viewer.ready; + assert.equal(f.elements.status.attributes.get('role'), 'alert'); + assert.match(f.elements.status.textContent, /Invalid PDF structure/); + f.viewer.cancel(); +}); + +test('cancellation prevents a late document load from drawing or replacing its status', async () => { + const f = fixture(); + f.elements.cancel.click(); + f.loadingGate.resolve(f.pdf); + await f.viewer.ready; + assert.equal(f.renders.length, 0); + assert.equal(f.elements.next.disabled, true); + assert.match(f.elements.status.textContent, /cancelled/); + assert.equal(f.destroyed(), true); +}); + +test('releases a previous page only after cancelled canvas and text work settle', async () => { + const f = fixture({ delayTextCancellation: true }); + f.loadingGate.resolve(f.pdf); + await tick(); + f.elements.next.click(); + await tick(); + assert.equal(f.renders[0].cancelled, true); + assert.equal(f.texts[0].cancelled, true); + assert.equal(f.pages.get(1).cleanupCount, 0); + assert.equal(f.renders.length, 1); + f.texts[0].settle(); + await tick(); + assert.equal(f.pages.get(1).cleanupCount, 1); + assert.equal(f.renders[1].number, 2); + f.renders[1].done(); + f.texts[1].settle(); + await tick(); + f.elements.next.click(); + await tick(); + assert.equal(f.pages.get(2).cleanupCount, 1); + assert.equal(f.renders[2].number, 3); + f.renders[2].done(); + f.texts[2].settle(); + await tick(); + f.viewer.cancel(); + await tick(); + assert.equal(f.pages.get(3).cleanupCount, 1); +}); + +test('cleans a stale getPage result before a newer request can reuse that cached page', async () => { + const f = fixture(); + const page = await f.pdf.getPage(1); + const gate = Promise.withResolvers(); + let calls = 0; + f.pdf.getPage = async () => ++calls === 1 ? gate.promise : page; + f.loadingGate.resolve(f.pdf); + await tick(); + f.elements.fit.click(); + gate.resolve(page); + await tick(); + assert.equal(page.cleanupCount, 1); + assert.equal(f.renders.length, 1); + f.renders[0].done(); + await tick(); + assert.equal(page.cleanupCount, 1); + assert.equal(f.elements.status.textContent, 'Page 1 of 3'); + f.viewer.cancel(); + await tick(); + assert.equal(page.cleanupCount, 2); +}); diff --git a/dor/viewer/controller.mjs b/dor/viewer/controller.mjs new file mode 100644 index 000000000..006611bbd --- /dev/null +++ b/dor/viewer/controller.mjs @@ -0,0 +1,190 @@ +const MAX_PIXELS = 16 * 1024 * 1024; +const MAX_DIMENSION = 8192; + +/** Bound both canvas dimensions and backing pixels, including unusual page + * sizes and high-DPI displays. CSS zoom does not allocate a larger bitmap. */ +export function canvasSize(width, height, deviceScale) { + const ratio = Math.min(Math.max(1, deviceScale || 1), 2, + MAX_DIMENSION / width, MAX_DIMENSION / height, Math.sqrt(MAX_PIXELS / (width * height))); + return { width: Math.max(1, Math.floor(width * ratio)), height: Math.max(1, Math.floor(height * ratio)), ratio }; +} + +export function startPdfViewer(pdfjs, document = globalThis.document, window = globalThis.window) { + const element = id => document.getElementById(id); + const status = element('status'); + const pageBox = element('page'); + const canvas = element('canvas'); + const text = element('text'); + const pageInput = element('page-number'); + const passwordForm = element('password-form'); + const controls = ['previous', 'next', 'zoom-out', 'fit', 'zoom-in', 'page-number']; + let pdf; + let pageNumber = 1; + let zoom = 1; + let generation = 0; + let active; + let renderQueue = Promise.resolve(); + let heldPage; + let heldPageNumber; + let stopped = false; + let resizeTimer; + let updatePassword; + + const message = (value, error = false) => { + status.textContent = value; + status.setAttribute('role', error ? 'alert' : 'status'); + }; + const updateControls = () => { + for (const id of controls) element(id).disabled = !pdf || stopped; + element('previous').disabled ||= pageNumber <= 1; + element('next').disabled ||= pageNumber >= (pdf?.numPages ?? 1); + element('zoom-out').disabled ||= zoom <= .25; + element('zoom-in').disabled ||= zoom >= 4; + pageInput.value = String(pageNumber); + }; + const isCurrent = request => !stopped && generation === request; + const releasePage = () => { + heldPage?.cleanup(); + heldPage = heldPageNumber = undefined; + }; + function render() { + if (!pdf || stopped) return Promise.resolve(); + const request = ++generation; + const number = pageNumber; + const requestedZoom = zoom; + active?.canvas?.cancel(); + active?.text?.cancel(); + updateControls(); + message(`Loading page ${number}…`); + pageBox.setAttribute('aria-busy', 'true'); + // Serial jobs prevent a stale getPage result from cleaning a PDFPageProxy + // already reused by a newer request. Obsolete queued jobs do no work. + renderQueue = renderQueue.catch(() => {}).then(async () => { + if (!isCurrent(request)) return; + const job = {}; + const tasks = []; + active = job; + let page; + try { + if (heldPageNumber !== number) releasePage(); + page = heldPage ?? await pdf.getPage(number); + if (!isCurrent(request)) return; + heldPage = page; + heldPageNumber = number; + const natural = page.getViewport({ scale: 1 }); + const fit = Math.max(1, element('pages').clientWidth - 24) / natural.width; + const scale = Math.min(fit * requestedZoom, MAX_DIMENSION / Math.max(natural.width, natural.height)); + const viewport = page.getViewport({ scale }); + const size = canvasSize(viewport.width, viewport.height, window.devicePixelRatio); + canvas.width = size.width; + canvas.height = size.height; + canvas.style.width = `${viewport.width}px`; + canvas.style.height = `${viewport.height}px`; + pageBox.style.width = `${viewport.width}px`; + pageBox.style.height = `${viewport.height}px`; + pageBox.style.setProperty('--scale-factor', String(scale)); + pageBox.style.setProperty('--user-unit', String(viewport.userUnit ?? 1)); + pageBox.hidden = false; + text.replaceChildren(); + text.setAttribute('aria-label', `Page ${number} text`); + const context = canvas.getContext('2d'); + if (!context) throw new Error('Canvas is unavailable in this browser.'); + job.canvas = page.render({ canvasContext: context, viewport, transform: [size.ratio, 0, 0, size.ratio, 0, 0] }); + tasks.push(job.canvas.promise); + job.text = new pdfjs.TextLayer({ textContentSource: page.streamTextContent(), container: text, viewport }); + tasks.push(job.text.render()); + await Promise.all(tasks); + if (!isCurrent(request)) return; + pageBox.setAttribute('aria-busy', 'false'); + message(`Page ${number} of ${pdf.numPages}`); + } catch (error) { + if (!isCurrent(request) || error?.name === 'RenderingCancelledException') return; + pageBox.hidden = true; + message(`Unable to display this page: ${error?.message || 'PDF rendering failed.'}`, true); + } finally { + // Cancelling a task is asynchronous. Free operator lists/images only + // after both painting and text extraction have stopped using the page. + await Promise.allSettled(tasks); + if (active === job) active = undefined; + if (page && page !== heldPage) page.cleanup(); + if (!isCurrent(request)) releasePage(); + } + }); + return renderQueue; + } + + const asset = path => new URL(`./pdfjs/${path}`, window.location.href).href; + pdfjs.GlobalWorkerOptions.workerSrc = asset('pdf.worker.mjs'); + const loading = pdfjs.getDocument({ + url: new URL(document.body.dataset.document, window.location.href).href, + cMapUrl: asset('cmaps/'), cMapPacked: true, + standardFontDataUrl: asset('standard_fonts/'), + wasmUrl: asset('wasm/'), iccUrl: asset('iccs/'), + // No annotation actions, embedded document scripting, or XFA forms. + enableXfa: false, + }); + loading.onPassword = (update, reason) => { + if (stopped) return; + updatePassword = update; + passwordForm.hidden = false; + message(reason === pdfjs.PasswordResponses.INCORRECT_PASSWORD ? 'Incorrect password. Try again.' : 'This PDF needs a password.'); + element('password').focus(); + }; + passwordForm.addEventListener('submit', event => { + event.preventDefault(); + if (!updatePassword || stopped) return; + const password = element('password'); + const value = password.value; + password.value = ''; + passwordForm.hidden = true; + const update = updatePassword; + updatePassword = undefined; + message('Loading PDF…'); + update(value); + }); + function cancel() { + if (stopped) return; + stopped = true; + ++generation; + window.clearTimeout(resizeTimer); + active?.canvas?.cancel(); + active?.text?.cancel(); + void renderQueue.finally(releasePage); + void loading.destroy().catch(() => {}); + pageBox.hidden = true; + passwordForm.hidden = true; + element('password').value = ''; + canvas.width = canvas.height = 1; + text.replaceChildren(); + updateControls(); + element('cancel').disabled = true; + message('PDF preview cancelled. Reload to open it again.'); + } + element('cancel').addEventListener('click', cancel); + element('previous').addEventListener('click', () => { if (pageNumber > 1) { --pageNumber; void render(); } }); + element('next').addEventListener('click', () => { if (pdf && pageNumber < pdf.numPages) { ++pageNumber; void render(); } }); + pageInput.addEventListener('change', () => { + const requested = Number(pageInput.value); + if (pdf && Number.isInteger(requested) && requested >= 1 && requested <= pdf.numPages) pageNumber = requested; + void render(); + }); + element('zoom-out').addEventListener('click', () => { zoom = Math.max(.25, zoom / 1.25); void render(); }); + element('zoom-in').addEventListener('click', () => { zoom = Math.min(4, zoom * 1.25); void render(); }); + element('fit').addEventListener('click', () => { zoom = 1; void render(); }); + window.addEventListener('resize', () => { + window.clearTimeout(resizeTimer); + resizeTimer = window.setTimeout(() => { void render(); }, 100); + }); + window.addEventListener('pagehide', cancel, { once: true }); + updateControls(); + const ready = loading.promise.then(async result => { + if (stopped) return; + pdf = result; + pageInput.max = String(pdf.numPages); + element('page-count').textContent = `of ${pdf.numPages}`; + await render(); + }).catch(error => { + if (!stopped) message(`Unable to open this PDF: ${error?.message || 'Invalid PDF file.'}`, true); + }); + return { ready, cancel }; +} diff --git a/dor/viewer/viewer.css b/dor/viewer/viewer.css new file mode 100644 index 000000000..9bddab672 --- /dev/null +++ b/dor/viewer/viewer.css @@ -0,0 +1,14 @@ +:root { color-scheme: light dark; font: 14px/1.4 system-ui, sans-serif; } +body { margin: 0; color: CanvasText; background: Canvas; } +header { display: flex; align-items: center; flex-wrap: wrap; gap: .4rem; padding: .6rem; border-bottom: 1px solid GrayText; } +button, input { font: inherit; color: ButtonText; background: ButtonFace; border: 1px solid GrayText; border-radius: .25rem; padding: .25rem .45rem; } +button { cursor: pointer; } +button:disabled { opacity: .5; cursor: default; } +input[type=number] { width: 4rem; } +#status, #password-form { margin: .5rem .75rem; } +#status[role=alert] { font-weight: 600; } +#pages { overflow: auto; padding: 12px; } +#page { position: relative; margin: 0 auto; background: white; --user-unit: 1; --total-scale-factor: calc(var(--scale-factor) * var(--user-unit)); --scale-round-x: 1px; --scale-round-y: 1px; } +#canvas { display: block; } +#text { position: absolute; inset: 0; } +[hidden] { display: none !important; } diff --git a/dor/viewer/viewer.html b/dor/viewer/viewer.html new file mode 100644 index 000000000..f1fd2e9b6 --- /dev/null +++ b/dor/viewer/viewer.html @@ -0,0 +1,31 @@ + + + + + +__DOR_PDF_TITLE__ + + + + +
+ + +of — + + + + + +
+

Loading PDF…

+ +
+ +
+ + + diff --git a/dor/viewer/viewer.mjs b/dor/viewer/viewer.mjs new file mode 100644 index 000000000..845d860b0 --- /dev/null +++ b/dor/viewer/viewer.mjs @@ -0,0 +1,9 @@ +import { startPdfViewer } from './controller.mjs'; + +try { + startPdfViewer(await import('./pdf.mjs')); +} catch (error) { + const status = document.getElementById('status'); + status.setAttribute('role', 'alert'); + status.textContent = `Unable to start the PDF viewer: ${error.message || 'Renderer unavailable.'}`; +} diff --git a/lib/package.json b/lib/package.json index 59f1c2b88..905aa90af 100644 --- a/lib/package.json +++ b/lib/package.json @@ -11,7 +11,7 @@ "build": "tsc -b && vite build", "build:pocket": "vite build --config vite.pocket.config.ts && vite build --config vite.sw.config.ts && node scripts/assert-pocket-worker.mjs", "preview": "vite preview", - "pretest": "pnpm --filter dor-lib-common build && pnpm --filter remote-lib-common build", + "pretest": "pnpm --filter dor-lib-common build && pnpm --filter remote-lib-common build && pnpm --filter dor build:pdf-viewer", "test": "pnpm typecheck && vitest run", "test:watch": "vitest", "storybook": "storybook dev -p 6006 --no-open --ci", @@ -51,6 +51,7 @@ "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^6.0.2", "chromatic": "^17.0.0", + "dor": "workspace:*", "fake-indexeddb": "^6.2.5", "remark-gfm": "^4.0.1", "storybook": "^10.4.0", diff --git a/lib/src/components/wall/SurfacePaneHeader.test.tsx b/lib/src/components/wall/SurfacePaneHeader.test.tsx index 5485d8696..845e6bb3e 100644 --- a/lib/src/components/wall/SurfacePaneHeader.test.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.test.tsx @@ -6,6 +6,10 @@ import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { PaneProps } from './pane-props'; import { SurfacePaneHeader } from './SurfacePaneHeader'; +import { ToolPaneHeader } from './ToolPaneHeader'; +import { FakePtyAdapter } from '../../lib/platform/fake-adapter'; +import { setPlatform } from '../../lib/platform'; +import { addPlainNote, clearAllNotepads, getOpenNotepadId } from '../../lib/notepad/notepad-store'; import { registerAgentBrowserScreen, type ChromeSnapshot, @@ -38,22 +42,35 @@ function headerProps(id: string, title: string): PaneProps { let container: HTMLDivElement; let root: Root; +let resizeHeader: (width: number) => void; + beforeEach(() => { + setPlatform(new FakePtyAdapter()); + clearAllNotepads(); container = document.createElement('div'); document.body.appendChild(container); root = createRoot(container); + vi.stubGlobal('ResizeObserver', class { + constructor(private callback: ResizeObserverCallback) {} + observe(target: Element) { + resizeHeader = width => this.callback([{ target, borderBoxSize: [{ inlineSize: width }] } as unknown as ResizeObserverEntry], this as unknown as ResizeObserver); + resizeHeader(620); + } + disconnect() {} + }); }); afterEach(() => { act(() => root.unmount()); container.remove(); + vi.unstubAllGlobals(); }); function renderHeader( props: PaneProps, actions: WallActions, - state: { active?: boolean; zoomedId?: string | null } = {}, + state: { active?: boolean; zoomedId?: string | null; tool?: boolean } = {}, ) { act(() => { root.render( @@ -63,7 +80,7 @@ function renderHeader( - + {state.tool ? : } @@ -75,6 +92,107 @@ function renderHeader( } describe('SurfacePaneHeader — browser chrome', () => { + it('adapts to pane resizes in a wide window and keeps compact controls keyboard reachable', async () => { + const registration = register('pane-resize', { ...CHROME, key: 'a'.repeat(300) }); + const actions = stubActions(); + renderHeader({ ...headerProps('pane-resize', 'Browser'), params: { surfaceType: 'tool', url: CHROME.url } }, actions, { tool: true }); + expect(container.querySelector('[aria-label="Terminal context"]')).not.toBeNull(); + const viewport = window.innerWidth; + expect(container.querySelector('[aria-label="Back"]')).not.toBeNull(); + expect(container.querySelector('[aria-label="Zoom"]')).not.toBeNull(); + act(() => resizeHeader(400)); + expect(container.querySelector('[aria-label="Back"]')).not.toBeNull(); + expect(container.querySelector('[aria-label="Zoom"]')).toBeNull(); + act(() => resizeHeader(340)); + expect(container.querySelector('[aria-label="Back"]')).toBeNull(); + // A 103px Tool leaves 79px beside its Terminal Context button. + act(() => resizeHeader(79)); + const overflow = container.querySelector('[aria-label="Browser controls"]')!; + expect(overflow).not.toBeNull(); + for (const label of ['Minimize', 'Kill']) { + act(() => overflow.click()); + act(() => container.querySelector(`[aria-label="${label}"]`)!.click()); + expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); + } + expect(actions.onMinimize).toHaveBeenCalledWith('pane-resize'); + expect(actions.onKill).toHaveBeenCalledWith('pane-resize'); + act(() => overflow.click()); + let dialog = document.querySelector('[role="dialog"][aria-label="Browser controls"]')!; + expect(dialog).not.toBeNull(); + expect(dialog.contains(document.activeElement)).toBe(true); + const firstControl = document.activeElement; + act(() => firstControl!.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', bubbles: true }))); + expect(document.activeElement).not.toBe(firstControl); + expect(dialog.contains(document.activeElement)).toBe(true); + expect(dialog.querySelector('[aria-label="Back"]')).not.toBeNull(); + await act(async () => { dialog.querySelector('[aria-label="Split left/right"]')!.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); + expect(actions.onSplitH).toHaveBeenCalledWith('pane-resize'); + expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); + act(() => overflow.click()); + dialog = document.querySelector('[role="dialog"][aria-label="Browser controls"]')!; + const url = dialog.querySelector('[role="button"]')!; + act(() => { url.focus(); url.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); }); + expect(dialog.querySelector('input')).not.toBeNull(); + act(() => dialog.querySelector('input')!.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))); + expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); + expect(document.activeElement).toBe(overflow); + act(() => addPlainNote('pane-resize', 'A saved note')); + expect(overflow.getAttribute('aria-label')).toBe('Browser controls, 1 note'); + act(() => overflow.click()); + expect(document.querySelector('[role="dialog"] input')).toBeNull(); + await act(async () => { document.querySelector('[role="dialog"] button[aria-label^="Notepad"]')!.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); + expect(getOpenNotepadId()).toBe('pane-resize'); + act(() => resizeHeader(56)); + expect(container.querySelector('[aria-label="Kill"]')).toBeNull(); + for (const label of ['Minimize', 'Kill']) { + act(() => overflow.click()); + await act(async () => { document.querySelector(`[role="dialog"] [aria-label="${label}"]`)!.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); + } + expect(actions.onMinimize).toHaveBeenCalledTimes(2); + expect(actions.onKill).toHaveBeenCalledTimes(2); + act(() => resizeHeader(620)); + expect(container.querySelector('[aria-label^="Browser controls"]')).toBeNull(); + expect(container.querySelector('[aria-label="Back"]')).not.toBeNull(); + expect(container.querySelector('[aria-label="Zoom"]')).not.toBeNull(); + expect(window.innerWidth).toBe(viewport); + registration.dispose(); + }); + + it('runs popup Zoom, Reload and Display before dismissal and preserves modal focus', async () => { + const modalControl = document.createElement('button'); + document.body.appendChild(modalControl); + const stillOwnsFocus = () => { + const popup = document.querySelector('[role="dialog"][aria-label="Browser controls"]'); + expect(popup?.contains(document.activeElement)).toBe(true); + }; + const onZoom = vi.fn(stillOwnsFocus); + const reload = vi.fn(stillOwnsFocus); + const openModal = vi.fn(() => { stillOwnsFocus(); modalControl.focus(); }); + const registration = registerAgentBrowserScreen('pane-popup-actions', { + snapshot: SCREEN, chrome: CHROME, hostCapable: true, + actions: { engageSync: vi.fn(), applyDevice: vi.fn(), applyViewport: vi.fn(), openModal }, + chromeActions: { navigate: vi.fn(), back: vi.fn(), forward: vi.fn(), reload }, + }); + try { + renderHeader(headerProps('pane-popup-actions', 'Browser'), stubActions({ onZoom })); + act(() => resizeHeader(79)); + const trigger = container.querySelector('[aria-label="Browser controls"]')!; + for (const selector of ['[aria-label="Zoom"]', '[aria-label="Reload"]', '[data-browser-display-trigger]']) { + act(() => trigger.click()); + const action = document.querySelector(`[role="dialog"] ${selector}`)!; + await act(async () => { action.focus(); action.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); + expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); + } + expect(onZoom).toHaveBeenCalledWith('pane-popup-actions'); + expect(reload).toHaveBeenCalledOnce(); + expect(openModal).toHaveBeenCalledOnce(); + expect(document.activeElement).toBe(modalControl); + } finally { + registration.dispose(); + modalControl.remove(); + } + }); + it('uses the shared capability-first icon pair for every browser display mode', () => { const cases = [ [{ ...SCREEN, renderMode: 'ab-screencast', syncEngaged: true }, 'ab-resize', 2], diff --git a/lib/src/components/wall/SurfacePaneHeader.tsx b/lib/src/components/wall/SurfacePaneHeader.tsx index 9ec95bfee..0597e27ce 100644 --- a/lib/src/components/wall/SurfacePaneHeader.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.tsx @@ -1,5 +1,11 @@ -import { useContext, useEffect, useState } from 'react'; +import { useCallback, useContext, useEffect, useLayoutEffect, useRef, useState, type CSSProperties, type ReactNode } from 'react'; +import { createPortal } from 'react-dom'; +import { usePopoverFocusTrap } from '../use-popover-focus-trap'; +import { useNoteCount } from '../use-notepad'; +import { clampOverlayPosition } from '../../lib/ui-geometry'; import { + DotsThreeIcon, + NotepadIcon, ArrowClockwiseIcon, ArrowLeftIcon, ArrowLineDownIcon, @@ -11,7 +17,7 @@ import { XIcon, } from '@phosphor-icons/react'; import { HeaderActionButton } from '../HeaderActionButton'; -import { HEADER_PALETTE_TRANSITION_CLASS, paneZoomButtonClass, TERMINAL_TOP_RADIUS_CLASS } from '../design'; +import { HEADER_PALETTE_TRANSITION_CLASS, POPUP_SURFACE_CLASS, paneZoomButtonClass, TERMINAL_TOP_RADIUS_CLASS } from '../design'; import { NotepadHeaderButton } from './NotepadHeaderButton'; import { useAgentBrowserChromeSnapshot, @@ -80,11 +86,30 @@ export function SurfacePaneHeader({ id, title }: PaneProps) { }; const closeUrlEditor = () => setEditingUrl(false); - return ( -
actions.onClickPanel(id)} - > + const headerRef = useRef(null); + const overflowRef = useRef(null); + const [width, setWidth] = useState(Number.POSITIVE_INFINITY); + const compact = width < 180; + const [menuAnchor, setMenuAnchor] = useState(null); + const noteCount = useNoteCount(id); + const overflowLabel = `Browser controls${noteCount ? `, ${noteCount} ${noteCount === 1 ? 'note' : 'notes'}` : ''}`; + const closeMenu = useCallback((restoreFocus = true) => { setMenuAnchor(null); setEditingUrl(false); if (restoreFocus) overflowRef.current?.focus(); }, []); + useLayoutEffect(() => { + const header = headerRef.current; + if (!header) return; + const initialWidth = header.getBoundingClientRect().width; + if (initialWidth > 0) setWidth(initialWidth); + const observer = new ResizeObserver(([entry]) => { + setWidth(entry.borderBoxSize?.[0]?.inlineSize ?? entry.contentRect.width); + setMenuAnchor(null); + setEditingUrl(false); + }); + observer.observe(header); + return () => observer.disconnect(); + }, []); + + const browserControls = ( + <> {screen && screenSnapshot && chrome ? ( <> {/* Render/screen chip → far left, out of the way of the nav controls. @@ -104,7 +129,7 @@ export function SurfacePaneHeader({ id, title }: PaneProps) { {/* Back / forward / refresh — native agent-browser commands; always enabled (no canGoBack/Forward in the stream). Collapse before the URL but after split/zoom. */} -
+ {(compact || width >= 360) &&
{ e.stopPropagation(); screen.chromeActions.back(); }} @@ -123,14 +148,14 @@ export function SurfacePaneHeader({ id, title }: PaneProps) { ariaLabel="Reload" tooltip="Reload" > -
+
} {/* --key indicator for non-default keys only — the key name inline, small + quiet (hover reveals `--key `), never a prefix on the persisted title. Raw --session surfaces show none. */} {chrome.key && chrome.key !== 'default' && ( {chrome.key} )} @@ -163,7 +188,7 @@ export function SurfacePaneHeader({ id, title }: PaneProps) { className="flex h-5 min-w-0 items-center gap-1 rounded px-1.5 text-xs transition-colors hover:bg-current/10" > {devServer.label} - {port != null && :{port}} + {port != null && :{port}} )} @@ -172,14 +197,17 @@ export function SurfacePaneHeader({ id, title }: PaneProps) { / full URL → tooltip. Gives up width (shrink-[10]) long before the command does. */} <span - className="min-w-0 shrink-[10] cursor-text truncate font-medium underline-offset-2 hover:underline" + className={`${compact ? 'basis-full' : ''} min-w-0 shrink-[10] cursor-text truncate font-medium underline-offset-2 hover:underline`} title={chrome.title ?? chrome.url ?? undefined} onMouseDown={(e) => e.stopPropagation()} + role="button" + tabIndex={0} + onKeyDown={event => { if (event.key === 'Enter' || event.key === ' ') { event.preventDefault(); setEditingUrl(true); } }} onClick={(e) => { e.stopPropagation(); setEditingUrl(true); }} >{urlText || title || id}</span> {/* Flexible spacer keeps the layout buttons right-aligned. */} - <div className="min-w-0 flex-1" /> + {!compact && <div className="min-w-0 flex-1" />} </> )} </> @@ -188,7 +216,7 @@ export function SurfacePaneHeader({ id, title }: PaneProps) { )} <NotepadHeaderButton surfaceId={id} /> - <div className="ml-1 hidden shrink-0 items-center gap-0.5 min-[420px]:flex"> + {(compact || width >= 420) && <div className="ml-1 flex shrink-0 items-center gap-0.5"> <HeaderActionButton className="flex h-5 min-w-5 items-center justify-center rounded transition-colors hover:bg-current/10" onClick={(e) => { e.stopPropagation(); actions.onSplitH(id); }} @@ -207,21 +235,79 @@ export function SurfacePaneHeader({ id, title }: PaneProps) { ariaLabel={zoomed ? 'Unzoom' : 'Zoom'} tooltip={zoomed ? 'Unzoom' : 'Zoom [z]'} >{zoomed ? <ArrowsInIcon size={14} /> : <ArrowsOutIcon size={14} />}</HeaderActionButton> - </div> - <div className="ml-1 flex shrink-0 items-center gap-0.5"> - <HeaderActionButton - className="flex h-5 min-w-5 items-center justify-center rounded transition-colors hover:bg-current/10" - onClick={(e) => { e.stopPropagation(); actions.onMinimize(id); }} - ariaLabel="Minimize" - tooltip="Minimize [m] or [d]" - ><ArrowLineDownIcon size={14} /></HeaderActionButton> - <HeaderActionButton - className="flex h-5 min-w-5 items-center justify-center rounded transition-colors hover:bg-error/10 hover:text-error" - onClick={(e) => { e.stopPropagation(); actions.onKill(id); }} - ariaLabel="Kill" - tooltip="Kill [k] or [x]" - ><XIcon size={14} /></HeaderActionButton> - </div> + </div>} + </> + ); + + const paneActions = ( + <div className="ml-auto flex shrink-0 items-center gap-0.5"> + <HeaderActionButton + className="flex h-5 min-w-5 items-center justify-center rounded transition-colors hover:bg-current/10" + onClick={(e) => { e.stopPropagation(); closeMenu(); actions.onMinimize(id); }} + ariaLabel="Minimize" + tooltip="Minimize [m] or [d]" + ><ArrowLineDownIcon size={14} /></HeaderActionButton> + <HeaderActionButton + className="flex h-5 min-w-5 items-center justify-center rounded transition-colors hover:bg-error/10 hover:text-error" + onClick={(e) => { e.stopPropagation(); closeMenu(); actions.onKill(id); }} + ariaLabel="Kill" + tooltip="Kill [k] or [x]" + ><XIcon size={14} /></HeaderActionButton> </div> ); + + return ( + <div + ref={headerRef} + className={`flex h-full min-w-0 flex-1 cursor-grab items-center ${compact ? 'gap-0.5 px-1' : 'gap-1.5 pl-2 pr-[5px]'} ${TERMINAL_TOP_RADIUS_CLASS} text-sm leading-none font-mono select-none active:cursor-grabbing ${HEADER_PALETTE_TRANSITION_CLASS} ${isActiveHeader ? 'bg-header-active-bg text-header-active-fg' : 'bg-header-inactive-bg text-header-inactive-fg'}`} + onMouseDown={() => actions.onClickPanel(id)} + > + {compact ? ( + <button ref={overflowRef} type="button" aria-label={overflowLabel} + aria-haspopup="dialog" aria-expanded={menuAnchor !== null} + title={overflowLabel} + className="flex h-5 min-w-5 shrink-0 items-center justify-center rounded hover:bg-current/10" + onMouseDown={event => event.stopPropagation()} + onClick={event => { event.stopPropagation(); if (menuAnchor) closeMenu(); else setMenuAnchor(event.currentTarget.getBoundingClientRect()); }}> + {noteCount ? <NotepadIcon size={14} weight="fill" /> : <DotsThreeIcon size={14} />} + </button> + ) : browserControls} + {width >= 72 && paneActions} + {compact && menuAnchor && <BrowserHeaderPopover anchor={menuAnchor} onClose={closeMenu}> + {browserControls} + {width < 72 && paneActions} + </BrowserHeaderPopover>} + </div> + ); +} + +function BrowserHeaderPopover({ anchor, onClose, children }: { anchor: DOMRect; onClose: (restoreFocus?: boolean) => void; children: ReactNode }) { + const ref = useRef<HTMLDivElement>(null); + const [position, setPosition] = useState<CSSProperties>({ position: 'fixed', left: anchor.left, top: anchor.bottom }); + useDialogKeyboardOwner(true); + usePopoverFocusTrap(ref, onClose); + useEffect(() => { + const resized = () => onClose(); + window.addEventListener('resize', resized); + return () => window.removeEventListener('resize', resized); + }, [onClose]); + useLayoutEffect(() => { + const rect = ref.current!.getBoundingClientRect(); + setPosition(clampOverlayPosition({ left: anchor.left, top: anchor.bottom + 4, width: rect.width, height: rect.height })); + ref.current!.querySelector<HTMLElement>('button, [tabindex="0"]')?.focus(); + }, [anchor]); + return createPortal( + <div ref={ref} role="dialog" aria-label="Browser controls" style={position} + className={`${POPUP_SURFACE_CLASS} flex max-h-[75dvh] w-80 max-w-[calc(100vw-2rem)] flex-wrap items-center gap-2 overflow-auto p-2 text-sm`} + onMouseDown={event => event.stopPropagation()} + onClickCapture={event => { + if (!(event.target as Element).closest('button')) return; + // Native clicks can drain microtasks between capture and bubble. Wait + // a task so the action runs before its target unmounts; a new modal + // keeps any focus it acquired in the action handler. + setTimeout(() => onClose(document.activeElement === document.body || !!ref.current?.contains(document.activeElement)), 0); + }}> + {children} + </div>, document.body, + ); } diff --git a/lib/src/host/file-viewer-proxy.test.ts b/lib/src/host/file-viewer-proxy.test.ts index 9cade49c1..2ef90a15f 100644 --- a/lib/src/host/file-viewer-proxy.test.ts +++ b/lib/src/host/file-viewer-proxy.test.ts @@ -76,7 +76,6 @@ it('retains the policy for an escaped text preview', async () => { it.each([ ['image.svg', '<svg xmlns="http://www.w3.org/2000/svg"/>', 'image/svg+xml'], - ['document.pdf', '%PDF-1.7 example bytes', 'application/pdf'], ['readme.png', 'image bytes', 'image/png'], ['video.mp4', 'video bytes', 'video/mp4'], ])('retains CSP, bytes, HEAD, and ranges for %s through the proxy', async (name, bytes, mime) => { @@ -96,3 +95,24 @@ it.each([ expect(range.headers['content-range']).toBe(`bytes 0-3/${Buffer.byteLength(bytes)}`); expect(range.body).toBe(bytes.slice(0, 4)); }); + +it('retains the PDF shell, module worker policy and descriptor ranges through the real proxy', async () => { + const url = await frame('document.pdf', '%PDF-1.7 example bytes'); + const shell = await read(url); + expectViewerPolicy(shell.headers); + expect(shell.headers['content-type']).toBe('text/html; charset=utf-8'); + expect(shell.body).toContain('data-document="./file/document.pdf"'); + expect(shell.body).toContain('__dormouse'); + expect(shell.headers['content-security-policy']).toContain("worker-src 'self'"); + expect(shell.headers['content-security-policy']).toContain("'wasm-unsafe-eval'"); + const worker = await read(new URL('./pdfjs/pdf.worker.mjs', url).href); + expect(worker.status).toBe(200); + expect(worker.headers['content-type']).toBe('text/javascript; charset=utf-8'); + expectViewerPolicy(worker.headers); + expect(worker.headers['content-security-policy']).toContain("'wasm-unsafe-eval'"); + const file = await read(new URL('./file/document.pdf', url).href, 'GET', { Range: 'bytes=0-3' }); + expect(file.status).toBe(206); + expect(file.body).toBe('%PDF'); + expect(file.headers['content-security-policy']).not.toContain("'wasm-unsafe-eval'"); + expect((await read(new URL('./pdfjs/manifest.json', url).href)).status).toBe(404); +}); diff --git a/lib/src/stories/BrowserChromeHeader.stories.tsx b/lib/src/stories/BrowserChromeHeader.stories.tsx index ff9c51ca7..4d0b8a05c 100644 --- a/lib/src/stories/BrowserChromeHeader.stories.tsx +++ b/lib/src/stories/BrowserChromeHeader.stories.tsx @@ -8,6 +8,8 @@ import { type WallActions, } from '../components/wall/wall-context'; import { SurfacePaneHeader } from '../components/wall/SurfacePaneHeader'; +import { ToolPaneHeader } from '../components/wall/ToolPaneHeader'; +import { PANE_HEADER_HEIGHT_PX } from '../components/design'; import { registerAgentBrowserScreen, type ChromeSnapshot, @@ -69,6 +71,8 @@ interface StoryArgs { hostCapable: boolean; /** Header width — shrink past 420/360 to watch split-zoom then nav collapse. */ width: number; + /** Include the Tool Terminal Context button beside the browser header. */ + tool: boolean; /** Whether the surface is the selected/active pane (header highlight). */ selected: boolean; } @@ -150,12 +154,13 @@ function BrowserChromeStory(args: StoryArgs) { story's un-zoomed header. */} <WallActionsContext.Provider value={loggingActions}> <div style={{ width: args.width }}> - <div className="bg-app-bg" style={{ height: 26 }}> - <SurfacePaneHeader + <div className="bg-app-bg" style={{ height: PANE_HEADER_HEIGHT_PX }}> + {args.tool ? <ToolPaneHeader id={surfaceId} title={args.htmlTitle} + params={{ surfaceType: 'tool', url: args.url }} /> : <SurfacePaneHeader id={surfaceId} title={args.htmlTitle || hostPathDisplay(args.url)} params={undefined} - /> + />} </div> </div> </WallActionsContext.Provider> @@ -177,8 +182,9 @@ const meta: Meta<typeof BrowserChromeStory> = { paneKey: { control: 'select', options: ['', 'default', 'storybook'] }, devServerLabel: { control: 'text' }, hostCapable: { control: 'boolean' }, - width: { control: { type: 'range', min: 200, max: 900, step: 10 } }, + width: { control: { type: 'range', min: 80, max: 900, step: 10 } }, selected: { control: 'boolean' }, + tool: { control: 'boolean' }, }, args: { renderMode: 'ab-screencast', @@ -190,6 +196,7 @@ const meta: Meta<typeof BrowserChromeStory> = { devServerLabel: 'pnpm dev', hostCapable: true, width: 620, + tool: false, selected: true, }, }; @@ -235,3 +242,16 @@ export const RawSession: Story = { export const Narrow: Story = { args: { width: 340 }, }; + +/** Real narrow split: the Tool context button leaves 79px for browser chrome. */ +export const TinyTool: Story = { + args: { width: 103, tool: true, paneKey: 'a-very-long-tool-identity', devServerLabel: 'pnpm --filter a-very-long-project-name dev' }, +}; + +export const TinyBrowser: Story = { + args: { width: 103, paneKey: 'a-very-long-browser-identity' }, +}; + +export const SmallestTool: Story = { + args: { width: 80, tool: true }, +}; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c01930ad0..843defb3e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4,6 +4,9 @@ settings: autoInstallPeers: true excludeLinksFromLockfile: false +overrides: + pdfjs-dist>@napi-rs/canvas: '-' + importers: .: @@ -63,6 +66,9 @@ importers: dor-lib-common: specifier: workspace:* version: link:../dor-lib-common + pdfjs-dist: + specifier: 6.3.289 + version: 6.3.289 devDependencies: '@types/node': specifier: ^24.13.4 @@ -180,6 +186,9 @@ importers: chromatic: specifier: ^17.0.0 version: 17.8.0 + dor: + specifier: workspace:* + version: link:../dor fake-indexeddb: specifier: ^6.2.5 version: 6.2.5 @@ -4002,6 +4011,10 @@ packages: resolution: {integrity: sha512-//nshmD55c46FuFw26xV/xFAaB5HF9Xdap7HJBBnrKdAd6/GxDBaNA1870O79+9ueg61cZLSVc+OaFlfmObYVQ==} engines: {node: '>= 14.16'} + pdfjs-dist@6.3.289: + resolution: {integrity: sha512-ZHjSVpDa3D6izMq8/04lvkhkATUmL9px6ChPaXc1k6nU2Mrhlg1/7F0bdUqCwUjw3NsPTfPZsMDUU6ZIcRaeQw==} + engines: {node: '>=22.13.0 || >=24'} + pend@1.2.0: resolution: {integrity: sha512-F3asv42UuXchdzt+xXqfW1OGlVBe+mxa2mqI0pg5yAHZPvFmY3Y6drSf/GQ1A86WgWEN9Kzh/WrgKa6iGcHXLg==} @@ -8133,6 +8146,8 @@ snapshots: pathval@2.0.1: {} + pdfjs-dist@6.3.289: {} + pend@1.2.0: {} picocolors@1.1.1: {} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 885ca5fe8..ecc588a40 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -33,3 +33,7 @@ peerDependencyRules: # sockets that would break first. "@hono/node-ws>@hono/node-server": ^2.0.0 minimumReleaseAge: 1440 + +# PDF.js runs only as staged browser assets; its optional Node canvas backend is unused. +overrides: + "pdfjs-dist>@napi-rs/canvas": "-" diff --git a/scripts/dor-tool-qc/server.mjs b/scripts/dor-tool-qc/server.mjs new file mode 100644 index 000000000..fc67ad6df --- /dev/null +++ b/scripts/dor-tool-qc/server.mjs @@ -0,0 +1,28 @@ +// Small real-process fixture for docs/testing/dor-tool-qc.md. +import http from 'node:http'; +const argv = process.argv.slice(2); +const option = (name, fallback) => { + const i = argv.indexOf(name); + return i === -1 ? fallback : argv[i + 1]; +}; +const label = option('--label', 'Tool QC'); +const count = Number(option('--ports', '1')); +const escape = text => text.replaceAll('&', '&').replaceAll('<', '<').replaceAll('"', '"'); +const servers = []; +for (let i = 0; i < count; i++) { + const server = http.createServer((req, res) => { + res.setHeader('content-type', 'text/html; charset=utf-8'); + res.end(`<html><head><title>${escape(label)}

${escape(label)}

${escape(req.url)}

${escape(JSON.stringify(argv))}
`); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + servers.push(server); +} +const ports = servers.map(server => server.address().port); +const announce = () => process.stdout.write(`\x1b]367;serve;${JSON.stringify({ v: 1, port: ports[0], path: option('--path', '/qc'), name: label })}\x1b\\`); +console.log(JSON.stringify({ event: 'qc-start', pid: process.pid, ports, argv })); +if (argv.includes('--announce')) announce(); +process.on('SIGUSR1', announce); +for (const signal of ['SIGINT', 'SIGTERM']) process.on(signal, () => { + for (const server of servers) server.close(); + process.exit(0); +}); diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 741516c09..842c0f242 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -7,9 +7,9 @@ "docs/specs/deploy.md": 1900, "docs/specs/dor-browser.md": 4700, "docs/specs/dor-cli.md": 6000, - "docs/specs/dor-tool.md": 3750, + "docs/specs/dor-tool.md": 3800, "docs/specs/glossary.md": 3000, - "docs/specs/layout.md": 8850, + "docs/specs/layout.md": 8900, "docs/specs/mobile-terminal-ui.md": 1950, "docs/specs/mouse-and-clipboard.md": 3800, "docs/specs/notepad.md": 4000, @@ -19,7 +19,7 @@ "docs/specs/remote-security-model.md": 4800, "docs/specs/security-audit.md": 1750, "docs/specs/security-ci.md": 2500, - "docs/specs/security-local.md": 3100, + "docs/specs/security-local.md": 3150, "docs/specs/security-remote.md": 5850, "docs/specs/security-supply-chain.md": 1200, "docs/specs/security.md": 1900, diff --git a/website/scripts/generate-deps.js b/website/scripts/generate-deps.js index 557c282da..86b60b106 100644 --- a/website/scripts/generate-deps.js +++ b/website/scripts/generate-deps.js @@ -306,6 +306,7 @@ const missingLicense = { "Solarized & Selenized": "MIT", }; const missingAuthor = { + "pdfjs-dist": "Mozilla Foundation and PDF.js contributors", "@hono/node-ws": "Hono middleware contributors", // The addon ships a `contributors` array rather than npm's singular `author` // field, and its prebuilt platform packages carry neither. diff --git a/website/src/data/dependencies-npm.json b/website/src/data/dependencies-npm.json index 24aaa5b46..4cf4c5358 100644 --- a/website/src/data/dependencies-npm.json +++ b/website/src/data/dependencies-npm.json @@ -335,6 +335,13 @@ "author": "Sindre Sorhus", "homepage": "https://github.com/sindresorhus/path-key" }, + { + "name": "pdfjs-dist", + "version": "6.3.289", + "license": "Apache-2.0", + "author": "Mozilla Foundation and PDF.js contributors", + "homepage": "https://mozilla.github.io/pdf.js/" + }, { "name": "picomatch", "version": "4.0.7", From 9987152f1232f08f83b96a909590187678f3c493 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 15 Sep 2026 22:13:08 -0700 Subject: [PATCH 02/11] Require a configured user Tool for PDF files --- docs/specs/dor-tool.md | 6 +- docs/specs/dor-tool.rationale.md | 2 +- docs/specs/security-local.md | 2 +- docs/specs/security-local.rationale.md | 2 - docs/testing/dor-tool-qc.md | 21 ++- dor/package.json | 8 +- dor/scripts/build-pdf-viewer.mjs | 45 ------ dor/src/commands/open.ts | 2 +- dor/src/file-viewer-format.ts | 5 +- dor/src/file-viewer.ts | 18 +-- dor/src/pdf-viewer-assets.ts | 30 ---- dor/test/file-viewer.test.mjs | 72 +++------- dor/test/pdf-viewer.test.mjs | 187 ------------------------ dor/test/snapshots/help/open.md | 2 +- dor/viewer/controller.mjs | 190 ------------------------- dor/viewer/viewer.css | 14 -- dor/viewer/viewer.html | 31 ---- dor/viewer/viewer.mjs | 9 -- lib/package.json | 3 +- lib/src/host/file-viewer-proxy.test.ts | 22 +-- lib/src/host/tool-open.test.ts | 13 ++ pnpm-lock.yaml | 15 -- pnpm-workspace.yaml | 4 - scripts/spec-word-budgets.json | 4 +- website/scripts/generate-deps.js | 1 - website/src/data/dependencies-npm.json | 7 - 26 files changed, 61 insertions(+), 654 deletions(-) delete mode 100644 dor/scripts/build-pdf-viewer.mjs delete mode 100644 dor/src/pdf-viewer-assets.ts delete mode 100644 dor/test/pdf-viewer.test.mjs delete mode 100644 dor/viewer/controller.mjs delete mode 100644 dor/viewer/viewer.css delete mode 100644 dor/viewer/viewer.html delete mode 100644 dor/viewer/viewer.mjs diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index 04e4a1fd6..95c40abd5 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -160,13 +160,13 @@ Source of truth: `toolCommand` in `dor/src/commands/tool.ts`; `dor/test/snapshot **Must prefer known extensions over filename-based text fallbacks; source extensions remain escaped previews.** -**Must run the built-in viewer as a Tool-owned `dor` process**, serving HTML, images, PDF/media, and escaped text/source previews. Markdown is source text; custom viewers may render it. Text previews and HTML/CSS dependency inspection are limited to 8 MiB per file. Text/source previews grant only their opened file and skip dependency inspection. (rationale) Oversized HTML and referenced CSS still stream without dependency inspection. The grant contains at most 256 files: the opened document and statically referenced relative HTML/CSS assets within its directory tree; exceeding that bound fails the open without serving a partial grant. Never expand the grant through root-relative, external, or dynamic references; requests can read only granted paths. +**Must run the built-in viewer as a Tool-owned `dor` process**, serving HTML, images, media, and escaped text/source previews. Markdown is source text; custom viewers may render it. Text previews and HTML/CSS dependency inspection are limited to 8 MiB per file. Text/source previews grant only their opened file and skip dependency inspection. (rationale) Oversized HTML and referenced CSS still stream without dependency inspection. The grant contains at most 256 files: the opened document and statically referenced relative HTML/CSS assets within its directory tree; exceeding that bound fails the open without serving a partial grant. Never expand the grant through root-relative, external, or dynamic references; requests can read only granted paths. -**Must render PDFs with the bundled PDF.js renderer inside the existing iframe sandbox**, with page navigation, fit/zoom, selectable page text, password input, and visible loading/error/cancellation states. Render one page at a time; cancel superseded work, release previous-page resources after rendering settles, and bound each canvas to 8,192 pixels per axis and 16 Mi pixels total. Renderer assets use an exact build inventory, separate from the document grant; no CDN or Node-side PDF renderer is used. (rationale) +**Must require a user Tool for PDFs**, including files named `README.pdf`; never pass them to the built-in viewer. (rationale) **Must retain the viewer's opened file descriptors until the Tool exits.** Refresh reads those files again, but atomic replacements and changes to the dependency graph require restarting the viewer. Cold restore runs the saved file command with a fresh URL capability; Workspace movement keeps the live binding. The listener's authority is `docs/specs/security-local.md` → Local-file viewer. -Source of truth: `openCommand` in `dor/src/commands/open.ts`; `resolveOpenTool` in `lib/src/host/tool-open.ts`; `parseToolFile` in `lib/src/host/tool-registry.ts`; `surface.tool` in `lib/src/components/wall/use-dor-control.ts`; `fileViewerFormat` in `dor/src/file-viewer-format.ts`; `startFileViewer` / `runFileViewer` in `dor/src/file-viewer.ts`. Tests: `lib/src/host/tool-open.test.ts`, `dor/test/cli-output.test.mjs`, `lib/src/components/Wall.test.tsx`, `dor/test/file-viewer.test.mjs`, `dor/test/pdf-viewer.test.mjs`. +Source of truth: `openCommand` in `dor/src/commands/open.ts`; `resolveOpenTool` in `lib/src/host/tool-open.ts`; `parseToolFile` in `lib/src/host/tool-registry.ts`; `surface.tool` in `lib/src/components/wall/use-dor-control.ts`; `fileViewerFormat` in `dor/src/file-viewer-format.ts`; `startFileViewer` / `runFileViewer` in `dor/src/file-viewer.ts`. Tests: `lib/src/host/tool-open.test.ts`, `dor/test/cli-output.test.mjs`, `lib/src/components/Wall.test.tsx`, `dor/test/file-viewer.test.mjs`. ## Take-over diff --git a/docs/specs/dor-tool.rationale.md b/docs/specs/dor-tool.rationale.md index 68d74e592..df8847fec 100644 --- a/docs/specs/dor-tool.rationale.md +++ b/docs/specs/dor-tool.rationale.md @@ -44,7 +44,7 @@ The September 2026 integration reuses Terminal Context for the Tool's primary te ## Opening local files -The native PDF plugin rendered a gray pane under the required iframe sandbox in the September 2026 browser harness, while the same 586-byte PDF rendered after removing that sandbox. A bundled PDF.js browser renderer preserves the framing boundary. Its assets ride the existing recursive CLI staging path, so the Node 18 host never imports a package whose Node renderer requires a newer runtime. The browser build excludes the optional native canvas backend and document-scripting sandbox. +Innerdogfood QC in Chromium (2026-09) showed the native PDF plugin failing inside the normal iframe sandbox. PDFs use configured user Tools; the built-in viewer carries no PDF renderer dependency. A CSS source preview escapes its contents, so its URLs cannot load assets. Scanning those references adds unused authority and can reject a small source file at the asset limit. CSS loaded by HTML is active, so its dependencies still enter the bounded grant. diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index bf1936add..a70e41dca 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -134,7 +134,7 @@ Source of truth: the shared rule and predicates — `isLoopbackHost`, `isOwnOrig **FAIL IF** `dor/src/file-viewer.ts` serves any request without the fresh 256-bit URL capability, its own case-insensitive loopback `Host`, an absent or same-listener `Origin`, and a GET/HEAD method. Compare capability prefixes by SHA-256 then `timingSafeEqual`, including malformed lengths. `allowsFileViewerRequest` in `dor/src/file-viewer-loopback-guard.ts` gates every route. Never grant CORS access to foreign origins, cache responses, or send the capability as a referrer. -**FAIL IF** the local-file viewer exposes directory listings, arbitrary path reads, writes, user files outside its opened-document grant, or renderer assets outside the build inventory. Grant construction permits only regular files, rejects symlinks escaping the canonical document directory, bounds static dependency discovery, and retains descriptors so later path replacement cannot widen the grant. Viewer resource loads are restricted by CSP to its own origin plus inline scripts/styles and data images, including through the iframe proxy; escaped text previews execute no document markup. Only the PDF shell and its bundled worker permit local WebAssembly compilation; never enable general JavaScript eval, external renderer assets, or PDF document scripting. The viewer opts into the proxy's upstream-policy preservation (`docs/specs/dor-browser.md` → Iframe Renderer). +**FAIL IF** the local-file viewer exposes directory listings, arbitrary path reads, writes, or a file outside its opened-document grant. Grant construction permits only regular files, rejects symlinks escaping the canonical document directory, bounds static dependency discovery, and retains descriptors so later path replacement cannot widen the grant. Viewer resource loads are restricted by CSP to its own origin plus inline scripts/styles and data images, including through the iframe proxy; escaped text previews execute no document markup. The viewer opts into the proxy's upstream-policy preservation (`docs/specs/dor-browser.md` → Iframe Renderer). **Must not describe the viewer CSP as confining active documents' navigation.** HTML/SVG scripts can navigate their frame to external URLs, including with granted contents; the resource policy is not a no-egress boundary. (rationale) diff --git a/docs/specs/security-local.rationale.md b/docs/specs/security-local.rationale.md index 1d32d05c2..c8dca10f8 100644 --- a/docs/specs/security-local.rationale.md +++ b/docs/specs/security-local.rationale.md @@ -171,6 +171,4 @@ What the snapshot tests cover. `restrict_to_owner_leaves_one_owner_only_ace` is ## Local-file viewer -PDF.js uses local WebAssembly decoders for embedded images and ICC color conversion. Their compilation permission is confined to the generated PDF shell and its worker response; raw document resources keep the ordinary viewer policy. Both the worker and its assets share the capability URL and listener origin, avoiding blob/CDN worker permissions. The build inventory excludes PDF.js document-scripting assets. - The viewer allows inline and granted scripts for interactive local reports. CSP fetch directives constrain resource requests, but do not prevent a script assigning an external URL to its own frame. `form-action` constrains form submissions, not arbitrary navigation. Preserving the policy through the proxy repairs the resource-load boundary; it does not establish that active documents cannot send granted contents outside the machine. [CSP3 navigation checks](https://www.w3.org/TR/CSP3/) and its multiple-policy rules distinguish these mechanisms. diff --git a/docs/testing/dor-tool-qc.md b/docs/testing/dor-tool-qc.md index 9707c7d9e..9e46e13bc 100644 --- a/docs/testing/dor-tool-qc.md +++ b/docs/testing/dor-tool-qc.md @@ -23,7 +23,7 @@ only to a mode-0600 local file; do not include them in reports. | Takeover | Type standalone `dor tool` at a plain prompt; compare compound line and `dor open` | Eligible Tool retains terminal/ref; other paths split | Pass: standalone Tool takes over; compound command and standalone open create separate Tools. | | Serving | Automatic single port, multiple-port conflict, announced port/path | Only owned ports frame; conflict explains refusal; announcement resolves it | Pass: automatic single port; three-port conflict; OSC announcement resolves the conflict. | | File dispatch | Ordered user rules, explicit handler, malformed config, project rule isolation | Correct user handler; useful errors; project config cannot intercept opens | Pass: first matching user rule, explicit override, malformed user errors; malformed/project associations do not intercept opens. | -| Built-in viewer | Text/Markdown, HTML+CSS+image, PDF/image/audio; awkward filename | Correct content, relative assets load, filename is one argument | Pass: text/Markdown source, HTML/CSS/image, SVG, audio and awkward names. PDF embedded font, image, three-page navigation, fit/zoom, password retry, cancel and malformed-file error pass with sandbox intact. | +| Built-in viewer | Text/Markdown, HTML+CSS+image, image/audio; awkward filename | Correct content, relative assets load, filename is one argument | Pass: text/Markdown source, HTML/CSS/image, SVG, audio and awkward names. | | Rejection/bounds | URL, directory, missing/unsupported file, oversized text | Clear errors and no leaked Tool/process | Pass: URL/directory/missing/unsupported rejected; oversized text reports 8 MiB limit and exits. | | UI and lifecycle | Narrow approval, Terminal Context, minimize/reveal, browser exit/refocus, renderer swap | Reachable controls; same Session; input reaches terminal after exit | Pass: approval at 249×203, same-session context, minimize/reveal state, exit/refocus, narrow popup Zoom/Unzoom/Display focus, and iframe↔screencast round trip. | | Workspace/reload | Live page reload, move serving Tool, cross-Workspace identity | State/PTY survive live reload; scoped reuse and correct movement | Pass: live reload preserves IDs/kinds/URLs; cross-Workspace identity is scoped. Tool transfer was not exercised; native-window transfer is unavailable in this harness. | @@ -41,21 +41,20 @@ before the selected action ran; dismissal now waits until that action completes. The clean-harness native-click retest passes: Zoom reaches 716×403 pixels, Unzoom returns to the compact header, Reload works, and Display retains modal focus. Header buttons remain inside every pane at the final 1200×800 viewport. Regression coverage lives in `lib/src/components/wall/SurfacePaneHeader.test.tsx` and narrow header stories. -### Sandboxed PDF rendering +### PDFs require a user Tool A valid one-page PDF showed Chromium's broken-document icon in the normal -sandboxed iframe. Temporarily removing the sandbox on that generated test -fixture confirmed the native PDF plugin was the incompatibility; the normal -sandbox was immediately restored. The fix bundles a browser-only PDF.js -renderer with capability-protected assets and retains the application sandbox. -Live retesting passes for an embedded font, JPEG image, three pages, navigation, fit/zoom, password rejection/retry, cancellation, and malformed PDF. The iframe retains its original sandbox attributes. Regression coverage includes descriptor-backed PDF -routes, renderer inventory, controller cancellation, and the real iframe proxy. +sandboxed iframe. PDFs now require a configured user Tool, per the selected +product behavior. The bundled renderer, dependency, assets and build plumbing +have been removed. Regression tests cover default rejection, explicit +`builtin:file` rejection, user associations, explicit user handlers, and +source-like names such as `README.pdf` and `LICENSE.PDF`. ### Evidence and limits Local screenshots and JSON observations are in the ignored `standalone/src-tauri/target/dor-tool-qc/` directory. Useful before-fix images: -`html.png`, `pdf-zoom.png`; the diagnostic PDF image is `pdf-diagnostic.png`. After-fix PDF evidence is `pdf-fixed.png` and `pdf-image.png`. +`html.png`, `pdf-zoom.png`; the diagnostic PDF image is `pdf-diagnostic.png`. `approval-error.png` captures the persisted error with no PTY. Reload snapshots are `before-reload.json` and `after-reload.json`. Captured CLI credentials are private runtime artifacts, not report material. @@ -66,9 +65,9 @@ real standalone sidecar, staged CLI, PTYs, and iframe proxy. ### Review and automated checks -- Full `pnpm test` and `pnpm build` passed after the first integrated fixes. +- Full `pnpm test` and `pnpm build` passed during the original QC run. PDF-policy follow-up validation is recorded below. - Focused header/Tool/iframe/Wall coverage: 139 tests pass. -- PDF review found two issues, both fixed and re-reviewed: build assets explicitly before lib's proxy tests, and release superseded PDF page resources. Final dor suite: 174 tests; final lib suite: 3,539 tests in 224 files. Typecheck, spec lint and diff checks pass. Navigation back to a released image page also passes live. +- PDF-policy follow-up: all 168 dor tests and 20 host dispatch/proxy tests pass; spec and public-doc lints pass. Both host CLI staging directories contain no PDF renderer assets. - An unexpected development-state reset occurred while the full build/test suite ran beside the harness: Tools appeared as terminals. Investigation confirmed `e2e-lint-selftest` temporarily mutates Vite inputs, including invalid root package JSON; the exact metadata-loss trigger was not captured. A clean harness restart restored normal operation. The final stable-build reload preserved every ID, kind, URL and Workspace (`final-before-reload.json` / `final-after-reload.json`). No claim of cold-restore or native-host coverage is made from the disturbed development reload. ## Cleanup diff --git a/dor/package.json b/dor/package.json index 2df59c746..258245704 100644 --- a/dor/package.json +++ b/dor/package.json @@ -13,9 +13,8 @@ ], "scripts": { "prebuild": "pnpm --filter dor-lib-common build && node ../scripts/generate-dor-version.mjs && node ../scripts/generate-dor-skill.mjs", - "build": "tsc -p tsconfig.json && esbuild src/dor.ts --bundle --format=esm --platform=node --outfile=dist/dor.js --banner:js=\"import { createRequire } from 'module'; const require = createRequire(import.meta.url);\" && pnpm build:pdf-viewer", - "test": "pnpm run build && node --test test/*.test.mjs", - "build:pdf-viewer": "node scripts/build-pdf-viewer.mjs" + "build": "tsc -p tsconfig.json && esbuild src/dor.ts --bundle --format=esm --platform=node --outfile=dist/dor.js --banner:js=\"import { createRequire } from 'module'; const require = createRequire(import.meta.url);\"", + "test": "pnpm run build && node --test test/*.test.mjs" }, "devDependencies": { "@types/node": "^24.13.4", @@ -24,7 +23,6 @@ }, "dependencies": { "@stricli/core": "^1.2.7", - "dor-lib-common": "workspace:*", - "pdfjs-dist": "6.3.289" + "dor-lib-common": "workspace:*" } } diff --git a/dor/scripts/build-pdf-viewer.mjs b/dor/scripts/build-pdf-viewer.mjs deleted file mode 100644 index cec897a79..000000000 --- a/dor/scripts/build-pdf-viewer.mjs +++ /dev/null @@ -1,45 +0,0 @@ -import { createRequire } from 'node:module'; -import { dirname, join, resolve } from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { cp, mkdir, readdir, rm, writeFile } from 'node:fs/promises'; - -const require = createRequire(import.meta.url); -const pdfjs = dirname(require.resolve('pdfjs-dist/package.json')); -const dor = fileURLToPath(new URL('../', import.meta.url)); -export async function buildPdfViewer(output = join(dor, 'dist/pdf-viewer')) { - await rm(output, { recursive: true, force: true }); - await mkdir(output, { recursive: true }); - // Browser artifacts only: never import PDF.js (or its optional native canvas) - // into the Node CLI. Both hosts stage this directory along with dor.js. - for (const [source, target] of [ - ['legacy/build/pdf.min.mjs', 'pdf.mjs'], - ['legacy/build/pdf.worker.min.mjs', 'pdf.worker.mjs'], - ['web/pdf_viewer.css', 'pdf_viewer.css'], - ['web/images', 'images'], - ['cmaps', 'cmaps'], - ['standard_fonts', 'standard_fonts'], - ['iccs', 'iccs'], - ['LICENSE', 'LICENSE'], - ]) await cp(join(pdfjs, source), join(output, target), { recursive: true }); - await mkdir(join(output, 'wasm')); - for (const name of [ - 'openjpeg.wasm', 'openjpeg_nowasm_fallback.js', 'jbig2.wasm', 'jbig2_nowasm_fallback.js', 'qcms_bg.wasm', - 'LICENSE_JBIG2', 'LICENSE_OPENJPEG', 'LICENSE_PDFJS_JBIG2', 'LICENSE_PDFJS_OPENJPEG', 'LICENSE_PDFJS_QCMS', 'LICENSE_QCMS', - ]) await cp(join(pdfjs, 'wasm', name), join(output, 'wasm', name)); - for (const name of ['viewer.html', 'viewer.css', 'viewer.mjs', 'controller.mjs']) { - await cp(join(dor, 'viewer', name), join(output, name)); - } - const files = []; - async function inventory(directory, prefix = '') { - for (const entry of await readdir(directory, { withFileTypes: true })) { - const name = prefix + entry.name; - if (entry.isDirectory()) await inventory(join(directory, entry.name), name + '/'); - else if (entry.isFile()) files.push(name); - else throw new Error(`Unexpected PDF viewer asset: ${name}`); - } - } - await inventory(output); - await writeFile(join(output, 'manifest.json'), JSON.stringify(files.sort()) + '\n'); -} - -if (process.argv[1] && fileURLToPath(import.meta.url) === resolve(process.argv[1])) await buildPdfViewer(); diff --git a/dor/src/commands/open.ts b/dor/src/commands/open.ts index 15df6d5ac..2407a1f6f 100644 --- a/dor/src/commands/open.ts +++ b/dor/src/commands/open.ts @@ -19,7 +19,7 @@ export const openCommand: Command = { brief: 'Open a local file with a Dor Tool.', fullDescription: `Opens one existing local file. Relative paths resolve from the caller's directory (or --cwd); symlink aliases resolve to the same file. URLs, directories, and Surface handles are not accepted. -The first matching rule in the user dormouse.yml selects a user Tool or builtin:file. --tool chooses a handler explicitly. Without a matching rule, the built-in viewer opens supported HTML, text/source, image, PDF, and media files. Use --tool builtin:file to select it explicitly. Markdown is shown as source text; a user Tool can provide rendered Markdown. Project associations and project Tools never participate in this lookup. The user file is $XDG_CONFIG_HOME/dormouse/dormouse.yml, or ~/.config/dormouse/dormouse.yml. +The first matching rule in the user dormouse.yml selects a user Tool or builtin:file. --tool chooses a handler explicitly. Without a matching rule, the built-in viewer opens supported HTML, text/source, image, and media files. PDFs require a user Tool association or --tool . Use --tool builtin:file to select it explicitly. Markdown is shown as source text; a user Tool can provide rendered Markdown. Project associations and project Tools never participate in this lookup. The user file is $XDG_CONFIG_HOME/dormouse/dormouse.yml, or ~/.config/dormouse/dormouse.yml. The ordered open list contains {match, tool} entries. Patterns without a slash match the filename; patterns with a slash match both the canonical absolute path and the path relative to the invocation directory. Matching uses picomatch glob syntax with forward slashes and case sensitivity. Dotfiles require explicit patterns. The built-in HTML viewer serves statically referenced relative assets within the document directory tree; root-relative and external resources are unavailable. Text previews are capped at 8 MiB. diff --git a/dor/src/file-viewer-format.ts b/dor/src/file-viewer-format.ts index c6d700a2a..47aba00db 100644 --- a/dor/src/file-viewer-format.ts +++ b/dor/src/file-viewer-format.ts @@ -2,7 +2,7 @@ * Tool association rather than being guessed to be text. */ const MIME: Record = { html: 'text/html; charset=utf-8', htm: 'text/html; charset=utf-8', - pdf: 'application/pdf', svg: 'image/svg+xml', png: 'image/png', jpg: 'image/jpeg', jpeg: 'image/jpeg', + svg: 'image/svg+xml', png: 'image/png', jpg: 'image/jpeg', jpeg: 'image/jpeg', gif: 'image/gif', webp: 'image/webp', avif: 'image/avif', ico: 'image/x-icon', css: 'text/css; charset=utf-8', js: 'text/javascript; charset=utf-8', mjs: 'text/javascript; charset=utf-8', json: 'application/json', woff: 'font/woff', woff2: 'font/woff2', ttf: 'font/ttf', otf: 'font/otf', @@ -20,6 +20,9 @@ export const VIEW_FILE_ARGV = '__view-file'; export function fileViewerFormat(path: string): { mime: string; text: boolean } | null { const name = path.replace(/\\/g, '/').split('/').pop()!.toLowerCase(); const ext = name.includes('.') ? name.split('.').pop()! : ''; + // PDF plugins cannot run inside the viewer's iframe sandbox. Exclude PDFs + // before source-name heuristics so README.pdf never becomes a text preview. + if (ext === 'pdf') return null; const knownMime = Object.prototype.hasOwnProperty.call(MIME, ext) ? MIME[ext] : undefined; const text = TEXT.has(ext) || (!knownMime && /^(readme|license|licence|makefile|dockerfile|\.gitignore|\.env)(\..*)?$/.test(name)); const mime = knownMime ?? (text ? 'text/plain; charset=utf-8' : null); diff --git a/dor/src/file-viewer.ts b/dor/src/file-viewer.ts index 1f6d17b72..e99478169 100644 --- a/dor/src/file-viewer.ts +++ b/dor/src/file-viewer.ts @@ -65,7 +65,6 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa const target = await realpath(input); const format = fileViewerFormat(target); if (!format) throw new Error('unsupported file format; configure a user Tool association'); - const pdf = format.mime === 'application/pdf' ? await (await import('./pdf-viewer-assets.js')).pdfViewerAssets() : null; // A source preview escapes the document; none of its references load. const inspectDependencies = !format.text; const root = dirname(target); @@ -131,21 +130,6 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa try { route = decodeURIComponent(new URL(req.url!, 'http://localhost').pathname.slice(prefix.length)); } catch { finish(res, 400); return; } if (route.includes('\\') || route.split('/').some(part => part === '..' || part === '.')) { finish(res, 403); return; } - if (pdf && (route === 'view' || route.startsWith('pdfjs/'))) { - // Only our PDF renderer compiles local decoder WASM. Document routes - // retain the ordinary viewer policy; iframe sandboxing is unchanged. - // unsafe-inline is required by the proxy's injected navigation/focus shim. - if (route === 'view' || route === 'pdfjs/pdf.worker.mjs') res.setHeader('Content-Security-Policy', "default-src 'none'; script-src 'self' 'unsafe-inline' 'wasm-unsafe-eval'; worker-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; connect-src 'self'; object-src 'none'; base-uri 'self'; form-action 'none'"); - const asset = route === 'view' ? { - bytes: Buffer.from(pdf.html.replace(/__DOR_PDF_(TITLE|DOCUMENT)__/g, (_, part: string) => escapeHtml(part === 'TITLE' - ? basename(target) : `./file/${encodeURIComponent(basename(target))}`))), - mime: 'text/html; charset=utf-8', - } : await pdf.read(route.slice('pdfjs/'.length)); - if (!asset) { finish(res, 404); return; } - res.writeHead(200, { 'Content-Type': asset.mime, 'Content-Length': asset.bytes.length }); - res.end(req.method === 'HEAD' ? undefined : asset.bytes); - return; - } if (route === 'view' && format.text) { const text = await readText(main.file); const body = `${escapeHtml(basename(target))}
${escapeHtml(text)}
`; @@ -191,7 +175,7 @@ export async function startFileViewer(input: string): Promise<{ port: number; pa await new Promise((yes, no) => { server.once('error', no); server.listen(0, '127.0.0.1', yes); }); port = (server.address() as { port: number }).port; let closing: Promise | undefined; - return { port, path: `${prefix}${format.text || pdf ? 'view' : `file/${encodeURIComponent(basename(target))}`}`, + return { port, path: `${prefix}${format.text ? 'view' : `file/${encodeURIComponent(basename(target))}`}`, close: () => closing ??= new Promise((yes, no) => { server.close(error => { void closeFiles().then(() => error ? no(error) : yes(), no); }); server.closeAllConnections(); diff --git a/dor/src/pdf-viewer-assets.ts b/dor/src/pdf-viewer-assets.ts deleted file mode 100644 index 43ba77db5..000000000 --- a/dor/src/pdf-viewer-assets.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { readFile } from 'node:fs/promises'; - -// Resolves beside dist/dor.js in the staged CLI and beside dist/file-viewer.js -// in tests. Source imports from lib tests use the same built artifacts. -const root = new URL('../dist/pdf-viewer/', import.meta.url); -const MIME: Record = { - mjs: 'text/javascript; charset=utf-8', js: 'text/javascript; charset=utf-8', - css: 'text/css; charset=utf-8', svg: 'image/svg+xml', wasm: 'application/wasm', -}; - -/** A build-generated exact inventory, never a directory/file URL supplied by - * the document. These application assets do not enlarge its file grant. */ -export async function pdfViewerAssets() { - let manifest: unknown; - try { manifest = JSON.parse(await readFile(new URL('manifest.json', root), 'utf8')); } - catch { throw new Error('PDF renderer is missing; rebuild or reinstall Dormouse'); } - if (!Array.isArray(manifest) || !manifest.every(name => typeof name === 'string' - && /^[a-zA-Z0-9_.\/-]+$/.test(name) && !name.split('/').some(part => part === '..' || part === '.' || part === ''))) { - throw new Error('Invalid PDF renderer asset inventory'); - } - const allowed = new Set(manifest); - const html = await readFile(new URL('viewer.html', root), 'utf8'); - return { - html, - async read(name: string): Promise<{ bytes: Buffer; mime: string } | null> { - if (!allowed.has(name) || name === 'viewer.html') return null; - return { bytes: await readFile(new URL(name, root)), mime: MIME[name.split('.').pop()!] ?? 'application/octet-stream' }; - }, - }; -} diff --git a/dor/test/file-viewer.test.mjs b/dor/test/file-viewer.test.mjs index 7b86b7efb..660b1ca83 100644 --- a/dor/test/file-viewer.test.mjs +++ b/dor/test/file-viewer.test.mjs @@ -5,11 +5,10 @@ import { join } from 'node:path'; import { request } from 'node:http'; import { spawn } from 'node:child_process'; import { once } from 'node:events'; +import { fileURLToPath } from 'node:url'; import { afterEach, beforeEach, test } from 'node:test'; import { startFileViewer } from '../dist/file-viewer.js'; import { fileViewerFormat } from '../dist/file-viewer-format.js'; -import { stageDorCli } from '../../scripts/stage-dor-cli.mjs'; -import { buildPdfViewer } from '../scripts/build-pdf-viewer.mjs'; let root; const viewers = []; @@ -39,7 +38,7 @@ async function get(viewer, path = viewer.path, headers = {}, method = 'GET') { const asset = (viewer, path) => viewer.path.replace(/\/file\/.*$/, `/file/${path}`); test('known formats override source-name heuristics without treating prototype keys as formats', () => { - for (const [name, mime] of [['README.pdf', 'application/pdf'], ['readme.png', 'image/png'], ['LICENSE.html', 'text/html; charset=utf-8']]) { + for (const [name, mime] of [['readme.png', 'image/png'], ['LICENSE.html', 'text/html; charset=utf-8']]) { assert.deepEqual(fileViewerFormat(name), { mime, text: false }); } for (const name of ['README', 'Dockerfile.dev', 'README.md', '.gitignore']) { @@ -49,6 +48,14 @@ test('known formats override source-name heuristics without treating prototype k assert.equal(fileViewerFormat('file.constructor'), null); }); +test('requires user Tools for PDFs even when their names resemble source files', async () => { + for (const name of ['report.pdf', 'README.pdf', 'LICENSE.PDF']) { + assert.equal(fileViewerFormat(name), null); + await writeFile(join(root, name), '%PDF-1.7 example bytes'); + await assert.rejects(startFileViewer(join(root, name)), /unsupported file format; configure a user Tool association/); + } +}); + test('renders text as escaped content and requires the per-run token on every method', async () => { const viewer = await start('README.md', ' & hello'); const good = await get(viewer); @@ -101,51 +108,18 @@ test('rejects parent-directory references and symlinks escaping the document dir assert.notEqual((await get(viewer, asset(viewer, '../secret.txt'))).status, 200); }); -test('keeps descriptor-backed PDF byte ranges and HEAD behind the rendered preview', async () => { - const opened = await start('README.pdf', '%PDF-1.7 example bytes'); - const viewer = { ...opened, path: opened.path.replace(/view$/, 'file/README.pdf') }; - assert.equal((await get(viewer)).headers['content-type'], 'application/pdf'); +test('supports byte ranges and HEAD for media presentation', async () => { + const viewer = await start('sample.wav', 'RIFF example bytes'); + assert.equal((await get(viewer)).headers['content-type'], 'audio/wav'); const range = await get(viewer, viewer.path, { Range: 'bytes=0-3' }); assert.equal(range.status, 206); - assert.equal(range.body, '%PDF'); + assert.equal(range.body, 'RIFF'); assert.equal((await get(viewer, viewer.path, { Range: 'bytes=-5' })).body, 'bytes'); assert.equal((await get(viewer, viewer.path, { Range: 'bytes=999-1000' })).status, 416); assert.equal((await get(viewer, viewer.path, { Range: 'bytes=0-1,4-6' })).status, 416); assert.equal((await get(viewer, viewer.path, {}, 'HEAD')).body, ''); }); -test('serves an exact capability-gated PDF renderer inventory with narrowly scoped WASM permission', async () => { - const viewer = await start('report & notes.pdf', '%PDF-1.7 example bytes'); - const prefix = viewer.path.slice(0, -'view'.length); - const shell = await get(viewer); - assert.equal(shell.headers['content-type'], 'text/html; charset=utf-8'); - assert.match(shell.body, /report & notes.pdf/); - assert.match(shell.body, /data-document="\.\/file\/report%20%26%20notes.pdf"/); - assert.match(shell.body, /Page number/); - assert.match(shell.headers['content-security-policy'], /worker-src 'self'/); - assert.match(shell.headers['content-security-policy'], /'wasm-unsafe-eval'/); - assert.doesNotMatch(shell.headers['content-security-policy'], /(?:^| )'unsafe-eval'/); - for (const name of ['viewer.mjs', 'controller.mjs', 'pdf.mjs', 'pdf.worker.mjs', 'pdf_viewer.css', 'viewer.css', - 'cmaps/Adobe-Japan1-UCS2.bcmap', 'standard_fonts/LiberationSans-Regular.ttf', 'wasm/openjpeg.wasm', 'LICENSE']) { - const path = `${prefix}pdfjs/${name}`; - const response = await get(viewer, path); - assert.equal(response.status, 200, name); - assert.ok(response.body.length > 0, name); - assert.equal(response.headers['cache-control'], 'no-store'); - assert.equal(response.headers['referrer-policy'], 'no-referrer'); - assert.equal(response.headers['content-security-policy'].includes("'wasm-unsafe-eval'"), name === 'pdf.worker.mjs'); - assert.equal((await get(viewer, path, {}, 'HEAD')).body, ''); - assert.equal((await get(viewer, path, { Origin: 'https://evil.test' })).status, 403); - assert.equal((await get(viewer, path.replace(prefix, '/wrong/'))).status, 403); - } - for (const name of ['manifest.json', 'viewer.html', '../package.json', '%2e%2e/package.json', 'pdf.sandbox.mjs', 'wasm/quickjs-eval.wasm']) { - assert.notEqual((await get(viewer, `${prefix}pdfjs/${name}`)).status, 200, name); - } - assert.equal((await get(viewer, `${prefix}file/private.pdf`)).status, 404); - const text = await start('plain.txt', 'text'); - assert.equal((await get(text, text.path.replace(/view$/, 'pdfjs/pdf.mjs'))).status, 404); -}); - test('fails unsupported formats and oversized text before starting a viewer', async () => { await writeFile(join(root, 'unknown.bin'), 'binary'); await assert.rejects(startFileViewer(join(root, 'unknown.bin')), /unsupported/); @@ -201,17 +175,10 @@ test('bounds the asset graph and keeps a grant on the opened file after path rep await assert.rejects(startFileViewer(html), /256 referenced files/); }); -test('the staged private entry serves PDF assets without node_modules and exits on termination', { timeout: 10_000 }, async () => { - const file = join(root, 'cli.pdf'); - await writeFile(file, '%PDF-1.7 example bytes'); - const staged = join(root, 'staged'); - await stageDorCli(staged); - // The library pretest runs this asset-only prerequisite on a clean checkout. - // Rebuild into an empty staged directory, without relying on dor's artifacts. - const assets = join(staged, 'dist/pdf-viewer'); - await rm(assets, { recursive: true }); - await buildPdfViewer(assets); - const child = spawn(process.execPath, [join(staged, 'dist/dor.js'), '__view-file', file], { stdio: ['ignore', 'pipe', 'pipe'] }); +test('the bundled private entry announces its port and path, then exits on termination', { timeout: 10_000 }, async () => { + const file = join(root, 'cli.txt'); + await writeFile(file, 'cli preview'); + const child = spawn(process.execPath, [fileURLToPath(new URL('../dist/dor.js', import.meta.url)), '__view-file', file], { stdio: ['ignore', 'pipe', 'pipe'] }); try { let output = ''; const announce = await new Promise((resolve, reject) => { @@ -223,8 +190,7 @@ test('the staged private entry serves PDF assets without node_modules and exits if (match) resolve(JSON.parse(match[1])); }); }); - assert.match((await get(announce)).body, /PDF controls/); - assert.equal((await get(announce, announce.path.replace(/view$/, 'pdfjs/pdf.worker.mjs'))).status, 200); + assert.equal((await get(announce)).status, 200); const exited = once(child, 'exit'); child.kill('SIGTERM'); await exited; diff --git a/dor/test/pdf-viewer.test.mjs b/dor/test/pdf-viewer.test.mjs deleted file mode 100644 index 318158d81..000000000 --- a/dor/test/pdf-viewer.test.mjs +++ /dev/null @@ -1,187 +0,0 @@ -import assert from 'node:assert/strict'; -import { test } from 'node:test'; -import { canvasSize, startPdfViewer } from '../viewer/controller.mjs'; - -const tick = () => new Promise(resolve => setImmediate(resolve)); -class Element extends EventTarget { - disabled = false; - hidden = false; - value = ''; - textContent = ''; - clientWidth = 824; - attributes = new Map(); - style = { setProperty() {} }; - dataset = {}; - setAttribute(name, value) { this.attributes.set(name, value); } - getContext() { return {}; } - replaceChildren() {} - focus() { this.focused = true; } - click() { this.dispatchEvent(new Event('click')); } -} -function fixture({ delayTextCancellation = false } = {}) { - const elements = Object.fromEntries(['status', 'page', 'canvas', 'text', 'page-number', 'password-form', 'password', - 'previous', 'next', 'zoom-out', 'fit', 'zoom-in', 'cancel', 'pages', 'page-count'].map(id => [id, new Element()])); - const loadingGate = Promise.withResolvers(); - const renders = []; - const pages = new Map(); - const texts = []; - const options = []; - let destroyed = false; - const loading = { promise: loadingGate.promise, destroy: async () => { destroyed = true; } }; - const pdf = { - numPages: 3, - getPage: async number => { - if (!pages.has(number)) pages.set(number, { - cleanupCount: 0, - cleanup() { ++this.cleanupCount; }, - getViewport: ({ scale }) => ({ width: 600 * scale, height: 800 * scale, userUnit: 1 }), - streamTextContent: () => ({}), - render: args => { - const gate = Promise.withResolvers(); - const task = { number, args, promise: gate.promise, done: gate.resolve, - cancel() { this.cancelled = true; gate.reject(Object.assign(new Error('cancelled'), { name: 'RenderingCancelledException' })); } }; - renders.push(task); - return task; - }, - }); - return pages.get(number); - }, - }; - const pdfjs = { - GlobalWorkerOptions: {}, PasswordResponses: { INCORRECT_PASSWORD: 2 }, - getDocument: value => { options.push(value); return loading; }, - TextLayer: class { - gate = Promise.withResolvers(); - constructor() { texts.push(this); } - render() { return delayTextCancellation ? this.gate.promise : Promise.resolve(); } - cancel() { this.cancelled = true; } - settle() { this.gate.resolve(); } - }, - }; - const document = { getElementById: id => elements[id], body: { dataset: { document: './file/report.pdf' } } }; - const window = Object.assign(new EventTarget(), { location: { href: 'http://127.0.0.1:1234/cap/view' }, devicePixelRatio: 2, setTimeout, clearTimeout }); - const viewer = startPdfViewer(pdfjs, document, window); - return { elements, loading, loadingGate, pdf, pdfjs, options, renders, pages, texts, viewer, window, destroyed: () => destroyed }; -} - -test('bounds enormous page bitmaps at both per-axis and total-pixel limits', () => { - for (const [width, height, dpr] of [[600, 800, 2], [1e6, 1e6, 4], [1e7, 10, 2], [10, 1e7, 2]]) { - const size = canvasSize(width, height, dpr); - assert.ok(size.width <= 8192 && size.height <= 8192); - assert.ok(size.width * size.height <= 16 * 1024 * 1024); - } -}); - -test('loads only capability-relative PDF assets, cancels obsolete rendering and navigates/zooms', async () => { - const f = fixture(); - assert.equal(f.pdfjs.GlobalWorkerOptions.workerSrc, 'http://127.0.0.1:1234/cap/pdfjs/pdf.worker.mjs'); - assert.equal(f.options[0].url, 'http://127.0.0.1:1234/cap/file/report.pdf'); - assert.equal(f.options[0].enableXfa, false); - f.loadingGate.resolve(f.pdf); - await tick(); - assert.equal(f.renders[0].number, 1); - f.elements.next.click(); - await tick(); - assert.equal(f.renders[0].cancelled, true); - assert.equal(f.renders[1].number, 2); - f.renders[1].done(); - await tick(); - assert.equal(f.elements.status.textContent, 'Page 2 of 3'); - const originalWidth = f.elements.canvas.width; - f.elements['zoom-in'].click(); - await tick(); - assert.ok(f.elements.canvas.width > originalWidth); - f.renders.at(-1).done(); - await tick(); - f.elements.fit.click(); - await tick(); - assert.equal(f.elements.canvas.width, originalWidth); - f.renders.at(-1).done(); - await tick(); - f.elements['page-number'].value = '999'; - f.elements['page-number'].dispatchEvent(new Event('change')); - await tick(); - assert.equal(f.renders.at(-1).number, 2); - f.viewer.cancel(); - await f.viewer.ready; - assert.equal(f.destroyed(), true); -}); - -test('keeps loading failures visible and accepts password retries without keeping the entered password', async () => { - const f = fixture(); - let supplied; - f.loading.onPassword(password => { supplied = password; }, 2); - assert.equal(f.elements['password-form'].hidden, false); - assert.equal(f.elements.status.textContent, 'Incorrect password. Try again.'); - f.elements.password.value = 'secret'; - f.elements['password-form'].dispatchEvent(new Event('submit', { cancelable: true })); - assert.equal(supplied, 'secret'); - assert.equal(f.elements.password.value, ''); - f.loadingGate.reject(new Error('Invalid PDF structure')); - await f.viewer.ready; - assert.equal(f.elements.status.attributes.get('role'), 'alert'); - assert.match(f.elements.status.textContent, /Invalid PDF structure/); - f.viewer.cancel(); -}); - -test('cancellation prevents a late document load from drawing or replacing its status', async () => { - const f = fixture(); - f.elements.cancel.click(); - f.loadingGate.resolve(f.pdf); - await f.viewer.ready; - assert.equal(f.renders.length, 0); - assert.equal(f.elements.next.disabled, true); - assert.match(f.elements.status.textContent, /cancelled/); - assert.equal(f.destroyed(), true); -}); - -test('releases a previous page only after cancelled canvas and text work settle', async () => { - const f = fixture({ delayTextCancellation: true }); - f.loadingGate.resolve(f.pdf); - await tick(); - f.elements.next.click(); - await tick(); - assert.equal(f.renders[0].cancelled, true); - assert.equal(f.texts[0].cancelled, true); - assert.equal(f.pages.get(1).cleanupCount, 0); - assert.equal(f.renders.length, 1); - f.texts[0].settle(); - await tick(); - assert.equal(f.pages.get(1).cleanupCount, 1); - assert.equal(f.renders[1].number, 2); - f.renders[1].done(); - f.texts[1].settle(); - await tick(); - f.elements.next.click(); - await tick(); - assert.equal(f.pages.get(2).cleanupCount, 1); - assert.equal(f.renders[2].number, 3); - f.renders[2].done(); - f.texts[2].settle(); - await tick(); - f.viewer.cancel(); - await tick(); - assert.equal(f.pages.get(3).cleanupCount, 1); -}); - -test('cleans a stale getPage result before a newer request can reuse that cached page', async () => { - const f = fixture(); - const page = await f.pdf.getPage(1); - const gate = Promise.withResolvers(); - let calls = 0; - f.pdf.getPage = async () => ++calls === 1 ? gate.promise : page; - f.loadingGate.resolve(f.pdf); - await tick(); - f.elements.fit.click(); - gate.resolve(page); - await tick(); - assert.equal(page.cleanupCount, 1); - assert.equal(f.renders.length, 1); - f.renders[0].done(); - await tick(); - assert.equal(page.cleanupCount, 1); - assert.equal(f.elements.status.textContent, 'Page 1 of 3'); - f.viewer.cancel(); - await tick(); - assert.equal(page.cleanupCount, 2); -}); diff --git a/dor/test/snapshots/help/open.md b/dor/test/snapshots/help/open.md index df5292172..470c99040 100644 --- a/dor/test/snapshots/help/open.md +++ b/dor/test/snapshots/help/open.md @@ -9,7 +9,7 @@ USAGE Opens one existing local file. Relative paths resolve from the caller's directory (or --cwd); symlink aliases resolve to the same file. URLs, directories, and Surface handles are not accepted. -The first matching rule in the user dormouse.yml selects a user Tool or builtin:file. --tool chooses a handler explicitly. Without a matching rule, the built-in viewer opens supported HTML, text/source, image, PDF, and media files. Use --tool builtin:file to select it explicitly. Markdown is shown as source text; a user Tool can provide rendered Markdown. Project associations and project Tools never participate in this lookup. The user file is $XDG_CONFIG_HOME/dormouse/dormouse.yml, or ~/.config/dormouse/dormouse.yml. +The first matching rule in the user dormouse.yml selects a user Tool or builtin:file. --tool chooses a handler explicitly. Without a matching rule, the built-in viewer opens supported HTML, text/source, image, and media files. PDFs require a user Tool association or --tool . Use --tool builtin:file to select it explicitly. Markdown is shown as source text; a user Tool can provide rendered Markdown. Project associations and project Tools never participate in this lookup. The user file is $XDG_CONFIG_HOME/dormouse/dormouse.yml, or ~/.config/dormouse/dormouse.yml. The ordered open list contains {match, tool} entries. Patterns without a slash match the filename; patterns with a slash match both the canonical absolute path and the path relative to the invocation directory. Matching uses picomatch glob syntax with forward slashes and case sensitivity. Dotfiles require explicit patterns. The built-in HTML viewer serves statically referenced relative assets within the document directory tree; root-relative and external resources are unavailable. Text previews are capped at 8 MiB. diff --git a/dor/viewer/controller.mjs b/dor/viewer/controller.mjs deleted file mode 100644 index 006611bbd..000000000 --- a/dor/viewer/controller.mjs +++ /dev/null @@ -1,190 +0,0 @@ -const MAX_PIXELS = 16 * 1024 * 1024; -const MAX_DIMENSION = 8192; - -/** Bound both canvas dimensions and backing pixels, including unusual page - * sizes and high-DPI displays. CSS zoom does not allocate a larger bitmap. */ -export function canvasSize(width, height, deviceScale) { - const ratio = Math.min(Math.max(1, deviceScale || 1), 2, - MAX_DIMENSION / width, MAX_DIMENSION / height, Math.sqrt(MAX_PIXELS / (width * height))); - return { width: Math.max(1, Math.floor(width * ratio)), height: Math.max(1, Math.floor(height * ratio)), ratio }; -} - -export function startPdfViewer(pdfjs, document = globalThis.document, window = globalThis.window) { - const element = id => document.getElementById(id); - const status = element('status'); - const pageBox = element('page'); - const canvas = element('canvas'); - const text = element('text'); - const pageInput = element('page-number'); - const passwordForm = element('password-form'); - const controls = ['previous', 'next', 'zoom-out', 'fit', 'zoom-in', 'page-number']; - let pdf; - let pageNumber = 1; - let zoom = 1; - let generation = 0; - let active; - let renderQueue = Promise.resolve(); - let heldPage; - let heldPageNumber; - let stopped = false; - let resizeTimer; - let updatePassword; - - const message = (value, error = false) => { - status.textContent = value; - status.setAttribute('role', error ? 'alert' : 'status'); - }; - const updateControls = () => { - for (const id of controls) element(id).disabled = !pdf || stopped; - element('previous').disabled ||= pageNumber <= 1; - element('next').disabled ||= pageNumber >= (pdf?.numPages ?? 1); - element('zoom-out').disabled ||= zoom <= .25; - element('zoom-in').disabled ||= zoom >= 4; - pageInput.value = String(pageNumber); - }; - const isCurrent = request => !stopped && generation === request; - const releasePage = () => { - heldPage?.cleanup(); - heldPage = heldPageNumber = undefined; - }; - function render() { - if (!pdf || stopped) return Promise.resolve(); - const request = ++generation; - const number = pageNumber; - const requestedZoom = zoom; - active?.canvas?.cancel(); - active?.text?.cancel(); - updateControls(); - message(`Loading page ${number}…`); - pageBox.setAttribute('aria-busy', 'true'); - // Serial jobs prevent a stale getPage result from cleaning a PDFPageProxy - // already reused by a newer request. Obsolete queued jobs do no work. - renderQueue = renderQueue.catch(() => {}).then(async () => { - if (!isCurrent(request)) return; - const job = {}; - const tasks = []; - active = job; - let page; - try { - if (heldPageNumber !== number) releasePage(); - page = heldPage ?? await pdf.getPage(number); - if (!isCurrent(request)) return; - heldPage = page; - heldPageNumber = number; - const natural = page.getViewport({ scale: 1 }); - const fit = Math.max(1, element('pages').clientWidth - 24) / natural.width; - const scale = Math.min(fit * requestedZoom, MAX_DIMENSION / Math.max(natural.width, natural.height)); - const viewport = page.getViewport({ scale }); - const size = canvasSize(viewport.width, viewport.height, window.devicePixelRatio); - canvas.width = size.width; - canvas.height = size.height; - canvas.style.width = `${viewport.width}px`; - canvas.style.height = `${viewport.height}px`; - pageBox.style.width = `${viewport.width}px`; - pageBox.style.height = `${viewport.height}px`; - pageBox.style.setProperty('--scale-factor', String(scale)); - pageBox.style.setProperty('--user-unit', String(viewport.userUnit ?? 1)); - pageBox.hidden = false; - text.replaceChildren(); - text.setAttribute('aria-label', `Page ${number} text`); - const context = canvas.getContext('2d'); - if (!context) throw new Error('Canvas is unavailable in this browser.'); - job.canvas = page.render({ canvasContext: context, viewport, transform: [size.ratio, 0, 0, size.ratio, 0, 0] }); - tasks.push(job.canvas.promise); - job.text = new pdfjs.TextLayer({ textContentSource: page.streamTextContent(), container: text, viewport }); - tasks.push(job.text.render()); - await Promise.all(tasks); - if (!isCurrent(request)) return; - pageBox.setAttribute('aria-busy', 'false'); - message(`Page ${number} of ${pdf.numPages}`); - } catch (error) { - if (!isCurrent(request) || error?.name === 'RenderingCancelledException') return; - pageBox.hidden = true; - message(`Unable to display this page: ${error?.message || 'PDF rendering failed.'}`, true); - } finally { - // Cancelling a task is asynchronous. Free operator lists/images only - // after both painting and text extraction have stopped using the page. - await Promise.allSettled(tasks); - if (active === job) active = undefined; - if (page && page !== heldPage) page.cleanup(); - if (!isCurrent(request)) releasePage(); - } - }); - return renderQueue; - } - - const asset = path => new URL(`./pdfjs/${path}`, window.location.href).href; - pdfjs.GlobalWorkerOptions.workerSrc = asset('pdf.worker.mjs'); - const loading = pdfjs.getDocument({ - url: new URL(document.body.dataset.document, window.location.href).href, - cMapUrl: asset('cmaps/'), cMapPacked: true, - standardFontDataUrl: asset('standard_fonts/'), - wasmUrl: asset('wasm/'), iccUrl: asset('iccs/'), - // No annotation actions, embedded document scripting, or XFA forms. - enableXfa: false, - }); - loading.onPassword = (update, reason) => { - if (stopped) return; - updatePassword = update; - passwordForm.hidden = false; - message(reason === pdfjs.PasswordResponses.INCORRECT_PASSWORD ? 'Incorrect password. Try again.' : 'This PDF needs a password.'); - element('password').focus(); - }; - passwordForm.addEventListener('submit', event => { - event.preventDefault(); - if (!updatePassword || stopped) return; - const password = element('password'); - const value = password.value; - password.value = ''; - passwordForm.hidden = true; - const update = updatePassword; - updatePassword = undefined; - message('Loading PDF…'); - update(value); - }); - function cancel() { - if (stopped) return; - stopped = true; - ++generation; - window.clearTimeout(resizeTimer); - active?.canvas?.cancel(); - active?.text?.cancel(); - void renderQueue.finally(releasePage); - void loading.destroy().catch(() => {}); - pageBox.hidden = true; - passwordForm.hidden = true; - element('password').value = ''; - canvas.width = canvas.height = 1; - text.replaceChildren(); - updateControls(); - element('cancel').disabled = true; - message('PDF preview cancelled. Reload to open it again.'); - } - element('cancel').addEventListener('click', cancel); - element('previous').addEventListener('click', () => { if (pageNumber > 1) { --pageNumber; void render(); } }); - element('next').addEventListener('click', () => { if (pdf && pageNumber < pdf.numPages) { ++pageNumber; void render(); } }); - pageInput.addEventListener('change', () => { - const requested = Number(pageInput.value); - if (pdf && Number.isInteger(requested) && requested >= 1 && requested <= pdf.numPages) pageNumber = requested; - void render(); - }); - element('zoom-out').addEventListener('click', () => { zoom = Math.max(.25, zoom / 1.25); void render(); }); - element('zoom-in').addEventListener('click', () => { zoom = Math.min(4, zoom * 1.25); void render(); }); - element('fit').addEventListener('click', () => { zoom = 1; void render(); }); - window.addEventListener('resize', () => { - window.clearTimeout(resizeTimer); - resizeTimer = window.setTimeout(() => { void render(); }, 100); - }); - window.addEventListener('pagehide', cancel, { once: true }); - updateControls(); - const ready = loading.promise.then(async result => { - if (stopped) return; - pdf = result; - pageInput.max = String(pdf.numPages); - element('page-count').textContent = `of ${pdf.numPages}`; - await render(); - }).catch(error => { - if (!stopped) message(`Unable to open this PDF: ${error?.message || 'Invalid PDF file.'}`, true); - }); - return { ready, cancel }; -} diff --git a/dor/viewer/viewer.css b/dor/viewer/viewer.css deleted file mode 100644 index 9bddab672..000000000 --- a/dor/viewer/viewer.css +++ /dev/null @@ -1,14 +0,0 @@ -:root { color-scheme: light dark; font: 14px/1.4 system-ui, sans-serif; } -body { margin: 0; color: CanvasText; background: Canvas; } -header { display: flex; align-items: center; flex-wrap: wrap; gap: .4rem; padding: .6rem; border-bottom: 1px solid GrayText; } -button, input { font: inherit; color: ButtonText; background: ButtonFace; border: 1px solid GrayText; border-radius: .25rem; padding: .25rem .45rem; } -button { cursor: pointer; } -button:disabled { opacity: .5; cursor: default; } -input[type=number] { width: 4rem; } -#status, #password-form { margin: .5rem .75rem; } -#status[role=alert] { font-weight: 600; } -#pages { overflow: auto; padding: 12px; } -#page { position: relative; margin: 0 auto; background: white; --user-unit: 1; --total-scale-factor: calc(var(--scale-factor) * var(--user-unit)); --scale-round-x: 1px; --scale-round-y: 1px; } -#canvas { display: block; } -#text { position: absolute; inset: 0; } -[hidden] { display: none !important; } diff --git a/dor/viewer/viewer.html b/dor/viewer/viewer.html deleted file mode 100644 index f1fd2e9b6..000000000 --- a/dor/viewer/viewer.html +++ /dev/null @@ -1,31 +0,0 @@ - - - - - -__DOR_PDF_TITLE__ - - - - -
- - -of — - - - - - -
-

Loading PDF…

- -
- -
- - - diff --git a/dor/viewer/viewer.mjs b/dor/viewer/viewer.mjs deleted file mode 100644 index 845d860b0..000000000 --- a/dor/viewer/viewer.mjs +++ /dev/null @@ -1,9 +0,0 @@ -import { startPdfViewer } from './controller.mjs'; - -try { - startPdfViewer(await import('./pdf.mjs')); -} catch (error) { - const status = document.getElementById('status'); - status.setAttribute('role', 'alert'); - status.textContent = `Unable to start the PDF viewer: ${error.message || 'Renderer unavailable.'}`; -} diff --git a/lib/package.json b/lib/package.json index 905aa90af..59f1c2b88 100644 --- a/lib/package.json +++ b/lib/package.json @@ -11,7 +11,7 @@ "build": "tsc -b && vite build", "build:pocket": "vite build --config vite.pocket.config.ts && vite build --config vite.sw.config.ts && node scripts/assert-pocket-worker.mjs", "preview": "vite preview", - "pretest": "pnpm --filter dor-lib-common build && pnpm --filter remote-lib-common build && pnpm --filter dor build:pdf-viewer", + "pretest": "pnpm --filter dor-lib-common build && pnpm --filter remote-lib-common build", "test": "pnpm typecheck && vitest run", "test:watch": "vitest", "storybook": "storybook dev -p 6006 --no-open --ci", @@ -51,7 +51,6 @@ "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^6.0.2", "chromatic": "^17.0.0", - "dor": "workspace:*", "fake-indexeddb": "^6.2.5", "remark-gfm": "^4.0.1", "storybook": "^10.4.0", diff --git a/lib/src/host/file-viewer-proxy.test.ts b/lib/src/host/file-viewer-proxy.test.ts index 2ef90a15f..c8c7368cf 100644 --- a/lib/src/host/file-viewer-proxy.test.ts +++ b/lib/src/host/file-viewer-proxy.test.ts @@ -76,6 +76,7 @@ it('retains the policy for an escaped text preview', async () => { it.each([ ['image.svg', '', 'image/svg+xml'], + ['sound.wav', 'RIFF example bytes', 'audio/wav'], ['readme.png', 'image bytes', 'image/png'], ['video.mp4', 'video bytes', 'video/mp4'], ])('retains CSP, bytes, HEAD, and ranges for %s through the proxy', async (name, bytes, mime) => { @@ -95,24 +96,3 @@ it.each([ expect(range.headers['content-range']).toBe(`bytes 0-3/${Buffer.byteLength(bytes)}`); expect(range.body).toBe(bytes.slice(0, 4)); }); - -it('retains the PDF shell, module worker policy and descriptor ranges through the real proxy', async () => { - const url = await frame('document.pdf', '%PDF-1.7 example bytes'); - const shell = await read(url); - expectViewerPolicy(shell.headers); - expect(shell.headers['content-type']).toBe('text/html; charset=utf-8'); - expect(shell.body).toContain('data-document="./file/document.pdf"'); - expect(shell.body).toContain('__dormouse'); - expect(shell.headers['content-security-policy']).toContain("worker-src 'self'"); - expect(shell.headers['content-security-policy']).toContain("'wasm-unsafe-eval'"); - const worker = await read(new URL('./pdfjs/pdf.worker.mjs', url).href); - expect(worker.status).toBe(200); - expect(worker.headers['content-type']).toBe('text/javascript; charset=utf-8'); - expectViewerPolicy(worker.headers); - expect(worker.headers['content-security-policy']).toContain("'wasm-unsafe-eval'"); - const file = await read(new URL('./file/document.pdf', url).href, 'GET', { Range: 'bytes=0-3' }); - expect(file.status).toBe(206); - expect(file.body).toBe('%PDF'); - expect(file.headers['content-security-policy']).not.toContain("'wasm-unsafe-eval'"); - expect((await read(new URL('./pdfjs/manifest.json', url).href)).status).toBe(404); -}); diff --git a/lib/src/host/tool-open.test.ts b/lib/src/host/tool-open.test.ts index ac5622dc4..39a237df9 100644 --- a/lib/src/host/tool-open.test.ts +++ b/lib/src/host/tool-open.test.ts @@ -95,3 +95,16 @@ it('matches catch-all rules above the invocation directory and canonical absolut await writeConfig(viewerConfig(join(root, 'docs').replace(/\\/g, '/') + '/**')); expect(await host().handle(request)).toMatchObject({ status: 'ok', name: 'viewer' }); }); + +it('requires a user Tool for PDFs and passes the canonical file to configured handlers', async () => { + const target = join(root, 'README.pdf'); + await writeFile(target, '%PDF-1.7'); + await rm(config); + expect(await host().handle({ op: 'open', target, cwd: root })).toMatchObject({ status: 'error', message: expect.stringContaining('add an open rule') }); + expect(await host().handle({ op: 'open', target, cwd: root, tool: 'builtin:file' })).toMatchObject({ status: 'error', message: expect.stringContaining('does not support') }); + await writeConfig('open:\n - {match: "*.pdf", tool: "builtin:file"}\n'); + expect(await host().handle({ op: 'open', target, cwd: root })).toMatchObject({ status: 'error' }); + await writeConfig(viewerConfig('*.pdf')); + expect(await host().handle({ op: 'open', target, cwd: root })).toMatchObject({ status: 'ok', scope: 'user', run: ['view', target] }); + expect(await host().handle({ op: 'open', target, cwd: root, tool: 'viewer' })).toMatchObject({ status: 'ok', scope: 'user', run: ['view', target] }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 843defb3e..c01930ad0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4,9 +4,6 @@ settings: autoInstallPeers: true excludeLinksFromLockfile: false -overrides: - pdfjs-dist>@napi-rs/canvas: '-' - importers: .: @@ -66,9 +63,6 @@ importers: dor-lib-common: specifier: workspace:* version: link:../dor-lib-common - pdfjs-dist: - specifier: 6.3.289 - version: 6.3.289 devDependencies: '@types/node': specifier: ^24.13.4 @@ -186,9 +180,6 @@ importers: chromatic: specifier: ^17.0.0 version: 17.8.0 - dor: - specifier: workspace:* - version: link:../dor fake-indexeddb: specifier: ^6.2.5 version: 6.2.5 @@ -4011,10 +4002,6 @@ packages: resolution: {integrity: sha512-//nshmD55c46FuFw26xV/xFAaB5HF9Xdap7HJBBnrKdAd6/GxDBaNA1870O79+9ueg61cZLSVc+OaFlfmObYVQ==} engines: {node: '>= 14.16'} - pdfjs-dist@6.3.289: - resolution: {integrity: sha512-ZHjSVpDa3D6izMq8/04lvkhkATUmL9px6ChPaXc1k6nU2Mrhlg1/7F0bdUqCwUjw3NsPTfPZsMDUU6ZIcRaeQw==} - engines: {node: '>=22.13.0 || >=24'} - pend@1.2.0: resolution: {integrity: sha512-F3asv42UuXchdzt+xXqfW1OGlVBe+mxa2mqI0pg5yAHZPvFmY3Y6drSf/GQ1A86WgWEN9Kzh/WrgKa6iGcHXLg==} @@ -8146,8 +8133,6 @@ snapshots: pathval@2.0.1: {} - pdfjs-dist@6.3.289: {} - pend@1.2.0: {} picocolors@1.1.1: {} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index ecc588a40..885ca5fe8 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -33,7 +33,3 @@ peerDependencyRules: # sockets that would break first. "@hono/node-ws>@hono/node-server": ^2.0.0 minimumReleaseAge: 1440 - -# PDF.js runs only as staged browser assets; its optional Node canvas backend is unused. -overrides: - "pdfjs-dist>@napi-rs/canvas": "-" diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 842c0f242..10011195b 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -7,7 +7,7 @@ "docs/specs/deploy.md": 1900, "docs/specs/dor-browser.md": 4700, "docs/specs/dor-cli.md": 6000, - "docs/specs/dor-tool.md": 3800, + "docs/specs/dor-tool.md": 3750, "docs/specs/glossary.md": 3000, "docs/specs/layout.md": 8900, "docs/specs/mobile-terminal-ui.md": 1950, @@ -19,7 +19,7 @@ "docs/specs/remote-security-model.md": 4800, "docs/specs/security-audit.md": 1750, "docs/specs/security-ci.md": 2500, - "docs/specs/security-local.md": 3150, + "docs/specs/security-local.md": 3100, "docs/specs/security-remote.md": 5850, "docs/specs/security-supply-chain.md": 1200, "docs/specs/security.md": 1900, diff --git a/website/scripts/generate-deps.js b/website/scripts/generate-deps.js index 86b60b106..557c282da 100644 --- a/website/scripts/generate-deps.js +++ b/website/scripts/generate-deps.js @@ -306,7 +306,6 @@ const missingLicense = { "Solarized & Selenized": "MIT", }; const missingAuthor = { - "pdfjs-dist": "Mozilla Foundation and PDF.js contributors", "@hono/node-ws": "Hono middleware contributors", // The addon ships a `contributors` array rather than npm's singular `author` // field, and its prebuilt platform packages carry neither. diff --git a/website/src/data/dependencies-npm.json b/website/src/data/dependencies-npm.json index 4cf4c5358..24aaa5b46 100644 --- a/website/src/data/dependencies-npm.json +++ b/website/src/data/dependencies-npm.json @@ -335,13 +335,6 @@ "author": "Sindre Sorhus", "homepage": "https://github.com/sindresorhus/path-key" }, - { - "name": "pdfjs-dist", - "version": "6.3.289", - "license": "Apache-2.0", - "author": "Mozilla Foundation and PDF.js contributors", - "homepage": "https://mozilla.github.io/pdf.js/" - }, { "name": "picomatch", "version": "4.0.7", From 3ca4e34f1d032df6f1224eafc75777ea6fb06110 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Sep 2026 16:09:31 -0700 Subject: [PATCH 03/11] Identify superseded QC baseline behavior --- docs/testing/dor-tool-qc.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/testing/dor-tool-qc.md b/docs/testing/dor-tool-qc.md index 9e46e13bc..7ca2e15e7 100644 --- a/docs/testing/dor-tool-qc.md +++ b/docs/testing/dor-tool-qc.md @@ -12,7 +12,12 @@ ignored `standalone/src-tauri/target/dor-tool-qc/` directory. Never use the installed application's configuration or trust records. Capture credentials only to a mode-0600 local file; do not include them in reports. -## Test plan +## Original QC baseline results + +The flag and placement rows below record the original `4c7f9012` baseline. +They are superseded by the integrated stack: Tools are always available, and +a standalone `dor open` can take over an eligible caller like `dor tool`. +Current behavior is specified in `docs/specs/dor-tool.md`. | Area | Exercise | Expected result | Result | | --- | --- | --- | --- | From 9290e9fcbeaa3462cd947f5cf66974a72bd58b3c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Sep 2026 16:11:08 -0700 Subject: [PATCH 04/11] Dismiss browser controls when their Surface becomes hidden --- docs/specs/layout.md | 2 +- .../wall/SurfacePaneHeader.test.tsx | 30 ++++++++++++++++++- lib/src/components/wall/SurfacePaneHeader.tsx | 17 +++++++---- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/docs/specs/layout.md b/docs/specs/layout.md index 2240874dd..c9ee26958 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -50,7 +50,7 @@ A 30px header doubling as a drag handle: **a `pointerdown` past a 5px threshold **Must use browser chrome for a serving Tool, with a Terminal Context disclosure for its serving terminal.** Tool composition belongs to `docs/specs/dor-tool.md` → Lifecycle. -**Must size browser chrome by available pane width, excluding Tool context.** Hide inline split/zoom below 420px and navigation below 360px. Below 180px, move browser controls into a keyboard-accessible, viewport-clamped popover; minimize/kill remain inline until 72px, then join the popover. A filled notepad glyph and note count identify saved notes on its trigger. Long keys and connection labels yield before controls. (rationale) +**Must size browser chrome by available pane width, excluding Tool context.** Hide inline split/zoom below 420px and navigation below 360px. Below 180px, use a keyboard-accessible, viewport-clamped popover; minimize/kill remain inline until 72px, then join the popover. Dismiss without restoring focus when hidden. Its trigger identifies notes by filled notepad glyph and count. Long keys and connection labels yield before controls. (rationale) Source of truth: `SurfacePaneHeader` in `lib/src/components/wall/SurfacePaneHeader.tsx`; tests: `lib/src/components/wall/SurfacePaneHeader.test.tsx`; stories: `lib/src/stories/BrowserChromeHeader.stories.tsx`. diff --git a/lib/src/components/wall/SurfacePaneHeader.test.tsx b/lib/src/components/wall/SurfacePaneHeader.test.tsx index 845e6bb3e..a102f34e9 100644 --- a/lib/src/components/wall/SurfacePaneHeader.test.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.test.tsx @@ -18,6 +18,7 @@ import { import { setDevServerResolution } from './agent-browser-ports'; import { ModeContext, + WorkspaceActiveContext, SelectedIdContext, WallActionsContext, WindowFocusedContext, @@ -70,11 +71,12 @@ afterEach(() => { function renderHeader( props: PaneProps, actions: WallActions, - state: { active?: boolean; zoomedId?: string | null; tool?: boolean } = {}, + state: { active?: boolean; zoomedId?: string | null; tool?: boolean; workspaceActive?: boolean } = {}, ) { act(() => { root.render( + @@ -86,12 +88,38 @@ function renderHeader( + , ); }); } describe('SurfacePaneHeader — browser chrome', () => { + it.each(['workspace', 'parked'] as const)('dismisses compact controls without stealing focus when hidden by %s', hiddenBy => { + const id = 'pane-hidden-controls'; + const registration = register(id); + const props = headerProps(id, 'Browser'); + const actions = stubActions(); + const otherWorkspaceControl = document.createElement('button'); + document.body.appendChild(otherWorkspaceControl); + try { + renderHeader(props, actions); + act(() => resizeHeader(79)); + act(() => container.querySelector('[aria-label="Browser controls"]')!.click()); + expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).not.toBeNull(); + otherWorkspaceControl.focus(); + renderHeader({ ...props, parked: hiddenBy === 'parked' }, actions, { workspaceActive: hiddenBy !== 'workspace' }); + expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); + expect(document.activeElement).toBe(otherWorkspaceControl); + renderHeader(props, actions); + expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); + expect(container.querySelector('[aria-label="Browser controls"]')?.getAttribute('aria-expanded')).toBe('false'); + } finally { + otherWorkspaceControl.remove(); + registration.dispose(); + } + }); + it('adapts to pane resizes in a wide window and keeps compact controls keyboard reachable', async () => { const registration = register('pane-resize', { ...CHROME, key: 'a'.repeat(300) }); const actions = stubActions(); diff --git a/lib/src/components/wall/SurfacePaneHeader.tsx b/lib/src/components/wall/SurfacePaneHeader.tsx index 0597e27ce..772a87eff 100644 --- a/lib/src/components/wall/SurfacePaneHeader.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.tsx @@ -1,6 +1,7 @@ import { useCallback, useContext, useEffect, useLayoutEffect, useRef, useState, type CSSProperties, type ReactNode } from 'react'; import { createPortal } from 'react-dom'; import { usePopoverFocusTrap } from '../use-popover-focus-trap'; +import { useSurfaceVisibility } from './use-surface-visibility'; import { useNoteCount } from '../use-notepad'; import { clampOverlayPosition } from '../../lib/ui-geometry'; import { @@ -39,7 +40,10 @@ import { useDialogKeyboardOwner, } from './wall-context'; -export function SurfacePaneHeader({ id, title }: PaneProps) { +export function SurfacePaneHeader({ id, title, parked }: PaneProps) { + const visible = useSurfaceVisibility(parked); + const visibleRef = useRef(visible); + visibleRef.current = visible; const mode = useContext(ModeContext); const selectedId = useContext(SelectedIdContext); const windowFocused = useContext(WindowFocusedContext); @@ -74,7 +78,7 @@ export function SurfacePaneHeader({ id, title }: PaneProps) { // keyboard handler stands down (the panel's own key-forwarder skips editable // targets); the editor closes itself when the surface stops being a browser. const [editingUrl, setEditingUrl] = useState(false); - useDialogKeyboardOwner(editingUrl); + useDialogKeyboardOwner(editingUrl && visible); useEffect(() => { if (!screen && editingUrl) setEditingUrl(false); }, [screen, editingUrl]); @@ -93,7 +97,10 @@ export function SurfacePaneHeader({ id, title }: PaneProps) { const [menuAnchor, setMenuAnchor] = useState(null); const noteCount = useNoteCount(id); const overflowLabel = `Browser controls${noteCount ? `, ${noteCount} ${noteCount === 1 ? 'note' : 'notes'}` : ''}`; - const closeMenu = useCallback((restoreFocus = true) => { setMenuAnchor(null); setEditingUrl(false); if (restoreFocus) overflowRef.current?.focus(); }, []); + const closeMenu = useCallback((restoreFocus = true) => { setMenuAnchor(null); setEditingUrl(false); if (restoreFocus && visibleRef.current) overflowRef.current?.focus(); }, []); + useEffect(() => { + if (!visible) closeMenu(false); + }, [visible, closeMenu]); useLayoutEffect(() => { const header = headerRef.current; if (!header) return; @@ -264,7 +271,7 @@ export function SurfacePaneHeader({ id, title }: PaneProps) { > {compact ? ( ) : browserControls} {width >= 72 && paneActions} - {compact && menuAnchor && + {visible && compact && menuAnchor && {browserControls} {width < 72 && paneActions} } From db2f5ee352289fe1cd062505787cc5a52e039865 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 16 Sep 2026 16:33:45 -0700 Subject: [PATCH 05/11] Simplify the narrow browser header and its QC follow-ups The browser header now keeps a quantized tier in state instead of a raw width, through a shared `useHeaderTier` hook, so a sash drag or tween no longer re-renders the whole header every frame. The overflow popover follows the pane-header dismissal contract (`useDismissOverlay`): a press in another pane's header now dismisses it, a press on its trigger toggles it, and the trigger anchors it by ref instead of a cached rect. It reuses the popover height token, `chromeButton`, `POPOVER_FOCUSABLE_SELECTOR`, and a shared note-count phrase. Tests split the one long popover case into one per rule, share a driveable ResizeObserver stub with the terminal header suite, and drop assertions the pure format function already pins. The width rule moves into layout.md's responsive-sizing section as a table beside the terminal tiers; the rationale lead no longer states the superseded approach. The QC fixture is allowlisted in the loopback lint, which it had been failing, and named from the QC doc that runs it. Co-Authored-By: Claude Fable 5.1 --- docs/specs/dor-browser.md | 2 +- docs/specs/dor-tool.md | 2 +- docs/specs/layout.md | 20 +- docs/specs/layout.rationale.md | 4 +- docs/testing/dor-tool-qc.md | 4 +- dor/test/file-viewer.test.mjs | 11 +- lib/src/components/use-notepad.ts | 7 +- .../wall/SurfacePaneHeader.test.tsx | 190 ++++++++++-------- lib/src/components/wall/SurfacePaneHeader.tsx | 124 +++++++----- .../wall/TerminalPaneHeader.test.tsx | 28 +-- lib/src/components/wall/use-header-tier.ts | 33 +++ lib/src/components/wall/wall-test-utils.ts | 33 +++ lib/src/host/tool-open.test.ts | 5 +- .../stories/BrowserChromeHeader.stories.tsx | 16 +- scripts/loopback-lint.mjs | 5 + scripts/spec-word-budgets.json | 2 +- 16 files changed, 298 insertions(+), 188 deletions(-) create mode 100644 lib/src/components/wall/use-header-tier.ts diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index fcbcafd71..8f04a96ff 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -116,7 +116,7 @@ Header contract: - **Must keep back/forward/reload enabled.** Agent-browser uses native commands; iframe uses parent history and re-resolves its proxy. - **Must show non-default managed `--key` as a badge, never a title prefix.** -Header sizing and narrow-pane control placement follow `docs/specs/layout.md` → Pane header. +- Width tiers and the narrow-pane popover: `docs/specs/layout.md` → "Pane header responsive sizing". Source of truth: `lib/src/components/wall/SurfacePaneHeader.tsx`, `lib/src/components/wall/agent-browser-screen.ts`, diff --git a/docs/specs/dor-tool.md b/docs/specs/dor-tool.md index 8171debd1..4c8157622 100644 --- a/docs/specs/dor-tool.md +++ b/docs/specs/dor-tool.md @@ -163,7 +163,7 @@ Source of truth: `toolCommand` in `dor/src/commands/tool.ts`; `dor/test/snapshot **Must run the built-in viewer as a Tool-owned `dor` process**, serving HTML, images, media, and escaped text/source previews. Markdown is source text; custom viewers may render it. Text previews and HTML/CSS dependency inspection are limited to 8 MiB per file. Text/source previews grant only their opened file and skip dependency inspection. (rationale) Oversized HTML and referenced CSS still stream without dependency inspection. The grant contains at most 256 files: the opened document and statically referenced relative HTML/CSS assets within its directory tree; exceeding that bound fails the open without serving a partial grant. Never expand the grant through root-relative, external, or dynamic references; requests can read only granted paths. -**Must require a user Tool for PDFs**, including files named `README.pdf`; never pass them to the built-in viewer. (rationale) +**Must require a user Tool for PDFs**, including files named `README.pdf`. (rationale) **Must retain the viewer's opened file descriptors until the Tool exits.** Refresh reads those files again, but atomic replacements and changes to the dependency graph require restarting the viewer. Cold restore runs the saved file command with a fresh URL capability; Workspace movement keeps the live binding. The listener's authority is `docs/specs/security-local.md` → Local-file viewer. diff --git a/docs/specs/layout.md b/docs/specs/layout.md index c9ee26958..9f4fd4c22 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -50,11 +50,6 @@ A 30px header doubling as a drag handle: **a `pointerdown` past a 5px threshold **Must use browser chrome for a serving Tool, with a Terminal Context disclosure for its serving terminal.** Tool composition belongs to `docs/specs/dor-tool.md` → Lifecycle. -**Must size browser chrome by available pane width, excluding Tool context.** Hide inline split/zoom below 420px and navigation below 360px. Below 180px, use a keyboard-accessible, viewport-clamped popover; minimize/kill remain inline until 72px, then join the popover. Dismiss without restoring focus when hidden. Its trigger identifies notes by filled notepad glyph and count. Long keys and connection labels yield before controls. (rationale) - -Source of truth: `SurfacePaneHeader` in `lib/src/components/wall/SurfacePaneHeader.tsx`; tests: `lib/src/components/wall/SurfacePaneHeader.test.tsx`; stories: `lib/src/stories/BrowserChromeHeader.stories.tsx`. - - Elements left to right: derived label; alert bell; TODO pill (compact+); flexible gap; mouse-reporting override icon (compact+, only while the inside program requests mouse reporting); notepad icon (`docs/specs/notepad.md` → "Notepad UI"); split left/right, split top/bottom, zoom/unzoom (full only); minimize; kill (hover turns error-red). The label is the `DerivedHeader` from `deriveHeader(...)`; `docs/specs/terminal-state.md` owns the priority chain and disambiguator. Layout renders it: primary truncates with ellipsis, secondary muted beside it, a failed last command appends an error-colored glyph. Click renames/pins; right-click — or `>` in command mode — opens the header context menu. @@ -112,12 +107,25 @@ Both layers wear the leaf's own rounding (header radius on top, terminal radius ### Pane header responsive sizing -A ResizeObserver picks one of three tiers by header width: +**Must pick each header's tier from its own measured width, never the viewport** (rationale). A terminal header has three tiers: - **Full** (>280px): everything. - **Compact** (>160px): split, zoom, and unzoom hidden. - **Minimal** (≤160px): also hides the TODO pill and the mouse-override icon, leaving alert, minimize, and kill. **The notepad icon survives this tier only while the Surface has notes** (`docs/specs/notepad.md` → "Notepad UI"). The label truncates with ellipsis. +A browser header, including a Tool's (Terminal Context sits outside the measured width), collapses by border-box width: + +| Below | Change | +|---|---| +| 420px | Split and zoom hidden. | +| 360px | Navigation hidden. | +| 180px | Chrome moves into a viewport-clamped popover behind one trigger; minimize and kill stay inline. | +| 72px | Minimize and kill join the popover. | + +**Must keep the popover keyboard reachable** (focus enters on open, Tab stays inside, Escape returns it to the trigger) **and dismiss it on a pane resize or, without restoring focus, when its Surface is hidden**; otherwise `lib/src/components/wall/use-dismiss-overlay.ts` applies, and a control inside dismisses only after its action ran. The trigger shows a filled notepad glyph and the note count while the Surface has notes; long keys and connection labels truncate before controls. + +Source of truth: `SurfacePaneHeader` in `lib/src/components/wall/SurfacePaneHeader.tsx`; `useHeaderTier` in `lib/src/components/wall/use-header-tier.ts`; `lib/src/components/wall/SurfacePaneHeader.test.tsx`; `lib/src/stories/BrowserChromeHeader.stories.tsx`. + ## Baseboard The baseboard (`h-7`, 28px) sits below content, visible by default, with no top divider. A 2px theme-colored gap preserves pane corners; 7px horizontal padding aligns doors with panes. With no doors above 350px wide, it shows `LCmd → RCmd to enter command mode` on macOS and `LShift → RShift to enter command mode` elsewhere. diff --git a/docs/specs/layout.rationale.md b/docs/specs/layout.rationale.md index 102a9ccd7..01c387c6d 100644 --- a/docs/specs/layout.rationale.md +++ b/docs/specs/layout.rationale.md @@ -2,9 +2,9 @@ > Informative companion to [layout.md](layout.md): the evidence, measurements, and dead-approach history behind its rules, keyed by that spec's headings (AGENTS.md → "What, not why"). Nothing here is normative. -## Pane header +## Pane header responsive sizing -Viewport breakpoints keep every button visible when a wide window contains a narrow split. Tool headers have even less browser width because Terminal Context occupies its own button. Measuring the browser header and moving fixed controls together prevents long keys, note buttons, or renderer chips from pushing minimize/kill into a neighboring pane. +A viewport breakpoint says nothing about a narrow split inside a wide window: at a 1200px viewport every control stayed rendered in a 103px pane and overflowed into its neighbor (innerdogfood QC, 2026-09). Tool headers have even less browser width because Terminal Context occupies its own button. Measuring the header and moving fixed controls together keeps long keys, note buttons, and renderer chips from pushing minimize/kill into a neighboring pane; quantizing the measurement to a tier keeps the header from re-rendering on every frame of a sash drag or tween. ## Pane body diff --git a/docs/testing/dor-tool-qc.md b/docs/testing/dor-tool-qc.md index 7ca2e15e7..17a332209 100644 --- a/docs/testing/dor-tool-qc.md +++ b/docs/testing/dor-tool-qc.md @@ -5,7 +5,9 @@ Branch: `dor-tool-qc`, based on the reviewed stack at `4c7f9012`. ## Harness and isolation Run source-mutating root self-tests before starting the live harness. Run -`pnpm innerdogfood` in a visible `dor ensure` pane. Use its real sidecar, +`pnpm innerdogfood` in a visible `dor ensure` pane. The serving fixture is +`scripts/dor-tool-qc/server.mjs` (`--ports N`, `--label`, `--path`, +`--announce`; `SIGUSR1` re-announces over OSC 367). Use its real sidecar, PTYs, staged CLI, and browser UI through `dor ab`. Keep fixture files, captured inner CLI credentials, and the separate XDG user config under this worktree's ignored `standalone/src-tauri/target/dor-tool-qc/` directory. Never use the diff --git a/dor/test/file-viewer.test.mjs b/dor/test/file-viewer.test.mjs index 660b1ca83..82a23e2b2 100644 --- a/dor/test/file-viewer.test.mjs +++ b/dor/test/file-viewer.test.mjs @@ -37,10 +37,11 @@ async function get(viewer, path = viewer.path, headers = {}, method = 'GET') { } const asset = (viewer, path) => viewer.path.replace(/\/file\/.*$/, `/file/${path}`); -test('known formats override source-name heuristics without treating prototype keys as formats', () => { +test('known formats override source-name heuristics, PDFs never preview, and prototype keys are not formats', () => { for (const [name, mime] of [['readme.png', 'image/png'], ['LICENSE.html', 'text/html; charset=utf-8']]) { assert.deepEqual(fileViewerFormat(name), { mime, text: false }); } + for (const name of ['report.pdf', 'README.pdf', 'LICENSE.PDF']) assert.equal(fileViewerFormat(name), null, name); for (const name of ['README', 'Dockerfile.dev', 'README.md', '.gitignore']) { assert.deepEqual(fileViewerFormat(name), { mime: 'text/plain; charset=utf-8', text: true }); } @@ -48,14 +49,6 @@ test('known formats override source-name heuristics without treating prototype k assert.equal(fileViewerFormat('file.constructor'), null); }); -test('requires user Tools for PDFs even when their names resemble source files', async () => { - for (const name of ['report.pdf', 'README.pdf', 'LICENSE.PDF']) { - assert.equal(fileViewerFormat(name), null); - await writeFile(join(root, name), '%PDF-1.7 example bytes'); - await assert.rejects(startFileViewer(join(root, name)), /unsupported file format; configure a user Tool association/); - } -}); - test('renders text as escaped content and requires the per-run token on every method', async () => { const viewer = await start('README.md', ' & hello'); const good = await get(viewer); diff --git a/lib/src/components/use-notepad.ts b/lib/src/components/use-notepad.ts index 9d6028074..395a622fc 100644 --- a/lib/src/components/use-notepad.ts +++ b/lib/src/components/use-notepad.ts @@ -51,7 +51,12 @@ export function useOpenNotepadId(): string | null { * filled icon says how filled it is. */ export function notepadLabel(count: number): string { if (count === 0) return 'Notepad'; - return `Notepad · ${count} ${count === 1 ? 'note' : 'notes'}`; + return `Notepad · ${noteCountPhrase(count)}`; +} + +/** "1 note" / "3 notes" — the one plural rule every notepad trigger shares. */ +export function noteCountPhrase(count: number): string { + return `${count} ${count === 1 ? 'note' : 'notes'}`; } /** Fire-and-forget: the clipboard write is best effort by contract, and the diff --git a/lib/src/components/wall/SurfacePaneHeader.test.tsx b/lib/src/components/wall/SurfacePaneHeader.test.tsx index a102f34e9..b762524cf 100644 --- a/lib/src/components/wall/SurfacePaneHeader.test.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.test.tsx @@ -25,7 +25,7 @@ import { ZoomedIdContext, type WallActions, } from './wall-context'; -import { registerStubScreen, STUB_CHROME, STUB_SCREEN, stubWallActions as stubActions } from './wall-test-utils'; +import { registerStubScreen, STUB_CHROME, STUB_SCREEN, stubResizeObserver, stubWallActions as stubActions } from './wall-test-utils'; import { setNativeFieldValue } from '../../lib/dom'; globalThis.IS_REACT_ACT_ENVIRONMENT = true; @@ -45,21 +45,13 @@ let container: HTMLDivElement; let root: Root; let resizeHeader: (width: number) => void; - beforeEach(() => { setPlatform(new FakePtyAdapter()); clearAllNotepads(); container = document.createElement('div'); document.body.appendChild(container); root = createRoot(container); - vi.stubGlobal('ResizeObserver', class { - constructor(private callback: ResizeObserverCallback) {} - observe(target: Element) { - resizeHeader = width => this.callback([{ target, borderBoxSize: [{ inlineSize: width }] } as unknown as ResizeObserverEntry], this as unknown as ResizeObserver); - resizeHeader(620); - } - disconnect() {} - }); + resizeHeader = stubResizeObserver(620); }); afterEach(() => { @@ -77,23 +69,37 @@ function renderHeader( root.render( - - - - - - {state.tool ? : } - - - - - + + + + + + {state.tool ? : } + + + + + , ); }); } +/** The compact header's popover, portaled to `document.body`. */ +const popup = () => document.querySelector('[role="dialog"][aria-label="Browser controls"]'); +/** The compact header's trigger; its label grows a note count. */ +const overflowTrigger = () => container.querySelector('[aria-label^="Browser controls"]')!; +const inPopup = (selector: string) => popup()?.querySelector(selector) ?? null; +/** Click, then let the popover's deferred dismissal (a 0ms task) run. */ +async function clickAndSettle(element: HTMLElement) { + await act(async () => { element.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); +} +function openPopup() { + act(() => overflowTrigger().click()); + expect(popup()).not.toBeNull(); +} + describe('SurfacePaneHeader — browser chrome', () => { it.each(['workspace', 'parked'] as const)('dismisses compact controls without stealing focus when hidden by %s', hiddenBy => { const id = 'pane-hidden-controls'; @@ -105,27 +111,24 @@ describe('SurfacePaneHeader — browser chrome', () => { try { renderHeader(props, actions); act(() => resizeHeader(79)); - act(() => container.querySelector('[aria-label="Browser controls"]')!.click()); - expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).not.toBeNull(); + openPopup(); otherWorkspaceControl.focus(); renderHeader({ ...props, parked: hiddenBy === 'parked' }, actions, { workspaceActive: hiddenBy !== 'workspace' }); - expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); + expect(popup()).toBeNull(); expect(document.activeElement).toBe(otherWorkspaceControl); renderHeader(props, actions); - expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); - expect(container.querySelector('[aria-label="Browser controls"]')?.getAttribute('aria-expanded')).toBe('false'); + expect(popup()).toBeNull(); + expect(overflowTrigger().getAttribute('aria-expanded')).toBe('false'); } finally { otherWorkspaceControl.remove(); registration.dispose(); } }); - it('adapts to pane resizes in a wide window and keeps compact controls keyboard reachable', async () => { + it('collapses chrome by its own width, excluding the Tool context button', () => { const registration = register('pane-resize', { ...CHROME, key: 'a'.repeat(300) }); - const actions = stubActions(); - renderHeader({ ...headerProps('pane-resize', 'Browser'), params: { surfaceType: 'tool', url: CHROME.url } }, actions, { tool: true }); + renderHeader({ ...headerProps('pane-resize', 'Browser'), params: { surfaceType: 'tool', url: CHROME.url } }, stubActions(), { tool: true }); expect(container.querySelector('[aria-label="Terminal context"]')).not.toBeNull(); - const viewport = window.innerWidth; expect(container.querySelector('[aria-label="Back"]')).not.toBeNull(); expect(container.querySelector('[aria-label="Zoom"]')).not.toBeNull(); act(() => resizeHeader(400)); @@ -135,64 +138,93 @@ describe('SurfacePaneHeader — browser chrome', () => { expect(container.querySelector('[aria-label="Back"]')).toBeNull(); // A 103px Tool leaves 79px beside its Terminal Context button. act(() => resizeHeader(79)); - const overflow = container.querySelector('[aria-label="Browser controls"]')!; - expect(overflow).not.toBeNull(); + expect(overflowTrigger()).not.toBeNull(); + expect(container.querySelector('[aria-label="Kill"]')).not.toBeNull(); + act(() => resizeHeader(56)); + expect(container.querySelector('[aria-label="Kill"]')).toBeNull(); + act(() => resizeHeader(620)); + expect(container.querySelector('[aria-label^="Browser controls"]')).toBeNull(); + expect(container.querySelector('[aria-label="Back"]')).not.toBeNull(); + expect(container.querySelector('[aria-label="Zoom"]')).not.toBeNull(); + registration.dispose(); + }); + + it('keeps the popover keyboard reachable and hands focus back to its trigger', async () => { + const registration = register('pane-popup'); + const actions = stubActions(); + renderHeader(headerProps('pane-popup', 'Browser'), actions); + act(() => resizeHeader(79)); + openPopup(); + expect(popup()!.contains(document.activeElement)).toBe(true); + const firstControl = document.activeElement!; + act(() => firstControl.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', bubbles: true }))); + expect(document.activeElement).not.toBe(firstControl); + expect(popup()!.contains(document.activeElement)).toBe(true); + expect(inPopup('[aria-label="Back"]')).not.toBeNull(); + await clickAndSettle(inPopup('[aria-label="Split left/right"]')!); + expect(actions.onSplitH).toHaveBeenCalledWith('pane-popup'); + expect(popup()).toBeNull(); + + openPopup(); + const url = inPopup('[role="button"]')!; + act(() => { url.focus(); url.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); }); + expect(inPopup('input')).not.toBeNull(); + act(() => inPopup('input')!.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))); + expect(popup()).toBeNull(); + expect(document.activeElement).toBe(overflowTrigger()); + + openPopup(); + act(() => document.body.dispatchEvent(new Event('pointerdown', { bubbles: true }))); + expect(popup()).toBeNull(); + + openPopup(); + // Separate acts: a browser flushes the pointerdown's state before the click. + act(() => overflowTrigger().dispatchEvent(new Event('pointerdown', { bubbles: true }))); + act(() => overflowTrigger().click()); + expect(popup()).toBeNull(); + expect(overflowTrigger().getAttribute('aria-expanded')).toBe('false'); + registration.dispose(); + }); + + it('names notes on the trigger and opens the notepad from the popover', async () => { + const registration = register('pane-notes'); + renderHeader(headerProps('pane-notes', 'Browser'), stubActions()); + act(() => resizeHeader(79)); + act(() => addPlainNote('pane-notes', 'A saved note')); + expect(overflowTrigger().getAttribute('aria-label')).toBe('Browser controls, 1 note'); + openPopup(); + expect(inPopup('input')).toBeNull(); + await clickAndSettle(inPopup('button[aria-label^="Notepad"]')!); + expect(getOpenNotepadId()).toBe('pane-notes'); + registration.dispose(); + }); + + it('closes the popover from Minimize and Kill wherever they render', async () => { + const registration = register('pane-actions'); + const actions = stubActions(); + renderHeader(headerProps('pane-actions', 'Browser'), actions); + act(() => resizeHeader(79)); for (const label of ['Minimize', 'Kill']) { - act(() => overflow.click()); + openPopup(); act(() => container.querySelector(`[aria-label="${label}"]`)!.click()); - expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); + expect(popup()).toBeNull(); } - expect(actions.onMinimize).toHaveBeenCalledWith('pane-resize'); - expect(actions.onKill).toHaveBeenCalledWith('pane-resize'); - act(() => overflow.click()); - let dialog = document.querySelector('[role="dialog"][aria-label="Browser controls"]')!; - expect(dialog).not.toBeNull(); - expect(dialog.contains(document.activeElement)).toBe(true); - const firstControl = document.activeElement; - act(() => firstControl!.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', bubbles: true }))); - expect(document.activeElement).not.toBe(firstControl); - expect(dialog.contains(document.activeElement)).toBe(true); - expect(dialog.querySelector('[aria-label="Back"]')).not.toBeNull(); - await act(async () => { dialog.querySelector('[aria-label="Split left/right"]')!.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); - expect(actions.onSplitH).toHaveBeenCalledWith('pane-resize'); - expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); - act(() => overflow.click()); - dialog = document.querySelector('[role="dialog"][aria-label="Browser controls"]')!; - const url = dialog.querySelector('[role="button"]')!; - act(() => { url.focus(); url.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); }); - expect(dialog.querySelector('input')).not.toBeNull(); - act(() => dialog.querySelector('input')!.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))); - expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); - expect(document.activeElement).toBe(overflow); - act(() => addPlainNote('pane-resize', 'A saved note')); - expect(overflow.getAttribute('aria-label')).toBe('Browser controls, 1 note'); - act(() => overflow.click()); - expect(document.querySelector('[role="dialog"] input')).toBeNull(); - await act(async () => { document.querySelector('[role="dialog"] button[aria-label^="Notepad"]')!.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); - expect(getOpenNotepadId()).toBe('pane-resize'); act(() => resizeHeader(56)); - expect(container.querySelector('[aria-label="Kill"]')).toBeNull(); for (const label of ['Minimize', 'Kill']) { - act(() => overflow.click()); - await act(async () => { document.querySelector(`[role="dialog"] [aria-label="${label}"]`)!.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); + openPopup(); + await clickAndSettle(inPopup(`[aria-label="${label}"]`)!); + expect(popup()).toBeNull(); } expect(actions.onMinimize).toHaveBeenCalledTimes(2); expect(actions.onKill).toHaveBeenCalledTimes(2); - act(() => resizeHeader(620)); - expect(container.querySelector('[aria-label^="Browser controls"]')).toBeNull(); - expect(container.querySelector('[aria-label="Back"]')).not.toBeNull(); - expect(container.querySelector('[aria-label="Zoom"]')).not.toBeNull(); - expect(window.innerWidth).toBe(viewport); + expect(actions.onKill).toHaveBeenCalledWith('pane-actions'); registration.dispose(); }); it('runs popup Zoom, Reload and Display before dismissal and preserves modal focus', async () => { const modalControl = document.createElement('button'); document.body.appendChild(modalControl); - const stillOwnsFocus = () => { - const popup = document.querySelector('[role="dialog"][aria-label="Browser controls"]'); - expect(popup?.contains(document.activeElement)).toBe(true); - }; + const stillOwnsFocus = () => expect(popup()?.contains(document.activeElement)).toBe(true); const onZoom = vi.fn(stillOwnsFocus); const reload = vi.fn(stillOwnsFocus); const openModal = vi.fn(() => { stillOwnsFocus(); modalControl.focus(); }); @@ -204,12 +236,12 @@ describe('SurfacePaneHeader — browser chrome', () => { try { renderHeader(headerProps('pane-popup-actions', 'Browser'), stubActions({ onZoom })); act(() => resizeHeader(79)); - const trigger = container.querySelector('[aria-label="Browser controls"]')!; for (const selector of ['[aria-label="Zoom"]', '[aria-label="Reload"]', '[data-browser-display-trigger]']) { - act(() => trigger.click()); - const action = document.querySelector(`[role="dialog"] ${selector}`)!; - await act(async () => { action.focus(); action.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); - expect(document.querySelector('[role="dialog"][aria-label="Browser controls"]')).toBeNull(); + openPopup(); + const action = inPopup(selector)!; + action.focus(); + await clickAndSettle(action); + expect(popup()).toBeNull(); } expect(onZoom).toHaveBeenCalledWith('pane-popup-actions'); expect(reload).toHaveBeenCalledOnce(); diff --git a/lib/src/components/wall/SurfacePaneHeader.tsx b/lib/src/components/wall/SurfacePaneHeader.tsx index 772a87eff..a348580f6 100644 --- a/lib/src/components/wall/SurfacePaneHeader.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.tsx @@ -1,9 +1,11 @@ -import { useCallback, useContext, useEffect, useLayoutEffect, useRef, useState, type CSSProperties, type ReactNode } from 'react'; +import { useCallback, useContext, useEffect, useLayoutEffect, useRef, useState, type CSSProperties, type ReactNode, type RefObject } from 'react'; import { createPortal } from 'react-dom'; -import { usePopoverFocusTrap } from '../use-popover-focus-trap'; +import { POPOVER_FOCUSABLE_SELECTOR, usePopoverFocusTrap } from '../use-popover-focus-trap'; +import { useDismissOverlay } from './use-dismiss-overlay'; +import { useHeaderTier } from './use-header-tier'; import { useSurfaceVisibility } from './use-surface-visibility'; -import { useNoteCount } from '../use-notepad'; -import { clampOverlayPosition } from '../../lib/ui-geometry'; +import { noteCountPhrase, useNoteCount } from '../use-notepad'; +import { clampOverlayPosition, OVERLAY_VIEWPORT_MARGIN_PX } from '../../lib/ui-geometry'; import { DotsThreeIcon, NotepadIcon, @@ -18,7 +20,7 @@ import { XIcon, } from '@phosphor-icons/react'; import { HeaderActionButton } from '../HeaderActionButton'; -import { HEADER_PALETTE_TRANSITION_CLASS, POPUP_SURFACE_CLASS, paneZoomButtonClass, TERMINAL_TOP_RADIUS_CLASS } from '../design'; +import { chromeButton, HEADER_PALETTE_TRANSITION_CLASS, OVERLAY_MAX_HEIGHT, POPUP_SURFACE_CLASS, paneZoomButtonClass, TERMINAL_TOP_RADIUS_CLASS } from '../design'; import { NotepadHeaderButton } from './NotepadHeaderButton'; import { useAgentBrowserChromeSnapshot, @@ -40,6 +42,14 @@ import { useDialogKeyboardOwner, } from './wall-context'; +/** What the browser chrome shows inline at a header width (`docs/specs/layout.md` + * → "Pane header responsive sizing"). Below `minimal` the chrome moves into the + * popover, where it renders at `full`. */ +type InlineTier = 'full' | 'compact' | 'minimal'; +type BrowserHeaderTier = InlineTier | 'overflow' | 'tiny'; +const browserHeaderTier = (width: number): BrowserHeaderTier => + width >= 420 ? 'full' : width >= 360 ? 'compact' : width >= 180 ? 'minimal' : width >= 72 ? 'overflow' : 'tiny'; + export function SurfacePaneHeader({ id, title, parked }: PaneProps) { const visible = useSurfaceVisibility(parked); const visibleRef = useRef(visible); @@ -78,7 +88,7 @@ export function SurfacePaneHeader({ id, title, parked }: PaneProps) { // keyboard handler stands down (the panel's own key-forwarder skips editable // targets); the editor closes itself when the surface stops being a browser. const [editingUrl, setEditingUrl] = useState(false); - useDialogKeyboardOwner(editingUrl && visible); + useDialogKeyboardOwner(editingUrl); useEffect(() => { if (!screen && editingUrl) setEditingUrl(false); }, [screen, editingUrl]); @@ -90,32 +100,28 @@ export function SurfacePaneHeader({ id, title, parked }: PaneProps) { }; const closeUrlEditor = () => setEditingUrl(false); + // Below the `minimal` tier the chrome lives in a popover behind one trigger; + // a pane resize or a hidden Surface closes it, the latter without pulling + // focus back to a trigger nobody can see. `closeMenu` stays identity-stable + // (reading `visibleRef`) so the popover's listeners subscribe once. const headerRef = useRef(null); const overflowRef = useRef(null); - const [width, setWidth] = useState(Number.POSITIVE_INFINITY); - const compact = width < 180; - const [menuAnchor, setMenuAnchor] = useState(null); + const [menuOpen, setMenuOpen] = useState(false); + const closeMenu = useCallback((restoreFocus = true) => { + setMenuOpen(false); + setEditingUrl(false); + if (restoreFocus && visibleRef.current) overflowRef.current?.focus(); + }, []); + const tier = useHeaderTier(headerRef, browserHeaderTier, () => closeMenu(false)); + const inline: InlineTier | null = tier === 'overflow' || tier === 'tiny' ? null : tier; + const popoverOpen = visible && inline === null && menuOpen; const noteCount = useNoteCount(id); - const overflowLabel = `Browser controls${noteCount ? `, ${noteCount} ${noteCount === 1 ? 'note' : 'notes'}` : ''}`; - const closeMenu = useCallback((restoreFocus = true) => { setMenuAnchor(null); setEditingUrl(false); if (restoreFocus && visibleRef.current) overflowRef.current?.focus(); }, []); + const overflowLabel = `Browser controls${noteCount ? `, ${noteCountPhrase(noteCount)}` : ''}`; useEffect(() => { if (!visible) closeMenu(false); }, [visible, closeMenu]); - useLayoutEffect(() => { - const header = headerRef.current; - if (!header) return; - const initialWidth = header.getBoundingClientRect().width; - if (initialWidth > 0) setWidth(initialWidth); - const observer = new ResizeObserver(([entry]) => { - setWidth(entry.borderBoxSize?.[0]?.inlineSize ?? entry.contentRect.width); - setMenuAnchor(null); - setEditingUrl(false); - }); - observer.observe(header); - return () => observer.disconnect(); - }, []); - const browserControls = ( + const renderBrowserControls = (placement: InlineTier | 'popover') => ( <> {screen && screenSnapshot && chrome ? ( <> @@ -136,7 +142,7 @@ export function SurfacePaneHeader({ id, title, parked }: PaneProps) { {/* Back / forward / refresh — native agent-browser commands; always enabled (no canGoBack/Forward in the stream). Collapse before the URL but after split/zoom. */} - {(compact || width >= 360) &&
+ {placement !== 'minimal' &&
{ e.stopPropagation(); screen.chromeActions.back(); }} @@ -204,7 +210,7 @@ export function SurfacePaneHeader({ id, title, parked }: PaneProps) { / full URL → tooltip. Gives up width (shrink-[10]) long before the command does. */} <span - className={`${compact ? 'basis-full' : ''} min-w-0 shrink-[10] cursor-text truncate font-medium underline-offset-2 hover:underline`} + className={`${placement === 'popover' ? 'basis-full' : ''} min-w-0 shrink-[10] cursor-text truncate font-medium underline-offset-2 hover:underline`} title={chrome.title ?? chrome.url ?? undefined} onMouseDown={(e) => e.stopPropagation()} role="button" @@ -214,7 +220,7 @@ export function SurfacePaneHeader({ id, title, parked }: PaneProps) { >{urlText || title || id}</span> {/* Flexible spacer keeps the layout buttons right-aligned. */} - {!compact && <div className="min-w-0 flex-1" />} + {placement !== 'popover' && <div className="min-w-0 flex-1" />} </> )} </> @@ -223,7 +229,7 @@ export function SurfacePaneHeader({ id, title, parked }: PaneProps) { )} <NotepadHeaderButton surfaceId={id} /> - {(compact || width >= 420) && <div className="ml-1 flex shrink-0 items-center gap-0.5"> + {(placement === 'popover' || placement === 'full') && <div className="ml-1 flex shrink-0 items-center gap-0.5"> <HeaderActionButton className="flex h-5 min-w-5 items-center justify-center rounded transition-colors hover:bg-current/10" onClick={(e) => { e.stopPropagation(); actions.onSplitH(id); }} @@ -266,53 +272,67 @@ export function SurfacePaneHeader({ id, title, parked }: PaneProps) { return ( <div ref={headerRef} - className={`flex h-full min-w-0 flex-1 cursor-grab items-center ${compact ? 'gap-0.5 px-1' : 'gap-1.5 pl-2 pr-[5px]'} ${TERMINAL_TOP_RADIUS_CLASS} text-sm leading-none font-mono select-none active:cursor-grabbing ${HEADER_PALETTE_TRANSITION_CLASS} ${isActiveHeader ? 'bg-header-active-bg text-header-active-fg' : 'bg-header-inactive-bg text-header-inactive-fg'}`} + className={`flex h-full min-w-0 flex-1 cursor-grab items-center ${inline ? 'gap-1.5 pl-2 pr-[5px]' : 'gap-0.5 px-1'} ${TERMINAL_TOP_RADIUS_CLASS} text-sm leading-none font-mono select-none active:cursor-grabbing ${HEADER_PALETTE_TRANSITION_CLASS} ${isActiveHeader ? 'bg-header-active-bg text-header-active-fg' : 'bg-header-inactive-bg text-header-inactive-fg'}`} onMouseDown={() => actions.onClickPanel(id)} > - {compact ? ( + {inline ? renderBrowserControls(inline) : ( <button ref={overflowRef} type="button" aria-label={overflowLabel} - aria-haspopup="dialog" aria-expanded={visible && menuAnchor !== null} + aria-haspopup="dialog" aria-expanded={popoverOpen} title={overflowLabel} - className="flex h-5 min-w-5 shrink-0 items-center justify-center rounded hover:bg-current/10" + className={`${chromeButton()} shrink-0`} + /* A press on the trigger toggles; it must not dismiss first and then reopen. */ + onPointerDown={event => event.stopPropagation()} onMouseDown={event => event.stopPropagation()} - onClick={event => { event.stopPropagation(); if (menuAnchor) closeMenu(); else setMenuAnchor(event.currentTarget.getBoundingClientRect()); }}> + onClick={event => { event.stopPropagation(); if (menuOpen) closeMenu(); else setMenuOpen(true); }}> {noteCount ? <NotepadIcon size={14} weight="fill" /> : <DotsThreeIcon size={14} />} </button> - ) : browserControls} - {width >= 72 && paneActions} - {visible && compact && menuAnchor && <BrowserHeaderPopover anchor={menuAnchor} onClose={closeMenu}> - {browserControls} - {width < 72 && paneActions} + )} + {tier !== 'tiny' && paneActions} + {popoverOpen && <BrowserHeaderPopover anchorRef={overflowRef} onClose={closeMenu}> + {renderBrowserControls('popover')} + {tier === 'tiny' && paneActions} </BrowserHeaderPopover>} </div> ); } -function BrowserHeaderPopover({ anchor, onClose, children }: { anchor: DOMRect; onClose: (restoreFocus?: boolean) => void; children: ReactNode }) { +/** Gap between the trigger's bottom edge and the popover. */ +const POPOVER_GAP_PX = 4; + +function BrowserHeaderPopover({ anchorRef, onClose, children }: { + anchorRef: RefObject<HTMLElement | null>; + onClose: (restoreFocus?: boolean) => void; + children: ReactNode; +}) { const ref = useRef<HTMLDivElement>(null); - const [position, setPosition] = useState<CSSProperties>({ position: 'fixed', left: anchor.left, top: anchor.bottom }); + const [position, setPosition] = useState<CSSProperties>({ position: 'fixed' }); useDialogKeyboardOwner(true); usePopoverFocusTrap(ref, onClose); - useEffect(() => { - const resized = () => onClose(); - window.addEventListener('resize', resized); - return () => window.removeEventListener('resize', resized); - }, [onClose]); + useDismissOverlay(onClose, ref); useLayoutEffect(() => { + const anchor = anchorRef.current!.getBoundingClientRect(); const rect = ref.current!.getBoundingClientRect(); - setPosition(clampOverlayPosition({ left: anchor.left, top: anchor.bottom + 4, width: rect.width, height: rect.height })); - ref.current!.querySelector<HTMLElement>('button, [tabindex="0"]')?.focus(); - }, [anchor]); + setPosition(clampOverlayPosition({ left: anchor.left, top: anchor.bottom + POPOVER_GAP_PX, width: rect.width, height: rect.height })); + ref.current!.querySelector<HTMLElement>(POPOVER_FOCUSABLE_SELECTOR)?.focus(); + }, [anchorRef]); return createPortal( - <div ref={ref} role="dialog" aria-label="Browser controls" style={position} - className={`${POPUP_SURFACE_CLASS} flex max-h-[75dvh] w-80 max-w-[calc(100vw-2rem)] flex-wrap items-center gap-2 overflow-auto p-2 text-sm`} + <div ref={ref} role="dialog" aria-label="Browser controls" + style={{ ...position, maxWidth: `calc(100vw - ${OVERLAY_VIEWPORT_MARGIN_PX * 2}px)` }} + className={`${POPUP_SURFACE_CLASS} ${OVERLAY_MAX_HEIGHT.popover} flex w-80 flex-wrap items-center gap-2 overflow-auto p-2 text-sm`} + /* Presses inside survive the dismissal contract and never start a pane drag. */ + onPointerDown={event => event.stopPropagation()} onMouseDown={event => event.stopPropagation()} onClickCapture={event => { + // Only real buttons dismiss: the URL is a `role="button"` span whose + // click opens the editor here, inside the popover. if (!(event.target as Element).closest('button')) return; // Native clicks can drain microtasks between capture and bubble. Wait // a task so the action runs before its target unmounts; a new modal // keeps any focus it acquired in the action handler. - setTimeout(() => onClose(document.activeElement === document.body || !!ref.current?.contains(document.activeElement)), 0); + setTimeout(() => { + const focusStillOurs = document.activeElement === document.body || !!ref.current?.contains(document.activeElement); + onClose(focusStillOurs); + }, 0); }}> {children} </div>, document.body, diff --git a/lib/src/components/wall/TerminalPaneHeader.test.tsx b/lib/src/components/wall/TerminalPaneHeader.test.tsx index 0703a6ae1..ad98dfda5 100644 --- a/lib/src/components/wall/TerminalPaneHeader.test.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.test.tsx @@ -7,7 +7,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { PaneProps } from './pane-props'; import { TerminalPaneHeader } from './TerminalPaneHeader'; import { RenamingIdContext, WallActionsContext, type WallActions } from './wall-context'; -import { ensureResizeObserver, stubWallActions as stubActions } from './wall-test-utils'; +import { ensureResizeObserver, stubResizeObserver, stubWallActions as stubActions } from './wall-test-utils'; import { FakePtyAdapter } from '../../lib/platform/fake-adapter'; import { setPlatform } from '../../lib/platform'; import { setNativeFieldValue } from '../../lib/dom'; @@ -154,27 +154,14 @@ describe('TerminalPaneHeader — inline rename', () => { describe('TerminalPaneHeader — notepad icon', () => { // The tier is ResizeObserver-driven, so the suite's inert stub can only ever // show `full`. This one reports a width the test picks. - let headerWidth = 400; - let previousObserver: typeof ResizeObserver; + let resizeHeader: (width: number) => void; beforeEach(() => { - headerWidth = 400; - previousObserver = globalThis.ResizeObserver; - globalThis.ResizeObserver = class { - constructor(private readonly callback: ResizeObserverCallback) {} - observe(target: Element): void { - this.callback( - [{ target, contentRect: { width: headerWidth } } as unknown as ResizeObserverEntry], - this as unknown as ResizeObserver, - ); - } - unobserve(): void {} - disconnect(): void {} - } as unknown as typeof ResizeObserver; + resizeHeader = stubResizeObserver(400); }); afterEach(() => { - globalThis.ResizeObserver = previousObserver; + vi.unstubAllGlobals(); // Still mounted at this point (the outer hook unmounts), so both stores // notify a live header. act(() => { @@ -220,14 +207,11 @@ describe('TerminalPaneHeader — notepad icon', () => { }); it('keeps its place at the compact tier and yields it at minimal only when empty', () => { - headerWidth = 200; renderHeader(stubActions(), null); + act(() => resizeHeader(200)); expect(notepadButton()).not.toBeNull(); - headerWidth = 100; - act(() => root.unmount()); - root = createRoot(container); - renderHeader(stubActions(), null); + act(() => resizeHeader(100)); expect(notepadButton()).toBeNull(); // Notes are never invisible: the icon comes back to carry them. diff --git a/lib/src/components/wall/use-header-tier.ts b/lib/src/components/wall/use-header-tier.ts new file mode 100644 index 000000000..a4c1c8a32 --- /dev/null +++ b/lib/src/components/wall/use-header-tier.ts @@ -0,0 +1,33 @@ +import { useLayoutEffect, useRef, useState, type RefObject } from 'react'; + +/** + * A pane header's responsive tier, quantized from its own border-box width + * (`docs/specs/layout.md` → "Pane header responsive sizing"). The Lath animator + * resizes leaves every frame of a tween or sash drag, so the observer keeps the + * raw width out of React state: the header re-renders only when `tierFor` + * changes its answer. The first measurement is synchronous so a narrow pane + * never paints one frame of full-width chrome; a zero width (a hidden leaf) + * keeps the previous tier. `onResize` fires on every observed resize regardless. + */ +export function useHeaderTier<T>( + ref: RefObject<HTMLElement | null>, + tierFor: (width: number) => T, + onResize?: () => void, +): T { + const [tier, setTier] = useState<T>(() => tierFor(Number.POSITIVE_INFINITY)); + const latest = useRef({ tierFor, onResize }); + latest.current = { tierFor, onResize }; + useLayoutEffect(() => { + const header = ref.current; + if (!header) return; + const measure = (width: number) => { if (width > 0) setTier(latest.current.tierFor(width)); }; + measure(header.getBoundingClientRect().width); + const observer = new ResizeObserver(([entry]) => { + measure(entry.borderBoxSize?.[0]?.inlineSize ?? entry.contentRect.width); + latest.current.onResize?.(); + }); + observer.observe(header); + return () => observer.disconnect(); + }, [ref]); + return tier; +} diff --git a/lib/src/components/wall/wall-test-utils.ts b/lib/src/components/wall/wall-test-utils.ts index 82c9a84f5..42460ddfd 100644 --- a/lib/src/components/wall/wall-test-utils.ts +++ b/lib/src/components/wall/wall-test-utils.ts @@ -40,6 +40,39 @@ export function ensureResizeObserver(): void { } as unknown as typeof ResizeObserver; } +/** + * A ResizeObserver whose width the test drives: every observed element is told + * `initialWidth` on observe, and the returned setter re-delivers a new width to + * all of them. Stubbed through `vi.stubGlobal`, so `vi.unstubAllGlobals()` in + * `afterEach` restores jsdom. + */ +export function stubResizeObserver(initialWidth: number): (width: number) => void { + let width = initialWidth; + const deliveries = new Set<() => void>(); + vi.stubGlobal('ResizeObserver', class { + private readonly delivery = new Set<() => void>(); + constructor(private readonly callback: ResizeObserverCallback) {} + observe(target: Element): void { + const deliver = () => this.callback([{ + target, + borderBoxSize: [{ inlineSize: width, blockSize: 0 }], + contentRect: { width }, + } as unknown as ResizeObserverEntry], this as unknown as ResizeObserver); + this.delivery.add(deliver); + deliveries.add(deliver); + deliver(); + } + unobserve(): void {} + disconnect(): void { + for (const deliver of this.delivery) deliveries.delete(deliver); + } + }); + return (next) => { + width = next; + for (const deliver of deliveries) deliver(); + }; +} + export interface WallHarness { container: HTMLDivElement; root: Root; diff --git a/lib/src/host/tool-open.test.ts b/lib/src/host/tool-open.test.ts index 39a237df9..03080ded9 100644 --- a/lib/src/host/tool-open.test.ts +++ b/lib/src/host/tool-open.test.ts @@ -96,15 +96,12 @@ it('matches catch-all rules above the invocation directory and canonical absolut expect(await host().handle(request)).toMatchObject({ status: 'ok', name: 'viewer' }); }); -it('requires a user Tool for PDFs and passes the canonical file to configured handlers', async () => { +it('requires a user Tool for PDFs', async () => { const target = join(root, 'README.pdf'); await writeFile(target, '%PDF-1.7'); await rm(config); expect(await host().handle({ op: 'open', target, cwd: root })).toMatchObject({ status: 'error', message: expect.stringContaining('add an open rule') }); expect(await host().handle({ op: 'open', target, cwd: root, tool: 'builtin:file' })).toMatchObject({ status: 'error', message: expect.stringContaining('does not support') }); - await writeConfig('open:\n - {match: "*.pdf", tool: "builtin:file"}\n'); - expect(await host().handle({ op: 'open', target, cwd: root })).toMatchObject({ status: 'error' }); await writeConfig(viewerConfig('*.pdf')); expect(await host().handle({ op: 'open', target, cwd: root })).toMatchObject({ status: 'ok', scope: 'user', run: ['view', target] }); - expect(await host().handle({ op: 'open', target, cwd: root, tool: 'viewer' })).toMatchObject({ status: 'ok', scope: 'user', run: ['view', target] }); }); diff --git a/lib/src/stories/BrowserChromeHeader.stories.tsx b/lib/src/stories/BrowserChromeHeader.stories.tsx index 4d0b8a05c..39f2c5827 100644 --- a/lib/src/stories/BrowserChromeHeader.stories.tsx +++ b/lib/src/stories/BrowserChromeHeader.stories.tsx @@ -146,6 +146,7 @@ function BrowserChromeStory(args: StoryArgs) { setDevServerResolution(port, label ? { paneId: 'term-dev', label } : null); }, [port, args.devServerLabel]); + const Header = args.tool ? ToolPaneHeader : SurfacePaneHeader; return ( <ModeContext.Provider value="passthrough"> <SelectedIdContext.Provider value={args.selected ? surfaceId : null}> @@ -155,12 +156,11 @@ function BrowserChromeStory(args: StoryArgs) { <WallActionsContext.Provider value={loggingActions}> <div style={{ width: args.width }}> <div className="bg-app-bg" style={{ height: PANE_HEADER_HEIGHT_PX }}> - {args.tool ? <ToolPaneHeader id={surfaceId} title={args.htmlTitle} - params={{ surfaceType: 'tool', url: args.url }} /> : <SurfacePaneHeader + <Header id={surfaceId} title={args.htmlTitle || hostPathDisplay(args.url)} - params={undefined} - />} + params={args.tool ? { surfaceType: 'tool', url: args.url } : undefined} + /> </div> </div> </WallActionsContext.Provider> @@ -243,15 +243,13 @@ export const Narrow: Story = { args: { width: 340 }, }; -/** Real narrow split: the Tool context button leaves 79px for browser chrome. */ +/** Real narrow split: the Tool context button leaves 79px for browser chrome, + * so the chrome sits behind one trigger while minimize/kill stay inline. */ export const TinyTool: Story = { args: { width: 103, tool: true, paneKey: 'a-very-long-tool-identity', devServerLabel: 'pnpm --filter a-very-long-project-name dev' }, }; -export const TinyBrowser: Story = { - args: { width: 103, paneKey: 'a-very-long-browser-identity' }, -}; - +/** Below 72px minimize/kill join the popover too. */ export const SmallestTool: Story = { args: { width: 80, tool: true }, }; diff --git a/scripts/loopback-lint.mjs b/scripts/loopback-lint.mjs index 8bb63ba46..0bf35a5e5 100644 --- a/scripts/loopback-lint.mjs +++ b/scripts/loopback-lint.mjs @@ -76,6 +76,11 @@ const ALLOWED = { + 'it vouches for no one. It skips the Host check on purpose: rebinding ' + 'exists to make same-origin-looking requests, which buys nothing against ' + 'an unguessable one-shot secret. See lib/src/host/loopback-guard.ts.', + 'scripts/dor-tool-qc/server.mjs': + 'A hand-run QC fixture (docs/testing/dor-tool-qc.md), never shipped or ' + + 'started by a host. It serves one static page echoing its own argv and ' + + 'the request path, holds no state and reaches nothing, so a page that ' + + 'finds the port learns only what the QC operator typed.', }; const GUARD_REFERENCES = ['loopback-guard', 'dev-host-guard']; diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 10011195b..ebc7e47a8 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -9,7 +9,7 @@ "docs/specs/dor-cli.md": 6000, "docs/specs/dor-tool.md": 3750, "docs/specs/glossary.md": 3000, - "docs/specs/layout.md": 8900, + "docs/specs/layout.md": 9000, "docs/specs/mobile-terminal-ui.md": 1950, "docs/specs/mouse-and-clipboard.md": 3800, "docs/specs/notepad.md": 4000, From 46e77877b71fe0f0dba4b6cc9f0cd78771c3ef2f Mon Sep 17 00:00:00 2001 From: Ned Twigg <ned.twigg@diffplug.com> Date: Wed, 16 Sep 2026 16:56:28 -0700 Subject: [PATCH 06/11] Share header tier observation and preserve QC evidence separately --- docs/specs/dor-tool.rationale.md | 39 ++++++ docs/specs/layout.md | 4 +- docs/specs/layout.rationale.md | 2 + docs/testing/dor-tool-qc.md | 116 ++++++------------ lib/src/components/wall/SurfacePaneHeader.tsx | 10 +- .../wall/TerminalPaneHeader.test.tsx | 31 ++++- .../components/wall/TerminalPaneHeader.tsx | 19 +-- lib/src/components/wall/use-header-tier.ts | 20 ++- lib/src/components/wall/wall-test-utils.ts | 4 +- 9 files changed, 135 insertions(+), 110 deletions(-) diff --git a/docs/specs/dor-tool.rationale.md b/docs/specs/dor-tool.rationale.md index 395beffe9..6b6f93b22 100644 --- a/docs/specs/dor-tool.rationale.md +++ b/docs/specs/dor-tool.rationale.md @@ -40,6 +40,45 @@ A hardcoded Storybook port can disagree with the port it obtains under contentio ## Lifecycle +### September 2026 innerdogfood QC record + +The `dor-tool-qc` run began at `4c7f9012` and used the real standalone sidecar, +staged CLI, PTYs, and iframe proxy. At that historical baseline the Tools flag +could reject creation and standalone `dor open` split; both behaviors were +subsequently superseded by always-enabled Tools and eligible inline opening. + +Observed passes covered project approval (pending dedupe, decline/re-prompt, +folder-only permission, failed spawn without a PTY, repair and Retry); literal +argv and canonical symlink targets; three concurrent keyed invocations sharing +one Tool, fresh instances, idle/fast-command restart with stable refs; automatic +single-port serving, three-port refusal, and announced port/path selection. +User-rule ordering and explicit overrides worked; malformed user configuration +failed, and project associations did not intercept file opens. Text/Markdown +source, HTML/CSS/image, SVG, audio, and awkward filenames rendered; URL, +directory, missing/unsupported-file and oversized-text cases failed usefully. + +Approval controls remained usable at 249×203 pixels. Terminal Context, +minimize/reveal, exit/refocus, and iframe/screencast round trips passed. A clean +harness reload preserved every ID, kind, URL, and Workspace; cross-Workspace +identity stayed scoped. The four final viewer processes and earlier fixture +listeners exited when their Tools closed; both owned harnesses stopped and +private credential captures were deleted. + +Full `pnpm test` and `pnpm build` passed during the run, plus 139 focused UI +checks. The PDF-policy follow-up passed 168 CLI tests, 20 host dispatch/proxy +tests, and spec/public-doc lints; staged hosts contained no PDF renderer assets. +These counts describe that run, not the current test inventory. + +One development-state reset made Tools appear as terminals while root tests +and builds ran beside the harness. Investigation confirmed that +`e2e-lint-selftest` temporarily mutates Vite inputs, including invalid root +package JSON; the exact metadata-loss trigger was not captured. A clean +restart and stable-build reload passed. This run did not exercise Tool transfer, +native-window movement, native Tauri/VS Code rendering, Windows shells, or cold +restore. Screenshots and raw JSON were local ignored artifacts, not portable +verification evidence. The reusable recipe is `docs/testing/dor-tool-qc.md`. + + The September 2026 integration reuses Terminal Context for the Tool's primary terminal. The auxiliary helper's automatic refresh, Reset, and Promote semantics do not describe a serving command, whose Session also owns the browser and remote terminal identity. Sharing the presentation avoids introducing a second navigation mechanism or a second shell. ## Opening local files diff --git a/docs/specs/layout.md b/docs/specs/layout.md index 9f36a001c..6616ee9cf 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -107,7 +107,7 @@ Both layers wear the leaf's own rounding (header radius on top, terminal radius ### Pane header responsive sizing -**Must pick each header's tier from its own measured width, never the viewport** (rationale). A terminal header has three tiers: +**Must measure each header's own width, never the viewport, retaining its tier at zero width** (rationale). Terminal content-box tiers: - **Full** (>280px): everything. - **Compact** (>160px): split, zoom, and unzoom hidden. @@ -124,7 +124,7 @@ A browser header, including a Tool's (Terminal Context sits outside the measured **Must keep the popover keyboard reachable** (focus enters on open, Tab stays inside, Escape returns it to the trigger) **and dismiss it on a pane resize or, without restoring focus, when its Surface is hidden**; otherwise `lib/src/components/wall/use-dismiss-overlay.ts` applies, and a control inside dismisses only after its action ran. The trigger shows a filled notepad glyph and the note count while the Surface has notes; long keys and connection labels truncate before controls. -Source of truth: `SurfacePaneHeader` in `lib/src/components/wall/SurfacePaneHeader.tsx`; `useHeaderTier` in `lib/src/components/wall/use-header-tier.ts`; `lib/src/components/wall/SurfacePaneHeader.test.tsx`; `lib/src/stories/BrowserChromeHeader.stories.tsx`. +Source of truth: `SurfacePaneHeader` in `lib/src/components/wall/SurfacePaneHeader.tsx`; `TerminalPaneHeader` in `lib/src/components/wall/TerminalPaneHeader.tsx`; `useHeaderTier` in `lib/src/components/wall/use-header-tier.ts`; `lib/src/components/wall/SurfacePaneHeader.test.tsx`; `lib/src/components/wall/TerminalPaneHeader.test.tsx`; `lib/src/stories/BrowserChromeHeader.stories.tsx`. ## Baseboard diff --git a/docs/specs/layout.rationale.md b/docs/specs/layout.rationale.md index 01c387c6d..eae7a6e56 100644 --- a/docs/specs/layout.rationale.md +++ b/docs/specs/layout.rationale.md @@ -6,6 +6,8 @@ A viewport breakpoint says nothing about a narrow split inside a wide window: at a 1200px viewport every control stayed rendered in a 103px pane and overflowed into its neighbor (innerdogfood QC, 2026-09). Tool headers have even less browser width because Terminal Context occupies its own button. Measuring the header and moving fixed controls together keeps long keys, note buttons, and renderer chips from pushing minimize/kill into a neighboring pane; quantizing the measurement to a tier keeps the header from re-rendering on every frame of a sash drag or tween. +In the same run, real clicks exposed premature popup dismissal before the action ran. After repair, Zoom reached 716×403 pixels, Unzoom returned to the compact header, Reload worked, and Display retained modal focus. Header buttons stayed within their panes at the final 1200×800 viewport. + ## Pane body xterm.js paints only its own rendered surface, and integer row fitting leaves a sub-row remainder at the bottom of the pane: a host background differing from the terminal screen shows as a stripe under the last row, and an unclipped host squares off the rounded bottom corners. diff --git a/docs/testing/dor-tool-qc.md b/docs/testing/dor-tool-qc.md index 17a332209..452afd95b 100644 --- a/docs/testing/dor-tool-qc.md +++ b/docs/testing/dor-tool-qc.md @@ -1,86 +1,44 @@ -# Dor Tool innerdogfood QC +# Dor Tool innerdogfood QC recipe -Branch: `dor-tool-qc`, based on the reviewed stack at `4c7f9012`. +Use this fixture with the current contracts in `docs/specs/dor-tool.md` and +`docs/specs/layout.md`. Historical September 2026 results and coverage limits +live in `docs/specs/dor-tool.rationale.md` → Lifecycle; header findings live in +`docs/specs/layout.rationale.md` → Pane header responsive sizing. ## Harness and isolation -Run source-mutating root self-tests before starting the live harness. Run -`pnpm innerdogfood` in a visible `dor ensure` pane. The serving fixture is -`scripts/dor-tool-qc/server.mjs` (`--ports N`, `--label`, `--path`, -`--announce`; `SIGUSR1` re-announces over OSC 367). Use its real sidecar, -PTYs, staged CLI, and browser UI through `dor ab`. Keep fixture files, captured -inner CLI credentials, and the separate XDG user config under this worktree's -ignored `standalone/src-tauri/target/dor-tool-qc/` directory. Never use the -installed application's configuration or trust records. Capture credentials -only to a mode-0600 local file; do not include them in reports. - -## Original QC baseline results - -The flag and placement rows below record the original `4c7f9012` baseline. -They are superseded by the integrated stack: Tools are always available, and -a standalone `dor open` can take over an eligible caller like `dor tool`. -Current behavior is specified in `docs/specs/dor-tool.md`. - -| Area | Exercise | Expected result | Result | -| --- | --- | --- | --- | -| Boot and flag | Start harness; run Tool with flag off, then enable it | Clear disabled error; healthy terminal and CLI after enable | Pass: disabled rejection, then real PTY and Tool startup; disabling later preserves existing Tools. | -| Project trust | Invoke named Tool; repeat pending invocation; decline, then allow folder | No execution before approval; pending dedupes; decline records nothing; approved command serves | Pass: pending deduplication, decline/re-prompt, folder-only approval, failure without PTY, repair and Retry. | -| Inputs | argv with spaces, quotes, dollar signs; canonical target and symlink | Exact argument values; no shell expansion; canonical file identity | Pass: literal shell metacharacters survive; symlink resolves to canonical target and reuse key. | -| Identity | Keyed live reuse, concurrent invocations, `--fresh`, idle restart | One keyed live process; fresh splits; restart keeps Surface/ref | Pass: three concurrent invocations reuse one live Tool; fresh splits; idle and fast-command restarts retain ref. | -| Takeover | Type standalone `dor tool` at a plain prompt; compare compound line and `dor open` | Eligible Tool retains terminal/ref; other paths split | Pass: standalone Tool takes over; compound command and standalone open create separate Tools. | -| Serving | Automatic single port, multiple-port conflict, announced port/path | Only owned ports frame; conflict explains refusal; announcement resolves it | Pass: automatic single port; three-port conflict; OSC announcement resolves the conflict. | -| File dispatch | Ordered user rules, explicit handler, malformed config, project rule isolation | Correct user handler; useful errors; project config cannot intercept opens | Pass: first matching user rule, explicit override, malformed user errors; malformed/project associations do not intercept opens. | -| Built-in viewer | Text/Markdown, HTML+CSS+image, image/audio; awkward filename | Correct content, relative assets load, filename is one argument | Pass: text/Markdown source, HTML/CSS/image, SVG, audio and awkward names. | -| Rejection/bounds | URL, directory, missing/unsupported file, oversized text | Clear errors and no leaked Tool/process | Pass: URL/directory/missing/unsupported rejected; oversized text reports 8 MiB limit and exits. | -| UI and lifecycle | Narrow approval, Terminal Context, minimize/reveal, browser exit/refocus, renderer swap | Reachable controls; same Session; input reaches terminal after exit | Pass: approval at 249×203, same-session context, minimize/reveal state, exit/refocus, narrow popup Zoom/Unzoom/Display focus, and iframe↔screencast round trip. | -| Workspace/reload | Live page reload, move serving Tool, cross-Workspace identity | State/PTY survive live reload; scoped reuse and correct movement | Pass: live reload preserves IDs/kinds/URLs; cross-Workspace identity is scoped. Tool transfer was not exercised; native-window transfer is unavailable in this harness. | -| Cleanup/regression | Close owned Tools, verify listeners retire; rerun affected tests/builds | No orphan fixture servers; fixes covered and retested live | Pass: all final viewer PIDs exited on close; both owned harness runs stopped. Full suite/build and final affected-package suites pass. | - -## Findings and evidence - -### Narrow browser and Tool headers - -At a 103-pixel pane width, header controls extended into the neighboring pane; -Zoom could not be clicked. The original breakpoints measured the viewport. -The fix measures available header width and exposes overflow controls in a -keyboard-accessible popup. Real clicks also exposed premature popup dismissal -before the selected action ran; dismissal now waits until that action completes. -The clean-harness native-click retest passes: Zoom reaches 716×403 pixels, Unzoom returns to the compact header, Reload works, and Display retains modal focus. Header buttons remain inside every pane at the final 1200×800 viewport. Regression coverage lives in -`lib/src/components/wall/SurfacePaneHeader.test.tsx` and narrow header stories. - -### PDFs require a user Tool - -A valid one-page PDF showed Chromium's broken-document icon in the normal -sandboxed iframe. PDFs now require a configured user Tool, per the selected -product behavior. The bundled renderer, dependency, assets and build plumbing -have been removed. Regression tests cover default rejection, explicit -`builtin:file` rejection, user associations, explicit user handlers, and -source-like names such as `README.pdf` and `LICENSE.PDF`. - -### Evidence and limits - -Local screenshots and JSON observations are in the ignored -`standalone/src-tauri/target/dor-tool-qc/` directory. Useful before-fix images: -`html.png`, `pdf-zoom.png`; the diagnostic PDF image is `pdf-diagnostic.png`. -`approval-error.png` captures the persisted error with no PTY. Reload snapshots -are `before-reload.json` and `after-reload.json`. Captured CLI credentials are -private runtime artifacts, not report material. - -Native Tauri/VS Code rendering, native-window transfer, and Windows shell -behavior are outside this browser harness's direct coverage. The tests use the -real standalone sidecar, staged CLI, PTYs, and iframe proxy. - -### Review and automated checks - -- Full `pnpm test` and `pnpm build` passed during the original QC run. PDF-policy follow-up validation is recorded below. -- Focused header/Tool/iframe/Wall coverage: 139 tests pass. -- PDF-policy follow-up: all 168 dor tests and 20 host dispatch/proxy tests pass; spec and public-doc lints pass. Both host CLI staging directories contain no PDF renderer assets. -- An unexpected development-state reset occurred while the full build/test suite ran beside the harness: Tools appeared as terminals. Investigation confirmed `e2e-lint-selftest` temporarily mutates Vite inputs, including invalid root package JSON; the exact metadata-loss trigger was not captured. A clean harness restart restored normal operation. The final stable-build reload preserved every ID, kind, URL and Workspace (`final-before-reload.json` / `final-after-reload.json`). No claim of cold-restore or native-host coverage is made from the disturbed development reload. +1. Run source-mutating root self-tests before starting the live harness. Start + `dor ensure -- pnpm innerdogfood` from the checkout under test. Use the + printed browser command through `dor ab`; the harness provides real sidecar + PTYs, a staged CLI, and the iframe proxy. +2. Keep generated files and a separate XDG user configuration under the ignored + `standalone/src-tauri/target/dor-tool-qc/` directory. Use that configuration + for inner CLI invocations; leave the installed application's configuration + and trust records untouched. Capture inner CLI credentials only to a + mode-0600 local file, never to a report. +3. From an inner terminal, start the fixture as a Tool: + `dor tool -- node scripts/dor-tool-qc/server.mjs --label QC --ports 1`. + Adjust the fixture path if that terminal starts outside the checkout. + +## Serving and interaction checks + +The fixture accepts `--ports N`, `--label TEXT`, `--path PATH`, and `--announce`. +It prints its PID, listening ports, and argv. `--ports 3` creates a port conflict; +`--announce` selects the first port through OSC 367. On POSIX, send `SIGUSR1` to +the printed PID to announce after startup. The page echoes the requested path +and argv, and its text input makes document-state retention visible. + +Use the fixture to exercise narrow header controls, popup actions and focus, +Terminal Context, minimize/reveal, renderer changes, and live reload. Add local +files and temporary user/project `dormouse.yml` declarations for approval, +argument quoting, keyed reuse/restart, and file dispatch. Record the tested +commit, observed outcomes, and coverage limits when collecting new evidence. +The browser harness does not establish native Tauri/VS Code rendering, +native-window transfer, or Windows shell behavior. ## Cleanup -The four final viewer processes exited after their verified Tool Surfaces were -killed; `cleanup-result.json` records the check. Earlier closed fixture listeners -also exited, and both QC harness processes were stopped. Private connection -captures were deleted after shutdown. Screenshot and result artifacts remain -ignored locally for inspection. +Close every Tool created by the run, verify its printed PID and listeners have +exited, then stop the owned harness. Remove private credential captures. Keep +any screenshots and raw observations ignored locally; summarize durable +findings in the owning spec's rationale rather than linking private artifacts. diff --git a/lib/src/components/wall/SurfacePaneHeader.tsx b/lib/src/components/wall/SurfacePaneHeader.tsx index a348580f6..eb8ce3a39 100644 --- a/lib/src/components/wall/SurfacePaneHeader.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.tsx @@ -45,8 +45,8 @@ import { /** What the browser chrome shows inline at a header width (`docs/specs/layout.md` * → "Pane header responsive sizing"). Below `minimal` the chrome moves into the * popover, where it renders at `full`. */ -type InlineTier = 'full' | 'compact' | 'minimal'; -type BrowserHeaderTier = InlineTier | 'overflow' | 'tiny'; +type BrowserInlineTier = 'full' | 'compact' | 'minimal'; +type BrowserHeaderTier = BrowserInlineTier | 'overflow' | 'tiny'; const browserHeaderTier = (width: number): BrowserHeaderTier => width >= 420 ? 'full' : width >= 360 ? 'compact' : width >= 180 ? 'minimal' : width >= 72 ? 'overflow' : 'tiny'; @@ -112,8 +112,8 @@ export function SurfacePaneHeader({ id, title, parked }: PaneProps) { setEditingUrl(false); if (restoreFocus && visibleRef.current) overflowRef.current?.focus(); }, []); - const tier = useHeaderTier(headerRef, browserHeaderTier, () => closeMenu(false)); - const inline: InlineTier | null = tier === 'overflow' || tier === 'tiny' ? null : tier; + const tier = useHeaderTier(headerRef, browserHeaderTier, { onResize: () => closeMenu(false) }); + const inline: BrowserInlineTier | null = tier === 'overflow' || tier === 'tiny' ? null : tier; const popoverOpen = visible && inline === null && menuOpen; const noteCount = useNoteCount(id); const overflowLabel = `Browser controls${noteCount ? `, ${noteCountPhrase(noteCount)}` : ''}`; @@ -121,7 +121,7 @@ export function SurfacePaneHeader({ id, title, parked }: PaneProps) { if (!visible) closeMenu(false); }, [visible, closeMenu]); - const renderBrowserControls = (placement: InlineTier | 'popover') => ( + const renderBrowserControls = (placement: BrowserInlineTier | 'popover') => ( <> {screen && screenSnapshot && chrome ? ( <> diff --git a/lib/src/components/wall/TerminalPaneHeader.test.tsx b/lib/src/components/wall/TerminalPaneHeader.test.tsx index ad98dfda5..22a973c9a 100644 --- a/lib/src/components/wall/TerminalPaneHeader.test.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.test.tsx @@ -157,11 +157,12 @@ describe('TerminalPaneHeader — notepad icon', () => { let resizeHeader: (width: number) => void; beforeEach(() => { - resizeHeader = stubResizeObserver(400); + resizeHeader = stubResizeObserver(400, 13); }); afterEach(() => { vi.unstubAllGlobals(); + vi.restoreAllMocks(); // Still mounted at this point (the outer hook unmounts), so both stores // notify a live header. act(() => { @@ -219,6 +220,34 @@ describe('TerminalPaneHeader — notepad icon', () => { expect(notepadButton()).not.toBeNull(); }); + it('preserves content-box breakpoints and the previous tier while hidden', () => { + renderHeader(stubActions(), null); + const split = () => container.querySelector('[aria-label="Split left/right"]'); + act(() => resizeHeader(280)); + expect(split()).toBeNull(); + expect(notepadButton()).not.toBeNull(); + act(() => resizeHeader(0)); + expect(split()).toBeNull(); + expect(notepadButton()).not.toBeNull(); + act(() => resizeHeader(281)); + expect(split()).not.toBeNull(); + act(() => resizeHeader(160)); + expect(notepadButton()).toBeNull(); + act(() => resizeHeader(161)); + expect(notepadButton()).not.toBeNull(); + }); + + it('measures the initial content width before ResizeObserver delivers', () => { + stubResizeObserver(0); + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue(new DOMRect(0, 0, 293, 30)); + vi.spyOn(window, 'getComputedStyle').mockReturnValue({ + paddingLeft: '8px', paddingRight: '5px', borderLeftWidth: '0px', borderRightWidth: '0px', + } as CSSStyleDeclaration); + renderHeader(stubActions(), null); + expect(container.querySelector('[aria-label="Split left/right"]')).toBeNull(); + expect(notepadButton()).not.toBeNull(); + }); + it('toggles the one open notepad', () => { renderHeader(stubActions(), null); diff --git a/lib/src/components/wall/TerminalPaneHeader.tsx b/lib/src/components/wall/TerminalPaneHeader.tsx index 3c20239ca..5a7b2722c 100644 --- a/lib/src/components/wall/TerminalPaneHeader.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.tsx @@ -16,6 +16,7 @@ import { HeaderActionButton } from '../HeaderActionButton'; import { HEADER_PALETTE_TRANSITION_CLASS, paneZoomButtonClass, POPUP_SURFACE_CLASS, TERMINAL_TOP_RADIUS_CLASS, TODO_PILL_TRACKING_CLASS } from '../design'; import { AlertBell } from '../AlertBell'; import { useTodoPillContent } from '../TodoPillBody'; +import { useHeaderTier } from './use-header-tier'; import { NotepadHeaderButton } from './NotepadHeaderButton'; import type { PaneProps } from './pane-props'; import { IllegalRenameWarning, type RenameRejection } from './IllegalRenameWarning'; @@ -64,7 +65,8 @@ const tabVariant = tv({ }, }); -type HeaderTier = 'full' | 'compact' | 'minimal'; +type TerminalHeaderTier = 'full' | 'compact' | 'minimal'; +const terminalHeaderTier = (width: number): TerminalHeaderTier => width > 280 ? 'full' : width > 160 ? 'compact' : 'minimal'; // WATCHING is a rule on the running command, so the bell says which command it // would act on rather than naming an abstract toggle (`docs/specs/alert.md`). @@ -126,7 +128,7 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { const isRenaming = renamingId === id; const tabRef = useRef<HTMLDivElement>(null); const suppressAlertClickRef = useRef(false); - const [tier, setTier] = useState<HeaderTier>('full'); + const tier = useHeaderTier(tabRef, terminalHeaderTier, { box: 'content-box' }); const [todoPreviewRect, setTodoPreviewRect] = useState<DOMRect | null>(null); const [renameWarning, setRenameWarning] = useState<{ rect: DOMRect; reason: RenameRejection; value: string } | null>(null); const todoPill = useTodoPillContent(activity.todo); @@ -169,19 +171,6 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { } }, [actions, id, context]); - useEffect(() => { - const el = tabRef.current; - if (!el) return; - const ro = new ResizeObserver(([entry]) => { - const w = entry.contentRect.width; - if (w > 280) setTier('full'); - else if (w > 160) setTier('compact'); - else setTier('minimal'); - }); - ro.observe(el); - return () => ro.disconnect(); - }, []); - useEffect(() => { if (!activity.notification) setTodoPreviewRect(null); }, [activity.notification]); diff --git a/lib/src/components/wall/use-header-tier.ts b/lib/src/components/wall/use-header-tier.ts index a4c1c8a32..c77486d61 100644 --- a/lib/src/components/wall/use-header-tier.ts +++ b/lib/src/components/wall/use-header-tier.ts @@ -1,7 +1,7 @@ import { useLayoutEffect, useRef, useState, type RefObject } from 'react'; /** - * A pane header's responsive tier, quantized from its own border-box width + * A pane header's responsive tier, quantized from its own measured width * (`docs/specs/layout.md` → "Pane header responsive sizing"). The Lath animator * resizes leaves every frame of a tween or sash drag, so the observer keeps the * raw width out of React state: the header re-renders only when `tierFor` @@ -12,7 +12,7 @@ import { useLayoutEffect, useRef, useState, type RefObject } from 'react'; export function useHeaderTier<T>( ref: RefObject<HTMLElement | null>, tierFor: (width: number) => T, - onResize?: () => void, + { onResize, box = 'border-box' }: { onResize?: () => void; box?: 'border-box' | 'content-box' } = {}, ): T { const [tier, setTier] = useState<T>(() => tierFor(Number.POSITIVE_INFINITY)); const latest = useRef({ tierFor, onResize }); @@ -21,13 +21,21 @@ export function useHeaderTier<T>( const header = ref.current; if (!header) return; const measure = (width: number) => { if (width > 0) setTier(latest.current.tierFor(width)); }; - measure(header.getBoundingClientRect().width); + let initialWidth = header.getBoundingClientRect().width; + if (box === 'content-box') { + // Terminal tiers predate this hook and exclude their horizontal chrome. + const style = getComputedStyle(header); + for (const value of [style.paddingLeft, style.paddingRight, style.borderLeftWidth, style.borderRightWidth]) { + initialWidth -= Number.parseFloat(value) || 0; + } + } + measure(initialWidth); const observer = new ResizeObserver(([entry]) => { - measure(entry.borderBoxSize?.[0]?.inlineSize ?? entry.contentRect.width); + measure(box === 'content-box' ? entry.contentRect.width : entry.borderBoxSize?.[0]?.inlineSize ?? entry.contentRect.width); latest.current.onResize?.(); }); - observer.observe(header); + observer.observe(header, { box }); return () => observer.disconnect(); - }, [ref]); + }, [ref, box]); return tier; } diff --git a/lib/src/components/wall/wall-test-utils.ts b/lib/src/components/wall/wall-test-utils.ts index 42460ddfd..00f0aaa24 100644 --- a/lib/src/components/wall/wall-test-utils.ts +++ b/lib/src/components/wall/wall-test-utils.ts @@ -46,7 +46,7 @@ export function ensureResizeObserver(): void { * all of them. Stubbed through `vi.stubGlobal`, so `vi.unstubAllGlobals()` in * `afterEach` restores jsdom. */ -export function stubResizeObserver(initialWidth: number): (width: number) => void { +export function stubResizeObserver(initialWidth: number, borderInset = 0): (width: number) => void { let width = initialWidth; const deliveries = new Set<() => void>(); vi.stubGlobal('ResizeObserver', class { @@ -55,7 +55,7 @@ export function stubResizeObserver(initialWidth: number): (width: number) => voi observe(target: Element): void { const deliver = () => this.callback([{ target, - borderBoxSize: [{ inlineSize: width, blockSize: 0 }], + borderBoxSize: [{ inlineSize: width + borderInset, blockSize: 0 }], contentRect: { width }, } as unknown as ResizeObserverEntry], this as unknown as ResizeObserver); this.delivery.add(deliver); From 0ef0c804b0e4e05edf9ca557db658a4d5ed1daf4 Mon Sep 17 00:00:00 2001 From: Ned Twigg <ned.twigg@diffplug.com> Date: Wed, 16 Sep 2026 17:12:15 -0700 Subject: [PATCH 07/11] Distinguish tiny visible headers from hidden panes --- docs/specs/layout.md | 8 ++-- docs/specs/layout.rationale.md | 2 + .../wall/TerminalPaneHeader.test.tsx | 39 ++++++++++++++----- .../components/wall/TerminalPaneHeader.tsx | 6 ++- lib/src/components/wall/use-header-tier.ts | 20 +++------- lib/src/components/wall/wall-test-utils.ts | 4 +- 6 files changed, 47 insertions(+), 32 deletions(-) diff --git a/docs/specs/layout.md b/docs/specs/layout.md index 6616ee9cf..3ac048cfb 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -107,11 +107,11 @@ Both layers wear the leaf's own rounding (header radius on top, terminal radius ### Pane header responsive sizing -**Must measure each header's own width, never the viewport, retaining its tier at zero width** (rationale). Terminal content-box tiers: +**Must measure each header's own border-box width, never the viewport, retaining its tier at zero width** (rationale). Terminal tiers: -- **Full** (>280px): everything. -- **Compact** (>160px): split, zoom, and unzoom hidden. -- **Minimal** (≤160px): also hides the TODO pill and the mouse-override icon, leaving alert, minimize, and kill. **The notepad icon survives this tier only while the Surface has notes** (`docs/specs/notepad.md` → "Notepad UI"). The label truncates with ellipsis. +- **Full** (>293px): everything. +- **Compact** (>173px): split, zoom, and unzoom hidden. +- **Minimal** (≤173px): also hides the TODO pill and the mouse-override icon, leaving alert, minimize, and kill. **The notepad icon survives this tier only while the Surface has notes** (`docs/specs/notepad.md` → "Notepad UI"). The label truncates with ellipsis. A browser header, including a Tool's (Terminal Context sits outside the measured width), collapses by border-box width: diff --git a/docs/specs/layout.rationale.md b/docs/specs/layout.rationale.md index eae7a6e56..fd655b3dd 100644 --- a/docs/specs/layout.rationale.md +++ b/docs/specs/layout.rationale.md @@ -8,6 +8,8 @@ A viewport breakpoint says nothing about a narrow split inside a wide window: at In the same run, real clicks exposed premature popup dismissal before the action ran. After repair, Zoom reached 716×403 pixels, Unzoom returned to the compact header, Reload worked, and Display retained modal focus. Header buttons stayed within their panes at the final 1200×800 viewport. +Terminal border-box thresholds of 293/173 pixels preserve the former 280/160 content-box thresholds plus 13 pixels of horizontal padding. A content box can clamp to zero in a visible tiny leaf; treating that as hidden retained the full tier. Positive border-box width distinguishes that case from a hidden leaf. + ## Pane body xterm.js paints only its own rendered surface, and integer row fitting leaves a sub-row remainder at the bottom of the pane: a host background differing from the terminal screen shows as a stripe under the last row, and an unclipped host squares off the rounded bottom corners. diff --git a/lib/src/components/wall/TerminalPaneHeader.test.tsx b/lib/src/components/wall/TerminalPaneHeader.test.tsx index 22a973c9a..792c161b3 100644 --- a/lib/src/components/wall/TerminalPaneHeader.test.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.test.tsx @@ -157,7 +157,7 @@ describe('TerminalPaneHeader — notepad icon', () => { let resizeHeader: (width: number) => void; beforeEach(() => { - resizeHeader = stubResizeObserver(400, 13); + resizeHeader = stubResizeObserver(400); }); afterEach(() => { @@ -220,34 +220,53 @@ describe('TerminalPaneHeader — notepad icon', () => { expect(notepadButton()).not.toBeNull(); }); - it('preserves content-box breakpoints and the previous tier while hidden', () => { + it('preserves visual breakpoints and the previous tier while hidden', () => { renderHeader(stubActions(), null); const split = () => container.querySelector('[aria-label="Split left/right"]'); - act(() => resizeHeader(280)); + act(() => resizeHeader(293)); expect(split()).toBeNull(); expect(notepadButton()).not.toBeNull(); act(() => resizeHeader(0)); expect(split()).toBeNull(); expect(notepadButton()).not.toBeNull(); - act(() => resizeHeader(281)); + act(() => resizeHeader(294)); expect(split()).not.toBeNull(); - act(() => resizeHeader(160)); + act(() => resizeHeader(173)); expect(notepadButton()).toBeNull(); - act(() => resizeHeader(161)); + act(() => resizeHeader(174)); expect(notepadButton()).not.toBeNull(); }); - it('measures the initial content width before ResizeObserver delivers', () => { + it('measures the initial border width before ResizeObserver delivers', () => { stubResizeObserver(0); vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue(new DOMRect(0, 0, 293, 30)); - vi.spyOn(window, 'getComputedStyle').mockReturnValue({ - paddingLeft: '8px', paddingRight: '5px', borderLeftWidth: '0px', borderRightWidth: '0px', - } as CSSStyleDeclaration); renderHeader(stubActions(), null); expect(container.querySelector('[aria-label="Split left/right"]')).toBeNull(); expect(notepadButton()).not.toBeNull(); }); + it.each([true, false])('handles zero content width with borderBoxSize available=%s', (hasBorderBox) => { + let resize: (width: number) => void; + const rect = vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue(new DOMRect(0, 0, 400, 30)); + vi.stubGlobal('ResizeObserver', class { + constructor(private readonly callback: ResizeObserverCallback) {} + observe(target: Element) { + resize = (width) => this.callback([{ + target, + borderBoxSize: hasBorderBox ? [{ inlineSize: width, blockSize: 30 }] : undefined, + contentRect: { width: Math.max(0, width - 13) }, + } as unknown as ResizeObserverEntry], this as unknown as ResizeObserver); + } + disconnect() {} + }); + renderHeader(stubActions(), null); + expect(container.querySelector('[aria-label="Split left/right"]')).not.toBeNull(); + rect.mockReturnValue(new DOMRect(0, 0, 8, 30)); + act(() => resize(8)); + expect(container.querySelector('[aria-label="Split left/right"]')).toBeNull(); + expect(notepadButton()).toBeNull(); + }); + it('toggles the one open notepad', () => { renderHeader(stubActions(), null); diff --git a/lib/src/components/wall/TerminalPaneHeader.tsx b/lib/src/components/wall/TerminalPaneHeader.tsx index 5a7b2722c..39018d16a 100644 --- a/lib/src/components/wall/TerminalPaneHeader.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.tsx @@ -66,7 +66,9 @@ const tabVariant = tv({ }); type TerminalHeaderTier = 'full' | 'compact' | 'minimal'; -const terminalHeaderTier = (width: number): TerminalHeaderTier => width > 280 ? 'full' : width > 160 ? 'compact' : 'minimal'; +// Includes the header's 8px left + 5px right padding; the former content-box +// boundaries were 280/160px. Border-box width distinguishes tiny from hidden. +const terminalHeaderTier = (width: number): TerminalHeaderTier => width > 293 ? 'full' : width > 173 ? 'compact' : 'minimal'; // WATCHING is a rule on the running command, so the bell says which command it // would act on rather than naming an abstract toggle (`docs/specs/alert.md`). @@ -128,7 +130,7 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { const isRenaming = renamingId === id; const tabRef = useRef<HTMLDivElement>(null); const suppressAlertClickRef = useRef(false); - const tier = useHeaderTier(tabRef, terminalHeaderTier, { box: 'content-box' }); + const tier = useHeaderTier(tabRef, terminalHeaderTier); const [todoPreviewRect, setTodoPreviewRect] = useState<DOMRect | null>(null); const [renameWarning, setRenameWarning] = useState<{ rect: DOMRect; reason: RenameRejection; value: string } | null>(null); const todoPill = useTodoPillContent(activity.todo); diff --git a/lib/src/components/wall/use-header-tier.ts b/lib/src/components/wall/use-header-tier.ts index c77486d61..3f89a2dfb 100644 --- a/lib/src/components/wall/use-header-tier.ts +++ b/lib/src/components/wall/use-header-tier.ts @@ -1,7 +1,7 @@ import { useLayoutEffect, useRef, useState, type RefObject } from 'react'; /** - * A pane header's responsive tier, quantized from its own measured width + * A pane header's responsive tier, quantized from its own border-box width * (`docs/specs/layout.md` → "Pane header responsive sizing"). The Lath animator * resizes leaves every frame of a tween or sash drag, so the observer keeps the * raw width out of React state: the header re-renders only when `tierFor` @@ -12,7 +12,7 @@ import { useLayoutEffect, useRef, useState, type RefObject } from 'react'; export function useHeaderTier<T>( ref: RefObject<HTMLElement | null>, tierFor: (width: number) => T, - { onResize, box = 'border-box' }: { onResize?: () => void; box?: 'border-box' | 'content-box' } = {}, + { onResize }: { onResize?: () => void } = {}, ): T { const [tier, setTier] = useState<T>(() => tierFor(Number.POSITIVE_INFINITY)); const latest = useRef({ tierFor, onResize }); @@ -21,21 +21,13 @@ export function useHeaderTier<T>( const header = ref.current; if (!header) return; const measure = (width: number) => { if (width > 0) setTier(latest.current.tierFor(width)); }; - let initialWidth = header.getBoundingClientRect().width; - if (box === 'content-box') { - // Terminal tiers predate this hook and exclude their horizontal chrome. - const style = getComputedStyle(header); - for (const value of [style.paddingLeft, style.paddingRight, style.borderLeftWidth, style.borderRightWidth]) { - initialWidth -= Number.parseFloat(value) || 0; - } - } - measure(initialWidth); + measure(header.getBoundingClientRect().width); const observer = new ResizeObserver(([entry]) => { - measure(box === 'content-box' ? entry.contentRect.width : entry.borderBoxSize?.[0]?.inlineSize ?? entry.contentRect.width); + measure(entry.borderBoxSize?.[0]?.inlineSize ?? header.getBoundingClientRect().width); latest.current.onResize?.(); }); - observer.observe(header, { box }); + observer.observe(header, { box: 'border-box' }); return () => observer.disconnect(); - }, [ref, box]); + }, [ref]); return tier; } diff --git a/lib/src/components/wall/wall-test-utils.ts b/lib/src/components/wall/wall-test-utils.ts index 00f0aaa24..42460ddfd 100644 --- a/lib/src/components/wall/wall-test-utils.ts +++ b/lib/src/components/wall/wall-test-utils.ts @@ -46,7 +46,7 @@ export function ensureResizeObserver(): void { * all of them. Stubbed through `vi.stubGlobal`, so `vi.unstubAllGlobals()` in * `afterEach` restores jsdom. */ -export function stubResizeObserver(initialWidth: number, borderInset = 0): (width: number) => void { +export function stubResizeObserver(initialWidth: number): (width: number) => void { let width = initialWidth; const deliveries = new Set<() => void>(); vi.stubGlobal('ResizeObserver', class { @@ -55,7 +55,7 @@ export function stubResizeObserver(initialWidth: number, borderInset = 0): (widt observe(target: Element): void { const deliver = () => this.callback([{ target, - borderBoxSize: [{ inlineSize: width + borderInset, blockSize: 0 }], + borderBoxSize: [{ inlineSize: width, blockSize: 0 }], contentRect: { width }, } as unknown as ResizeObserverEntry], this as unknown as ResizeObserver); this.delivery.add(deliver); From 5e2e6d9c5f4b0e8a0caa1b9c5b6b4a89bbd36945 Mon Sep 17 00:00:00 2001 From: Ned Twigg <ned.twigg@diffplug.com> Date: Wed, 16 Sep 2026 17:27:51 -0700 Subject: [PATCH 08/11] Reclamp browser controls as popover content resizes --- docs/specs/layout.md | 2 +- .../wall/SurfacePaneHeader.test.tsx | 48 +++++++++++++++++++ lib/src/components/wall/SurfacePaneHeader.tsx | 17 +++++-- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/docs/specs/layout.md b/docs/specs/layout.md index 3ac048cfb..d5a96ceeb 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -119,7 +119,7 @@ A browser header, including a Tool's (Terminal Context sits outside the measured |---|---| | 420px | Split and zoom hidden. | | 360px | Navigation hidden. | -| 180px | Chrome moves into a viewport-clamped popover behind one trigger; minimize and kill stay inline. | +| 180px | Chrome moves behind one trigger into a viewport-clamped popover, remeasured on content resize; minimize/kill stay inline. | | 72px | Minimize and kill join the popover. | **Must keep the popover keyboard reachable** (focus enters on open, Tab stays inside, Escape returns it to the trigger) **and dismiss it on a pane resize or, without restoring focus, when its Surface is hidden**; otherwise `lib/src/components/wall/use-dismiss-overlay.ts` applies, and a control inside dismisses only after its action ran. The trigger shows a filled notepad glyph and the note count while the Surface has notes; long keys and connection labels truncate before controls. diff --git a/lib/src/components/wall/SurfacePaneHeader.test.tsx b/lib/src/components/wall/SurfacePaneHeader.test.tsx index b762524cf..f8a3465c1 100644 --- a/lib/src/components/wall/SurfacePaneHeader.test.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.test.tsx @@ -58,6 +58,7 @@ afterEach(() => { act(() => root.unmount()); container.remove(); vi.unstubAllGlobals(); + vi.restoreAllMocks(); }); function renderHeader( @@ -186,6 +187,53 @@ describe('SurfacePaneHeader — browser chrome', () => { registration.dispose(); }); + it('reclamps changing popup content near the viewport edge without moving editor focus', () => { + const registration = register('pane-popup-geometry'); + vi.stubGlobal('innerWidth', 300); + vi.stubGlobal('innerHeight', 300); + const originalRect = HTMLElement.prototype.getBoundingClientRect; + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function (this: HTMLElement) { + if (this.getAttribute('role') === 'dialog') { + return new DOMRect(0, 0, 276, this.querySelector('input') ? 40 : 80); + } + if (this.getAttribute('aria-label')?.startsWith('Browser controls')) return new DOMRect(280, 240, 20, 20); + return originalRect.call(this); + }); + renderHeader(headerProps('pane-popup-geometry', 'Browser'), stubActions()); + act(() => resizeHeader(79)); + let resizePopup: () => void; + const disconnect = vi.fn(); + vi.stubGlobal('ResizeObserver', class { + constructor(private readonly callback: ResizeObserverCallback) {} + observe(target: Element) { + resizePopup = () => this.callback([{ target } as ResizeObserverEntry], this as unknown as ResizeObserver); + } + disconnect = disconnect; + }); + try { + openPopup(); + expect(popup()!.style.top).toBe('208px'); + expect(popup()!.style.left).toBe('12px'); + expect(popup()!.style.maxWidth).toBe('calc(100vw - 24px)'); + act(() => inPopup('[role="button"]')!.click()); + const input = inPopup('input')!; + expect(document.activeElement).toBe(input); + act(() => resizePopup()); + expect(popup()!.style.top).toBe('248px'); + expect(document.activeElement).toBe(input); + act(() => inPopup('[aria-label="Back"]')!.focus()); + expect(inPopup('input')).toBeNull(); + act(() => resizePopup()); + expect(popup()!.style.top).toBe('208px'); + expect(Number.parseFloat(popup()!.style.top) + 80).toBe(288); + act(() => overflowTrigger().click()); + expect(popup()).toBeNull(); + expect(disconnect).toHaveBeenCalled(); + } finally { + registration.dispose(); + } + }); + it('names notes on the trigger and opens the notepad from the popover', async () => { const registration = register('pane-notes'); renderHeader(headerProps('pane-notes', 'Browser'), stubActions()); diff --git a/lib/src/components/wall/SurfacePaneHeader.tsx b/lib/src/components/wall/SurfacePaneHeader.tsx index eb8ce3a39..aa5bdeab1 100644 --- a/lib/src/components/wall/SurfacePaneHeader.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.tsx @@ -310,10 +310,19 @@ function BrowserHeaderPopover({ anchorRef, onClose, children }: { usePopoverFocusTrap(ref, onClose); useDismissOverlay(onClose, ref); useLayoutEffect(() => { - const anchor = anchorRef.current!.getBoundingClientRect(); - const rect = ref.current!.getBoundingClientRect(); - setPosition(clampOverlayPosition({ left: anchor.left, top: anchor.bottom + POPOVER_GAP_PX, width: rect.width, height: rect.height })); - ref.current!.querySelector<HTMLElement>(POPOVER_FOCUSABLE_SELECTOR)?.focus(); + const element = ref.current!; + const positionPopover = () => { + const anchor = anchorRef.current!.getBoundingClientRect(); + const rect = element.getBoundingClientRect(); + setPosition(clampOverlayPosition({ left: anchor.left, top: anchor.bottom + POPOVER_GAP_PX, width: rect.width, height: rect.height })); + }; + positionPopover(); + const observer = new ResizeObserver(positionPopover); + observer.observe(element, { box: 'border-box' }); + // Content resizing (URL editing, notes, or connection labels) changes only + // geometry. Moving focus again would cancel the URL editor on its blur. + element.querySelector<HTMLElement>(POPOVER_FOCUSABLE_SELECTOR)?.focus(); + return () => observer.disconnect(); }, [anchorRef]); return createPortal( <div ref={ref} role="dialog" aria-label="Browser controls" From e8a3509ff6f2335cc51fb8eafc65c6e7c87f6dab Mon Sep 17 00:00:00 2001 From: Ned Twigg <ned.twigg@diffplug.com> Date: Wed, 16 Sep 2026 17:37:16 -0700 Subject: [PATCH 09/11] Pin popover observer cleanup at close --- docs/specs/layout.md | 4 ++-- lib/src/components/wall/SurfacePaneHeader.test.tsx | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/specs/layout.md b/docs/specs/layout.md index d5a96ceeb..ef78e0952 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -119,10 +119,10 @@ A browser header, including a Tool's (Terminal Context sits outside the measured |---|---| | 420px | Split and zoom hidden. | | 360px | Navigation hidden. | -| 180px | Chrome moves behind one trigger into a viewport-clamped popover, remeasured on content resize; minimize/kill stay inline. | +| 180px | Chrome moves into a viewport-clamped popover behind one trigger; minimize and kill stay inline. | | 72px | Minimize and kill join the popover. | -**Must keep the popover keyboard reachable** (focus enters on open, Tab stays inside, Escape returns it to the trigger) **and dismiss it on a pane resize or, without restoring focus, when its Surface is hidden**; otherwise `lib/src/components/wall/use-dismiss-overlay.ts` applies, and a control inside dismisses only after its action ran. The trigger shows a filled notepad glyph and the note count while the Surface has notes; long keys and connection labels truncate before controls. +**Must reclamp the popover when its content resizes and keep it keyboard reachable** (focus enters on open, Tab stays inside, Escape returns it to the trigger) **and dismiss it on a pane resize or, without restoring focus, when its Surface is hidden**; otherwise `lib/src/components/wall/use-dismiss-overlay.ts` applies, and a control inside dismisses only after its action ran. The trigger shows a filled notepad glyph and the note count while the Surface has notes; long keys and connection labels truncate before controls. Source of truth: `SurfacePaneHeader` in `lib/src/components/wall/SurfacePaneHeader.tsx`; `TerminalPaneHeader` in `lib/src/components/wall/TerminalPaneHeader.tsx`; `useHeaderTier` in `lib/src/components/wall/use-header-tier.ts`; `lib/src/components/wall/SurfacePaneHeader.test.tsx`; `lib/src/components/wall/TerminalPaneHeader.test.tsx`; `lib/src/stories/BrowserChromeHeader.stories.tsx`. diff --git a/lib/src/components/wall/SurfacePaneHeader.test.tsx b/lib/src/components/wall/SurfacePaneHeader.test.tsx index f8a3465c1..034f21f02 100644 --- a/lib/src/components/wall/SurfacePaneHeader.test.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.test.tsx @@ -226,9 +226,10 @@ describe('SurfacePaneHeader — browser chrome', () => { act(() => resizePopup()); expect(popup()!.style.top).toBe('208px'); expect(Number.parseFloat(popup()!.style.top) + 80).toBe(288); + const disconnectsBeforeClose = disconnect.mock.calls.length; act(() => overflowTrigger().click()); expect(popup()).toBeNull(); - expect(disconnect).toHaveBeenCalled(); + expect(disconnect.mock.calls.length).toBeGreaterThan(disconnectsBeforeClose); } finally { registration.dispose(); } From 8fb697dbafa21d672a2372848490e49578c0eefa Mon Sep 17 00:00:00 2001 From: Ned Twigg <ned.twigg@diffplug.com> Date: Wed, 16 Sep 2026 19:23:16 -0700 Subject: [PATCH 10/11] Restore browser header snapshot thickness --- lib/src/stories/BrowserChromeHeader.stories.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/stories/BrowserChromeHeader.stories.tsx b/lib/src/stories/BrowserChromeHeader.stories.tsx index 39f2c5827..0a38c9ffb 100644 --- a/lib/src/stories/BrowserChromeHeader.stories.tsx +++ b/lib/src/stories/BrowserChromeHeader.stories.tsx @@ -9,7 +9,6 @@ import { } from '../components/wall/wall-context'; import { SurfacePaneHeader } from '../components/wall/SurfacePaneHeader'; import { ToolPaneHeader } from '../components/wall/ToolPaneHeader'; -import { PANE_HEADER_HEIGHT_PX } from '../components/design'; import { registerAgentBrowserScreen, type ChromeSnapshot, @@ -155,7 +154,8 @@ function BrowserChromeStory(args: StoryArgs) { story's un-zoomed header. */} <WallActionsContext.Provider value={loggingActions}> <div style={{ width: args.width }}> - <div className="bg-app-bg" style={{ height: PANE_HEADER_HEIGHT_PX }}> + {/* Preserve the compact 26px visual baseline for these isolated headers. */} + <div className="bg-app-bg" style={{ height: 26 }}> <Header id={surfaceId} title={args.htmlTitle || hostPathDisplay(args.url)} From 229c08ae65ad12efc03ddc64d98869bc755c8be6 Mon Sep 17 00:00:00 2001 From: Ned Twigg <ned.twigg@diffplug.com> Date: Wed, 16 Sep 2026 20:21:16 -0700 Subject: [PATCH 11/11] Preserve inline browser header icon spacing --- lib/src/components/wall/SurfacePaneHeader.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/src/components/wall/SurfacePaneHeader.tsx b/lib/src/components/wall/SurfacePaneHeader.tsx index aa5bdeab1..f2d5311cf 100644 --- a/lib/src/components/wall/SurfacePaneHeader.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.tsx @@ -252,8 +252,9 @@ export function SurfacePaneHeader({ id, title, parked }: PaneProps) { </> ); + // Preserve the 4px separation with inline chrome; collapsed controls align right. const paneActions = ( - <div className="ml-auto flex shrink-0 items-center gap-0.5"> + <div className={`${inline ? 'ml-1' : 'ml-auto'} flex shrink-0 items-center gap-0.5`}> <HeaderActionButton className="flex h-5 min-w-5 items-center justify-center rounded transition-colors hover:bg-current/10" onClick={(e) => { e.stopPropagation(); closeMenu(); actions.onMinimize(id); }}