gsadmin/pad
1
0
Fork 0
mirror of https://github.com/PerpetualSoftware/pad.git synced 2026-09-24 19:32:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(attachments): complete the MIME matrix and address fence (TASK-2434)

Browse Source 
TASK-2433 made the inline-image activation path resolve the attachment's
MIME before emitting and refuse anything not positively allowlisted. That
refusal was binary: `ok` + raster opened the viewer and every other result
returned silently, leaving two states where a focused, button-announced
image swallowed the gesture — a `transient` probe, and a resolved MIME the
viewer will not take.

This completes the four-branch matrix, each arm with a destination:

  - `ok` + allowlisted raster → the viewer, unchanged.
  - `ok` + anything else      → the options PANEL (DR-7). A REDIRECT, not a
    refusal: an SVG or a PDF referenced as an inline image is a real
    attachment with real options, it is just not something to hand a viewer
    that would execute it. The image therefore stays a real activation
    target and its accessible name names the panel — taking the semantics
    off (as the binary gate did) would hide a working control, and would
    make the redirect fire exactly once before the recorded MIME closed the
    gate on every later tap.
  - `missing` (authoritative 404) → the permanent placeholder, latched,
    nothing opened.
  - `transient` → the RETRYABLE placeholder. Never an open and never a
    latch: only a 404 is authoritative (DR-17).

The fence is hardened to the FULL address. The whole address is captured
before the await and compared after, and both emissions stamp the CAPTURED
values — the reader is live (`CommentEditor` is reused across an item
switch) so a re-read can address the wrong host. The continuation also
re-checks `deleted` on its own terms: a delete does not change which
attachment the node points at, so a probe issued before it can resolve `ok`
afterwards with the uuid still current. Check and emit stay adjacent and
synchronous — no timer, no microtask between them.

Also adds the minimal pending contract the await needs: `aria-busy` plus a
wait cursor while the MIME resolves, cleared by the resolution's finalizer
and by a uuid swap. No new chrome.

Seam with PLAN-2411, stated in comments and deliberately not built: the
`deleted` latch is cleared ONLY by an authoritative restore signal on
2411's channel, never by editor undo — DR-17 requires Ctrl-Z to leave an
inert placeholder rather than resurrect a working attachment.

