fix(editor): block-drag global listeners defer to a frontmost viewer (BUG-2453) (#1373)

`block-drag-handle.ts` installs four WINDOW-level listeners and none of them
asked whether anything was in front of the editor. Background inertness stops
NEW interaction with the editor; it does not stop a gesture that already owns
the pointer. So a block drag in flight when the attachment viewer opened kept
running against an editor that was `inert` and invisible behind the viewer's
backdrop — and the drop still landed.

This is the deferred half of TASK-2430, which taught seven global owners to
stand down and closed with "Deferred: svelte-dnd-action + editor block-drag
global gestures."

FOUR OWNERS, NOT THE TWO THE BUG NAMES. It names the mouse pair; `touchmove`
and `touchend` are installed on the same line with the identical hole, so a
mouse-only fix would have left every touch device unguarded.

Each handler consults the shared arbitration helper — `isBlockedByModal`, never
a raw lease query — with `editorView.dom` as the owner, the surface asking to
act rather than `event.target`, which during a straddling gesture is whatever
the pointer happens to be over.

ABORT, NOT EARLY-RETURN, the choice `ItemGraph.abortPan` already makes for its
captured pan. An early return leaves `dragging` latched, the block at 20%
opacity, the ghost under the cursor and `pointerEvents: none` on the editor — a
half-drag frozen onto a surface that is now behind a viewer, with nothing left
to deliver the event that would end it. The teardown reuses `cancelDrag`, which
already existed and had NO callers; unlike `endDrag` it never calls
`executeMove`, so aborting cannot commit the reorder.

The gates sit above the branch structure, not inside the `dragging` arm:
`mouseup` and `touchend` each also open the block menu on a press that never
moved, and a guard placed in the drag arm would still pop a menu onto the inert
editor beneath the viewer.

`isBlockedByModal` returns false on an empty lease stack, so with no viewer open
every path is untouched. Each of the four owners has an inert-while-frontmost
leg and the contract's empty-stack regression, and the empty-stack legs assert
what ships TODAY — the trap TASK-2430 shipped and reverted twice, where a test
labelled a regression actually specified a change.

Codex round 2 closed two more, both in the abort's own teardown: `abortDrag`
left `activeBlock` populated where `endDrag` nulls it, and the comment beside
it claimed to clear two pending flags when `cancelDrag` already owned one of
them. The first is consistency rather than a defect closed — no path was found
that acts on a stale `activeBlock` — and the comment now says so instead of
implying a bug was fixed.

Six mutants, each killed by its own legs: removing any of the four gates;
replacing the abort with a bare early return; and pointing the owner at a
viewer-contained node.

The reorder itself is proven suppressed, on both `mouseup` and `touchend`, by
legs that drag the SECOND block. That detail is load-bearing: jsdom gives every
element a zero `getBoundingClientRect`, so `dropPosAtY` scores all candidates
alike and keeps the first — for a drag of block 1 that is the slot the node is
already in, so the drop runs and changes nothing and a doc-equality assertion
passes either way. An earlier revision of this change asserted exactly that and
was green with the gate deleted. Dragging block 2 makes the first candidate
position 0, the paragraphs swap, and the assertion has something to see; each
reorder leg is paired with an empty-stack leg proving the drag still reorders
with no viewer, so a future harness break cannot turn it back into a vacuous
pass.

Claude-Session: https://claude.ai/code/session_019EmDd3Wy8b3cPG1TZBSzBu
This commit is contained in:
xarmian
2026-09-14 17:15:07 -04:00
committed by GitHub
parent 7fef073326
commit 4e81480d0a
2 changed files with 400 additions and 0 deletions
@@ -13,6 +13,11 @@ import { Extension } from '@tiptap/core';
import type { Editor } from '@tiptap/core';
import { Plugin, PluginKey } from '@tiptap/pm/state';
import type { EditorView } from '@tiptap/pm/view';
// The SHARED arbitration helper, never a raw lease query — the contract
// TASK-2430 set for all seven global owners it taught to stand down. This
// plugin was deferred from that task ("svelte-dnd-action + editor block-drag
// global gestures") and is the last of them; BUG-2453 is that deferral.
import { isBlockedByModal } from '$lib/a11y/viewerBackdrop';
// Side-effect imports to register tiptap command-type augmentations on
// `@tiptap/core`'s `ChainedCommands` interface in this file's compilation
// context. Required since TS 6 stopped propagating module augmentations
@@ -825,6 +830,66 @@ export const BlockDragHandle = Extension.create({
let mouseDownOnHandle = false;
let mouseStartY = 0;
/**
* Should this plugin's WINDOW-level listeners stand down? (BUG-2453)
*
* The owner is the editor surface asking to act — `editorView.dom` —
* never `event.target`, which during a straddling gesture is
* whatever the pointer happens to be over. A viewer portaled to
* `<body>` does not contain the editor, so the helper answers true
* and these handlers decline.
*
* Returns FALSE on an empty lease stack, so with no viewer open
* every path below behaves exactly as it did before — which is the
* property the empty-stack legs of the test pin.
*/
function dragBlocked(): boolean {
return isBlockedByModal(editorView.dom);
}
/**
* Tear a block gesture down mid-flight (BUG-2453).
*
* ABORT RATHER THAN EARLY-RETURN, the same choice `ItemGraph`'s
* `abortPan` makes for its captured pan. An early return alone
* leaves `dragging` latched, the block at 20% opacity, the ghost
* under the user's cursor and `pointerEvents: none` on the editor
* — a half-drag frozen onto a surface that is now behind a viewer,
* with nothing left to deliver the event that would have ended it.
*
* `cancelDrag` is the existing teardown and is what makes this a
* small change: it restores every visual and, unlike `endDrag`,
* never calls `executeMove`. Aborting therefore cannot commit the
* reorder — which is the actual defect here, since the drop landed
* in a document the user could not see.
*
* `mouseDownOnHandle` is cleared HERE because it is the one piece
* of gesture state `cancelDrag` does not own — it describes a
* mouse press that has not become a drag yet, and leaving it set
* would let the first mouseup after the viewer closes open a menu
* for a pointer-down the user has long forgotten. (`pendingDrag`,
* its touch counterpart, IS cleared by `cancelDrag`, so it is not
* repeated here.)
*
* `activeBlock` is nulled to match `endDrag`, which does the same
* as its last act. This is CONSISTENCY, not a defect being closed:
* no path was found that acts on a stale `activeBlock` after an
* abort, because every consumer either re-derives it (`update`,
* the wrapper `mousemove`) or is reached only through a flag this
* function has just cleared. Left populated it would still be a
* teardown that ends in a different state depending on which door
* it came through, which is how the next such path becomes a bug.
*/
function abortDrag() {
mouseDownOnHandle = false;
cancelDrag();
activeBlock = null;
// Matches `endDrag`'s own last act. The handle is invisible
// behind the viewer either way; hiding it means the editor is
// not left mid-hover when the viewer closes.
hideHandle();
}
handle.addEventListener('mousedown', (e) => {
e.preventDefault();
e.stopPropagation();
@@ -833,6 +898,14 @@ export const BlockDragHandle = Extension.create({
});
function onMouseMoveGlobal(e: MouseEvent) {
// STRADDLE GATE (BUG-2453) — the press may have landed before
// the viewer opened. Guarded ABOVE the threshold check, so a
// press that has not yet become a drag cannot become one while
// something is in front of the editor.
if (dragBlocked()) {
if (dragging || mouseDownOnHandle) abortDrag();
return;
}
if (mouseDownOnHandle && !dragging) {
if (Math.abs(e.clientY - mouseStartY) > 5) {
mouseDownOnHandle = false;
@@ -845,6 +918,16 @@ export const BlockDragHandle = Extension.create({
}
function onMouseUpGlobal(e: MouseEvent) {
// STRADDLE GATE (BUG-2453), above the click branch and not
// inside the `dragging` one. This handler has TWO jobs: it ends
// a drag, and on a press that never moved it opens the block
// menu. A guard placed inside the drag arm would still let a
// mouseup released over a frontmost viewer pop a menu open on
// the inert editor beneath it.
if (dragBlocked()) {
if (dragging || mouseDownOnHandle) abortDrag();
return;
}
if (mouseDownOnHandle && !dragging) {
// It was a click — open menu
mouseDownOnHandle = false;
@@ -868,6 +951,14 @@ export const BlockDragHandle = Extension.create({
}, { passive: false });
function onTouchMove(e: TouchEvent) {
// STRADDLE GATE (BUG-2453). Above the `preventDefault()` below:
// while a viewer is in front the touch belongs to it — swallowing
// the move here would break scrolling inside the thing the user
// is actually looking at.
if (dragBlocked()) {
if (dragging || pendingDrag) abortDrag();
return;
}
const touchY = e.touches[0].clientY;
if (pendingDrag && !dragging) {
if (Math.abs(touchY - pendingTouchY) > 8) {
@@ -883,6 +974,13 @@ export const BlockDragHandle = Extension.create({
}
function onTouchEnd(e: TouchEvent) {
// STRADDLE GATE (BUG-2453) — the tap-opens-menu branch below is
// this handler's version of the mouseup one, and needs the same
// gate for the same reason.
if (dragBlocked()) {
if (dragging || pendingDrag) abortDrag();
return;
}
if (pendingDrag) {
// Tap on handle → open menu
pendingDrag = false;
@@ -0,0 +1,302 @@
import { describe, it, expect, afterEach } from 'vitest';
import { Editor } from '@tiptap/core';
import StarterKit from '@tiptap/starter-kit';
import { BlockDragHandle } from './block-drag-handle';
import { acquire, __resetViewerBackdropForTests } from '$lib/a11y/viewerBackdrop';
/**
* BUG-2453 — the block-drag global listeners defer to a frontmost viewer.
*
* TASK-2430 taught seven global owners to stand down for a frontmost
* attachment viewer and DEFERRED two: svelte-dnd-action and this plugin's
* editor block-drag gestures. This is that deferral.
*
* The state is not theoretical. Background inertness stops NEW interaction with
* the editor; it does not stop a gesture that already owns the pointer. So a
* block drag in flight when the viewer opens kept running against an `inert`,
* invisible editor — and the drop still landed.
*
* FOUR owners, not the two the bug names: the mouse pair (`mousemove` /
* `mouseup`) and the touch pair (`touchmove` / `touchend`) are installed on the
* same line and had the identical hole.
*
* ACCEPTANCE IS PER-OWNER, per that task's contract: each owner proven inert
* while a viewer is frontmost, AND each with an empty-stack regression.
*
* WHAT THE EMPTY-STACK LEGS PIN, stated because that task shipped and reverted
* two tests that got this wrong: they assert TODAY'S behaviour, unchanged — not
* behaviour this change would prefer. `isBlockedByModal` returns false on an
* empty lease stack, so with no viewer every path below is untouched, and a leg
* that failed here would mean the guard had leaked into the ordinary case.
*
* Driven through a real Tiptap editor rather than mocks, like its sibling
* `blockDragHandleFreeze.svelte.test.ts`, because the handle and menu are
* imperative DOM the plugin appends. The SELECTION path arms the handle;
* the hover path needs `posAtCoords`, which jsdom lacks.
*/
function makeEditor(element: HTMLElement): Editor {
return new Editor({
element,
extensions: [StarterKit, BlockDragHandle],
content: '<p>alpha</p><p>bravo</p>',
editable: true,
});
}
function armHandle(editor: Editor, wrapper: HTMLElement, at: number): HTMLElement {
wrapper.dispatchEvent(new MouseEvent('click', { bubbles: true }));
editor.commands.setTextSelection(at);
return wrapper.querySelector('.block-drag-handle') as HTMLElement;
}
/** Inside "alpha" (block 1) and inside "bravo" (block 2). */
const IN_FIRST_BLOCK = 3;
const IN_SECOND_BLOCK = 10;
/**
* WHICH BLOCK A REORDER LEG MUST DRAG, and why it is not arbitrary.
*
* jsdom gives every element a zero `getBoundingClientRect`, so `dropPosAtY`
* scores every candidate identically and keeps the FIRST it considers. For a
* drag of block 1 that is the position immediately AFTER block 1 — a move to
* where the node already is, which `executeMove` performs as a no-op. The drop
* genuinely runs and changes nothing, so a doc-equality assertion passes
* whether or not the guard fired.
*
* Dragging block 2 inverts it: the first candidate is position 0, ahead of
* block 1, so the paragraphs swap and the assertion has something to see.
* Measured both ways before this leg was written.
*/
/** A body-portaled viewer root, exactly as `Lightbox` mounts one. */
function mountViewer(): HTMLElement {
const root = document.createElement('div');
root.className = 'attachment-viewer';
root.setAttribute('role', 'dialog');
document.body.appendChild(root);
return root;
}
function touchEvent(type: string, clientY: number): TouchEvent {
const e = new Event(type, { bubbles: true, cancelable: true }) as unknown as {
touches: { clientY: number }[];
} & TouchEvent;
e.touches = [{ clientY }];
return e;
}
describe('BUG-2453 — block-drag global listeners defer to a frontmost viewer', () => {
let editor: Editor | null = null;
let element: HTMLElement | null = null;
afterEach(() => {
editor?.destroy();
element?.remove();
editor = null;
element = null;
document.querySelectorAll('.attachment-viewer').forEach((n) => n.remove());
__resetViewerBackdropForTests();
});
function setup(at: number = IN_FIRST_BLOCK) {
element = document.body.appendChild(document.createElement('div'));
editor = makeEditor(element);
const handle = armHandle(editor, element, at);
return { editor, wrapper: element, handle };
}
/** Press the handle and move past the 5px threshold — a drag is now in flight. */
function startMouseDrag(handle: HTMLElement) {
handle.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, clientY: 100 }));
window.dispatchEvent(new MouseEvent('mousemove', { bubbles: true, clientY: 130 }));
}
/**
* The observable signature of a drag in flight, taken from `startDrag`'s own
* writes. Deliberately NOT a ghost selector: the ghost is an UNCLASSED
* `cloneNode` of the block styled by inline `cssText`, so it matches no
* class at all — an earlier draft of this file asserted `.block-drag-ghost`
* and that selector was null in every state, which made this predicate
* always false and the teardown assertions vacuously true.
*/
function dragIsInFlight(editor: Editor, handle: HTMLElement): boolean {
return (
handle.classList.contains('active') &&
editor.view.dom.style.pointerEvents === 'none' &&
firstBlockDom(editor).style.opacity === '0.2'
);
}
/** The paragraph `startDrag` dims to 0.2 and every teardown path restores. */
function firstBlockDom(editor: Editor): HTMLElement {
return editor.view.dom.querySelector('p') as HTMLElement;
}
/** The block `startDrag` dimmed — the one whose handle was armed. */
function draggedBlockDom(editor: Editor, nth: number): HTMLElement {
return editor.view.dom.querySelectorAll('p')[nth] as HTMLElement;
}
/** Paragraph text in document order — the observable a reorder changes. */
function blockText(editor: Editor): string[] {
return [...editor.view.dom.querySelectorAll('p')].map((n) => n.textContent ?? '');
}
/** The menu is built once at plugin init and toggled by `display`. */
function menuIsOpen(): boolean {
const menu = document.querySelector('.block-context-menu') as HTMLElement | null;
return !!menu && menu.style.display !== 'none';
}
describe('mousemove — the straddling drag', () => {
it('aborts the in-flight drag when a viewer opens, leaving no half-drag behind', () => {
const { editor, handle } = setup();
startMouseDrag(handle);
// PRECONDITION: without this the rest proves nothing — a test that
// never started a drag would pass against the unfixed code.
expect(dragIsInFlight(editor, handle)).toBe(true);
const before = JSON.stringify(editor.getJSON());
const bodyChildrenDuringDrag = document.body.childElementCount;
acquire(mountViewer());
window.dispatchEvent(new MouseEvent('mousemove', { bubbles: true, clientY: 400 }));
// Not merely "stopped" — torn down. An early return would leave every
// one of these latched.
expect(handle.classList.contains('active')).toBe(false);
expect(editor.view.dom.style.pointerEvents).toBe('');
expect(firstBlockDom(editor).style.opacity).toBe('');
// The ghost is removed: one fewer body child than mid-drag, allowing
// for the viewer root this leg just mounted.
expect(document.body.childElementCount).toBe(bodyChildrenDuringDrag);
expect(JSON.stringify(editor.getJSON())).toBe(before);
});
it('EMPTY STACK: a drag proceeds exactly as before when no viewer exists', () => {
const { editor, handle } = setup();
startMouseDrag(handle);
expect(dragIsInFlight(editor, handle)).toBe(true);
window.dispatchEvent(new MouseEvent('mousemove', { bubbles: true, clientY: 400 }));
expect(dragIsInFlight(editor, handle)).toBe(true);
});
});
describe('mouseup — the drop, and the menu branch', () => {
it('does not commit the reorder when the mouseup lands under a viewer', () => {
const { editor, handle } = setup(IN_SECOND_BLOCK);
startMouseDrag(handle);
// PRECONDITION: a drag really is in flight on the SECOND block.
expect(draggedBlockDom(editor, 1).style.opacity).toBe('0.2');
acquire(mountViewer());
window.dispatchEvent(new MouseEvent('mouseup', { bubbles: true, clientY: 400 }));
// The defect this bug was filed for: the drop landed in a document
// the user could not see.
expect(blockText(editor)).toEqual(['alpha', 'bravo']);
expect(editor.view.dom.style.pointerEvents).toBe('');
});
it('EMPTY STACK: the same drag DOES reorder when no viewer exists', () => {
const { editor, handle } = setup(IN_SECOND_BLOCK);
startMouseDrag(handle);
window.dispatchEvent(new MouseEvent('mouseup', { bubbles: true, clientY: 400 }));
// The positive half. Without this the leg above could pass because
// the drag never worked rather than because the guard stopped it.
expect(blockText(editor)).toEqual(['bravo', 'alpha']);
});
it('does not open the block menu on a press that never moved', () => {
const { handle } = setup();
handle.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, clientY: 100 }));
acquire(mountViewer());
window.dispatchEvent(new MouseEvent('mouseup', { bubbles: true, clientY: 100 }));
// The non-drag branch of this handler. A guard placed inside the
// `dragging` arm would leave this menu open over the viewer.
expect(menuIsOpen()).toBe(false);
});
it('EMPTY STACK: a press that never moved still opens the menu', () => {
const { handle } = setup();
handle.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, clientY: 100 }));
window.dispatchEvent(new MouseEvent('mouseup', { bubbles: true, clientY: 100 }));
expect(menuIsOpen()).toBe(true);
});
});
describe('touchmove / touchend — the same hole on every touch device', () => {
it('does not begin a drag from a pending touch while a viewer is frontmost', () => {
const { editor, handle } = setup();
handle.dispatchEvent(touchEvent('touchstart', 100));
acquire(mountViewer());
window.dispatchEvent(touchEvent('touchmove', 140));
expect(handle.classList.contains('active')).toBe(false);
expect(editor.view.dom.style.pointerEvents).toBe('');
expect(firstBlockDom(editor).style.opacity).toBe('');
});
it('does not swallow the touch it declines to handle', () => {
const { handle } = setup();
handle.dispatchEvent(touchEvent('touchstart', 100));
acquire(mountViewer());
const move = touchEvent('touchmove', 140);
window.dispatchEvent(move);
// The touch belongs to the viewer. Calling preventDefault here would
// break scrolling inside the thing the user is actually looking at.
expect(move.defaultPrevented).toBe(false);
});
it('does not open the block menu on a tap under a viewer', () => {
const { handle } = setup();
handle.dispatchEvent(touchEvent('touchstart', 100));
acquire(mountViewer());
window.dispatchEvent(touchEvent('touchend', 100));
expect(menuIsOpen()).toBe(false);
});
it('does not commit the reorder when the touchend lands under a viewer', () => {
const { editor, handle } = setup(IN_SECOND_BLOCK);
handle.dispatchEvent(touchEvent('touchstart', 100));
window.dispatchEvent(touchEvent('touchmove', 140));
expect(draggedBlockDom(editor, 1).style.opacity).toBe('0.2');
acquire(mountViewer());
window.dispatchEvent(touchEvent('touchend', 400));
expect(blockText(editor)).toEqual(['alpha', 'bravo']);
});
it('EMPTY STACK: the same touch drag DOES reorder', () => {
const { editor, handle } = setup(IN_SECOND_BLOCK);
handle.dispatchEvent(touchEvent('touchstart', 100));
window.dispatchEvent(touchEvent('touchmove', 140));
window.dispatchEvent(touchEvent('touchend', 400));
expect(blockText(editor)).toEqual(['bravo', 'alpha']);
});
it('EMPTY STACK: a tap on the handle still opens the menu', () => {
const { handle } = setup();
handle.dispatchEvent(touchEvent('touchstart', 100));
window.dispatchEvent(touchEvent('touchend', 100));
expect(menuIsOpen()).toBe(true);
});
});
});