From 57caca287bdabb4c4fb4b2662ab9eb97776aee6b Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Sat, 22 Aug 2026 11:47:20 +1000 Subject: [PATCH 1/6] fix(table): measure drag travel against the content, not the viewport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pointer travel was compared between two viewport points, so a gesture the content moved under — a scroll mid-drag, a host-driven scrollIntoView — measured near zero and was judged a plain click, dropping the selection. Take both endpoints relative to contentDOM's live rect instead. One frame, so a pointer that follows the moving content still measures zero and stays a click, and both operands are visual pixels — which matters because CodeMirror supports being CSS transformed, where mixing a client coordinate with a layout-space scroll offset would break the threshold. --- src/webview/cm/table/table-widget.ts | 41 ++++++++++++-- .../table/cm-table-widget-drag.test.ts | 55 +++++++++++++++++++ test/webview/table/helpers/widget-fixtures.ts | 45 ++++++++++++++- 3 files changed, 132 insertions(+), 9 deletions(-) diff --git a/src/webview/cm/table/table-widget.ts b/src/webview/cm/table/table-widget.ts index 8347e77b..17b91f5d 100644 --- a/src/webview/cm/table/table-widget.ts +++ b/src/webview/cm/table/table-widget.ts @@ -59,8 +59,20 @@ const DRAG_THRESHOLD_PX = 4; * `updateDOM` can do and the closure cannot. A WeakMap so a discarded widget * root takes its entry with it. */ interface PendingDrag { - readonly x: number; - readonly y: number; + /** The press point RELATIVE TO THE CONTENT, not to the viewport. The content + * can move under a held pointer — a scroll mid-gesture, a `scrollIntoView` + * from an incoming host message, CodeMirror's own scrolling — and a pointer + * that stayed still through one of those HAS crossed text. Two `clientX/Y` + * pairs cannot see that, and called the gesture a click. + * + * Relative to `contentDOM`'s rect rather than `scrollDOM`'s scroll offsets so + * that both operands are VISUAL pixels: CodeMirror supports being CSS + * transformed (`view.scaleX` / `scaleY` exist for exactly that), and mixing a + * viewport coordinate with a layout-space scroll offset breaks the threshold + * under any scale but 1. Measuring both ends the same way makes the scale + * cancel instead of needing a correction. */ + readonly contentX: number; + readonly contentY: number; readonly point: CellPoint | null; } const pendingDrag = new WeakMap(); @@ -126,6 +138,20 @@ function dispatchSelection(view: EditorView, selection: { anchor: number; head?: } } +/** A pointer position in the CONTENT's frame of reference. */ +function contentPoint(view: EditorView, event: MouseEvent): { x: number; y: number } { + const origin = view.contentDOM.getBoundingClientRect(); + return { x: event.clientX - origin.left, y: event.clientY - origin.top }; +} + +/** Manhattan pointer travel since the press, over the CONTENT. One measurement + * in one frame, so a pointer that follows moving content cancels out and stays + * a click. Manhattan (not Euclidean) to match the threshold the suite pins. */ +function travelSince(view: EditorView, pending: PendingDrag, event: MouseEvent): number { + const now = contentPoint(view, event); + return Math.abs(now.x - pending.contentX) + Math.abs(now.y - pending.contentY); +} + /** The RANGE a completed pointer gesture describes, or `null` when this gesture * is not a drag at all (no armed anchor, unmappable anchor, keyboard / * programmatic click, sub-threshold travel), when the head has no mapping, or @@ -152,8 +178,7 @@ function dragRange( if (event.detail === 0) { return null; } - const travel = Math.abs(event.clientX - pending.x) + Math.abs(event.clientY - pending.y); - if (travel < DRAG_THRESHOLD_PX) { + if (travelSince(view, pending, event) < DRAG_THRESHOLD_PX) { return null; } const head = cellPointAt( @@ -283,9 +308,13 @@ export class TableBlockWidget extends WidgetType { } // No dispatch here: dispatching would fire the reveal mid-drag and pull // the widget out from under the pointer. + const at = contentPoint(view, event); pendingDrag.set(root, { - x: event.clientX, - y: event.clientY, + contentX: at.x, + contentY: at.y, + // `cellPointAt` keeps taking VIEWPORT coordinates — it feeds + // `caretPositionFromPoint`, which is a viewport API. Only the travel + // measurement changes frame. point: cellPointAt( root, event.clientX, diff --git a/test/webview/table/cm-table-widget-drag.test.ts b/test/webview/table/cm-table-widget-drag.test.ts index 6ae5df38..9ae89c57 100644 --- a/test/webview/table/cm-table-widget-drag.test.ts +++ b/test/webview/table/cm-table-widget-drag.test.ts @@ -451,6 +451,61 @@ describe("TableBlockWidget drag-selection", () => { ]); }); + // (2) of the TODO entry. Travel was measured between two VIEWPORT points, so + // a gesture the CONTENT moved under — a scroll mid-drag, a host-driven + // scrollIntoView, CodeMirror's own scrolling — measured ~0 and was judged a + // plain click. The pointer moved relative to the TEXT, which is the only + // space the gesture means anything in. + it("a drag the content scrolled under is a drag, even with a stationary pointer", () => { + const dispatched: unknown[] = []; + const { mount, scrollContentBy } = stubViewWithCaret(dispatched, [ + { text: "alpha", offset: 2 }, + { text: "alpha", offset: 5 }, + ]); + const dom = mount(makeWidget(SRC)); + const td = dom.querySelectorAll("td")[0] as HTMLElement; + press(td, "mousedown", 30, 30); + scrollContentBy(0, 40); // the text moved 40px under a pointer that did not + press(td, "click", 30, 30); + expect(dispatched).toEqual([ + { selection: { anchor: SRC.indexOf("alpha") + 2, head: SRC.indexOf("alpha") + 5 } }, + ]); + }); + + // The mirror image, and the reason this is ONE measurement in the content's + // frame rather than two gates added together: a pointer that follows the + // scroll exactly has not moved over the text at all, so it is still a click. + // A scroll-aware threshold that summed magnitudes would call this a drag. + it("a pointer that tracks the scrolling content exactly is still a click", () => { + const dispatched: unknown[] = []; + const { mount, scrollContentBy } = stubViewWithCaret(dispatched, [ + { text: "alpha", offset: 2 }, + { text: "alpha", offset: 5 }, + ]); + const dom = mount(makeWidget(SRC)); + const td = dom.querySelectorAll("td")[0] as HTMLElement; + press(td, "mousedown", 30, 30); + scrollContentBy(0, 40); + press(td, "click", 30, -10); // followed the text up by exactly 40px + expect(dispatched).toEqual([{ selection: { anchor: SRC.indexOf("alpha") } }]); + }); + + it("horizontal content movement counts too", () => { + const dispatched: unknown[] = []; + const { mount, scrollContentBy } = stubViewWithCaret(dispatched, [ + { text: "alpha", offset: 2 }, + { text: "alpha", offset: 5 }, + ]); + const dom = mount(makeWidget(SRC)); + const td = dom.querySelectorAll("td")[0] as HTMLElement; + press(td, "mousedown", 30, 30); + scrollContentBy(25, 0); + press(td, "click", 30, 30); + expect(dispatched).toEqual([ + { selection: { anchor: SRC.indexOf("alpha") + 2, head: SRC.indexOf("alpha") + 5 } }, + ]); + }); + // Aborted-gesture guard. A press released OUTSIDE the widget delivers no // click to the root, so the armed anchor survives with stale coordinates. // The only click that can then reach this handler without a mousedown of its diff --git a/test/webview/table/helpers/widget-fixtures.ts b/test/webview/table/helpers/widget-fixtures.ts index 56b5a4e0..ebe2b143 100644 --- a/test/webview/table/helpers/widget-fixtures.ts +++ b/test/webview/table/helpers/widget-fixtures.ts @@ -121,7 +121,16 @@ export function stubView( return { state: EditorState.create({ extensions }), dispatch: (tr: unknown) => dispatched.push(tr), - } satisfies Pick as unknown as EditorViewType; + // The widget measures pointer travel against this element's rect when it + // arms a gesture, so a stub without one throws inside a DOM listener — + // where the throw is swallowed and the test sees a missing dispatch rather + // than an error. happy-dom reports an all-zero rect, which is exactly right + // for the suites using this stub: content that never moves. + contentDOM: document.createElement("div"), + } satisfies Pick< + EditorViewType, + "state" | "dispatch" | "contentDOM" + > as unknown as EditorViewType; } /** The shared throwaway-recorder `stubView` — for the display-only paths, where @@ -205,10 +214,31 @@ export function stubViewWithCaret( ); return null; }; + // happy-dom has no layout engine, so `getBoundingClientRect` answers zeros for + // everything. The widget only ever reads `left`/`top` off this rect, so a + // scripted origin is a complete stand-in — and it is the ONLY way to express + // "the content moved under a stationary pointer" without a layout engine. + const contentDOM = document.createElement("div"); + let origin = { left: 0, top: 0 }; + contentDOM.getBoundingClientRect = () => + ({ + ...origin, + x: origin.left, + y: origin.top, + width: 0, + height: 0, + right: 0, + bottom: 0, + }) as DOMRect; + const view = { state: EditorState.create({ extensions: [quollTableCaretResolver.of(resolve), ...extensions] }), dispatch: (tr: unknown) => dispatched.push(tr), - } satisfies Pick as unknown as EditorViewType; + contentDOM, + } satisfies Pick< + EditorViewType, + "state" | "dispatch" | "contentDOM" + > as unknown as EditorViewType; /** Mount a widget the way every drag test needs it: rendered, resolver root * wired, attached to the body (the caret resolver needs a live tree). The @@ -241,7 +271,16 @@ export function stubViewWithCaret( const update = (dom: HTMLElement, next: TableBlockWidget, prev: TableBlockWidget): boolean => next.updateDOM(dom, view, prev); - return { mount, update }; + /** Move the CONTENT by (dx, dy) — a scroll, a `scrollIntoView` from the host, + * anything that shifts the text under a held pointer. Scrolling DOWN by 40 + * moves the content UP, so the origin goes negative: a pointer that has not + * moved is then 40px further into the document than where it pressed, which + * is the travel the old viewport measurement could not see. */ + const scrollContentBy = (dx: number, dy: number): void => { + origin = { left: origin.left - dx, top: origin.top - dy }; + }; + + return { mount, update, scrollContentBy }; } /** Dispatch a mouse event carrying coordinates — the movement threshold reads From 3d5abd16eb382efcd4a39acf87822db01ff7dbad Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Sat, 22 Aug 2026 11:51:47 +1000 Subject: [PATCH 2/6] fix(table): dispatch the drag range when the release lands outside the widget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A drag that starts in a rendered cell and is released outside the widget never delivered a click to the widget root — per UI-events the click retargets to the nearest common ancestor of the press and release targets, measured here as .cm-content — so the gesture was lost and the editor was left with whatever caret its observer happened to park at the release point. Add a second dispatch seam: a document-level mouseup, armed at mousedown, that dispatches only when the release landed outside the root. An inside release still belongs to the click listener, so the modifier-click open-external path is untouched. Four things disarm the seam, each pinned by a test that reddens when its guard is removed: the release itself, a native drag-and-drop (which ends in dragend, never mouseup), WidgetType.destroy, and the next press anywhere. That last one is registered in the CAPTURE phase deliberately — four sibling widgets stop mousedown propagation while leaving mouseup alone, and a bubble-phase disarm starved by one of those would let a stale gesture dispatch a range the user never drew. --- src/webview/cm/table/table-widget.ts | 216 +++++++++-- .../table/cm-table-widget-release.test.ts | 334 ++++++++++++++++++ test/webview/table/helpers/widget-fixtures.ts | 55 ++- 3 files changed, 581 insertions(+), 24 deletions(-) create mode 100644 test/webview/table/cm-table-widget-release.test.ts diff --git a/src/webview/cm/table/table-widget.ts b/src/webview/cm/table/table-widget.ts index 17b91f5d..50849943 100644 --- a/src/webview/cm/table/table-widget.ts +++ b/src/webview/cm/table/table-widget.ts @@ -91,6 +91,15 @@ const pendingDrag = new WeakMap(); * rationale, as image-widget.ts's `blockStart`. */ const blockStart = new WeakMap(); +/** Aborts the document-level listeners armed for the gesture in flight on this + * root. Kept OUT of `PendingDrag` because the two have different lifetimes: + * `updateDOM` drops the pending anchor when a doc edit invalidates it while the + * gesture is still physically in progress, and that release must still be heard + * (it dispatches the block-start caret). A WeakMap so a discarded root takes its + * controller with it — though the listeners are removed by the gesture's own end + * and by `destroy`, not left to garbage collection. */ +const armedRelease = new WeakMap(); + /** Margin-click caret: the block start this root currently points at. * * Falling back to the toDOM-time `widget.docFrom` totalizes the @@ -152,33 +161,50 @@ function travelSince(view: EditorView, pending: PendingDrag, event: MouseEvent): return Math.abs(now.x - pending.contentX) + Math.abs(now.y - pending.contentY); } -/** The RANGE a completed pointer gesture describes, or `null` when this gesture - * is not a drag at all (no armed anchor, unmappable anchor, keyboard / - * programmatic click, sub-threshold travel), when the head has no mapping, or - * when the range collapses after snapping. Every `null` answer means the same - * thing to the caller: dispatch the plain collapsed caret instead. */ -function dragRange( +/** A `PendingDrag` whose press landed on a cell — the only kind either seam can + * build a range from. */ +interface ArmedDrag extends PendingDrag { + readonly point: CellPoint; +} + +/** Is this completed gesture a DRAG (rather than a click, a keyboard or + * programmatic activation, or nothing this widget armed)? ONE definition for + * both seams, so the click and the outside release can never disagree about + * what a drag is. + * + * `detail === 0` means the event was NOT produced by a pointer gesture — + * keyboard activation of an in-cell ``, or a programmatic `.click()` / + * `dispatchEvent`. Its clientX/Y are 0, so pairing it with an armed anchor + * would read a large bogus travel and dispatch a range the user never drew. */ +function isDrag( view: EditorView, - root: HTMLElement, event: MouseEvent, pending: PendingDrag | null -): { anchor: number; head: number } | null { +): pending is ArmedDrag { if (pending === null || pending.point === null) { - return null; + return false; } - // `detail === 0` means this click was NOT produced by a pointer gesture — - // keyboard activation of an in-cell ``, or a programmatic `.click()`. Its - // clientX/Y are 0, so pairing it with an armed anchor would read a large - // bogus travel and dispatch a range the user never drew. This is also what - // closes the aborted-gesture window: a press released OUTSIDE the widget - // delivers no click to `root`, leaving the entry armed, and the only clicks - // that can then reach the handler without a fresh mousedown of their own are - // keyboard/programmatic ones (a click retargeted to a common ancestor above - // the widget never runs that listener at all). if (event.detail === 0) { - return null; + return false; } - if (travelSince(view, pending, event) < DRAG_THRESHOLD_PX) { + return travelSince(view, pending, event) >= DRAG_THRESHOLD_PX; +} + +/** The RANGE a completed pointer gesture describes, or `null` when this gesture + * is not a drag at all (see `isDrag`), when the head has no mapping, or when + * the range collapses after snapping. Every `null` answer means the same thing + * to the caller: dispatch the plain collapsed caret instead. + * + * The aborted-gesture window this used to close through `detail === 0` is now + * ALSO closed structurally: the release seam disarms itself on every mouseup, + * and any press elsewhere disarms a gesture whose release never arrived. */ +function dragRange( + view: EditorView, + root: HTMLElement, + event: MouseEvent, + pending: PendingDrag | null +): { anchor: number; head: number } | null { + if (!isDrag(view, event, pending)) { return null; } const head = cellPointAt( @@ -228,6 +254,48 @@ function dragRange( return from === to ? null : { anchor: from, head: to }; } +/** The RANGE a gesture RELEASED OUTSIDE this widget describes, or `null` when it + * is not a drag, when the press was not on a cell, or when the editor cannot + * place the release point — every one of which the caller answers with the + * collapsed caret, the same degrade the click seam uses. + * + * The head comes from `view.posAtCoords` rather than from `cellPointAt`: the + * release is outside the widget by construction, so there is no cell to map and + * the editor's own coordinate lookup IS the answer. It returns `null` for a + * point it cannot resolve (an unrendered block, a viewport gap) — but NOT for + * an overshoot: past the last line it clamps to `doc.length`, and above the + * first to `0` (@codemirror/view 6.43.0), which is why a release below the + * document still draws a range to the end rather than degrading. + * + * Direction comes from comparing that document position with the cell, for the + * same reason the across-cells arm of `dragRange` uses cell order: an + * unmappable anchor has no offset to compare, and it snaps OUTWARD — away from + * the release — so the range still covers the cell the pointer started in. */ +function releaseRange( + view: EditorView, + event: MouseEvent, + pending: PendingDrag | null +): { anchor: number; head: number } | null { + if (!isDrag(view, event, pending)) { + return null; + } + let head: number | null; + try { + head = view.posAtCoords({ x: event.clientX, y: event.clientY }); + } catch (err) { + // Same contract as `dispatchSelection`'s catch: a coordinate lookup against + // a view torn down mid-gesture must cost the gesture, not the editor. + console.error("[quoll] table widget release lookup failed", { err }); + return null; + } + if (head === null) { + return null; + } + const start = pending.point; + const anchor = start.offset ?? (head > start.cellFrom ? start.cellFrom : start.cellTo); + return anchor === head ? null : { anchor, head }; +} + export class TableBlockWidget extends WidgetType { constructor( readonly table: Table, @@ -322,6 +390,102 @@ export class TableBlockWidget extends WidgetType { view.state.facet(quollTableCaretResolver) ), }); + + // The SECOND dispatch seam. A gesture released outside this root never + // delivers a `click` here — measured in real Chromium: the click is + // retargeted to `.cm-content`, the nearest common ancestor of the press + // and release targets — so the release has to be heard on the document, + // where every mouseup lands. Armed per gesture rather than kept + // permanently, so the listener that can dispatch is exactly the one + // belonging to the press in flight. Any controller left over from a + // previous press is aborted first, so at most one is ever armed. + armedRelease.get(root)?.abort(); + const release = new AbortController(); + armedRelease.set(root, release); + const doc = root.ownerDocument; + const armingPress = event; + doc.addEventListener( + "mouseup", + (up: MouseEvent) => { + // BEFORE the abort, not after: a right-button press-and-release while + // the left button is held delivers a mouseup this gesture did not end. + // Aborting first would leave the real release with no listener. + if (up.button !== 0) { + return; + } + release.abort(); // one-shot: this gesture is over either way + // The root left the document mid-gesture — CodeMirror rebuilds a + // widget by REPLACING its root, and a detached root's stamps have + // stopped tracking the document. The click seam never had to check + // (a detached root receives no clicks); a document-level one does. + if (!root.isConnected) { + return; + } + // Released INSIDE: the click WILL reach this root, and it owns the + // dispatch — including the modifier-link `open-external` branch, which + // stays exactly where it was. Leave `pendingDrag` armed for it. + if (root.contains(up.target as Node | null)) { + return; + } + const pending = pendingDrag.get(root) ?? null; + pendingDrag.delete(root); + dispatchSelection( + view, + releaseRange(view, up, pending) ?? { + anchor: pending?.point?.cellFrom ?? blockStartCaret(root, this), + } + ); + }, + { signal: release.signal } + ); + // ⚠️ The guard that makes the seam safe rather than merely useful. + // + // A release this document never sees — the pointer leaves the webview + // iframe, focus is lost, Cmd+Tab — leaves the listener above armed, and + // the user's NEXT unrelated release would be read as this gesture's end: + // a range from a table cell to a point nobody dragged to. A press is the + // one thing that must precede any such release, so disarming here covers + // every focus-loss path, including the ones nobody enumerated. + // + // CAPTURE, and that is the load-bearing part. Four sibling widgets in this + // editor call stopPropagation() on mousedown — the task checkbox, the + // fenced-code copy and collapse buttons, the language picker — and NONE of + // them stops mouseup. A bubble-phase disarm is therefore starved by + // exactly those presses while the release still arrives, which is the one + // combination that dispatches a range the user never drew (measured: + // bubble 0, capture 1, mouseup delivered). Capture runs document → target, + // so nothing downstream can starve it. + // + // The identity check guards the other direction: this listener is added + // DURING the dispatch of the arming press. In capture that press has + // already passed the document, so it cannot reach here — but the check + // costs one line, says out loud what must stay true, and keeps the guard + // correct if the phase is ever changed back. Comparing the event OBJECT, + // not the target, which a second press in the same cell would match too. + doc.addEventListener( + "mousedown", + (down: MouseEvent) => { + if (down === armingPress || down.button !== 0) { + return; + } + release.abort(); + pendingDrag.delete(root); + }, + { signal: release.signal, capture: true } + ); + // A native drag-and-drop ends in `dragend`, NOT in a mouseup. Measured: a + // plain cell drag starts no DnD at all, so this is for a press that begins + // on an in-cell or , both natively draggable. Nothing is + // preventDefault'ed — the selection seam simply stands down, because a + // drag-and-drop is not a text selection. + doc.addEventListener( + "dragstart", + () => { + release.abort(); + pendingDrag.delete(root); + }, + { signal: release.signal, capture: true } + ); }); // Root click handler — the sole dispatch seam for the caret/range contract @@ -536,4 +700,16 @@ export class TableBlockWidget extends WidgetType { ignoreEvent(): boolean { return true; } + + /** CodeMirror's documented teardown for a widget instance. The gesture + * listeners live on the DOCUMENT and close over `root`, so without this a + * widget destroyed mid-gesture keeps both the listeners and the DOM alive — + * and the listeners keep answering for an editor that has forgotten them. + * The `isConnected` guard in the release seam is not a substitute: `destroy` + * can be called while the DOM is still in the tree. */ + destroy(dom: HTMLElement): void { + armedRelease.get(dom)?.abort(); + armedRelease.delete(dom); + pendingDrag.delete(dom); + } } diff --git a/test/webview/table/cm-table-widget-release.test.ts b/test/webview/table/cm-table-widget-release.test.ts new file mode 100644 index 00000000..35052905 --- /dev/null +++ b/test/webview/table/cm-table-widget-release.test.ts @@ -0,0 +1,334 @@ +// @vitest-environment happy-dom +// The OUTSIDE-RELEASE seam: a drag that starts in a rendered cell and is +// released outside the widget root. Per UI-events a `click` is delivered to the +// nearest common ancestor of the mousedown and mouseup targets — MEASURED in +// real Chromium, not inferred: the release lands on a `.cm-line` and the click +// on `.cm-content`, so the root's click listener never runs and the gesture was +// dispatched as nothing. table-widget.ts therefore also listens for `mouseup` +// on the document, and that listener dispatches ONLY when the release landed +// outside the root; an inside release is left to the click listener, which is +// where the modifier-click `open-external` path lives and stays. +// +// Four things disarm the seam, and each has a row below: the release itself, +// a native drag-and-drop, the widget's destruction, and the NEXT press +// anywhere. That last one is the load-bearing guard — see its row. +// The click-seam contract is cm-table-widget-drag.test.ts. +import { describe, expect, it } from "vitest"; + +import { parseTable } from "../../../src/markdown/table/index.js"; +import { quollOpenExternalSink } from "../../../src/webview/cm/open-external.js"; +import { TableBlockWidget } from "../../../src/webview/cm/table/table-widget.js"; +import { makeWidget, press, SRC, stubViewWithCaret } from "./helpers/widget-fixtures.js"; + +/** A document position in the prose BELOW the table, as `view.posAtCoords` + * would answer it. Any offset outside the table's own source works. */ +const BELOW = 500; + +describe("TableBlockWidget release outside the widget", () => { + it("a drag released over the prose below dispatches cell-offset → release position", () => { + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret( + dispatched, + [{ text: "alpha", offset: 2 }], + [], + () => BELOW + ); + const dom = mount(makeWidget(SRC)); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + // Released on the BODY: no click is delivered to the widget root at all, + // so without the mouseup seam this gesture dispatches nothing. + press(document.body, "mouseup", 30, 400); + expect(dispatched).toEqual([{ selection: { anchor: SRC.indexOf("alpha") + 2, head: BELOW } }]); + }); + + it("a release INSIDE the widget dispatches nothing — the click seam owns it", () => { + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret(dispatched, [ + { text: "alpha", offset: 2 }, + { text: "alpha", offset: 5 }, + ]); + const dom = mount(makeWidget(SRC)); + const td = dom.querySelectorAll("td")[0] as HTMLElement; + press(td, "mousedown", 30, 30); + press(td, "mouseup", 80, 30); + expect(dispatched, "the mouseup seam stands down").toEqual([]); + press(td, "click", 80, 30); + expect(dispatched, "and the click seam dispatches exactly once").toEqual([ + { selection: { anchor: SRC.indexOf("alpha") + 2, head: SRC.indexOf("alpha") + 5 } }, + ]); + }); + + // The Done-when's "modifier-click open-external path proven unchanged", as a + // WHOLE gesture: press, release and click all on the link. The mouseup lands + // inside the root, so the new seam must not consume the pending anchor, must + // not dispatch, and must leave the click listener's link branch untouched. + it("a modifier-click on an in-cell link still opens through the sink, mouseup and all", () => { + const src = "| L |\n| - |\n| [x](https://example.com) |"; + const dispatched: unknown[] = []; + const opened: string[] = []; + const { mount } = stubViewWithCaret( + dispatched, + [{ text: "x", offset: 0 }], + [quollOpenExternalSink.of((href: string) => opened.push(href))] + ); + const dom = mount(makeWidget(src)); + const a = dom.querySelector("a") as HTMLElement; + press(a, "mousedown", 10, 10); + press(a, "mouseup", 10, 10); + const click = press(a, "click", 10, 10, { metaKey: true }); + expect(click.defaultPrevented).toBe(true); + expect(opened).toEqual(["https://example.com"]); + expect(dispatched).toEqual([]); + }); + + // ⚠️ The most important row in this file (Codex 94 / Fable 85 / + // error-handler 83, independently). + // + // A release the webview's document never sees — the pointer leaves the + // iframe, focus is lost, Cmd+Tab — leaves the listener armed. The user's NEXT + // unrelated release would then be read as THIS gesture's end and dispatch a + // range from a table cell to a point the user never dragged to: exactly the + // "selection the user did not draw" the whole widget is built to refuse. + // + // The guard is a document-level `mousedown` disarm rather than a list of + // `blur` / `visibilitychange` / `pointercancel` handlers, because a stray + // mouseup is ALWAYS preceded by a mousedown. That makes the guard total: it + // cannot be outrun by a focus-loss path nobody enumerated. + it("a press elsewhere disarms a gesture whose release never arrived", () => { + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret( + dispatched, + [{ text: "alpha", offset: 2 }], + [], + () => BELOW + ); + const dom = mount(makeWidget(SRC)); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + // ...the mouseup goes to the workbench, outside this document. Later, an + // ordinary unrelated click somewhere else in the editor: + press(document.body, "mousedown", 500, 900); + press(document.body, "mouseup", 500, 900); + expect(dispatched, "the stale gesture must not be resurrected").toEqual([]); + }); + + // The other half of that guard. The trap it names is real but phase- + // dependent: the arming mousedown BUBBLES to the document, so a bubble-phase + // disarm would cancel the gesture it was just armed for — and every + // outside-release row above would go green for the wrong reason (nothing + // dispatched, caret expected nowhere). The capture phase this seam uses + // sidesteps it (the press has already passed the document by then), which is + // exactly why this row stays green either way and cannot be the pin for the + // identity check. It pins something else, and something worth pinning: that + // arming and disarming coexist at all. + it("the arming press does not disarm its own gesture", () => { + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret( + dispatched, + [{ text: "alpha", offset: 2 }], + [], + () => BELOW + ); + const dom = mount(makeWidget(SRC)); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + press(document.body, "mouseup", 30, 400); + expect(dispatched).toEqual([{ selection: { anchor: SRC.indexOf("alpha") + 2, head: BELOW } }]); + }); + + // Fable 80. A right-button press-and-release while the left button is held + // delivers a `button === 2` mouseup to the document. Consuming it would end + // the gesture at the wrong point — and because the one-shot `abort()` runs + // first, the real left release would then find no listener at all. + it("a non-primary release is not the end of the gesture", () => { + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret( + dispatched, + [{ text: "alpha", offset: 2 }], + [], + () => BELOW + ); + const dom = mount(makeWidget(SRC)); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + press(document.body, "mouseup", 200, 200, { button: 2 }); + expect(dispatched, "the right-button release is ignored").toEqual([]); + press(document.body, "mouseup", 30, 400); + expect(dispatched, "and the real release still lands the range").toEqual([ + { selection: { anchor: SRC.indexOf("alpha") + 2, head: BELOW } }, + ]); + }); + + // The phase of the disarm, pinned. No other row here distinguishes capture + // from bubble — they all disarm through presses nothing interferes with. + it("a press that stops propagation still disarms the seam", () => { + // Sibling widgets in this editor — the task checkbox, the fenced-code copy + // and collapse buttons, the language picker — all call stopPropagation() on + // mousedown, and NONE of them stops mouseup. A bubble-phase disarm is + // starved by exactly those presses while the release still arrives: the one + // combination that dispatches a range the user never drew. Capture cannot be + // starved from below. (Measured: bubble 0, capture 1, mouseup delivered.) + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret( + dispatched, + [{ text: "alpha", offset: 2 }], + [], + () => BELOW + ); + const dom = mount(makeWidget(SRC)); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + // ...the release was lost outside this document. Later, a press on a + // sibling widget that swallows mousedown: + const swallower = document.createElement("button"); + document.body.appendChild(swallower); + swallower.addEventListener("mousedown", (event) => event.stopPropagation()); + press(swallower, "mousedown", 500, 900); + press(document.body, "mouseup", 500, 900); + expect(dispatched).toEqual([]); + }); + + it("a native drag-and-drop disarms the seam (no mouseup follows a dragstart)", () => { + // MEASURED (2026-08-22): a plain cell drag fires no `dragstart` at all, so + // this guard is for a press that starts on an in-cell or , both + // natively draggable. A browser that starts a DnD delivers `dragend`, never + // `mouseup`. Nothing here preventDefaults the drag — the seam simply stands + // down, because a drag-and-drop is not a text selection. + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret( + dispatched, + [{ text: "alpha", offset: 2 }], + [], + () => BELOW + ); + const dom = mount(makeWidget(SRC)); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + document.body.dispatchEvent(new Event("dragstart", { bubbles: true })); + press(document.body, "mouseup", 30, 400); + expect(dispatched).toEqual([]); + }); + + it("destroying the widget disarms its gesture", () => { + // `WidgetType.destroy` is CodeMirror's documented removal hook. Without the + // override the closure keeps `root` alive and the listeners keep firing for + // a widget the editor has already forgotten. + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret( + dispatched, + [{ text: "alpha", offset: 2 }], + [], + () => BELOW + ); + const widget = makeWidget(SRC); + const dom = mount(widget); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + widget.destroy(dom); // still IN the document — so `isConnected` cannot mask this + expect(dom.isConnected, "the guard under test is destroy(), not isConnected").toBe(true); + press(document.body, "mouseup", 30, 400); + expect(dispatched).toEqual([]); + }); + + it("a detached root's gesture dispatches nothing", () => { + // CodeMirror rebuilds a widget by REPLACING its root, and the old root's + // offsets stop tracking the document the moment it leaves it. On the click + // seam that was self-enforcing — a detached root receives no clicks — but a + // DOCUMENT-level listener still fires, so the guard has to be explicit. + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret( + dispatched, + [{ text: "alpha", offset: 2 }], + [], + () => BELOW + ); + const dom = mount(makeWidget(SRC)); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + dom.remove(); + press(document.body, "mouseup", 30, 400); + expect(dispatched).toEqual([]); + }); + + it("the seam is one-shot: a second release dispatches nothing more", () => { + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret( + dispatched, + [{ text: "alpha", offset: 2 }], + [], + () => BELOW + ); + const dom = mount(makeWidget(SRC)); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + press(document.body, "mouseup", 30, 400); + expect(dispatched).toHaveLength(1); + press(document.body, "mouseup", 30, 400); + expect(dispatched, "the listener left with the gesture").toHaveLength(1); + }); + + it("a sub-threshold release just outside the widget falls back to the cell caret", () => { + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret(dispatched, [{ text: "alpha", offset: 2 }]); + const dom = mount(makeWidget(SRC)); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + press(document.body, "mouseup", 31, 30); // 1px — a click that slipped off the edge + expect(dispatched).toEqual([{ selection: { anchor: SRC.indexOf("alpha") } }]); + }); + + it("a release the editor cannot place falls back to the cell caret", () => { + // `posAtCoords` answers null for a point it cannot resolve — an unrendered + // block, a gap in the viewport. It does NOT answer null for an overshoot: + // past the last line it clamps to `doc.length` (@codemirror/view 6.43.0), + // which is why the browser suite pins the overshoot as a real RANGE and + // this row scripts the null explicitly. + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret(dispatched, [{ text: "alpha", offset: 2 }], [], () => null); + const dom = mount(makeWidget(SRC)); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + press(document.body, "mouseup", 30, 4000); + expect(dispatched).toEqual([{ selection: { anchor: SRC.indexOf("alpha") } }]); + }); + + it("a press on the widget's own padding falls back to the block-start caret", () => { + // No cell under the press, so there is no anchor to span FROM: `cellPointAt` + // answered null and the gesture has only the block start to offer. The + // caret still reveals this table, which is the whole point of dispatching + // anything at all here. + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret(dispatched, [null], [], () => BELOW); + const dom = mount(makeWidget(SRC, 7)); + press(dom, "mousedown", 30, 30); // the root itself: padding, no cell + press(document.body, "mouseup", 30, 400); + expect(dispatched).toEqual([{ selection: { anchor: 7 } }]); + }); + + it("updateDOM cancels an outside release too (stale-offset guard)", () => { + // The click seam's counterpart of this row is in cm-table-widget-drag.ts. + // A doc edit landing mid-gesture moves every stamp, so the anchor captured + // under the old ones must not be paired with a head resolved under the new. + const dispatched: unknown[] = []; + const { mount, update } = stubViewWithCaret( + dispatched, + [{ text: "alpha", offset: 2 }], + [], + () => BELOW + ); + const first = new TableBlockWidget(parseTable(SRC, 0, SRC.length)!, SRC, 0, 0); + const dom = mount(first); + press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + update(dom, new TableBlockWidget(parseTable(SRC, 0, SRC.length)!, SRC, 5, 5), first); + press(document.body, "mouseup", 30, 400); + // The caret comes from the RE-POINTED block start, not from the anchor. + expect(dispatched).toEqual([{ selection: { anchor: 5 } }]); + }); + + it("a backwards release above the table snaps an unmappable anchor OUTWARD", () => { + // Same rule as the across-cells arm of `dragRange`: direction comes from + // comparing the release position with the cell, and the end that cannot be + // placed exactly snaps AWAY from the other one, so the range still covers + // the cell the pointer started in. + const cell = "a![i](https://x.test/a.png)b"; + const src = `| A |\n| - |\n| ${cell} |`; + const dispatched: unknown[] = []; + const { mount } = stubViewWithCaret(dispatched, [{ text: "b", offset: 0 }], [], () => 0); + const dom = mount(makeWidget(src)); + press(dom.querySelector("td") as HTMLElement, "mousedown", 30, 30); + press(document.body, "mouseup", 30, 0); // released ABOVE the table + expect(dispatched).toEqual([ + { selection: { anchor: src.indexOf(cell) + cell.length, head: 0 } }, + ]); + }); +}); diff --git a/test/webview/table/helpers/widget-fixtures.ts b/test/webview/table/helpers/widget-fixtures.ts index ebe2b143..6f9e9798 100644 --- a/test/webview/table/helpers/widget-fixtures.ts +++ b/test/webview/table/helpers/widget-fixtures.ts @@ -73,8 +73,20 @@ export function drainResolverFailures(): void { // than a spurious one. (Before that probe, deleting this hook left all 78 tests // green: cross-widget capture is prevented by the resolver's private root, not // by body cleanliness, so nothing observed the body between cases.) +// Disposers for widgets mounted through `stubViewWithCaret`. A widget owns +// DOCUMENT-level listeners while a gesture is in flight, and `replaceChildren` +// only unparents its DOM — `destroy` is what CodeMirror itself would call, and +// what actually takes those listeners with it. Without this every suite leaves +// its last mount's listeners attached to the document for the whole file. +const mountedWidgets: Array<() => void> = []; + afterEach(() => { // Cleanup FIRST, so a throwing drain never also leaks DOM into the next test. + // Destroy BEFORE unparenting: the widget's own teardown is what removes the + // document listeners, and it must not depend on the DOM still being attached. + for (const dispose of mountedWidgets.splice(0)) { + dispose(); + } document.body.replaceChildren(); drainResolverFailures(); }); @@ -166,7 +178,17 @@ export const mockView = stubView([]); export function stubViewWithCaret( dispatched: unknown[], script: Array<{ text: string; offset: number } | null>, - extensions: Extension[] = [] + extensions: Extension[] = [], + /** What `view.posAtCoords` answers for a release point OUTSIDE the widget — + * the document position the outside-release seam uses as the range head. + * Scripted per suite, like the caret resolver, because happy-dom has no + * layout and CodeMirror is not mounted here at all. + * + * The default is NOT `() => null`: a null answer degrades to the collapsed + * caret, which is the expected value of several rows in the release suite, so + * an unscripted call would pass vacuously. It records misuse through the same + * channel the caret resolver uses, so an unscripted call is audible instead. */ + posAtCoords?: (x: number, y: number) => number | null ) { let root: HTMLElement | null = null; let i = 0; @@ -235,9 +257,28 @@ export function stubViewWithCaret( state: EditorState.create({ extensions: [quollTableCaretResolver.of(resolve), ...extensions] }), dispatch: (tr: unknown) => dispatched.push(tr), contentDOM, + // ⚠️ `EditorView.posAtCoords` is OVERLOADED — `(coords, precise: false): + // number` and `(coords): number | null` — and a single-signature stub is not + // assignable to the FIRST overload, so `satisfies` rejects it with TS2322 + // ("Type 'number | null' is not assignable to type 'number'"). Measured with + // tsc, and worth knowing WHY this cast is here rather than "cleaning it up": + // vitest is transpile-only, so no test run would catch its removal — the + // error surfaces only at `pnpm compile`. + // + // Only this ONE member is cast, and the key stays IN the `Pick` below, so an + // omitted member is still an error. Casting the whole literal instead would + // give up the typo check (`dispath`) for every member at once, which is the + // entire reason the `satisfies` clause exists. + posAtCoords: ((coords: { x: number; y: number }) => { + if (posAtCoords === undefined) { + resolverFailures.push("view.posAtCoords called with no scripted answer"); + return null; + } + return posAtCoords(coords.x, coords.y); + }) as EditorViewType["posAtCoords"], } satisfies Pick< EditorViewType, - "state" | "dispatch" | "contentDOM" + "state" | "dispatch" | "contentDOM" | "posAtCoords" > as unknown as EditorViewType; /** Mount a widget the way every drag test needs it: rendered, resolver root @@ -263,6 +304,7 @@ export function stubViewWithCaret( const dom = widget.toDOM(view); root = dom; document.body.appendChild(dom); + mountedWidgets.push(() => widget.destroy(dom)); return dom; }; @@ -287,10 +329,15 @@ export function stubViewWithCaret( * them, and happy-dom defaults them to 0. `detail: 1` by default because a * real pointer click always carries a click count; `detail: 0` is reserved for * keyboard/programmatic activation, which the drag path deliberately ignores - * (override it explicitly to exercise that guard). */ + * (override it explicitly to exercise that guard). + * + * `mouseup` is what the OUTSIDE-release seam listens for, and it is dispatched + * on whatever element the pointer was released over — usually NOT the widget, + * which is the whole point of that seam. Aim it at `document.body` to model a + * release that landed outside the table. */ export function press( el: HTMLElement, - type: "mousedown" | "click", + type: "mousedown" | "mouseup" | "click", x: number, y: number, init: MouseEventInit = {} From e2aed4d60a288b6e1d0aff12748fee7824335981 Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Sat, 22 Aug 2026 11:59:25 +1000 Subject: [PATCH 3/6] test(table): pin the outside-release drag with a trusted pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The happy-dom suites drive the widget with hand-built MouseEvents, so none of them observes what a real pointer gesture delivers. These two rows use Playwright's own mousedown/mousemove/mouseup/click over real geometry. The emptiness assertion comes first deliberately: before the fix the editor already parked a collapsed caret at the release position, so asserting the head alone would have passed without the seam existing. The second row covers the overshoot below the last line, which the unit suite cannot see — posAtCoords clamps a point past the document to doc.length rather than answering null, so that gesture is a real range. --- .../helpers/table-drag-harness.ts | 13 ++++++ .../table-drag-selection.browser.test.ts | 43 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/test/webview-browser/helpers/table-drag-harness.ts b/test/webview-browser/helpers/table-drag-harness.ts index 2bb0c2b8..94f55c83 100644 --- a/test/webview-browser/helpers/table-drag-harness.ts +++ b/test/webview-browser/helpers/table-drag-harness.ts @@ -38,6 +38,9 @@ export const PLAIN = DOC.indexOf("plain"); /** Source offset of the table block's first byte — the caret the widget falls * back to when a gesture maps to no cell at all (`blockStartCaret`). */ export const TABLE_BLOCK_START = DOC.indexOf("| Alpha"); +/** Source offset of the paragraph AFTER the table — the release point for the + * gesture "select the table plus the paragraph below". */ +export const TAIL = DOC.indexOf("tail"); /** Production extension order for the table island (editor.ts: skeleton field * BEFORE the block field). Caret parked at doc end so the line-level reveal is @@ -82,6 +85,16 @@ export function widgetRoot(v: EditorView): HTMLElement { return root as HTMLElement; } +/** The rendered `.cm-line` holding `text`, as ordinary editable prose OUTSIDE + * the table widget — the release target for a drag that leaves the widget. */ +export function proseLine(v: EditorView, text: string): HTMLElement { + const line = [...v.contentDOM.querySelectorAll(".cm-line")].find( + (l) => l.textContent === text + ); + expect(line, `prose line "${text}" must be rendered`).toBeDefined(); + return line as HTMLElement; +} + export function cellByText(v: EditorView, text: string): HTMLElement { const cell = [...widgetRoot(v).querySelectorAll("th, td")].find( (c) => c.textContent === text diff --git a/test/webview-browser/table-drag-selection.browser.test.ts b/test/webview-browser/table-drag-selection.browser.test.ts index ce5c9172..7d298c2a 100644 --- a/test/webview-browser/table-drag-selection.browser.test.ts +++ b/test/webview-browser/table-drag-selection.browser.test.ts @@ -33,8 +33,10 @@ import { PLAIN, pointAtChar, pointInWidgetPadding, + proseLine, revealed, TABLE_BLOCK_START, + TAIL, unmount, widgetRoot, } from "./helpers/table-drag-harness.js"; @@ -186,6 +188,47 @@ describe.each(ARMS)("table drag selection — trusted pointer, %s", (_name, arm) expect(revealed(view), "the fallback caret still reveals this table").toBe(true); }); + it("a drag from a cell RELEASED over the paragraph below spans from the cell into the prose", async () => { + // The gesture that was silently lost. Measured before the fix: the release + // lands on a `.cm-line`, the click is retargeted to `.cm-content` so the + // root's click listener never runs, and CodeMirror's own observer parks a + // COLLAPSED CARET at the release position. That last detail is why the + // emptiness assertion comes first: today's caret already sits at TAIL + 2, + // so asserting the head alone would pass without the seam existing. + view = mount(arm); + await settled(); + const cell = cellByText(view, "gamma"); + const tail = proseLine(view, "tail"); + await dragPointer(cell, pointAtChar(cell, 1), tail, pointAtChar(tail, 2)); + await settled(); + + const sel = view.state.selection.main; + expect(sel.empty, "the gesture must land a RANGE, not the caret it lands today").toBe(false); + expect(sel.anchor, "anchor is the pressed cell's source offset").toBe(GAMMA + 1); + expect(sel.head, "head is the release position in the prose below").toBe(TAIL + 2); + expect(revealed(view), "and the range still fires the table's reveal").toBe(true); + }); + + it("a drag released BELOW the last line runs to the end of the document", async () => { + // The commonest live overshoot, and the one the unit suite cannot see: + // `posAtCoords` clamps a point past the document to `doc.length` rather than + // answering null (@codemirror/view 6.43.0), so this is a real range, not a + // degrade. Released well below the editor's own box. + view = mount(arm); + await settled(); + const cell = cellByText(view, "gamma"); + const box = view.dom.getBoundingClientRect(); + await dragPointer(cell, pointAtChar(cell, 1), document.body, { + x: box.left + box.width / 2, + y: box.bottom + 40, + }); + await settled(); + + const sel = view.state.selection.main; + expect(sel.anchor).toBe(GAMMA + 1); + expect(sel.head, "clamped to the document end, not refused").toBe(view.state.doc.length); + }); + it("a press and release at the SAME point stays a click: collapsed caret at the cell start", async () => { // Non-vacuity control for every drag above: the ranges they assert come // from pointer TRAVEL past DRAG_THRESHOLD_PX, not from any click reaching From 911884e4942f9958678d223307aea5be00b904fb Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Sat, 22 Aug 2026 12:47:57 +1000 Subject: [PATCH 4/6] fix(table): address review findings on the drag-release seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Type soundness: isDrag's false arm narrowed on conditions that are not properties of its argument (detail and travel), so TS's negative inference lied. Split into a pure isArmed guard plus armedDragFor, which returns a value and therefore has no false arm at all. Coordinate and offset spaces: contentPoint returned a bare {x, y}, structurally identical to the viewport points passed to posAtCoords and cellPointAt two lines away. Named ContentPoint makes the mis-pass a compile error. releaseRange was the one absolute-space entry in this family with no asAbsoluteOffset mint, which the module's own policy requires be said out loud; DragSelection now names the shape both seams produce. Diagnostics: the release-lookup catch logged {err} alone, below the standard this file sets twice. It now carries the cell offset and the coordinates. Phase: the dispatching mouseup was bubble while both disarms were capture, resting on an enumeration of today's siblings — the reasoning the adjacent comment rejects. It is capture now. CodeMirror registers no document mouseup for a gesture that starts in an ignoreEvent widget, so the ordering change is inert; the browser suite measures unchanged at 64. Coverage: four paths had no test — the forward arm of the outward snap (a mutation to a constant left every row green), the release-lookup catch, the multi-widget disarm ordering, and the fixture's posAtCoords misuse channel. A byte-identical duplicate row that could not pin what it named is replaced by one pinning re-arm after a completed gesture. Comments: the file header documented only the click seam while the code had two; 'sole dispatch seam' sat 35 lines above 'The SECOND dispatch seam'; and several internal line references had rotted. Bare :NNN refs are replaced by file-and-symbol references, which do not rot. --- src/webview/cm/table/table-widget.ts | 242 +++++++++++++----- .../table-drag-selection.browser.test.ts | 6 +- .../table/cm-table-widget-drag.test.ts | 31 ++- .../table/cm-table-widget-release.test.ts | 202 +++++++++++++-- .../helpers/widget-fixtures-guards.test.ts | 24 ++ test/webview/table/helpers/widget-fixtures.ts | 45 +++- 6 files changed, 443 insertions(+), 107 deletions(-) diff --git a/src/webview/cm/table/table-widget.ts b/src/webview/cm/table/table-widget.ts index 50849943..3350c3cb 100644 --- a/src/webview/cm/table/table-widget.ts +++ b/src/webview/cm/table/table-widget.ts @@ -17,6 +17,19 @@ // snapping, fall back to the same caret dispatch as a plain click. (See // cell-point.ts for the pointer→source-offset mapping and why the widget must // do this itself rather than reading the browser selection at mouseup.) +// A gesture RELEASED OUTSIDE this widget delivers no click here — the click is +// retargeted to the nearest common ancestor of the press and release targets, +// `.cm-content` — so `mousedown` also arms a DOCUMENT-level `mouseup` seam. +// Same drag test, but the head comes from `view.posAtCoords` (the release is on +// no cell, so there is nothing for the source map to answer) and an unmappable +// anchor snaps outward against it. That this widget owns listeners on the +// DOCUMENT — which outlive its own DOM, since CodeMirror can replace a widget +// root mid-gesture — is the most surprising fact about this module, and it is +// exactly why `destroy()` and the seam's `isConnected` guard exist: without +// them a discarded widget keeps answering for an editor that has forgotten it. +// The seam is armed per gesture and disarmed by four paths — the gesture's own +// PRIMARY-button release, any later press (capture phase), `dragstart`, and +// `destroy` — each with its own comment at the site. // The dispatched selection — caret or range — is what fires tableBlockField's // line-level reveal-on-caret, surfacing the source for editing. // @@ -37,6 +50,8 @@ import { type Align, type Cell, type Table, tableAlign } from "../../../markdown import { quollResourceBaseUri } from "../image/resource-base.js"; import { quollOpenExternalSink } from "../open-external.js"; import { + type AbsoluteOffset, + asAbsoluteOffset, type CellPoint, cellPointAt, quollTableCaretResolver, @@ -51,28 +66,46 @@ import { renderCellInto } from "./cell-render.js"; // collapsed caret lands — a regression of the existing click contract. const DRAG_THRESHOLD_PX = 4; -/** Where a drag started, remembered between `mousedown` and `click`. +/** A pointer position RELATIVE TO THE CONTENT, not to the viewport. The content + * can move under a held pointer — a scroll mid-gesture, a `scrollIntoView` + * from an incoming host message, CodeMirror's own scrolling — and a pointer + * that stayed still through one of those HAS crossed text. Two `clientX/Y` + * pairs cannot see that, and called the gesture a click. + * + * Relative to `contentDOM`'s rect rather than `scrollDOM`'s scroll offsets so + * that both operands are VISUAL pixels: CodeMirror supports being CSS + * transformed (`view.scaleX` / `scaleY` exist for exactly that), and mixing a + * viewport coordinate with a layout-space scroll offset breaks the threshold + * under any scale but 1. Measuring both ends the same way makes the scale + * cancel instead of needing a correction. + * + * Named `contentX`/`contentY` rather than `x`/`y` so the two frames differ by + * SHAPE, not merely by this comment: a viewport point and a content point are + * both pairs of numbers, so a structural `{x, y}` lets + * `view.posAtCoords(contentPoint(view, event))` — a viewport API handed a + * content point — compile silently, and the bug it produces (a threshold that + * misreads travel whenever the editor is scrolled) is invisible until someone + * scrolls. This is cell-source-map.ts's "every crossing between spaces is said + * out loud" policy applied to coordinates; the rename alone makes each + * mis-pairing a missing-property error, so no `unique symbol` is minted. */ +interface ContentPoint { + readonly contentX: number; + readonly contentY: number; +} + +/** Where a drag started, remembered between `mousedown` and whichever seam ends + * the gesture — the root's `click` for a release INSIDE this widget, the + * document `mouseup` for a release outside. Both terminating seams read the + * entry and delete it; naming only the click would tell a reader that a + * gesture ending outside the widget cannot consult it, when those are exactly + * the gestures the outside-release seam exists for. * * Keyed on the widget's root ELEMENT rather than held in the `toDOM` closure * because `updateDOM` reuses that element across widget instances: the entry * has to be invalidated exactly when the cell stamps move, which is something * `updateDOM` can do and the closure cannot. A WeakMap so a discarded widget * root takes its entry with it. */ -interface PendingDrag { - /** The press point RELATIVE TO THE CONTENT, not to the viewport. The content - * can move under a held pointer — a scroll mid-gesture, a `scrollIntoView` - * from an incoming host message, CodeMirror's own scrolling — and a pointer - * that stayed still through one of those HAS crossed text. Two `clientX/Y` - * pairs cannot see that, and called the gesture a click. - * - * Relative to `contentDOM`'s rect rather than `scrollDOM`'s scroll offsets so - * that both operands are VISUAL pixels: CodeMirror supports being CSS - * transformed (`view.scaleX` / `scaleY` exist for exactly that), and mixing a - * viewport coordinate with a layout-space scroll offset breaks the threshold - * under any scale but 1. Measuring both ends the same way makes the scale - * cancel instead of needing a correction. */ - readonly contentX: number; - readonly contentY: number; +interface PendingDrag extends ContentPoint { readonly point: CellPoint | null; } const pendingDrag = new WeakMap(); @@ -147,10 +180,12 @@ function dispatchSelection(view: EditorView, selection: { anchor: number; head?: } } -/** A pointer position in the CONTENT's frame of reference. */ -function contentPoint(view: EditorView, event: MouseEvent): { x: number; y: number } { +/** THE constructor of a {@link ContentPoint} — the one sanctioned crossing from + * the viewport frame into the content frame, mirroring `asAbsoluteOffset`'s + * role in cell-point.ts. */ +function contentPoint(view: EditorView, event: MouseEvent): ContentPoint { const origin = view.contentDOM.getBoundingClientRect(); - return { x: event.clientX - origin.left, y: event.clientY - origin.top }; + return { contentX: event.clientX - origin.left, contentY: event.clientY - origin.top }; } /** Manhattan pointer travel since the press, over the CONTENT. One measurement @@ -158,7 +193,7 @@ function contentPoint(view: EditorView, event: MouseEvent): { x: number; y: numb * a click. Manhattan (not Euclidean) to match the threshold the suite pins. */ function travelSince(view: EditorView, pending: PendingDrag, event: MouseEvent): number { const now = contentPoint(view, event); - return Math.abs(now.x - pending.contentX) + Math.abs(now.y - pending.contentY); + return Math.abs(now.contentX - pending.contentX) + Math.abs(now.contentY - pending.contentY); } /** A `PendingDrag` whose press landed on a cell — the only kind either seam can @@ -167,44 +202,77 @@ interface ArmedDrag extends PendingDrag { readonly point: CellPoint; } -/** Is this completed gesture a DRAG (rather than a click, a keyboard or - * programmatic activation, or nothing this widget armed)? ONE definition for - * both seams, so the click and the outside release can never disagree about - * what a drag is. +/** Pure type guard: false ⟺ not an `ArmedDrag`. Sound in BOTH arms, so the + * compiler's negative inference stays true however it is called. */ +function isArmed(pending: PendingDrag | null): pending is ArmedDrag { + return pending !== null && pending.point !== null; +} + +/** The armed anchor this completed gesture is a DRAG from, or `null` when it is + * a click, a keyboard/programmatic activation, or nothing this widget armed. + * ONE definition for both seams, so the click and the outside release can never + * disagree about what a drag is. * * `detail === 0` means the event was NOT produced by a pointer gesture — * keyboard activation of an in-cell ``, or a programmatic `.click()` / * `dispatchEvent`. Its clientX/Y are 0, so pairing it with an armed anchor - * would read a large bogus travel and dispatch a range the user never drew. */ -function isDrag( + * would read a large bogus travel and dispatch a range the user never drew. + * + * Returns a VALUE rather than asserting `pending is ArmedDrag`: two of the + * three gates are properties of `event`, not of `pending`, so a predicate would + * license tsc to conclude "this press did not land on a cell" from "this + * gesture did not travel far enough" — an unsound negative inference that + * happens to be harmless only because both call sites pass the wide type + * today. Handing back the narrowed anchor removes the false arm entirely + * instead of relying on that. */ +function armedDragFor( view: EditorView, event: MouseEvent, pending: PendingDrag | null -): pending is ArmedDrag { - if (pending === null || pending.point === null) { - return false; - } - if (event.detail === 0) { - return false; +): ArmedDrag | null { + if (!isArmed(pending) || event.detail === 0) { + return null; } - return travelSince(view, pending, event) >= DRAG_THRESHOLD_PX; + return travelSince(view, pending, event) >= DRAG_THRESHOLD_PX ? pending : null; +} + +/** A NON-COLLAPSED selection range in ABSOLUTE document space — the only thing + * either drag seam produces, and the reason both end in an explicit collapse + * check: a zero-width result degrades to the caret path instead of being + * dispatched as a range. + * + * Shared by `dragRange` and `releaseRange` rather than named for just one of + * them: the two differ only in where the HEAD comes from (a cell's source map + * vs. `view.posAtCoords`), while the contract they hand the caller — absolute + * space, anchor ≠ head, `null` means "dispatch the caret instead" — is + * identical. Naming it once means a third seam inherits that contract rather + * than re-deriving it, and picking either producer to own the name would leave + * the next author guessing which one to copy. */ +interface DragSelection { + readonly anchor: AbsoluteOffset; + readonly head: AbsoluteOffset; } /** The RANGE a completed pointer gesture describes, or `null` when this gesture - * is not a drag at all (see `isDrag`), when the head has no mapping, or when - * the range collapses after snapping. Every `null` answer means the same thing - * to the caller: dispatch the plain collapsed caret instead. + * is not a drag at all (see `armedDragFor`), when the head has no mapping, or + * when the range collapses after snapping. Every `null` answer means the same + * thing to the caller: dispatch the plain collapsed caret instead. * * The aborted-gesture window this used to close through `detail === 0` is now - * ALSO closed structurally: the release seam disarms itself on every mouseup, - * and any press elsewhere disarms a gesture whose release never arrived. */ + * ALSO closed structurally: the release seam is one-shot on the gesture's own + * PRIMARY-button release — a non-primary mouseup returns BEFORE the abort, so + * a right-button click while the left is held cannot end a gesture it never + * belonged to, and a release inside the root leaves the anchor armed for the + * click seam that owns it — and any press elsewhere disarms a gesture whose + * release never arrived. */ function dragRange( view: EditorView, root: HTMLElement, event: MouseEvent, pending: PendingDrag | null -): { anchor: number; head: number } | null { - if (!isDrag(view, event, pending)) { +): DragSelection | null { + const armed = armedDragFor(view, event, pending); + if (armed === null) { return null; } const head = cellPointAt( @@ -216,7 +284,7 @@ function dragRange( if (head === null) { return null; } - const start = pending.point; + const start = armed.point; if (start.cellFrom === head.cellFrom) { // ONE cell. An unmappable end carries no direction here: a rendered offset // beside a construct that renders no text measures the SAME on both sides @@ -275,23 +343,46 @@ function releaseRange( view: EditorView, event: MouseEvent, pending: PendingDrag | null -): { anchor: number; head: number } | null { - if (!isDrag(view, event, pending)) { +): DragSelection | null { + const armed = armedDragFor(view, event, pending); + if (armed === null) { return null; } - let head: number | null; + let raw: number | null; try { - head = view.posAtCoords({ x: event.clientX, y: event.clientY }); + raw = view.posAtCoords({ x: event.clientX, y: event.clientY }); } catch (err) { - // Same contract as `dispatchSelection`'s catch: a coordinate lookup against - // a view torn down mid-gesture must cost the gesture, not the editor. - console.error("[quoll] table widget release lookup failed", { err }); + // Same contract as `dispatchSelection`'s catch: a failed coordinate lookup + // must cost the gesture, not the editor. + // + // The payload carries what it takes to REPRODUCE the degrade, for the reason + // `blockStartCaret` logs `slice` — a document can hold many tables, and the + // error alone would not say which one, nor where the pointer was. It matters + // more here than at either sibling site because `posAtCoords` has a + // SYSTEMATIC throw mode, not only a teardown one: it calls `readMeasured()`, + // which throws "Reading the editor layout isn't allowed during an update" + // whenever `updateState === Updating` (@codemirror/view 6.43.0) — and a DOM + // mouseup is exactly the thing an in-progress update can deliver. Every such + // drag silently collapses to a caret, so the log is the only trace. + console.error("[quoll] table widget release lookup failed", { + cellFrom: armed.point.cellFrom, + x: event.clientX, + y: event.clientY, + err, + }); return null; } - if (head === null) { + if (raw === null) { return null; } - const start = pending.point; + // The crossing into absolute document space, minted here for the reason + // cell-point.ts mints at its own one legal crossing: a grep for + // `asAbsoluteOffset` must enumerate EVERY entry into this space, and this is + // the only one in the family that lacked a mint. CodeMirror answers a real + // document position — clamped to `[0, doc.length]`, never a fraction — so + // there is nothing further to validate, only to say out loud. + const head = asAbsoluteOffset(raw); + const start = armed.point; const anchor = start.offset ?? (head > start.cellFrom ? start.cellFrom : start.cellTo); return anchor === head ? null : { anchor, head }; } @@ -356,9 +447,11 @@ export class TableBlockWidget extends WidgetType { root.appendChild(table); // Two root listeners, one gesture. `mousedown` only ARMS the gesture - // (anchor point + coordinates); `click` — which fires after mouseup and - // already owns the caret dispatch and the modifier-link path — is the sole - // dispatch seam. + // (anchor point + coordinates) and arms the document-level release seam + // below; `click` — which fires after mouseup and already owns the caret + // dispatch and the modifier-link path — is the dispatch seam for a gesture + // RELEASED INSIDE this root. A release outside never delivers a click here, + // so that case is dispatched by the document `mouseup` seam instead. // // Deliberately NOT preventDefault'ed: the native mousedown default is what // moves focus into CodeMirror's contenteditable, and without focus the @@ -376,10 +469,8 @@ export class TableBlockWidget extends WidgetType { } // No dispatch here: dispatching would fire the reveal mid-drag and pull // the widget out from under the pointer. - const at = contentPoint(view, event); pendingDrag.set(root, { - contentX: at.x, - contentY: at.y, + ...contentPoint(view, event), // `cellPointAt` keeps taking VIEWPORT coordinates — it feeds // `caretPositionFromPoint`, which is a viewport API. Only the travel // measurement changes frame. @@ -436,7 +527,23 @@ export class TableBlockWidget extends WidgetType { } ); }, - { signal: release.signal } + // CAPTURE, for the same reason the disarm below is: this seam must not + // be starvable by a sibling widget that stops `mouseup`. Bubble rested + // on "none of today's siblings stops mouseup" — an enumeration, which is + // the class of assumption the disarm's own comment rejects — and the + // failure would be silent, because a starved seam dispatches nothing, + // which is precisely the pre-fix behaviour. + // + // Capture also moves this ahead of CodeMirror's own document `mouseup` + // (`MouseSelection.up`), and that reordering is a no-op HERE by + // construction, not by luck: `eventBelongsToEditor` walks from the event + // target up to `contentDOM` and bails at any widget whose + // `ignoreEvent()` is true (@codemirror/view 6.43.0), so this widget's + // mousedown never reaches CM's `handlers.mousedown`, no `MouseSelection` + // is constructed, and its constructor is the ONLY thing that registers + // that document listener. For a gesture armed in this widget, CM has no + // document mouseup listener to be ordered against. + { signal: release.signal, capture: true } ); // ⚠️ The guard that makes the seam safe rather than merely useful. // @@ -488,8 +595,10 @@ export class TableBlockWidget extends WidgetType { ); }); - // Root click handler — the sole dispatch seam for the caret/range contract - // described at the top of this file. + // Root click handler — the dispatch seam for a gesture released INSIDE this + // root (the caret/range contract described at the top of this file); its + // outside-release counterpart is the document `mouseup` seam armed in the + // `mousedown` listener above. // // Modifier-click on a live `` (external nav — cell-render left it // un-preventDefault'd because the href is absolute AND within @@ -632,9 +741,11 @@ export class TableBlockWidget extends WidgetType { blockStart.set(dom, this.docFrom); // Pure positional shift: the bytes are identical (from.slice === this.slice) // and only the absolute offsets moved. Re-stamp data-cell-from on each cell - // and reuse the rendered inline children verbatim — skip patchRow's - // textContent="" + renderCellInto re-tokenize (its own design comment, - // :16-18). This is the hot path when typing in a paragraph ABOVE the table. + // and reuse the rendered inline children verbatim — skip `patchRow`'s + // `renderCellInto` call, which clears the cell and re-tokenizes it + // (cell-render.ts's header: `renderCellInto` is the only supported way to + // fill a cell, so clearing is ITS job, not `patchRow`'s). This is the hot + // path when typing in a paragraph ABOVE the table. if (from.slice === this.slice) { this.stampRow(headerRows[0], this.table.header.cells); for (let rowIdx = 0; rowIdx < this.table.rows.length; rowIdx++) { @@ -642,8 +753,11 @@ export class TableBlockWidget extends WidgetType { } return true; } - // Content edit (slice changed): full re-render. patchRow re-stamps cellFrom - // itself (:198), so offsets stay correct on this path too. + // Content edit (slice changed): full re-render. `patchRow` re-stamps + // `data-cell-from` / `data-cell-to` itself before each `renderCellInto`, so + // offsets stay correct on this path too. (Named by symbol rather than by + // line: bare `:NNN` refs in this file rotted twice as the gesture seams + // grew above them.) const resourceBase = view.state.facet(quollResourceBaseUri); const align = tableAlign(this.table); this.patchRow(headerRows[0], this.table.header.cells, align, resourceBase); diff --git a/test/webview-browser/table-drag-selection.browser.test.ts b/test/webview-browser/table-drag-selection.browser.test.ts index 7d298c2a..11ce5de9 100644 --- a/test/webview-browser/table-drag-selection.browser.test.ts +++ b/test/webview-browser/table-drag-selection.browser.test.ts @@ -14,8 +14,10 @@ // rectangles measured with a DOM Range, and the facet's real caret-from-point // resolvers. // -// Both caret-from-point arms run, because there are two floor-dependent ones -// (see `ARMS` below), not one. +// Every contract below runs TWICE, once per caret-from-point arm: the default +// (`caretPositionFromPoint`, what this runner's Chromium takes) and the +// `caretRangeFromPoint` fallback, which is the one arm LIVE on the extension's +// floor and would otherwise never be exercised here. See `ARMS` below. import type { Extension } from "@codemirror/state"; import type { EditorView } from "@codemirror/view"; import { afterEach, describe, expect, it } from "vitest"; diff --git a/test/webview/table/cm-table-widget-drag.test.ts b/test/webview/table/cm-table-widget-drag.test.ts index 9ae89c57..ebdb8ff9 100644 --- a/test/webview/table/cm-table-widget-drag.test.ts +++ b/test/webview/table/cm-table-widget-drag.test.ts @@ -246,9 +246,15 @@ describe("TableBlockWidget drag-selection", () => { expect(opened).toEqual(["https://example.com"]); expect(dispatched).toEqual([]); // The NEXT click, far away and with no mousedown of its own, must not - // resurrect the cleared anchor. If it leaked, this click sees moved=true - // with a non-null point (link cell → offset null → whole-cell snap) and - // dispatches a RANGE spanning the cell instead of the caret below. + // resurrect the cleared anchor. Which mechanism reddens the row matters, + // because it is NOT the assertion below: with the anchor leaked, + // `armedDragFor` sees 390px of travel against a non-null point and hands + // `dragRange` an armed anchor, `dragRange` then resolves the HEAD through + // `cellPointAt` — a SECOND resolver call this vehicle's one-step script + // cannot answer. The fixture records "resolver call #2 ran off the end of a + // 1-step script" and returns null, so `head === null`, `dragRange` returns + // null, a caret is dispatched and the expectation below PASSES. The leak + // surfaces only through `drainResolverFailures` in the shared `afterEach`. press(dom.querySelector("td") as HTMLElement, "click", 400, 10); expect(dispatched).toEqual([ { selection: { anchor: src.indexOf("[x](https://example.com)") } }, @@ -506,12 +512,19 @@ describe("TableBlockWidget drag-selection", () => { ]); }); - // Aborted-gesture guard. A press released OUTSIDE the widget delivers no - // click to the root, so the armed anchor survives with stale coordinates. - // The only click that can then reach this handler without a mousedown of its - // own is a keyboard/programmatic one — `detail === 0`, clientX/Y 0 — which - // would otherwise read a huge bogus travel and dispatch a range the user - // never drew. It must take the caret path instead. + // Non-gesture activation. A keyboard or programmatic click — `detail === 0`, + // clientX/Y 0 — can reach this handler while an anchor is still armed (a + // press whose release the document never saw: the pointer left the webview + // iframe, focus was lost, Cmd+Tab), and would then read a huge bogus travel + // and dispatch a range the user never drew. It must take the caret path + // instead. + // + // ⚠️ Do NOT re-justify this row with "a press released OUTSIDE the widget + // leaves the anchor armed" — that window is closed structurally now: the + // outside-release seam deletes `pendingDrag` itself, and any later press + // disarms what a lost release did not (cm-table-widget-release.test.ts). + // What `detail === 0` still guards is the genuine non-pointer activation + // above. it("a detail-0 click (keyboard / programmatic) never takes the drag path", () => { const dispatched: unknown[] = []; const { mount } = stubViewWithCaret(dispatched, [ diff --git a/test/webview/table/cm-table-widget-release.test.ts b/test/webview/table/cm-table-widget-release.test.ts index 35052905..e25ce002 100644 --- a/test/webview/table/cm-table-widget-release.test.ts +++ b/test/webview/table/cm-table-widget-release.test.ts @@ -13,7 +13,7 @@ // a native drag-and-drop, the widget's destruction, and the NEXT press // anywhere. That last one is the load-bearing guard — see its row. // The click-seam contract is cm-table-widget-drag.test.ts. -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { parseTable } from "../../../src/markdown/table/index.js"; import { quollOpenExternalSink } from "../../../src/webview/cm/open-external.js"; @@ -111,27 +111,56 @@ describe("TableBlockWidget release outside the widget", () => { expect(dispatched, "the stale gesture must not be resurrected").toEqual([]); }); - // The other half of that guard. The trap it names is real but phase- - // dependent: the arming mousedown BUBBLES to the document, so a bubble-phase - // disarm would cancel the gesture it was just armed for — and every - // outside-release row above would go green for the wrong reason (nothing - // dispatched, caret expected nowhere). The capture phase this seam uses - // sidesteps it (the press has already passed the document by then), which is - // exactly why this row stays green either way and cannot be the pin for the - // identity check. It pins something else, and something worth pinning: that - // arming and disarming coexist at all. - it("the arming press does not disarm its own gesture", () => { + // The far side of the one-shot row below: that row pins that a release with + // no new press behind it dispatches nothing, and this one pins that a release + // WITH one does. Together they are the whole cycle — arm, fire, re-arm — and + // neither half implies the other. + // + // ⚠️ It replaces a row titled "the arming press does not disarm its own + // gesture", which was byte-identical to the first row of this file. The + // knowledge that row carried is worth keeping even though the row was not: + // the arming mousedown BUBBLES to the document, so a bubble-phase disarm + // would cancel the gesture it was just armed for, and every outside-release + // row in this file would then go green for the wrong reason (nothing + // dispatched, caret expected nowhere). The `down === armingPress` identity + // check in table-widget.ts guards exactly that, and under the `capture: true` + // this seam uses it is UNREACHABLE — the press has already passed the + // document before the listener exists. So no row here can pin it, and the old + // title claimed a pin that does not exist. (Measured: dropping the identity + // check leaves the suite green.) + // + // ⚠️ What THIS row pins, stated honestly: the observable re-arm cycle, not + // the `armedRelease.get(root)?.abort()` line that runs at arm time. That line + // is unobservable here — gesture 1's controller was already aborted by its + // own mouseup, so aborting it again is a no-op. No single-line mutation of + // the seam was found that reddens this row (measured: dropping the arm-time + // abort, dropping `armedRelease.set`, and dropping the mouseup's own + // `release.abort()` all leave it green — the last two redden the destroy and + // one-shot rows instead). It is a behavioural tripwire for the cycle, and the + // scripted head makes it discriminating about WHICH gesture answered. + it("a completed gesture re-arms: the next press draws its own range", () => { const dispatched: unknown[] = []; + // The head is the release's own Y, so the two gestures cannot be confused + // for one another: a second dispatch carrying the FIRST release's head + // would mean the stale listener answered, not a freshly armed one. const { mount } = stubViewWithCaret( dispatched, - [{ text: "alpha", offset: 2 }], + [ + { text: "alpha", offset: 2 }, + { text: "alpha", offset: 2 }, + ], [], - () => BELOW + (_x, y) => y ); - const dom = mount(makeWidget(SRC)); - press(dom.querySelectorAll("td")[0] as HTMLElement, "mousedown", 30, 30); + const td = mount(makeWidget(SRC)).querySelectorAll("td")[0] as HTMLElement; + press(td, "mousedown", 30, 30); press(document.body, "mouseup", 30, 400); - expect(dispatched).toEqual([{ selection: { anchor: SRC.indexOf("alpha") + 2, head: BELOW } }]); + press(td, "mousedown", 30, 30); + press(document.body, "mouseup", 30, 600); + expect(dispatched).toEqual([ + { selection: { anchor: SRC.indexOf("alpha") + 2, head: 400 } }, + { selection: { anchor: SRC.indexOf("alpha") + 2, head: 600 } }, + ]); }); // Fable 80. A right-button press-and-release while the left button is held @@ -156,8 +185,11 @@ describe("TableBlockWidget release outside the widget", () => { ]); }); - // The phase of the disarm, pinned. No other row here distinguishes capture - // from bubble — they all disarm through presses nothing interferes with. + // The phase of the DISARM listener (the document `mousedown`), pinned. No + // other row here distinguishes capture from bubble for it — they all disarm + // through presses nothing interferes with. Its pair is the row below, which + // pins the phase of the DISPATCH listener (the document `mouseup`); the two + // read alike and are not interchangeable, so each names its listener. it("a press that stops propagation still disarms the seam", () => { // Sibling widgets in this editor — the task checkbox, the fenced-code copy // and collapse buttons, the language picker — all call stopPropagation() on @@ -184,6 +216,82 @@ describe("TableBlockWidget release outside the widget", () => { expect(dispatched).toEqual([]); }); + // Every other disarm row presses on `document.body` or a bare `