Claude-Session: https://claude.ai/code/session_01LmbFxQFDjcYKBLcTnor6DC
This commit is contained in:
xarmian
2026-08-05 23:57:09 +00:00
parent 30fa9fc3dd
commit 03e0edb605
3 changed files with 1001 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/lib/components/editor/attachment-image.ts
+302 -43
View File
@@ -38,7 +38,11 @@ import {
mimeToFormat
} from './attachment-metadata';
import { openCropModal, type CropResult } from './attachment-crop-modal';
import { notifyViewerOpen, registerAttachmentDeletionListener } from '$lib/attachments/events';
import {
notifyAttachmentPanelOpen,
notifyViewerOpen,
registerAttachmentDeletionListener
} from '$lib/attachments/events';
import {
type AttachmentHostAddressReader,
readUnaddressed
@@ -290,6 +294,16 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
// Latched by a confirmed deletion (not by a mere load failure).
// Deletion is authoritative: a load still in flight when it lands
// must not be allowed to paint the image back (Codex round 15).
//
// SEAM WITH PLAN-2411 (stated, not built here): this latch is cleared
// ONLY by an authoritative RESTORE signal on 2411's channel — never by
// editor undo. DR-17 requires Ctrl-Z to leave an inert placeholder
// rather than resurrect a working attachment: the delete was a REST row
// mutation that Tiptap/Yjs history cannot roll back, so an undo that
// re-inserted the node would otherwise present a live-looking image for
// a row the server no longer has. `deleted` is closure-private and the
// bus is deletion-only, so 2411 must extend both; that is its work, and
// nothing in THIS file may clear the latch on a document event.
let deleted = false;
// True once the NodeView is torn down. Async continuations (HEAD
// probes, transform results) must not touch DOM after that.
@@ -329,22 +343,24 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
* role+tabindex would be a focus stop that announces itself as a
* button and does nothing. (The placeholder itself carries the
* retry affordance while the failure is still transient.)
* - a KNOWN MIME outside the viewer allowlist — activate() refuses it
* (DR-16), so the same dead stop applies. An UNPROBED node keeps the
* semantics: "not yet asked" is not "not viewable", and the probe is
* lazy. This mirrors ItemTimeline.svelte's semantics pass, which is
* likewise able to take the semantics back OFF when a probe resolves.
*
* Consequence, accepted deliberately: because the probe is lazy, a
* refused image can be FOCUSED before its MIME resolves and lose
* focus when it does. The alternative is leaving an SVG as a focus
* stop that announces itself as a button and does nothing, which is
* the failure this whole rule exists to prevent — and the same
* trade the placeholder below already makes.
* A KNOWN MIME outside the viewer allowlist is NOT one of them, as of
* TASK-2434. It used to be: the gate was binary — viewer or nothing —
* so a resolved `image/svg+xml` made the image a control that refused,
* and the semantics had to come off. The matrix replaced that refusal
* with a REDIRECT (DR-7): a non-allowlisted attachment is in the
* options PANEL's scope, so the image stays a perfectly real activation
* target — only its destination changes. Taking the semantics off now
* would hide a working control instead of retiring a dead one, and
* would break it for the mouse too, since click and key share one gate:
* the first tap would open the panel, resolve the MIME, and leave the
* image inert for every tap after it.
*
* The accessible name comes from alt with a GENERIC fallback: there is
* no filename on the node's attrs and the HEAD metadata does not carry
* one either, so the filename form DR-12 sketches has no source here.
* It also has to name the DESTINATION rather than always promising a
* viewer — a resolved non-raster type announces the panel it actually
* opens.
*/
/**
* Is this image an activation target right now?
@@ -356,6 +372,13 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
* below was, briefly, only in the semantics half — so the placeholder
* hid the image and stripped its role, while a stale or synthetic
* event on the hidden <img> still opened a viewer.
*
* It answers "does activation DO something", not "does it open the
* viewer". The MIME decides WHICH surface (see activate()), and it is
* deliberately absent from here: this predicate is also what a
* post-await continuation re-checks, and folding a viewer-only clause
* into it would make the panel branch unreachable the moment it wrote
* the MIME it was branching on.
*/
function canActivate(): boolean {
return (
@@ -364,11 +387,20 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
// Hidden means the placeholder has taken over: either a
// confirmed deletion or a load failure. Neither has an image
// to show, so neither has anything to open.
img.style.display !== 'none' &&
!(knownMime && !canOpenInViewer(knownMime))
img.style.display !== 'none'
);
}
/**
* What activation would open, as far as anything KNOWN says. Unprobed
* reads as the viewer — "not yet asked" is not "not viewable", and the
* probe is lazy — which is exactly what the name has to say before the
* HEAD lands. activate() never trusts it: it resolves the MIME itself.
*/
function announcesPanel(): boolean {
return !!knownMime && !canOpenInViewer(knownMime);
}
function applyImageSemantics() {
if (!canActivate()) {
// Removing tabindex from the focused element would strand focus on
@@ -383,10 +415,37 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
img.setAttribute('tabindex', '0');
img.setAttribute(
'aria-label',
currentAlt ? `View image: ${currentAlt}` : 'View attachment image'
announcesPanel()
? currentAlt
? `Attachment options: ${currentAlt}`
: 'Attachment options'
: currentAlt
? `View image: ${currentAlt}`
: 'View attachment image'
);
}
/**
* The pending contract, minimal (TASK-2434).
*
* Activation awaits a HEAD. On a cache hit that is one microtask and
* nothing is visible; cold, it is a round trip during which a click
* that does nothing reads as broken. This NodeView has no
* metadata-pending affordance — the placeholder below is for LOAD
* failure, a different thing — so the smallest honest one: `aria-busy`
* for assistive tech and a wait cursor for everyone else. No spinner
* chrome in a parity commit.
*/
function setActivationPending(pending: boolean): void {
if (pending) {
img.setAttribute('aria-busy', 'true');
img.style.cursor = 'progress';
return;
}
img.removeAttribute('aria-busy');
img.style.cursor = '';
}
function showMissing() {
missing.textContent = `📎 ${currentAlt || 'Attachment unavailable'}`;
// Distinct copy per cause: a confirmed deletion is permanent and
@@ -533,6 +592,48 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
const base = opts.getDownloadUrl(currentUuid, 'thumb-md');
loadImage(`${base}${base.includes('?') ? '&' : '?'}retry=${Date.now()}`);
}
/**
* The `transient` arm of the matrix (TASK-2434 / DR-17).
*
* `transient` says NOTHING about whether the row exists — a 5xx, a
* proxy hiccup, a network throw — so it must never latch and must
* never open. Before this task it also did nothing at all, which left
* the worst of the three: a focused image announcing itself as a button
* whose activation silently returned, with no way for the user to learn
* that anything failed or to try again. Repeat presses would have gone
* on doing nothing forever.
*
* So it hands over to the RETRYABLE placeholder — the one affordance
* this NodeView already has for "this did not work, click to retry",
* reached here by a different route than a failed `load`. Nothing is
* latched: `deleted` stays false, the transient result is not cached
* (`fetchAttachmentMetadata` evicts it on settle), and Retry re-issues
* both the image load and, on a second failure, the HEAD.
*
* Retry does NOT resume the activation, deliberately. It is the
* placeholder's existing affordance and it means "load this image
* again", not "open it" — a reload control that opened a viewer would
* be doing something the user did not ask for. The user gets the image
* back and activates it again if they still want to, and that second
* gesture really does re-probe: a transient result is evicted from the
* metadata cache as it settles, so nothing replays the failure.
*
* The cost, accepted: an image that had rendered FINE is replaced by
* the placeholder when only its HEAD failed. It is one click to undo
* and it is recoverable; a control that silently does nothing is
* neither. Anything gentler would be a third failure state — an inline
* error, a toast — which is new chrome this commit does not add.
*/
function showTransientProbeFailure(): void {
// Keyboard activation is the case that matters: showMissing() takes
// the semantics off the <img> and blurs it, so without this the
// keypress that reported the failure also drops focus to <body>.
// The placeholder is the retry control, so focus belongs on it.
const hadFocus = document.activeElement === img;
showMissing();
if (hadFocus) missing.focus();
}
missing.addEventListener('click', retryLoad);
missing.addEventListener('keydown', (event) => {
if (event.key === 'Enter' || event.key === ' ') {
@@ -570,8 +671,12 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
* viewer that mounts and renders no image. The producer's half of that
* contract is this function: either the cache answers (the common
* case — the toolbar probe and the strip both warm the same entry) or
* we await one HEAD, and we emit ONLY on a positively-known
* allowlisted answer.
* we await one HEAD, and we ask the VIEWER for only a
* positively-known allowlisted answer. (Since TASK-2434 a
* positively-known NON-allowlisted answer is not dropped — it goes to
* the options panel instead. The rule the viewer cares about is
* unchanged: nothing unresolved and nothing outside the allowlist
* reaches it.)
*
* The channel enforces the same rule at the boundary as of TASK-2433 —
* `notifyViewerOpen` takes a set whose `mime_type` is non-nullable and
@@ -580,14 +685,26 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
* open. It is still where the answer is OBTAINED, and the payload needs
* it either way.
*
* What that COSTS, deliberately and temporarily: an image whose probe
* comes back `transient` (or whose surface has no workspace to probe
* with) keeps its button semantics and does not open. That is a dead
* focus stop, and it is TASK-2434's — the four-branch matrix
* (ok → viewer, unsafe → panel redirect, missing → inert placeholder,
* transient → retryable) is what makes this gate TOTAL. This task is
* the surface swap, and the swap must not be the thing that reopens
* the hole TASK-2431 closed.
* TASK-2434 made the gate TOTAL rather than binary. Every probe result
* now has a destination, and none of them is "return quietly":
*
* - `ok` + allowlisted raster → the viewer.
* - `ok` + anything else → the options PANEL (DR-7). A
* REDIRECT, not a refusal: an SVG or a PDF referenced as an inline
* image is a real attachment with real options, it is just not
* something to hand a viewer that would execute it.
* - `missing` (an authoritative 404) → the permanent placeholder,
* latched. Nothing opens.
* - `transient` → the RETRYABLE placeholder. Never an open, and
* never a latch: only a 404 is authoritative (DR-17).
*
* The two that were dead focus stops before this — `transient`, and a
* resolved non-allowlisted MIME — are the two the matrix exists for.
*
* The one remaining silent return is a surface with NO WORKSPACE to
* probe with (SSR / preview). It is not a dead stop in practice: those
* surfaces have no host mounted to receive either event, so there is
* nothing to route to and nothing to say.
*
* It also closes a mid-phase bypass Codex found: the old gate read
* `knownMime` only when truthy, so a click landing before the lazy
@@ -608,13 +725,26 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
// from, so probing under one workspace and emitting under another
// would serve ws1's click from ws2's endpoint. Snapshot once, then
// re-check at emit (below) rather than re-reading and trusting it.
const from = opts.address();
//
// DESTRUCTURED INTO PRIMITIVES, not held as the returned object.
// `opts.address()` is a reader the HOST supplies, and both live
// implementations happen to build a fresh object literal per call —
// so holding the reference would be safe today. It would be safe
// only for that reason. A host that returned a stable object it
// mutated in place would rewrite the very snapshot this fence
// compares against, and the comparison would pass unconditionally
// while looking exactly as it looks now. Three string copies buy
// independence from a property no interface states and no test
// could plausibly catch.
const { workspaceSlug: fromWs, itemId: fromItem, hostToken: fromHost } =
opts.address();
// No workspace ⇒ no probe ⇒ nothing can be positively known. An
// SSR/preview surface simply does not open a viewer.
if (!from.workspaceSlug) return;
if (!fromWs) return;
activating = true;
const seq = ++activationSeq;
void fetchAttachmentMetadata(from.workspaceSlug, forUuid, opts.getDownloadUrl)
setActivationPending(true);
void fetchAttachmentMetadata(fromWs, forUuid, opts.getDownloadUrl)
.then((result) => {
// Everything the gate asserted at gesture time has to still
// hold at emit time: the NodeView can be torn down, the node
@@ -629,32 +759,145 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
// finds its own uuid in place and emits for a gesture two
// swaps ago.
if (activationSeq !== seq) return;
// DELETION, checked explicitly and BEFORE the result is read.
// It is the one invalidation the uuid comparison structurally
// cannot catch: a delete does not change which attachment the
// node points at, so a probe issued before it can resolve `ok`
// afterwards with `forUuid` still current — describing a row
// the server has since dropped. `canActivate()` restates this
// below; it is spelled out here because the branches between
// the two must not act on that answer either.
//
// The two guards happen to be equivalent TODAY — every path
// that sets `deleted` also hides the image — but nothing
// enforces that, and inferring "the row is gone" from "the
// placeholder is showing" would silently stop holding the
// moment a deletion state exists that does not hide.
if (deleted) return;
// AUTHORITATIVE 404. The row is gone: latch the permanent
// placeholder (the same end state the deletion broadcast
// reaches) and open nothing. This runs even when the image is
// already hidden behind a retryable placeholder — upgrading a
// transient failure to a confirmed one is exactly what a 404
// is for, so it deliberately precedes the presentability
// check below.
if (result.status === 'missing') {
latchMissing(forUuid);
return;
}
// NOT AUTHORITATIVE. Stay retryable, latch nothing, open
// nothing — and stop being a control that silently does
// nothing (see showTransientProbeFailure).
if (result.status === 'transient') {
showTransientProbeFailure();
return;
}
// A positively-known MIME. Record it: the semantics pass and
// the transform toolbar both read `knownMime`, and this
// activation is often the FIRST thing to learn it (the
// toolbar's own probe only runs once the node is selected).
// Without this the image would keep announcing "View image"
// for something that opens the panel.
//
// The SEMANTICS follow, and deliberately nothing else. The
// rotate/crop toolbar reads `knownMime` too, but refreshing
// it from here would settle its per-format gating earlier
// than it does today — a behaviour change this task's
// contract does not ask for. It is also unnecessary: a
// toolbar only exists once the node has been selected, and
// selection runs its own probe for the same uuid which
// refreshes on arrival. The window is a moment of staleness
// that closes itself.
knownMime = result.mime;
applyImageSemantics();
// The gesture-time gate, restated on state that may have
// moved: a load failure inside the await window hands over to
// the placeholder, and there is then no image to act on.
if (!canActivate()) return;
// `transient` and `missing` are both "not positively known".
if (result.status !== 'ok') return;
// DR-16, restated on the resolved answer rather than on the
// absence of one: `image/svg+xml` can carry active content.
if (!canOpenInViewer(result.mime)) return;
// The host may have MOVED while the HEAD was in flight — the
// comment composer is deliberately reused across an item
// switch (see hostAddress.ts), so its address is live. The
// gesture belonged to the old address: emitting there opens a
// viewer over a pane the user has left, and emitting at the
// surface over a pane the user has left, and emitting at the
// new one attributes the gesture to a different item. Neither
// is what the user did, so drop it.
//
// Read ONCE, compared against the FULL captured address, and
// every emission below uses the CAPTURED values — never a
// re-read. The check and the emit are adjacent and
// synchronous; deferring either behind a timer or a microtask
// would reopen the window this closes.
//
// WHAT THIS FENCE DOES NOT CATCH, stated because it is a real
// gap and not an oversight: it compares VALUES, so an address
// that leaves and RETURNS (A→B→A) reads as unchanged. That is
// reachable — the pane's `ItemDetail` has no `{#key}` (PLAN-2105
// / TASK-2112), so an A→B→A item switch keeps one host token,
// and the comment composer it owns is reused across it.
//
// It is left as-is deliberately, on two grounds. First, the
// outcome differs from what the fence exists to prevent: the
// user has NOT left the pane (they are back on it), and the
// gesture is NOT re-attributed (same node, same attachment,
// same host), so what opens is the image they clicked over the
// pane they are looking at. Compare the uuid A→B→A case just
// below, which IS fenced by `activationSeq` — there the
// SUBJECT of the gesture changes, which is a different and
// worse thing than a destination that round-trips to itself.
//
// Second, it is not fixable from inside this NodeView. Telling
// A→B→A from A needs a generation that advances on every
// address CHANGE, and this NodeView cannot observe one: it
// only READS the address (`AttachmentHostAddressReader` is a
// getter, with no subscription and no epoch), so a B that
// arrives and leaves between the two reads is invisible here
// by construction. Closing it means adding an epoch to
// `AttachmentHostAddress` and bumping it in every host — a
// cross-surface API change that also lands on the chip
// NodeView, which is outside this task's contract. Tracked as
// a fence-completeness follow-up rather than smuggled in here.
const to = opts.address();
if (
to.workspaceSlug !== from.workspaceSlug ||
to.itemId !== from.itemId ||
to.hostToken !== from.hostToken
to.workspaceSlug !== fromWs ||
to.itemId !== fromItem ||
to.hostToken !== fromHost
) {
return;
}
// DR-16 / DR-7, on the resolved answer rather than the absence
// of one: `image/svg+xml` can carry active content, so it does
// not go to the viewer — but it is still an attachment the
// user just activated, and the options panel is the surface
// that owns everything the viewer will not take. A redirect,
// not a refusal.
//
// `filename` is null for the same reason it is on the viewer
// payload: the node's attrs carry none and the HEAD does not
// either. The panel fetches what it needs itself — its three
// metadata fields are nullable precisely for emitters like
// this one.
if (!canOpenInViewer(result.mime)) {
notifyAttachmentPanelOpen({
attachmentId: forUuid,
itemId: fromItem,
hostToken: fromHost,
anchor: img,
filename: null,
mime_type: result.mime,
size_bytes: result.size,
});
return;
}
notifyViewerOpen({
attachmentId: forUuid,
workspaceSlug: from.workspaceSlug,
itemId: from.itemId,
hostToken: from.hostToken,
workspaceSlug: fromWs,
itemId: fromItem,
hostToken: fromHost,
// A single-image set: this NodeView knows about ITS node
// and nothing else. The body's other images are a set the
// editor could offer, but assembling one here would make
@@ -688,8 +931,19 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
// Only if this activation is still the one holding the latch.
// A uuid swap bumps the counter, so a stale resolution
// landing afterwards cannot unlock the request that
// replaced it.
if (activationSeq === seq) activating = false;
// replaced it — nor clear a pending state the request that
// replaced it is still entitled to show.
if (activationSeq !== seq) return;
activating = false;
// The DOM write, unlike the latch release, is subject to
// this file's teardown rule: `destroyed` means every async
// continuation stops touching the node. Harmless in
// practice — the element is detached — but the `.then()`
// above fences on it and a finalizer that does not is the
// kind of asymmetry that stops being harmless the first
// time someone puts something real in here.
if (destroyed) return;
setActivationPending(false);
});
}
@@ -998,6 +1252,11 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
// the user made two swaps ago.
activationSeq += 1;
activating = false;
// And the pending affordance goes with the latch: the bump
// above retires the old request's finalizer, so leaving
// `aria-busy` on would strand the NEW image as permanently
// busy.
setActivationPending(false);
resetMissing();
if (newUuid) {
loadImage(opts.getDownloadUrl(newUuid, 'thumb-md'));
web/src/lib/components/editor/attachmentImageKeyboard.svelte.test.ts
+606 -9
View File
@@ -40,8 +40,16 @@ const deletionListeners = new Set<(uuid: string) => void>();
// Open-viewer requests, captured RAW — before the channel's addressability
// filter, so what is asserted is what THIS NodeView produced.
const emitted: Array<Record<string, unknown>> = [];
// Open-the-PANEL requests, same treatment. TASK-2434 makes this NodeView a
// producer on BOTH channels — a non-allowlisted MIME is redirected to the
// options panel rather than refused — so a spec that captured only the viewer
// channel could not tell a redirect from a silent drop, which is exactly the
// distinction the matrix is about.
const panelEmitted: Array<Record<string, unknown>> = [];
vi.mock('$lib/attachments/events', () => ({
notifyAttachmentPanelOpen: () => {},
notifyAttachmentPanelOpen: (event: Record<string, unknown>) => {
panelEmitted.push(event);
},
notifyViewerOpen: (event: Record<string, unknown>) => {
emitted.push(event);
},
@@ -66,9 +74,16 @@ type ProbeResult =
const probeMock = vi.fn<(ws?: string, uuid?: string) => Promise<ProbeResult>>(async () => ({
status: 'transient',
}));
// The load-failure path's REVALIDATION, separable from the cached read above.
// It delegates to `probeMock` by default, so for almost every test the two are
// one mock and answer alike. Exactly one test needs them to differ: proving
// that a 404 reaching the ACTIVATION branch upgrades an already-hidden
// placeholder requires the error path's own probe not to be the thing that
// latches it, or the assertion could not tell the two routes apart.
const revalidateMock = vi.fn<(ws?: string, uuid?: string) => Promise<ProbeResult>>();
vi.mock('./attachment-metadata', () => ({
fetchAttachmentMetadata: (ws: string, uuid: string) => probeMock(ws, uuid),
revalidateAttachmentMetadata: (ws: string, uuid: string) => probeMock(ws, uuid),
revalidateAttachmentMetadata: (ws: string, uuid: string) => revalidateMock(ws, uuid),
invalidateAttachmentMetadata: () => {},
mimeToFormat: () => null,
}));
@@ -133,7 +148,15 @@ function makeCommentEditor(element: HTMLElement): Editor {
});
}
/** How many times activation actually fired. One request per open. */
/**
* How many times activation asked for the VIEWER. One request per open.
*
* Deliberately not "how many times activation fired": since TASK-2434 an
* activation can instead land on the panel channel, which `panelEmitted`
* counts. Reading this as a count of activations would make every redirect
* look like a dropped gesture — the exact confusion the two arrays exist to
* keep apart.
*/
function openCount(): number {
return emitted.length;
}
@@ -208,7 +231,10 @@ describe('inline body image — keyboard activation (DR-12)', () => {
// `transient` default would make every "opens exactly once" test below
// assert 0 for a reason that has nothing to do with the keyboard.
probeMock.mockResolvedValue({ status: 'ok' as const, mime: 'image/png', size: 4096 });
revalidateMock.mockClear();
revalidateMock.mockImplementation((ws?: string, uuid?: string) => probeMock(ws, uuid));
emitted.length = 0;
panelEmitted.length = 0;
host = document.body.appendChild(document.createElement('div'));
target = host.appendChild(document.createElement('div'));
});
@@ -218,6 +244,7 @@ describe('inline body image — keyboard activation (DR-12)', () => {
editor = undefined;
host.remove();
emitted.length = 0;
panelEmitted.length = 0;
document.querySelectorAll('.timeline-viewer-stub').forEach((d) => d.remove());
});
@@ -465,7 +492,7 @@ describe('inline body image — keyboard activation (DR-12)', () => {
expect(await opened()).toBe(0);
});
it('refuses a probed non-raster type through the KEYBOARD, not just the mouse', async () => {
it('keeps a probed non-raster type OUT of the viewer through the KEYBOARD, not just the mouse', async () => {
// The gate used to live inside the click handler. A keyboard path that
// emitted on its own would have sailed straight past it, so the refusal
// is asserted on the route that would have bypassed it.
@@ -488,13 +515,571 @@ describe('inline body image — keyboard activation (DR-12)', () => {
press(image(), 'Enter');
expect(await opened()).toBe(0);
// And it stops being a focus stop at all, rather than announcing itself
// as a button that does nothing.
expect(image().getAttribute('role')).toBeNull();
expect(image().getAttribute('tabindex')).toBeNull();
expect(image().getAttribute('aria-label')).toBeNull();
// TASK-2434: it is a REDIRECT, not a refusal. Asserting only "the viewer
// did not open" would be satisfied by an implementation that dropped the
// gesture on the floor — which is what this used to do, and the dead
// focus stop the matrix exists to close.
expect(panelEmitted).toHaveLength(1);
// So it stays a real control, and it says where it goes.
expect(image().getAttribute('role')).toBe('button');
expect(image().getAttribute('tabindex')).toBe('0');
expect(image().getAttribute('aria-label')).toBe('Attachment options: A diagram');
});
it('redirects a non-allowlisted MIME to the panel, with the whole payload', async () => {
// The `ok` + not-allowlisted arm of the matrix, asserted on what is
// EMITTED. A count alone is satisfied by an event of any shape, and the
// panel's routing fields are exactly what a producer gets wrong: the
// address decides which of two mounted hosts opens it, and the three
// metadata fields are what the panel renders before its own fetch lands.
probeMock.mockResolvedValue({ status: 'ok', mime: 'application/pdf', size: 1234 });
editor = makeEditor(target);
const img = image();
press(img, 'Enter');
expect(await opened()).toBe(0);
expect(panelEmitted).toEqual([
{
attachmentId: 'uuid-1',
itemId: 'item-A',
hostToken: 'apanel-1',
anchor: img,
// No filename anywhere on this surface — the node's attrs carry
// none and the HEAD does not either. Null, not a fabricated one.
filename: null,
mime_type: 'application/pdf',
size_bytes: 1234,
},
]);
// It ASKED. A payload alone is satisfiable by an implementation that
// skipped the probe and assumed a MIME — which is precisely the gate
// this whole path exists to close.
expect(probeMock).toHaveBeenCalled();
// And the pending affordance is cleared on THIS branch too. The
// finalizer is shared, but a clear moved into the viewer branch would
// leave every redirect permanently `aria-busy`.
expect(img.getAttribute('aria-busy')).toBeNull();
expect(img.style.cursor).toBe('');
});
it('keeps redirecting on every activation, not just the first', async () => {
// The regression the semantics rewrite exists to prevent, and the one an
// attribute-only assertion would miss entirely. Activation now WRITES the
// resolved MIME onto the node. If the activation gate still refused a
// known non-allowlisted MIME — as it did before this task — the first tap
// would open the panel and every tap after it would silently do nothing,
// because the gate would be reading the answer the first tap recorded.
probeMock.mockResolvedValue({ status: 'ok', mime: 'image/svg+xml', size: 10 });
editor = makeEditor(target);
const img = image();
press(img, 'Enter');
await opened();
expect(panelEmitted).toHaveLength(1);
// The premise: the MIME really is on the node now (the label proves it,
// and it is the same state the gate would have been reading).
expect(img.getAttribute('aria-label')).toBe('Attachment options: A diagram');
press(img, 'Enter');
await opened();
click(img);
await opened();
expect(panelEmitted).toHaveLength(3);
expect(await opened()).toBe(0);
// The LAST one, not just the count: a redirect that kept firing with a
// payload frozen at the first gesture would be a count of three and a
// panel that opens on whatever the node used to be.
expect(panelEmitted[2]).toMatchObject({
attachmentId: 'uuid-1',
itemId: 'item-A',
hostToken: 'apanel-1',
anchor: img,
mime_type: 'image/svg+xml',
size_bytes: 10,
});
});
it('never opens the panel for a MIME the viewer WILL take', async () => {
// The other direction of the redirect, which a one-sided spec would leave
// free: an implementation that emitted BOTH events would satisfy every
// panel assertion above and open two surfaces from one gesture.
editor = makeEditor(target);
press(image(), 'Enter');
expect(await opened()).toBe(1);
expect(panelEmitted).toEqual([]);
// Reached by ASKING, not by assuming: an implementation that skipped the
// probe for images it liked the look of would pass the two assertions
// above and be exactly the bypass TASK-2433 closed.
expect(probeMock).toHaveBeenCalled();
});
it('latches the permanent placeholder on an authoritative 404, and opens nothing', async () => {
// The `missing` arm. `missing` is the ONLY result that may latch (DR-17),
// and the latch is what keeps editor undo from resurrecting a deleted
// attachment as a live-looking node.
probeMock.mockResolvedValue({ status: 'missing' });
editor = makeEditor(target);
const img = image();
press(img, 'Enter');
await opened();
expect(await opened()).toBe(0);
expect(panelEmitted).toEqual([]);
const placeholder = target.querySelector<HTMLElement>('.attachment-missing');
expect(img.style.display).toBe('none');
expect(placeholder?.style.display).not.toBe('none');
// PERMANENT, and the copy says so: a deleted row is not retryable, so the
// placeholder is deliberately NOT a control. A retryable placeholder here
// would invite a click that can only 404.
expect(placeholder?.title).toBe('This attachment has been deleted');
expect(placeholder?.getAttribute('role')).toBeNull();
expect(placeholder?.getAttribute('tabindex')).toBeNull();
// And the latch holds against BOTH ways back. A click cannot retry it...
placeholder?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
expect(img.style.display).toBe('none');
// ...and neither can a `load` that was already in flight when the 404
// landed. The implementation stops that twice over — the latch detaches
// the listener AND `resetMissing` refuses to run once `deleted` — so this
// pins the OUTCOME rather than either mechanism, which is the honest
// claim to make about a guard with a redundant partner.
img.dispatchEvent(new Event('load'));
expect(img.style.display).toBe('none');
expect(img.getAttribute('aria-busy')).toBeNull();
});
it('leaves a transient failure RETRYABLE — no open, no latch', async () => {
// The `transient` arm, and the dead focus stop TASK-2433 left behind: it
// emitted nothing and did nothing, so a focused image announced itself as
// a button and silently swallowed every press.
probeMock.mockResolvedValue({ status: 'transient' });
editor = makeEditor(target);
const img = image();
img.focus();
press(img, 'Enter');
await opened();
expect(await opened()).toBe(0);
expect(panelEmitted).toEqual([]);
const placeholder = target.querySelector<HTMLElement>('.attachment-missing');
expect(img.style.display).toBe('none');
expect(placeholder?.style.display).not.toBe('none');
// RETRYABLE, not latched — the copy, the semantics and the focus all say
// the same thing. This is the assertion that separates it from `missing`:
// an implementation that treated the two alike would pass every "did not
// open" check above and permanently strand a row that is perfectly fine.
expect(placeholder?.title).toContain('Click to retry');
expect(placeholder?.getAttribute('role')).toBe('button');
expect(placeholder?.getAttribute('tabindex')).toBe('0');
// The keypress that reported the failure must not drop focus to <body>.
expect(document.activeElement).toBe(placeholder);
// And the latch really is absent: Retry restores the image, which
// `missing` above cannot do.
expect(img.getAttribute('aria-busy')).toBeNull();
const srcBefore = img.getAttribute('src');
placeholder?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
expect(img.style.display).toBe('');
expect(placeholder?.style.display).toBe('none');
// Retry means a NEW REQUEST, not just an un-hidden element: without the
// cache-busting query the browser replays the failed entry and the retry
// is theatre. Restoring the DOM alone would pass the two lines above.
expect(img.getAttribute('src')).not.toBe(srcBefore);
expect(img.getAttribute('src')).toContain('retry=');
});
it('does not let a transient failure latch permanently across a recovery', async () => {
// DR-17's rule stated over TIME rather than over one result: only an
// authoritative 404 latches, so a blip followed by a healthy probe must
// leave the image fully openable again. An implementation that reused the
// `missing` path for `transient` would fail here and NOWHERE else — every
// single-shot assertion above would still pass.
probeMock.mockResolvedValue({ status: 'transient' });
editor = makeEditor(target);
const img = image();
press(img, 'Enter');
await opened();
const placeholder = target.querySelector<HTMLElement>('.attachment-missing');
// The premise, and it is load-bearing: without it this passes against an
// implementation whose `transient` arm does nothing at all — there would
// be no latch to survive, and the recovery below would prove nothing.
expect(img.style.display).toBe('none');
expect(placeholder?.getAttribute('role')).toBe('button');
placeholder?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
probeMock.mockResolvedValue({ status: 'ok', mime: 'image/png', size: 4096 });
press(img, 'Enter');
expect(await opened()).toBe(1);
});
it('shows a pending affordance while the MIME is still resolving', async () => {
// A cold probe is a round trip, and a click that does nothing for its
// duration reads as broken. Deliberately minimal: `aria-busy` and a
// cursor, no new chrome.
let release: () => void = () => {};
probeMock.mockImplementation(
() =>
new Promise((resolve) => {
release = () => resolve({ status: 'ok', mime: 'image/png', size: 4096 });
})
);
editor = makeEditor(target);
const img = image();
expect(img.getAttribute('aria-busy')).toBeNull();
press(img, 'Enter');
// Set SYNCHRONOUSLY with the gesture — a pending state that waits for a
// tick is not covering the wait it exists for.
expect(img.getAttribute('aria-busy')).toBe('true');
expect(img.style.cursor).toBe('progress');
release();
expect(await opened()).toBe(1);
// And cleared, or the image announces itself as permanently busy.
expect(img.getAttribute('aria-busy')).toBeNull();
expect(img.style.cursor).toBe('');
});
it('clears the pending affordance when a swap supersedes the request', async () => {
// The finalizer only runs for the request that still holds the latch, so
// a swap has to clear the pending state itself — otherwise a HEAD that
// never settles leaves the NEW image permanently `aria-busy`.
probeMock.mockImplementation(() => new Promise<never>(() => {}));
editor = makeEditor(target);
press(image(), 'Enter');
expect(image().getAttribute('aria-busy')).toBe('true');
// The SAME element across the swap: this NodeView deliberately survives a
// uuid change, and reacquiring by selector would let a destroy/recreate
// implementation pass without ever clearing anything.
const before = image();
editor.commands.setNodeSelection(1);
editor.commands.updateAttributes('attachmentImage', { uuid: 'uuid-2' });
expect(image()).toBe(before);
expect(before.getAttribute('aria-busy')).toBeNull();
expect(before.style.cursor).toBe('');
});
it('opens NOTHING when a delete lands mid-probe and the answer comes back ok', async () => {
// The race the uuid comparison structurally cannot catch: a delete does
// not change which attachment the node points at, so `forUuid` is still
// current when the probe resolves — with a perfectly valid `ok` describing
// a row the server has since dropped.
//
// The implementation states this TWICE — an explicit `deleted` re-check
// and `canActivate()`'s hidden-placeholder clause — and mutation testing
// confirms this spec fails only when BOTH are gone. That is the honest
// claim: it pins the BEHAVIOUR, not either line. (The two are equivalent
// today only because every path that sets `deleted` also hides the image;
// nothing enforces that, which is why the check is stated on its own
// terms rather than inferred from the placeholder.)
//
// A test that let the probe resolve BEFORE the delete would pass against
// an implementation with no `deleted` check at all, so the ordering here
// is the whole point: the answer is held until after the broadcast.
let release: () => void = () => {};
probeMock.mockImplementation(
() =>
new Promise((resolve) => {
release = () => resolve({ status: 'ok', mime: 'image/png', size: 4096 });
})
);
editor = makeEditor(target);
const img = image();
press(img, 'Enter');
for (const fn of deletionListeners) fn('uuid-1');
// The premise: the node still points at the very attachment the probe is
// about. Without this the drop could be the uuid fence doing the work.
expect(img.getAttribute('data-attachment-id')).toBe('uuid-1');
release();
expect(await opened()).toBe(0);
// BOTH channels. A `deleted` check placed after the allowlist branch
// would still leak the redirect.
expect(panelEmitted).toEqual([]);
});
it('opens NO PANEL either when a delete lands mid-probe on a non-raster type', async () => {
// The same race down the redirect branch, which is new surface: the
// `missing`/`transient`/`ok` split gave the continuation three more places
// to act on a row that is gone.
let release: () => void = () => {};
probeMock.mockImplementation(
() =>
new Promise((resolve) => {
release = () => resolve({ status: 'ok', mime: 'image/svg+xml', size: 10 });
})
);
editor = makeEditor(target);
press(image(), 'Enter');
for (const fn of deletionListeners) fn('uuid-1');
release();
await opened();
expect(panelEmitted).toEqual([]);
expect(await opened()).toBe(0);
// THE CONTROL, and it is the whole test: `expect no panel` is satisfied
// by an implementation that has no redirect at all — the pre-TASK-2434
// binary refusal passes it outright. So prove the branch works on an
// undeleted node under the identical probe.
editor.destroy();
probeMock.mockResolvedValue({ status: 'ok', mime: 'image/svg+xml', size: 10 });
editor = makeEditor(target);
press(image(), 'Enter');
await opened();
expect(panelEmitted).toHaveLength(1);
});
// The address fence, restated on the PANEL branch. It is a second emission
// site with its own copy of the captured address, so the three comparisons
// have to hold for it independently — a fence that only guarded the viewer
// would let a redirect open a panel over a pane the user has left.
for (const [label, moved] of [
['workspace', { workspaceSlug: 'ws2', itemId: 'item-A', hostToken: 'apanel-1' }],
['item', { workspaceSlug: 'ws', itemId: 'item-B', hostToken: 'apanel-1' }],
['owning mount', { workspaceSlug: 'ws', itemId: 'item-A', hostToken: 'apanel-2' }],
] as const) {
it(`drops the PANEL redirect when the ${label} moves mid-resolution`, async () => {
probeMock.mockResolvedValue({ status: 'ok', mime: 'image/svg+xml', size: 10 });
editor = makeCommentEditor(target);
press(image(), 'Enter');
address = { ...moved };
await opened();
expect(panelEmitted).toEqual([]);
// The control: settled, the next gesture DOES redirect — so the drop
// above is the fence, not a branch that never worked.
press(image(), 'Enter');
await opened();
expect(panelEmitted).toHaveLength(1);
expect(panelEmitted[0].itemId).toBe(moved.itemId);
expect(panelEmitted[0].hostToken).toBe(moved.hostToken);
});
}
it('emits with the address CAPTURED at the gesture, even if the reader mutates in place', async () => {
// The fence holds three PRIMITIVES, not the object the reader returned.
// Both live readers build a fresh object literal per call, so holding the
// reference would be safe — for that reason alone, which no interface
// states. This pins the independence: a reader that hands back ONE object
// and mutates it in place would, if the fence held the reference, rewrite
// the snapshot it compares against and pass unconditionally.
const shared = { workspaceSlug: 'ws', itemId: 'item-A', hostToken: 'apanel-1' };
address = shared;
let release: () => void = () => {};
probeMock.mockImplementation(
() =>
new Promise((resolve) => {
release = () => resolve({ status: 'ok', mime: 'image/png', size: 4096 });
})
);
editor = makeCommentEditor(target);
press(image(), 'Enter');
// The host "moves" by mutating the object the reader keeps handing out.
shared.itemId = 'item-B';
shared.hostToken = 'apanel-2';
release();
// Dropped. Holding the reference would compare item-B against item-B.
expect(await opened()).toBe(0);
});
it('DOES emit when the address leaves and returns (A→B→A) — documented, accepted', async () => {
// The fence compares VALUES, so an address that round-trips reads as
// unchanged. Reachable: the pane's `ItemDetail` has no `{#key}`
// (PLAN-2105 / TASK-2112), so an A→B→A item switch keeps one host token
// and the composer it owns is reused across it.
//
// PINNED AS THE CURRENT BEHAVIOUR, not asserted as ideal. It is accepted
// because the outcome differs from what the fence prevents — the user is
// back on the pane, and the gesture is not re-attributed: the same node,
// the same attachment, the same host. Contrast the uuid A→B→A case below,
// which IS dropped, because there the SUBJECT of the gesture changed.
//
// Telling A→B→A from A needs an epoch on `AttachmentHostAddress` that
// every host bumps; this NodeView only READS the address, so a B between
// the two reads is invisible to it by construction. When that epoch
// lands, this expectation flips to 0 and this comment is its changelog.
let release: () => void = () => {};
probeMock.mockImplementation(
() =>
new Promise((resolve) => {
release = () => resolve({ status: 'ok', mime: 'image/png', size: 4096 });
})
);
editor = makeCommentEditor(target);
const original = { ...address };
press(image(), 'Enter');
address = { workspaceSlug: 'ws', itemId: 'item-B', hostToken: 'apanel-1' };
address = { ...original };
release();
expect(await opened()).toBe(1);
// And it emits at A — not at the B it passed through.
expect(emitted[0].itemId).toBe(original.itemId);
expect(emitted[0].hostToken).toBe(original.hostToken);
});
it('UPGRADES an already-hidden retryable placeholder when a 404 arrives', async () => {
// The transient→missing transition, and the only DR-17 boundary that had
// no test. The mutant it exists for is exact: an implementation that
// processed `missing` only when `canActivate()` is true would pass every
// other test in this file, because the placeholder-showing state is
// precisely where `canActivate()` is already false. That is why the
// `missing` branch deliberately runs BEFORE the presentability check — a
// 404 is authoritative whatever the node happens to be showing.
//
// Reaching that state takes care. An activation cannot START while the
// placeholder is up (the gesture-time gate refuses), and Retry UN-hides
// the image — so a naive "fail, retry, 404" sequence lands the 404 on a
// VISIBLE image and tests nothing new. The image has to go behind the
// placeholder DURING the await, which is what the load `error` does.
let release: () => void = () => {};
probeMock.mockImplementation(
() =>
new Promise((resolve) => {
release = () => resolve({ status: 'missing' });
})
);
// The error path runs its OWN revalidation, which latches on a 404 too.
// Keeping it transient is what makes the latch below attributable to the
// activation branch rather than to the load failure's probe.
revalidateMock.mockResolvedValue({ status: 'transient' });
editor = makeEditor(target);
const img = image();
press(img, 'Enter');
img.dispatchEvent(new Event('error'));
await opened();
const placeholder = target.querySelector<HTMLElement>('.attachment-missing');
// The premise, and the whole point: hidden, and still RETRYABLE. Without
// both, the 404 below would be landing on a state some other test covers.
expect(img.style.display).toBe('none');
expect(placeholder?.getAttribute('role')).toBe('button');
expect(placeholder?.title).toContain('Click to retry');
// The 404 lands while the placeholder is up.
release();
await opened();
// Upgraded: permanent, inert, no longer inviting a retry that can only
// 404 again.
expect(placeholder?.title).toBe('This attachment has been deleted');
expect(placeholder?.getAttribute('role')).toBeNull();
expect(placeholder?.getAttribute('tabindex')).toBeNull();
// And the latch holds — a retry click cannot undo an authoritative 404.
placeholder?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
expect(img.style.display).toBe('none');
expect(await opened()).toBe(0);
expect(panelEmitted).toEqual([]);
});
it('opens nothing when a load failure hides the image mid-probe and the MIME is FINE', async () => {
// The post-await `canActivate()` guard, which nothing else reaches. The
// spec below ("will not ACTIVATE while the image is showing a
// load-failure placeholder") fails the load FIRST and gestures after, so
// it is stopped by the GESTURE-time gate and would pass with the
// post-await check deleted entirely.
//
// This is the other order, and it is the reachable one: the gesture lands
// on a healthy image, the load fails while the HEAD is in flight, and the
// HEAD comes back with a perfectly good `image/png`. Nothing is deleted
// and nothing is missing — the only reason not to open is that there is
// no longer an image on screen to open. Emitting here would put a viewer
// over a placeholder.
let release: () => void = () => {};
probeMock.mockImplementation(
() =>
new Promise((resolve) => {
release = () => resolve({ status: 'ok', mime: 'image/png', size: 4096 });
})
);
// The error path's own revalidation stays transient, so the placeholder
// remains the RETRYABLE one — this test is about a healthy MIME meeting a
// hidden image, not about a latch.
revalidateMock.mockResolvedValue({ status: 'transient' });
editor = makeEditor(target);
const img = image();
press(img, 'Enter');
img.dispatchEvent(new Event('error'));
const placeholder = target.querySelector<HTMLElement>('.attachment-missing');
// The premise: hidden, retryable, NOT deleted — so every other guard in
// the continuation (uuid, generation, `deleted`) is satisfied and only
// presentability is left to do the work.
expect(img.style.display).toBe('none');
expect(placeholder?.getAttribute('role')).toBe('button');
release();
await opened();
expect(await opened()).toBe(0);
expect(panelEmitted).toEqual([]);
// And it stayed retryable: refusing to open must not cost the user the
// retry affordance.
expect(placeholder?.getAttribute('role')).toBe('button');
});
it('emits SYNCHRONOUSLY with the fence check, not on a later turn', async () => {
// The fence's correctness rests on the check and the emit being
// adjacent: anything queued between them — a timer, a microtask hop — is
// a new window for the address to go stale across, which is the whole
// hazard the fence exists for. Nothing else in this file would notice a
// refactor that queued delivery, because every other assertion goes
// through `opened()`, which waits two MACROtask turns and would happily
// observe a `setTimeout(0)` emission.
//
// So: resolve the probe and read the count after a bounded number of
// MICROtasks. A delivery queued behind a timer cannot be observed here;
// a synchronous one always is.
let release: () => void = () => {};
probeMock.mockImplementation(
() =>
new Promise((resolve) => {
release = () => resolve({ status: 'ok', mime: 'image/png', size: 4096 });
})
);
editor = makeEditor(target);
press(image(), 'Enter');
release();
// Three microtasks is generous for the implementation's own `.then`
// chain and still strictly inside the current macrotask.
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
expect(emitted).toHaveLength(1);
});
// NOTE, deliberately not a test: "stamps the CAPTURED address rather than a
// re-read one" is UNOBSERVABLE from outside this function, and writing a test
// that appears to prove it would be worse than leaving the gap named. The
// fence refuses to emit whenever the two differ, so every emission happens in
// a state where captured and re-read are equal — a spec that moved the address
// away and back would assert the same values under either implementation. The
// enforceable half is the drop, and that is asserted per field above.
it('refuses an UNPROBED non-raster type — the gesture that beats the lazy probe', async () => {
// The bypass this task's revised gate closes, and the one the test above
// cannot see: `settleProbe()` selects the node, which builds the toolbar
@@ -520,6 +1105,10 @@ describe('inline body image — keyboard activation (DR-12)', () => {
// And it did ask — a refusal reached by never probing at all would be
// the same count for the wrong reason.
expect(probeMock).toHaveBeenCalled();
// TASK-2434: and neither gesture was DROPPED. Asserting only "no viewer"
// is satisfied by the silent return this task replaced; the unprobed
// gesture has to reach the panel exactly as the probed one does.
expect(panelEmitted).toHaveLength(2);
});
it('emits once when two gestures land inside one resolution window', async () => {
@@ -587,6 +1176,10 @@ describe('inline body image — keyboard activation (DR-12)', () => {
press(image(), 'Enter');
expect(await opened()).toBe(0);
// And nothing else opened either — `missing` is the one result with no
// destination at all. An implementation that fell through to the panel
// redirect would offer options for a row that is gone.
expect(panelEmitted).toEqual([]);
});
it('drops the request when the NodeView is torn down mid-resolution', async () => {
@@ -629,6 +1222,10 @@ describe('inline body image — keyboard activation (DR-12)', () => {
// working.
press(image(), 'Enter');
expect(await opened()).toBe(1);
// The probe ran under the address the GESTURE happened at — the
// workspace keys the metadata cache, so probing under the wrong one
// answers a question about a different workspace's row.
expect(probeMock).toHaveBeenLastCalledWith(moved.workspaceSlug, 'uuid-1');
expect(emitted[0].workspaceSlug).toBe(moved.workspaceSlug);
expect(emitted[0].itemId).toBe(moved.itemId);
expect(emitted[0].hostToken).toBe(moved.hostToken);
web/src/lib/components/editor/attachmentImageViewerHost.svelte.test.ts
+93
View File
@@ -24,6 +24,11 @@ import { Editor } from '@tiptap/core';
import StarterKit from '@tiptap/starter-kit';
import { __resetViewerBackdropForTests } from '$lib/a11y/viewerBackdrop';
import { _resetEscapeStackForTests } from '$lib/stores/escapeStack';
import {
isAttachmentPanelEventForHost,
registerAttachmentPanelListener,
type AttachmentPanelOpenEvent,
} from '$lib/attachments/events';
const UUID = '11111111-1111-4111-8111-111111111111';
const ITEM_ID = 'item-A';
@@ -43,6 +48,9 @@ const { AttachmentImage } = await import('./attachment-image');
const { default: AttachmentViewerHost } = await import(
'$lib/components/attachments/AttachmentViewerHost.svelte'
);
const { default: AttachmentPanelHost } = await import(
'$lib/components/attachments/AttachmentPanelHost.svelte'
);
let address = { workspaceSlug: 'ws', itemId: ITEM_ID, hostToken: HOST_TOKEN };
@@ -194,6 +202,91 @@ describe('inline image → viewer host → Lightbox', () => {
expect(viewers()).toHaveLength(0);
});
it('REDIRECTS a non-allowlisted type into the real panel host', async () => {
// The whole route for the redirect arm, with nothing between the NodeView
// and the panel stubbed: the real bus, the real `AttachmentPanelHost`, the
// real panel it mounts. The producer specs next door mock the bus, so a
// host that stopped consuming — or an address that could not route —
// would leave them green and leave the user with an image that does
// nothing, which is the exact failure this task replaced.
probeMock.mockResolvedValue({ status: 'ok', mime: 'image/svg+xml', size: 100 } as never);
mountHost({ itemId: ITEM_ID, hostToken: HOST_TOKEN });
const panelHost = mount(AttachmentPanelHost, {
target: hostTarget,
props: {
wsSlug: 'ws',
itemId: ITEM_ID,
hostToken: HOST_TOKEN,
mutationsEnabled: false,
},
}) as Record<string, unknown>;
try {
editor = makeEditor(editorTarget);
image().dispatchEvent(
new MouseEvent('click', { bubbles: true, cancelable: true, detail: 1 })
);
await settle();
// No viewer — the security half.
expect(viewers()).toHaveLength(0);
// And a real panel on screen, for the attachment that was activated —
// the completeness half. A redirect nobody consumes is still a tap
// that does nothing.
const panel = document.body.querySelector<HTMLElement>('[role="menu"] .ap-header');
expect(panel).not.toBeNull();
// And it is about THIS attachment, not merely present: the MIME the
// probe returned and a download target carrying the uuid. A panel that
// opened on the wrong row would satisfy a presence check.
expect(panel?.querySelector('.ap-meta')?.getAttribute('title')).toBe('image/svg+xml');
expect(
document.body.querySelector<HTMLAnchorElement>('[role="menu"] a[download]')?.getAttribute('href')
).toContain(UUID);
} finally {
unmount(panelHost);
}
});
it('REDIRECTS a non-allowlisted type onto the real panel channel', async () => {
// TASK-2434's redirect, asserted through the REAL bus rather than a mock.
// The producer specs next door mock `notifyAttachmentPanelOpen`, so they
// see the call and are blind to what the channel does with it — and the
// channel drops any emission it judges unaddressable. An event that never
// leaves the bus is indistinguishable, from the producer's side, from the
// silent refusal this task replaced.
const seen: AttachmentPanelOpenEvent[] = [];
const dispose = registerAttachmentPanelListener((e) => seen.push(e));
try {
probeMock.mockResolvedValue({ status: 'ok', mime: 'image/svg+xml', size: 100 } as never);
mountHost({ itemId: ITEM_ID, hostToken: HOST_TOKEN });
editor = makeEditor(editorTarget);
image().dispatchEvent(
new MouseEvent('click', { bubbles: true, cancelable: true, detail: 1 })
);
await settle();
// No viewer — the security half.
expect(viewers()).toHaveLength(0);
// And it went SOMEWHERE — the completeness half.
expect(seen).toHaveLength(1);
expect(seen[0].attachmentId).toBe(UUID);
expect(seen[0].mime_type).toBe('image/svg+xml');
// Addressed well enough for a host to claim it. The channel's own
// predicate, not a re-implementation of it: a payload that reached a
// raw subscriber but that no host would match is still a tap that
// does nothing.
expect(
isAttachmentPanelEventForHost(seen[0], { itemId: ITEM_ID, hostToken: HOST_TOKEN })
).toBe(true);
expect(
isAttachmentPanelEventForHost(seen[0], { itemId: ITEM_ID, hostToken: 'another-mount' })
).toBe(false);
} finally {
dispose();
}
});
it('does not open for a MIME the viewer would filter back out', async () => {
// The producer's gate and the viewer's are the same gate, stated twice
// on purpose (TASK-2431). If the producer ever emitted an SVG, the