diff --git a/e2e/tests/canvas/acceptance.spec.ts b/e2e/tests/canvas/acceptance.spec.ts index 200d9bfba0..182ed4dfee 100644 --- a/e2e/tests/canvas/acceptance.spec.ts +++ b/e2e/tests/canvas/acceptance.spec.ts @@ -40,7 +40,14 @@ import { mapFramePointToHost } from "./coordinate-mapping"; import { CanvasCapabilityError, dragPointerTo, + dragToZoneEdge, + dragUntilInsideZone, dragUntilTarget, + jitterAcrossEdge, + readShellState, + dwellAllowanceOf, + settledTarget, + settledValue, } from "./driver"; import type { CanvasChromeReader, CanvasDriver } from "./driver"; import { createPocChromeReader, createPocDriver } from "./poc-driver"; @@ -103,6 +110,35 @@ async function dragOntoZone(driver: CanvasDriver): Promise { return active; } +/** + * What a running drag looks like from outside, whichever engine is driving it. + * + * Deliberately reads only what BOTH a panel drag and a canvas drag can answer, + * so the two are comparable rather than merely both measured. Anything one side + * cannot report would make the comparison a statement about the harness. + * + * Containment rather than proximity for the target reading. `@dnd-kit/collision` + * resolves to a zone CONTAINING the pointer first and only ranks by overlap when + * none does, so a nearest-zone equality holds most of the time and fails next to + * a boundary — a flake that reads as an engine disagreement. + */ +async function engineSignature(driver: CanvasDriver): Promise<{ + dragging: boolean; + resolvesToContainingZone: boolean; +}> { + const containing = await driver.zoneContainingPointer(); + // SETTLED, because a canvas using the permitted dwell rather than a distance + // margin is entitled to keep the previous target for up to the allowance + // after the pointer moves into a new zone. Sampling immediately reports + // `resolvesToContainingZone: false` for a correct engine, which fails the + // panel-side control before the marker even when both drags share it. + const active = await settledTarget(driver); + return { + dragging: await driver.isDragging(), + resolvesToContainingZone: containing >= 0 && active === containing, + }; +} + test.describe("a canvas any Nextly editor could ship", () => { let driver: CanvasDriver; let chrome: CanvasChromeReader; @@ -267,7 +303,22 @@ test.describe("a canvas any Nextly editor could ship", () => { // missing most of its zones and activating one leaves the misses // invisible, and the samples that survive all agree. const containing = await driver.zoneContainingPointer(); - const owner = await driver.readActiveZoneOwner(); + // Settled wherever the pointer is INSIDE a zone, which is exactly where + // the assertions below read this. A stale reading there is wrong in + // both directions: a pointer that has just entered may legitimately + // have no owner yet, AND it may still be showing the PREVIOUS zone's + // owner. Settling only the absent case answers the first and takes the + // second at face value, recording a lingering owner as one the canvas + // chose. Outside a zone nothing asserts on the value, so the wait is + // not spent there. + const owner = + containing >= 0 + ? await settledValue( + () => driver.readActiveZoneOwner(), + dwellAllowanceOf(driver), + "active zone owner" + ) + : await driver.readActiveZoneOwner(); if (owner !== null) owners.push(owner); zoneChoices.push({ owner, @@ -361,14 +412,34 @@ test.describe("a canvas any Nextly editor could ship", () => { // `pressAt`, not `startDragAt`: the latter passes the drag threshold by // contract, so the drag would already have begun before the move below. - await driver.pressAt(await driver.dragSourceCentre()); + const source = await driver.dragSourceCentre(); + await driver.pressAt(source); // Below any sane activation distance. A canvas that begins dragging here // makes every click on a block a possible accidental move. await driver.moveBy(2, 2); - const dragging = await driver.isDragging(); + const subThreshold = await driver.isDragging(); + // The SAME press, carried past the threshold BY THE DRIVER. Without this + // the assertion below is satisfied by absence: a press that never landed — + // a moved handle, a changed selector, an overlay swallowing the pointerdown + // — reports "not dragging" exactly as a correct hysteresis does, and the + // target then passes on a gesture that never happened. Continuing the same + // gesture rather than starting a second one is what makes it a control: it + // proves the press this test made was live. + // + // The distance is the DRIVER's, not a number written here. Activation + // motion is explicitly each canvas's own, so a hard-coded displacement + // asserts this canvas's threshold on every replacement — and one whose + // activation distance is larger leaves the press below threshold, failing a + // control for a property it satisfies. + await driver.crossActivationThreshold(); + const pastThreshold = await driver.isDragging(); await driver.cancel(); - expect(dragging, "a 2px movement must not begin a drag").toBe(false); + expect( + pastThreshold, + "the press must be live, or 'not dragging' proves nothing" + ).toBe(true); + expect(subThreshold, "a 2px movement must not begin a drag").toBe(false); }); test("reaches a drop zone on the fixture the hysteresis probe uses", async ({ @@ -394,26 +465,92 @@ test.describe("a canvas any Nextly editor could ship", () => { test("holds its target through a jitter at a zone boundary", async ({ request, }) => { - note(PLAN_POINT.targetSwitchHysteresis, "B-7"); + note( + PLAN_POINT.targetSwitchHysteresis, + "B-7", + "this canvas has no switch margin: bracketed at an edge, the target " + + "flips on every 2px crossing" + ); await driver.mountTree(await seedPage(request, FLAT_LIST_FIXTURE)); - // Jittering from dead space counts the indicator appearing and vanishing - // as target changes, which looks exactly like the missing hysteresis this - // is meant to detect. The property is only observable from a live zone, - // and without this the run reported nine changes that were mostly the - // indicator blinking rather than moving. - await dragOntoZone(driver); - - const reader = await driver.recordActiveTargetTransitions(); - // A 2px oscillation across a boundary. With no switch margin the target - // flips on every crossing and the indicator stutters under a hand that is - // not perfectly still. - for (let cycle = 0; cycle < 6; cycle += 1) { - await driver.moveBy(0, 2); - await driver.moveBy(0, -2); + // Onto a zone, then to that zone's EDGE. Both halves are preconditions with + // teeth. Jittering from dead space counts the indicator appearing and + // vanishing as target changes, which looks exactly like the missing + // hysteresis this is meant to detect; jittering from the middle of a zone's + // catchment reports a stable target on a canvas with NO hysteresis, because + // nothing there was ever close to switching. The second is the weaker + // failure and the harder to see: it passes, and it passes for a reason that + // has nothing to do with the property. + await dragFromPanel(driver); + const edge = await dragToZoneEdge(driver); + expect( + edge.target, + "the drag must reach a zone before a boundary can be sought" + ).toBeGreaterThanOrEqual(0); + // The target must have CHANGED at least once. A canvas stuck on one target + // forever never crosses a boundary, and every jitter afterwards is stable — + // which reads exactly like the compliant switch margin this point asks for. + // Asserted rather than annotated, because that unusable implementation + // would otherwise produce the same green as a correct one. + expect( + edge.crossed, + "a boundary must be crossed, or this measures a target that never moves" + ).toBe(true); + // An unbracketed edge makes the jitter INCONCLUSIVE rather than weaker, so + // the run stops here. A resolver sticky in one direction only advances once + // and never retreats: it satisfies `crossed`, leaves this false, and then + // jitters perfectly stably from the middle of its catchment — which is what + // a compliant margin looks like. Failing would blame a canvas that may be + // correct; continuing would let a broken one read as correct the day the + // marker comes off. Neither is an answer, so neither is given. + test.info().annotations.push({ + type: "bracketed", + description: String(edge.bracketed), + }); + if (!edge.bracketed) { + await driver.cancel(); + test.skip( + true, + "the reverse search never found the edge, so a stable jitter cannot be told from a target that only ever advances" + ); + return; } - const transitions = await reader(); + + // The DWELL-AWARE probe, shared with the scenario suite. The requirement + // permits hysteresis expressed as a >100ms dwell instead of a distance + // margin, and each move is a CDP round trip whose duration belongs to the + // machine: on a loaded runner one move outlasts that dwell, the pointer + // rests long enough for a compliant timer to commit, and the flip that + // follows says nothing about hysteresis. An untimed jitter reports that as + // this canvas's known gap. + const probe = await jitterAcrossEdge(driver); await driver.cancel(); + // Unmeasurable is INCONCLUSIVE, not a shortfall. Failing here would report + // a missing canvas behaviour on evidence that cannot show one, for a reason + // living in the runner rather than the code. + if (probe.transitions === undefined) { + test.skip( + true, + `the jitter never outpaced the ${String(probe.dwellAllowanceMs)}ms dwell a canvas may use as hysteresis across ${String(probe.sweeps)} sweeps (slowest ${String(probe.slowestMoveMs)}ms), so this runner cannot tell a sticky target from a slow mouse` + ); + return; + } + const transitions = probe.transitions; + + // The indicator has to have been visible throughout: a log holding only a + // baseline of -1 reports no movement because nothing was ever shown. A + // precondition, so it runs BEFORE the expectation is marked. + expect( + transitions[0]?.index, + "the indicator must be visible to measure whether it moves" + ).toBeGreaterThanOrEqual(0); + + // Marked only now. Everything above ran unprotected, so a failed seed, a + // drag that never reached a zone, a target that never moved, or a probe the + // runner was too slow to take stays a real outcome of its own rather than + // becoming another expected failure. + test.fail(true, "the target flips on every 2px crossing of a zone edge"); + // The log's FIRST entry is the state when recording began, not a change, so // anything after it is motion the jitter caused. Comparing the whole log // instead asserts an array that can never be empty, which would keep this @@ -422,12 +559,6 @@ test.describe("a canvas any Nextly editor could ship", () => { transitions.slice(1).map(entry => entry.index), "a 2px jitter must not move the drop target" ).toEqual([]); - // And the indicator has to have been visible throughout: a log holding only - // a baseline of -1 reports no movement because nothing was ever shown. - expect( - transitions[0]?.index, - "the indicator must be visible to measure whether it moves" - ).toBeGreaterThanOrEqual(0); }); test("shifts no existing block when its drop zones appear", async ({ @@ -583,6 +714,19 @@ test.describe("a canvas any Nextly editor could ship", () => { // Marked only now. Everything above ran unprotected. test.fail(true, "nothing is shown over an illegal target"); + // NOT SEPARATING YET, and the reason is in the product rather than here. + // `canDrop` refuses a drop for four reasons, and the only one a panel drag + // can reach is `not-allowed-in-slot`, which needs a slot declaring + // `allowedBlocks`. Measured against the shipped registry: no block declares + // one, so every slot accepts every child and there is no illegal target for + // this drag to enter. The pointer therefore rests somewhere legal, and a + // canvas that answered `false` here forever would satisfy the assertion the + // day the reader starts working. + // + // Closing this needs a block whose slot restricts its children — a product + // decision, not a harness one. Until then the expected failure records a + // capability the canvas lacks and NOT a judgement about what it draws over + // an illegal target, because it is never over one. const explicit = await chrome.readsInvalidTarget(); await driver.cancel(); @@ -753,11 +897,37 @@ test.describe("a canvas any Nextly editor could ship", () => { CanvasCapabilityError ); + // THE CONTROL, with its own drop, entirely before the marker. A drop that + // changed nothing would make `after - before === 1` a statement about a + // gesture that did not happen — an editor logging an undo step for a no-op + // is worse than one logging none, because the author presses undo and + // watches nothing. + // + // It cannot share the measured drop below, because the count needs a + // reading BEFORE its own drop and `undoDepth` throws today. Under an active + // marker that throw would be recorded as the expected missing-undo failure + // and the control would never run at all. + const treeBefore = await driver.readTreeShape(); + // Onto a live zone, not merely over the canvas. `dragFromPanel` stops at + // the centre, which the file's own comment says is dead space as often as + // not — and a drop there inserts nothing. + await dragOntoZone(driver); + await driver.drop(); + // POLLED, because the insert is asynchronous: the canvas re-renders after + // the drop resolves, so a single read taken immediately sees the tree the + // drag started from and reports a working drop as having changed nothing. + await expect + .poll(async () => (await driver.readTreeShape()).length, { + message: + "a drop must change the document before any undo entry for it means anything", + }) + .toBe(treeBefore.length + 1); + // Marked only now. Everything above ran unprotected. test.fail(true, "this canvas keeps no undo history to count"); const before = await chrome.undoDepth(); - await dragFromPanel(driver); + await dragOntoZone(driver); await driver.drop(); const after = await chrome.undoDepth(); @@ -791,33 +961,86 @@ test.describe("a canvas any Nextly editor could ship", () => { chrome.startDragOfBlock(fixture.blockIds[1] ?? "") ).rejects.toThrow(CanvasCapabilityError); - // Marked only now. Everything above ran unprotected. + // BOTH drags, measured the same way, and compared against each other. + // Reading only the canvas drag asks whether it works, not whether it is the + // same engine — two independent implementations both satisfy that whenever + // the canvas one reports dragging, so it cannot separate them. The property + // is AGREEMENT, which is why neither side may be a constant written into + // this file. + // + // The panel side runs BEFORE the marker, with everything it depends on. + // Under an active `test.fail`, a panel-side harness regression — a broken + // `dragOntoZone`, a reader that stopped working — is recorded as the + // expected failure and the canvas comparison below is never reached, so the + // control stops controlling anything at exactly the moment it matters. + await dragOntoZone(driver); + // INSIDE a zone, not merely on one. `dragOntoZone` stops as soon as a + // target resolves, and that can be the overlap fallback with the pointer + // outside every zone — measured, this control read + // `resolvesToContainingZone: false` from a perfectly healthy panel drag. + // The signature's exact reading is only decidable from containment. + await dragUntilInsideZone(driver); + const panel = await engineSignature(driver); + await driver.cancel(); + + // The panel side is the reference, so it has to be a live drag resting + // inside a zone. Two dead readings compare equal, and `toEqual` below would + // report two engines agreeing when neither was running. + expect( + panel, + "the panel drag is the reference and must be live and on a zone" + ).toEqual({ dragging: true, resolvesToContainingZone: true }); + + // Marked only now, with the whole panel-side control behind it. test.fail( true, "dragging a block already in the canvas is not offered here" ); await chrome.startDragOfBlock(fixture.blockIds[1] ?? ""); - // The same observable state a panel drag produces. Two engines drift: - // one gains a hysteresis fix or an autoscroll tune and the other does - // not, and the canvas then behaves differently depending on where the - // block came from. - const dragging = await driver.isDragging(); - const active = await driver.readActiveTarget(); - const nearest = await driver.nearestZoneToPointer(); + // Advanced INSIDE a zone, exactly as the panel side is. Sampling the two + // under different conditions makes the comparison a statement about the + // harness: `dragUntilTarget` can resolve through overlap with the pointer + // outside every zone, so a canvas drag using the very same engine reports + // `resolvesToContainingZone: false` and stays an expected failure. + await dragUntilTarget(driver); + await dragUntilInsideZone(driver); + const canvas = await engineSignature(driver); await driver.cancel(); - expect(dragging, "a canvas drag reports the same drag state").toBe(true); - expect(active, "and resolves targets by the same rule").toBe(nearest); + // Two engines drift: one gains a hysteresis fix or an autoscroll tune and + // the other does not, and the canvas then behaves differently depending on + // where the block came from. + // + // WHAT THIS DOES NOT PROVE, stated because the title overreaches and a + // future reader should not take the green for more than it is: two + // independent engines that both report dragging and both resolve to the + // containing zone pass this. Agreement on observable behaviour is necessary + // for "one engine" and nowhere near sufficient, and the drift this case + // exists to catch — a hysteresis fix or an autoscroll tune landing on one + // side only — is not represented in either reading. + // + // Separating the two genuinely needs an identity boundary rather than a + // behavioural sample: one provider both drags resolve through, asserted at + // the architecture rather than through the DOM. That is a B-15 design + // decision and cannot be bolted on from the harness, so it is recorded here + // rather than approximated with more booleans. + expect(canvas, "a canvas drag behaves as a panel drag does").toEqual(panel); }); test("leaves the document and the editor intact when Escape cancels", async ({ + page, request, }) => { note(PLAN_POINT.escapeCancelsWithoutNavigating, "B-11"); await driver.mountTree(await seedPage(request, FLAT_LIST_FIXTURE)); const before = await driver.readTreeShape(); + // Read through the shared probe rather than assembled here. Two + // hand-written copies of one invariant drift: a correction applied to one + // never reaches the other, and both keep claiming the same property while + // disagreeing about it. + const shellBefore = await readShellState(page, driver); await dragFromPanel(driver); await driver.cancel(); @@ -828,10 +1051,14 @@ test.describe("a canvas any Nextly editor could ship", () => { expect(await driver.readTreeShape(), "Escape changes nothing").toEqual( before ); + // The LOCATION and the DOM together. A shell that treats Escape as go-back + // changes the location synchronously while the outgoing document stays + // mounted for a tick, so a presence check alone reads the editor on its way + // out and reports no navigation. expect( - await driver.isEditorPresent(), - "and the shell does not treat it as go-back" - ).toBe(true); + await readShellState(page, driver), + "Escape must not navigate away or unmount the editor" + ).toEqual({ url: shellBefore.url, hasEditor: true }); }); test("ends the drag when Escape cancels", async ({ request }) => { diff --git a/e2e/tests/canvas/driver.ts b/e2e/tests/canvas/driver.ts index 1135149ab4..fa9107cf45 100644 --- a/e2e/tests/canvas/driver.ts +++ b/e2e/tests/canvas/driver.ts @@ -141,6 +141,22 @@ export interface CanvasDriver { /** Move the pending insertion point with the keyboard. */ keyboardInsert(direction: "up" | "down"): Promise; + /** + * The longest this canvas may keep showing a previous reading after the + * pointer has moved, in milliseconds. + * + * Declared per driver because the requirement permits a dwell of MORE than + * 100ms and sets no upper bound, so no global constant is correct for every + * canvas. A canvas with a distance margin rather than a timer declares 0. + * + * Optional: a driver that omits it gets {@link DEFAULT_DWELL_ALLOWANCE_MS}. + * Understating it is self-punishing rather than self-serving — readings come + * back stale and this suite fails — which is why the settling helpers trust + * it while the jitter probe, which grades whether hysteresis exists at all, + * deliberately does not. + */ + dwellAllowanceMs?: number; + /** * Ordinal of the active drop zone among ALL drop zones in document order, or * -1 when none is active. Ordinal rather than id because the droppable id is @@ -199,13 +215,35 @@ export interface CanvasDriver { /** * Ordinal of the drop zone geometrically nearest the current pointer. * - * The exact form of "the indicator is where the pointer is": comparing the - * ACTIVE ordinal against this one needs no tolerance, and both the stale-rect - * (#1705) and unscaled-transform (#1706) failures select a zone that is not - * the nearest, so it catches them without a magic number. + * The APPROXIMATE reading, and the weaker of the two. Proximity is not a rule + * this canvas follows: `@dnd-kit/collision` resolves to a zone CONTAINING the + * pointer first and only ranks by the dragged shape's overlap when none does, + * so next to a boundary the nearest zone by centre distance and the resolved + * zone legitimately differ. Measured, one sample of 28 resolved one ordinal + * away with the pointer inside neither. + * + * So an equality assertion against this is a latent flake wherever the pointer + * may sit outside every zone. Use {@link zoneContainingPointer} for the exact + * claim, and bound this one to a single ordinal where only an approximation is + * available. */ nearestZoneToPointer(): Promise; + /** + * Carry an already-pressed pointer past THIS canvas's activation threshold. + * + * The distance is the driver's to know. `startDragAt` is contractually + * allowed to move by whatever its canvas requires, so a suite that hard-codes + * a displacement is asserting one canvas's threshold on every other: a + * replacement whose activation distance is larger leaves the press below + * threshold, and a positive control built on it fails while reporting a + * property that is perfectly satisfied. + * + * Used to prove a press is LIVE. A sub-threshold test reads "not dragging", + * which absence satisfies just as well as correct hysteresis does. + */ + crossActivationThreshold(): Promise; + /** `data-nx-id` of the container owning the active zone, or null. */ readActiveZoneOwner(): Promise; @@ -298,17 +336,499 @@ export interface CanvasChromeReader { * the measurement this prevents. */ export async function dragUntilTarget( - driver: CanvasDriver, + driver: EdgeSearchDriver, maxSteps = 90 ): Promise { for (let step = 0; step < maxSteps; step += 1) { await driver.moveBy(0, 8); - const active = await driver.readActiveTarget(); + // Given the dwell, not sampled. A resolver whose hysteresis is a timer + // starting from no target at all can have that timer RESET by each move, + // so a fixture of narrow candidates is traversed for every step without a + // target ever becoming active — and both hysteresis suites then fail their + // precondition before reaching the dwell-aware search they exist to run. + const active = await departureFrom( + () => driver.readActiveTarget(), + -1, + dwellAllowanceOf(driver) + ); if (active >= 0) return active; } return -1; } +/** + * The longest dwell a canvas may use INSTEAD of a distance margin. + * + * The requirement permits hysteresis expressed either way, so a compliant + * canvas is allowed to keep showing the previous target for this long after the + * pointer has moved. Every reader that asks "which target is active" therefore + * has to decide whether it is reading a settled answer or a permitted lag. + */ +export const PERMITTED_DWELL_FLOOR_MS = 100; + +/** + * The longest dwell this suite will WAIT for before calling a reading settled. + * + * Separate from {@link PERMITTED_DWELL_FLOOR_MS} because the two answer + * opposite questions, and one number cannot serve both: + * + * - Settling asks "has the canvas committed yet?", so it must wait at least as + * long as the longest dwell a compliant canvas may use. Too SMALL and a + * compliant slow canvas is read while still lagging. + * - The jitter probe asks "was that move fast enough that a compliant timer + * could NOT have committed?", so its bound must be no larger than the + * SHORTEST permitted dwell. Too LARGE and it accepts a sweep during which a + * compliant canvas legitimately switched, then reads that switch as missing + * hysteresis. + * + * One constant cannot serve both: any value large enough for the first is too + * large for the second, and any value small enough for the second is too small + * for the first. + * + * The requirement states a dwell of MORE than 100ms and gives no upper bound, + * so no finite wait is provably sufficient and no global constant can be + * correct for every canvas. This is the DEFAULT for a driver that does not say + * otherwise; a canvas that dwells longer declares it on the driver, the way the + * activation threshold already is. + * + * Which number each question uses is the load-bearing part. Settling takes the + * DRIVER's figure, because that is harness-side knowledge about the + * implementation and getting it wrong is self-punishing — understate it and + * readings come back stale and the suite fails. The jitter probe takes the + * REQUIREMENT's floor instead, never the driver's, because it grades whether + * hysteresis exists at all: feeding it the canvas's own claim would let an + * implementation set the bar it is measured against. + */ +export const DEFAULT_DWELL_ALLOWANCE_MS = 3 * PERMITTED_DWELL_FLOOR_MS; + +/** + * How many times a stationary pointer may see the reading change before it is + * called unsettled. + * + * A canvas is entitled to one change, and to another if the first expiry moved + * the target somewhere that starts a second; past a few it is changing its mind + * with no input to justify it, which is a defect rather than permitted lag and + * must not be reported as a settled reading. + */ +const SETTLE_TRANSITIONS = 4; + +/** The capability these readers need, so a test can supply exactly it. */ +type TargetReader = Pick & + Partial>; + +/** + * What an edge search needs, which is less than a whole canvas. + * + * Declared as the capability rather than the whole interface so these searches + * can run against a simulated resolver as well as a real canvas. Their waiting + * behaviour is only observable against a canvas that declares a dwell, and the + * canvas this suite drives declares none. + */ +type EdgeSearchDriver = Pick & + Partial>; + +/** {@link EdgeSearchDriver} plus the in-page recorder the jitter probe needs. */ +type JitterDriver = EdgeSearchDriver & + Pick; + +/** + * Wait for `read` to return something other than `from`, or for the permitted + * dwell to pass. + * + * The one waiting loop in this file, because the two questions callers ask — + * "has it moved off X yet" and "what is it once it stops moving" — differ only + * in what they do with the answer, and two loops would drift. + * + * Returns the departed value, or `from` when the whole allowance passed without + * one. A caller can therefore distinguish the two by comparing with what it + * passed in, and "unchanged" now MEANS unchanged for the full permitted dwell + * rather than unchanged between two adjacent reads. + * + * Generic over the reading, because the dwell is a property of the CANVAS + * rather than of any one probe: the active target, the owning zone and anything + * else read straight after a move are all entitled to the same lag, and a + * version that only knew about target ordinals would leave the other readers to + * grow their own copy of this. + */ +async function departureFrom( + read: () => Promise, + from: T, + allowanceMs: number +): Promise { + const deadline = Date.now() + allowanceMs; + let current = await read(); + while (current === from && Date.now() < deadline) { + current = await read(); + } + // One reading taken strictly AFTER the deadline before concluding it never + // departed. Every read above may have SAMPLED the value before the deadline + // and resolved after it — a cross-frame read easily spans that boundary — so + // the loop can exit holding a value that was already stale when it was taken, + // and report a canvas that committed exactly on time as never having moved. + if (current === from) current = await read(); + return current; +} + +/** + * Read what a canvas has COMMITTED to, rather than what it is still entitled to + * be showing. + * + * A canvas whose hysteresis is a TIMER rather than a distance margin is allowed + * to lag: the pointer is over a new zone and the old reading stays correct for + * up to the dwell its driver declares. So "settled" cannot mean "two reads agreed" + * — during that lag EVERY read agrees, and they all return the pre-move value. + * Stability is only evidence once it has been observed across the whole interval + * the canvas was permitted to lag for, which is why this waits the allowance out + * rather than stopping at the first identical pair. + * + * Throws rather than returning when the reading never holds still: a value from + * a canvas that is still changing its mind is one no assertion downstream can + * qualify, and handing it back silently would let an unstable canvas produce an + * ordinary-looking green. + */ +export async function settledValue( + read: () => Promise, + allowanceMs: number, + subject = "reading" +): Promise { + let value = await read(); + // Bounded by TRANSITIONS, never by a clock. What is being tolerated is a + // canvas changing its mind a bounded number of times, and each observation + // already bounds its own wait by the allowance — so a wall-clock budget adds + // nothing and collapses to zero for a canvas that declares no dwell, where it + // would turn a single asynchronous re-render between two reads into a harness + // error instead of a settled reading. + // + // The count is of CHANGES, so the permitted number of them is followed by one + // more observation rather than ending on one. Ending on a transition would + // reject a reader that changed exactly the permitted number of times and then + // held perfectly still — asynchronous relayout does precisely that — and the + // refusal would land on the reading that finally settled. + for (let transition = 0; transition <= SETTLE_TRANSITIONS; transition += 1) { + const next = await departureFrom(read, value, allowanceMs); + if (next === value) return value; + value = next; + } + throw new Error( + `the ${subject} changed more than ${String(SETTLE_TRANSITIONS)} times with ` + + `a stationary pointer (last seen ${String(value)}), so nothing read here ` + + `is settled` + ); +} + +/** {@link settledValue} over the active drop target, at the driver's own dwell. */ +export async function settledTarget(driver: TargetReader): Promise { + return settledValue( + () => driver.readActiveTarget(), + dwellAllowanceOf(driver), + "active target" + ); +} + +/** + * The dwell a driver declares, or the default when it declares none. + * + * The field is optional, so a driver that declares nothing still has an + * allowance; read through one helper so that fallback is stated once rather + * than at each call, where the several copies would drift. + */ +export function dwellAllowanceOf(driver: Partial): number { + return driver.dwellAllowanceMs ?? DEFAULT_DWELL_ALLOWANCE_MS; +} + +/** + * Carry the drag until the pointer is INSIDE a zone, not merely until one is + * active. + * + * `dragUntilTarget` stops as soon as a target resolves, and + * `@dnd-kit/collision` resolves one by the dragged shape's OVERLAP when no zone + * contains the pointer. So "a target is active" and "the pointer is inside that + * target" are different states, and every exact claim about mapping — is the + * indicator where the pointer is, do two drags resolve the same way — is only + * decidable in the second. + * + * The distinction is not academic: it is where a stale-scroll or unscaled + * transform hides. Those implementations select a NEIGHBOURING zone, which any + * assertion tolerant of the overlap case accepts. + * + * Returns the containing zone's ordinal, or -1 if none was reached — a value the + * CALLER must assert on, for the same reason `dragUntilTarget` says so. + */ +export async function dragUntilInsideZone( + driver: CanvasDriver, + maxSteps = 40 +): Promise { + let containing = await driver.zoneContainingPointer(); + for (let step = 0; step < maxSteps && containing < 0; step += 1) { + await driver.moveBy(0, 4); + containing = await driver.zoneContainingPointer(); + } + return containing; +} + +/** The two readings that together say "the shell survived the gesture". */ +export interface ShellState { + readonly url: string; + readonly hasEditor: boolean; +} + +/** + * Read the shell state a cancelled gesture must not have changed. + * + * Both readings, because either alone passes while the other has already gone + * wrong: the editor can still be in the DOM one tick after navigation began, + * and a stable URL says nothing about the canvas having unmounted for some + * other reason. + * + * Offered as one reader so a correction reaches every caller. It does not yet + * have every caller: `checklist.spec.ts` builds the same two readings inline, + * so a fix made here does not reach it and the two can answer the same named + * question differently. Routing that one through here is the remaining half. + */ +export async function readShellState( + page: { url: () => string }, + driver: CanvasDriver +): Promise { + return { url: page.url(), hasEditor: await driver.isEditorPresent() }; +} + +/** Where a boundary search left the pointer. */ +export interface ZoneEdge { + /** Active target the pointer rests on, or -1 when no zone was reached. */ + readonly target: number; + /** + * Whether the forward walk ever saw the target CHANGE. + * + * Separate from {@link bracketed} because the two failures mean opposite + * things. A canvas whose collision resolution is stuck on one target forever + * never crosses a boundary, and every jitter afterwards is stable — which + * reads exactly like a compliant switch margin. Callers must assert this, or + * an unusable implementation produces the same green as a correct one. + */ + readonly crossed: boolean; + /** + * Whether the reverse search then located the edge to within a pixel. + * + * `false` makes any jitter INCONCLUSIVE, not weaker. A resolver that is + * sticky in one direction only — it advances once and never retreats — + * satisfies {@link crossed}, leaves this false, and then produces a perfectly + * stable jitter from the middle of its catchment. That is indistinguishable + * from a compliant switch margin, so the moment an expected-failure marker + * comes off, the broken resolver reads as correct. + * + * Callers must therefore treat `false` as "this run could not ask the + * question" rather than as evidence in either direction. + */ + readonly bracketed: boolean; +} + +/** + * Carries the drag to a zone BOUNDARY and leaves the pointer one pixel inside it. + * + * A jitter is only a test of hysteresis when it straddles an edge. Oscillating + * in the middle of a zone's catchment reports a stable target on a canvas with + * no hysteresis at all, because nothing there was ever close to switching — the + * assertion is satisfied by the pointer being far from any decision. + * + * Three steps, and each one is load-bearing: + * + * 1. Reach a zone, so the walk starts from a live target rather than dead + * space, where the indicator appearing and vanishing counts as a change. + * 2. Walk until the target CHANGES, which is the only way to know an edge was + * passed rather than assumed. + * 3. Step back a pixel at a time until it changes again, then one step in. The + * pointer is now within a pixel of the edge, so +/-2px lands on opposite + * sides of it. + * + * The search distance is deliberately past the largest margin the requirement + * allows, so failing to find the edge means the target is sticky rather than + * that the search was too short. + * + * Shared rather than repeated because the acceptance suite and the scenario + * suite ask the same question, and a per-suite copy is invisible when it is + * wrong: the drag still runs and still reports a number. + */ +export async function dragToZoneEdge( + driver: EdgeSearchDriver, + marginPx = 24 +): Promise { + const first = await dragUntilTarget(driver); + if (first < 0) return { target: -1, crossed: false, bracketed: false }; + + const FORWARD_STEP_PX = 4; + let crossed = -1; + let previous = first; + let seen = first; + for (let step = 0; step < 120; step += 1) { + await driver.moveBy(0, FORWARD_STEP_PX); + // Given the dwell to depart, not sampled. A canvas using the permitted dwell + // instead of a distance margin can be traversed across a narrow candidate + // region faster than its timer expires, so an immediate read keeps returning + // the previous target and the walk concludes the resolver never crosses + // anything. Both suites assert `crossed` before their marker, so that + // compliant implementation would fail the harness rather than the + // requirement. + // + // Departure rather than full settling: this loop only asks whether the + // target left `previous`, and returning the moment it does keeps the + // overshoot the reverse budget below has to carry down to one step. + // + // Two baselines, because the walk asks two different questions. `seen` is + // the last value OBSERVED, `-1` included, and departure is measured from + // it: leaving the baseline at the last real zone while the pointer sits in + // dead space makes every later read differ from it immediately, so the + // wait expires at once and the walk races through the next narrow zone + // before a compliant timer can activate it. `previous` is the last ZONE, + // which is what a crossing is measured against — arriving in dead space is + // not a crossing. + const current = await departureFrom( + () => driver.readActiveTarget(), + seen, + dwellAllowanceOf(driver) + ); + seen = current; + if (current >= 0 && current !== previous) { + crossed = current; + break; + } + if (current >= 0) previous = current; + } + if (crossed < 0) { + return { target: previous, crossed: false, bracketed: false }; + } + + // The reverse budget carries the FORWARD step's overshoot. A 4px scan first + // observes the new target up to 3px past the point where it switched, so + // walking back the margin alone falls short by that much and reports a + // compliant canvas as unbracketed. The distance to search is the margin the + // requirement allows plus however far the coarse step could have overshot it. + const reverseBudget = marginPx + FORWARD_STEP_PX - 1; + for (let step = 0; step < reverseBudget; step += 1) { + await driver.moveBy(0, -1); + // The dwell applies walking back too. These are one-pixel commands, so a + // compliant timer-based canvas can be carried through the whole reverse + // budget in less time than one dwell — every immediate read then still says + // `crossed`, the edge is never bracketed, and both hysteresis tests skip + // without having tested the implementation. + const stepped = await departureFrom( + () => driver.readActiveTarget(), + crossed, + dwellAllowanceOf(driver) + ); + if (stepped !== crossed) { + await driver.moveBy(0, 1); + return { target: crossed, crossed: true, bracketed: true }; + } + } + return { target: crossed, crossed: true, bracketed: false }; +} + +/** What a dwell-aware jitter observed, and whether it could observe anything. */ +export interface JitterProbe { + /** Target transitions recorded inside the page, or undefined if inconclusive. */ + readonly transitions: ActiveTargetTransition[] | undefined; + /** The slowest single move across the sweeps that ran, in milliseconds. */ + readonly slowestMoveMs: number; + /** The allowance a move had to stay under for the sweep to count. */ + readonly dwellAllowanceMs: number; + /** How many sweeps were attempted. */ + readonly sweeps: number; +} + +/** + * Oscillate across a bracketed edge, and report whether the probe was VALID. + * + * The requirement permits hysteresis expressed as a dwell of more than 100ms + * instead of a distance margin, and every `moveBy` is a CDP round trip whose + * duration belongs to the machine rather than to the canvas. On a loaded runner + * a single move can outlast that dwell, which means the pointer rested at an + * endpoint long enough for a COMPLIANT timer to commit — and any flip observed + * afterwards says nothing about hysteresis. + * + * So the sweep is timed and repeated, and only a sweep whose slowest move stayed + * inside the allowance is returned. Each sweep is balanced (ten moves of +4 + * against ten of -4) so it ends where it began and a repeat re-probes the same + * edge. + * + * Shared rather than reimplemented. An acceptance probe that jitters without + * timing reports a compliant dwell-based canvas as the known missing-hysteresis + * failure, on a machine property, and nothing in its output says so. + */ +export async function jitterAcrossEdge( + driver: JitterDriver, + { sweeps = 3, dwellAllowanceMs = PERMITTED_DWELL_FLOOR_MS } = {} +): Promise { + // To P-2 first, then alternating by 4, so the samples are P-2 and P+2 — + // genuinely opposite sides. Alternating +/-2 from P samples P+2 and P, both + // on the same side, which a canvas that switches the instant the pointer + // crosses would still pass. + await driver.moveBy(0, -2); + // Settled BEFORE the recorder exists, so no dwell started by the positioning + // move is still pending when observation begins. The interval between that + // move and the first timing mark is not covered by the sweep's own + // measurement, so a slow round trip there could let a compliant timer commit + // while the observer was active — putting a transition in the log that the + // jitter never provoked, inside a sweep whose measured moves all look fast + // enough to trust. That is the one thing this probe exists to distinguish. + await settledTarget(driver); + + let slowestMoveMs = Number.POSITIVE_INFINITY; + for (let sweep = 0; sweep < sweeps; sweep += 1) { + const readTransitions = await driver.recordActiveTargetTransitions(); + // CONTINUOUS marks, not per-command durations. A stopwatch around each + // `moveBy` measures only the time inside the command and misses the gap + // between them — and if the test process is descheduled in that gap, two + // fast commands still leave their browser events far apart. + // + // Marking the clock at every boundary makes the elapsed time cover the gaps + // too: `marks[i]` is the instant before move `i`, and the last mark is + // after the final one, so no wall-clock time between the first and last + // move is unaccounted for. + const marks: number[] = [Date.now()]; + for (let step = 0; step < 20; step += 1) { + await driver.moveBy(0, step % 2 === 0 ? 4 : -4); + marks.push(Date.now()); + } + const log = await readTransitions(); + // The TEARDOWN tail counts too. The recorder is still observing between the + // final move and the moment it disconnects, so a stall there lets a + // compliant dwell timer commit — and that commit lands in the log while + // every measured move window stays under the allowance. The probe would + // then read a terminal dwell as a jitter-induced transition, which is the + // one thing it exists to distinguish. + marks.push(Date.now()); + // The widest window that can hold two consecutive pointer EVENTS. Each + // event fires somewhere inside its own command, so the pair from move `i` + // and move `i+1` is contained by the span from BEFORE move `i` to AFTER + // move `i+1` — `marks[i+2] - marks[i]`. Because the marks are continuous, + // that span includes any time the process spent descheduled between the + // two commands, which a per-command stopwatch cannot see. + // + // It over-estimates, so some runs are skipped that could have been + // measured. That is the safe direction: the alternative is classifying a + // canvas with a permitted dwell as having no hysteresis at all. + slowestMoveMs = 0; + for (let index = 0; index + 2 < marks.length; index += 1) { + const window = (marks[index + 2] ?? 0) - (marks[index] ?? 0); + if (window > slowestMoveMs) slowestMoveMs = window; + } + if (slowestMoveMs < dwellAllowanceMs) { + return { + transitions: log, + slowestMoveMs, + dwellAllowanceMs, + sweeps: sweep + 1, + }; + } + } + return { + transitions: undefined, + slowestMoveMs, + dwellAllowanceMs, + sweeps, + }; +} + /** * Carries a panel drag to a point, measuring from where the pointer ACTUALLY is. * diff --git a/e2e/tests/canvas/dwelling-canvas.test.ts b/e2e/tests/canvas/dwelling-canvas.test.ts new file mode 100644 index 0000000000..2e846d3373 --- /dev/null +++ b/e2e/tests/canvas/dwelling-canvas.test.ts @@ -0,0 +1,128 @@ +/** + * The edge searches, run against a SIMULATED dwell-based canvas. + * + * The requirement permits hysteresis expressed as a distance margin OR as a + * dwell timer, and these searches have to work for both. The canvas this suite + * drives implements neither — it declares `dwellAllowanceMs: 0` because its + * indicator flips on every 2px move — so running the searches against it + * exercises only the zero-dwell path, and every wait they contain reduces to a + * no-op there. + * + * The other permitted form is therefore simulated. `dwellingCanvas` is a + * resolver that commits only after the pointer has rested in a new zone, which + * is what makes the waiting behaviour observable at all, and each test states + * which broken search it distinguishes. + * + * No browser, matching `oscillation.test.ts` and `settle.test.ts`. + */ +import { expect, test } from "@playwright/test"; + +import { dragToZoneEdge, dragUntilTarget } from "./driver"; + +/** A zone's extent along the drag axis, in the same pixels `moveBy` speaks. */ +interface Band { + readonly from: number; + readonly to: number; +} + +/** + * A canvas whose target commits only after the pointer has rested in a new + * zone for `dwellMs`. + * + * This is a COMPLIANT implementation, not a broken one: the requirement permits + * hysteresis expressed as a dwell instead of a distance margin. Everything + * asserted below is about the harness reading it correctly. + * + * Gaps between bands are dead space, which resolves to -1 — the state the + * forward search has to keep giving the dwell a chance from, rather than + * treating as a departure that needs no wait. + */ +function dwellingCanvas(bands: readonly Band[], dwellMs: number) { + let y = 0; + let committed = -1; + let pending = -1; + let pendingSince = Date.now(); + + const zoneAt = (at: number): number => + bands.findIndex(band => at >= band.from && at < band.to); + + const settleIfDue = (): number => { + if (pending !== committed && Date.now() - pendingSince >= dwellMs) { + committed = pending; + } + return committed; + }; + + return { + dwellAllowanceMs: dwellMs, + moveBy: (_dx: number, dy: number): Promise => { + y += dy; + const zone = zoneAt(y); + if (zone !== pending) { + pending = zone; + pendingSince = Date.now(); + } + // A read is what advances the clock elsewhere; moving alone commits + // nothing, which is what makes a fast traversal outrun the timer. + settleIfDue(); + return Promise.resolve(); + }, + readActiveTarget: (): Promise => Promise.resolve(settleIfDue()), + at: (): number => y, + }; +} + +test("acquires a first target on a resolver whose timer each move resets", async () => { + // `dragUntilTarget` moves 8px a step. With a dwell longer than a step's round + // trip, every move restarts the timer, so an acquisition that sampled + // immediately never sees a target become active: it exhausts the whole budget + // and reports none. Both hysteresis suites assert on a target before they + // begin, so they would fail that precondition without reaching the + // dwell-aware search they exist to run. + const canvas = dwellingCanvas([{ from: 0, to: 400 }], 60); + + expect(await dragUntilTarget(canvas)).toBe(0); +}); + +test("finds an edge past dead space instead of racing through it", async () => { + // The geometry is the point. The pointer starts in a wide zone, crosses a + // gap, and reaches a NARROW one — narrow enough that a search giving it no + // wait steps over it in a single move. + // + // Measuring departure from the last ZONE rather than the last value SEEN is + // what this separates: in dead space every read differs from a zone baseline + // at once, so the wait expires immediately for the rest of the walk and the + // narrow zone is never observed. `crossed` is then false and both hysteresis + // tests skip, on a canvas that is compliant. + const canvas = dwellingCanvas( + [ + { from: 0, to: 40 }, + { from: 56, to: 72 }, + ], + 40 + ); + + const edge = await dragToZoneEdge(canvas, 24); + + expect(edge.crossed, "the walk must observe the second zone").toBe(true); + expect(edge.target).toBe(1); +}); + +test("brackets that edge rather than exhausting the reverse budget", async () => { + // The reverse search steps one pixel at a time, so a compliant timer can be + // carried through the entire budget in less time than one dwell. An immediate + // read then returns the crossed target at every step, the edge is never + // bracketed, and the jitter that follows is discarded as inconclusive — + // silently, which is worse than failing. + const canvas = dwellingCanvas( + [ + { from: 0, to: 40 }, + { from: 56, to: 72 }, + ], + 40 + ); + + const edge = await dragToZoneEdge(canvas, 24); + + expect(edge.bracketed, "the reverse search must locate the edge").toBe(true); +}); diff --git a/e2e/tests/canvas/poc-driver.ts b/e2e/tests/canvas/poc-driver.ts index d51f519bc5..6e685f1f96 100644 --- a/e2e/tests/canvas/poc-driver.ts +++ b/e2e/tests/canvas/poc-driver.ts @@ -160,6 +160,13 @@ export function createPocDriver(page: Page): CanvasDriver { let pointer: Point = { x: 0, y: 0 }; const driver: CanvasDriver = { + // Zero, because this canvas has no target-switch hysteresis at all: + // `plugin-page-builder` registers no collision priority and no dwell, and a + // 2px jitter at a boundary flips the indicator on every move. Declaring a + // dwell it does not have would spend a wait per reading for lag that never + // happens, and would let a real hysteresis defect hide inside the wait. + dwellAllowanceMs: 0, + async mountTree(fixture: CanvasFixture) { await gotoAdmin(page, `/collections/pages/${fixture.entryId}`); await expect(page.locator("iframe")).toBeVisible({ timeout: 30_000 }); @@ -204,7 +211,21 @@ export function createPocDriver(page: Page): CanvasDriver { // dnd-kit flips aria-grabbed on the source while a drag is active, so the // signal is the library's own accessibility state rather than a class the // canvas happens to add. - return page.evaluate( + // + // BOTH documents, because the source can live in either. A panel drag's + // source is host chrome; a drag of a block already in the canvas has its + // source inside the iframe. Searching only the host reports `false` for a + // fully active canvas drag — and since that half of the engine-parity + // case runs under an expected-failure marker, the capability arriving + // would still look exactly like the capability missing. + if ( + await page.evaluate( + () => !!document.querySelector('[aria-grabbed="true"]') + ) + ) { + return true; + } + return canvasFrame().evaluate( () => !!document.querySelector('[aria-grabbed="true"]') ); }, @@ -341,8 +362,7 @@ export function createPocDriver(page: Page): CanvasDriver { pointer = { ...point }; await page.mouse.move(pointer.x, pointer.y); await page.mouse.down(); - pointer = { x: pointer.x + DRAG_THRESHOLD_PX, y: pointer.y }; - await page.mouse.move(pointer.x, pointer.y); + await driver.crossActivationThreshold(); }, async pressAt(point: Point) { @@ -351,6 +371,16 @@ export function createPocDriver(page: Page): CanvasDriver { await page.mouse.down(); }, + async crossActivationThreshold() { + // The one place this driver performs its activation motion. `startDragAt` + // calls it too, so the GESTURE is single-sourced rather than only the + // distance: sharing the constant alone leaves two code paths that agree + // today and diverge the moment activation needs a different direction, + // several moves, or another event — and both would still compile. + pointer = { x: pointer.x + DRAG_THRESHOLD_PX, y: pointer.y }; + await page.mouse.move(pointer.x, pointer.y); + }, + async moveBy(dx: number, dy: number) { pointer = { x: pointer.x + dx, y: pointer.y + dy }; await page.mouse.move(pointer.x, pointer.y); diff --git a/e2e/tests/canvas/scenarios.spec.ts b/e2e/tests/canvas/scenarios.spec.ts index 44bc6571b7..b4369073d1 100644 --- a/e2e/tests/canvas/scenarios.spec.ts +++ b/e2e/tests/canvas/scenarios.spec.ts @@ -11,7 +11,14 @@ */ import { expect, test } from "@playwright/test"; -import { dragPointerTo, dragUntilTarget } from "./driver"; +import { + dragPointerTo, + dragToZoneEdge, + dragUntilInsideZone, + dragUntilTarget, + jitterAcrossEdge, + settledTarget, +} from "./driver"; import type { ActiveTargetTransition, CanvasDriver } from "./driver"; import { EXTREME_RATIO_FIXTURE, FLAT_LIST_FIXTURE, seedPage } from "./fixtures"; import { createPocDriver } from "./poc-driver"; @@ -49,26 +56,67 @@ async function startPanelDrag(driver: CanvasDriver) { * produces. Comparing the indicator's live rect against the live pointer is * what makes the #1705 and #1706 guards real. */ -async function expectIndicatorAtPointer(driver: CanvasDriver, label: string) { +async function expectIndicatorAtPointer( + driver: CanvasDriver, + label: string +): Promise { + // Driven INTO a zone before the comparison, so the exact reading is always + // the one used. `@dnd-kit/collision` resolves to a zone containing the + // pointer first and only ranks by the dragged shape's overlap when none does, + // and these scenarios can stop on that fallback with the pointer outside + // every zone. + // + // Tolerating that case is what makes this guard weak. An adjacency bound + // accepts the NEIGHBOURING zone, which is precisely what a stale-scroll or + // unscaled-transform implementation selects — so the two failures this + // function exists to catch (#1705, #1706) can satisfy it. Inside a zone, + // containment is exact and needs no tolerance at all, and every geometry + // scenario can reach such a position. + const containing = await dragUntilInsideZone(driver); + + // SETTLED FIRST, then the indicator. `dragUntilInsideZone` has just moved the + // pointer into a zone and a canvas whose hysteresis is a dwell may still be + // showing the previous one, so comparing that lagging reading with + // `containing` would make scenarios 1-3 reject correct scroll, scale and + // cross-frame behaviour. + // + // The ORDER is part of the assertion. Reading the rectangle before the wait + // certifies a rectangle belonging to whatever was active before the dwell + // expired, so a canvas that activates the right containing zone and then + // hides or collapses its new indicator passes on the old target's rect — + // the two readings would describe different states while being asserted as + // one. + const active = await settledTarget(driver); + + // Read after the walk AND after the settle, so it belongs to the position and + // the target everything below is about. Reading before the walk would certify + // a rectangle from a position the pointer has since left. const rect = await driver.readIndicatorRect(); expect(rect, `${label}: an indicator must be visible`).not.toBeNull(); - - // Exact, not within a tolerance. A distance threshold is meaningless here: - // collision picks the NEAREST zone, so the pointer legitimately sits up to - // half the zone spacing away, and any threshold near that is either slack - // enough to accept a neighbour or tight enough to reject a correct answer. - // "Is the active zone the nearest one?" has neither problem, and both the - // stale-rect and unscaled-transform failures pick a non-nearest zone. - const active = await driver.readActiveTarget(); const nearest = await driver.nearestZoneToPointer(); test.info().annotations.push({ type: `${label}-zone`, - description: `active=${active} nearest=${nearest}`, + description: `active=${active} containing=${containing} nearest=${nearest}`, }); + + // A precondition, not a fallback. If no position inside a zone can be reached + // the mapping question cannot be asked exactly, and answering it approximately + // would certify the implementations this is meant to reject. + expect( + containing, + `${label}: the pointer must reach a position inside a zone for containment to be decidable` + ).toBeGreaterThanOrEqual(0); expect( active, - `${label}: the active zone must be the nearest to the pointer` - ).toBe(nearest); + `${label}: the zone containing the pointer must be the active one` + ).toBe(containing); + + // Handed back so a caller asserting on POSITION uses the target the pointer + // ended on. The containment walk can activate a different zone than the one + // the caller reached, and a drop then lands at the new target while an + // assertion written against the cached ordinal reports a failure the canvas + // did not cause. + return active; } test("scenario 1: a library block drags across the iframe boundary", async ({ @@ -87,12 +135,17 @@ test("scenario 1: a library block drags across the iframe boundary", async ({ // A drop target here is the whole cross-frame question: it means dnd-kit // resolved a droppable registered inside the iframe from a pointer event in // the host document. - const active = await dragUntilTarget(driver); + const reached = await dragUntilTarget(driver); test .info() - .annotations.push({ type: "active-target", description: String(active) }); - expect(active).toBeGreaterThanOrEqual(0); - await expectIndicatorAtPointer(driver, "cross-frame"); + .annotations.push({ type: "active-target", description: String(reached) }); + expect(reached).toBeGreaterThanOrEqual(0); + // The target the pointer ENDED on, which is what the drop will use. The + // containment walk inside this helper can activate a different zone than the + // one `dragUntilTarget` stopped at — the first may have come from the overlap + // fallback — and a position assertion written against the earlier ordinal + // then reports a failure for a drop that landed exactly where it was shown. + const active = await expectIndicatorAtPointer(driver, "cross-frame"); await driver.drop(); await expect @@ -144,7 +197,11 @@ test("scenario 2: droppable geometry survives a host scroll mid-drag", async ({ ).toBeGreaterThan(50); await driver.moveBy(0, 1); - const after = await driver.readActiveTarget(); + // The host has just scrolled and the pointer has just moved, so this is + // exactly where a dwell-based canvas is permitted to report nothing yet. An + // immediate read can catch that gap and fail the assertion below on a canvas + // that resolves correctly a moment later. + const after = await settledTarget(driver); test.info().annotations.push({ type: "scroll-targets", @@ -290,104 +347,60 @@ test("scenario 4b: a 2px jitter at a zone edge keeps the indicator stable", asyn await startPanelDrag(driver); - // Get onto a zone first, then walk until the target CHANGES. Both halves are - // required: jittering from a point with no active target measures dead space, - // and jittering from the middle of one zone's catchment is not a boundary at - // all. Either would report a clean run without testing the thing named in the - // title. - const first = await dragUntilTarget(driver); + // Onto a zone, then to that zone's EDGE. Both halves are required: jittering + // from a point with no active target measures dead space, and jittering from + // the middle of one zone's catchment is not a boundary at all. Either reports + // a clean run without testing the thing named in the title. + // + // Shared with the acceptance suite rather than written twice. The two suites + // ask the same question, and a second copy of this walk is invisible when it + // is wrong — the drag still runs and still reports an ordinal. + const edge = await dragToZoneEdge(driver); expect( - first, + edge.target, "the drag must reach a zone before seeking a boundary" ).toBeGreaterThanOrEqual(0); - - let crossed = -1; - let previous = first; - for (let step = 0; step < 120; step++) { - await driver.moveBy(0, 4); - const current = await driver.readActiveTarget(); - if (current >= 0 && current !== previous) { - crossed = current; - break; - } - if (current >= 0) previous = current; - } + // The target must have CHANGED. A canvas whose collision resolution is stuck + // on one target forever walks the whole search without a crossing, and every + // jitter afterwards is stable — indistinguishable from a compliant 8-12px + // margin or dwell. Without this, that unusable implementation produces the + // same eventual green as a correct one. expect( - crossed, + edge.crossed, "a boundary must actually be crossed, or this measures the middle of one zone" - ).toBeGreaterThanOrEqual(0); - - // Bracket the edge of `crossed`'s own catchment. Waiting for `previous` to - // return cannot work: the file documents that zones are separated by - // block-sized dead space, so the old zone is hundreds of pixels away and six - // 1px moves simply run out, leaving the pointer wherever it started. - // Search well past the largest hysteresis margin the requirement allows - // (8-12px). Failing to find the edge within that distance is not a test - // failure: it means the target is sticky, which is the behaviour being asked - // for. The jitter below then runs from wherever the pointer sits and simply - // observes no flip. - const HYSTERESIS_SEARCH_PX = 24; - let bracketed = false; - for (let step = 0; step < HYSTERESIS_SEARCH_PX; step++) { - await driver.moveBy(0, -1); - if ((await driver.readActiveTarget()) !== crossed) { - // One step back inside, so +/-2px straddles the edge. - await driver.moveBy(0, 1); - bracketed = true; - break; - } - } + ).toBe(true); + // An unbracketed edge is INCONCLUSIVE rather than weaker evidence: a resolver + // that advances once and never retreats satisfies `crossed`, leaves this + // false, and jitters stably from the middle of its catchment — exactly like a + // compliant margin. The scenario stops rather than reporting either verdict. test.info().annotations.push({ type: "bracketed", - description: String(bracketed), + description: String(edge.bracketed), }); - // Step to P-2 first, then alternate by 4px so the samples are P+2 and P-2 — - // genuinely opposite sides of the edge. Alternating +/-2 from P samples P+2 - // and P, both on the same side, which an implementation that switches the - // instant the pointer crosses would still pass. - await driver.moveBy(0, -2); - - // Recorded from inside the page, not sampled from the test. The requirement - // permits hysteresis expressed as a dwell of more than 100ms as an - // alternative to a distance margin, and a `readActiveTarget()` between two - // moves is a cross-frame round trip that holds the pointer still for the - // length of that trip. On a loaded runner that alone can outlast the dwell, - // so a canvas that implements the timer correctly would still be seen to - // switch on every sample and would stay classified as the known gap below. - // Recording separates the observation from the gesture; the moves then run - // back to back and the dwell is only as long as a mouse event takes. - // Whether the probe was valid at all, established rather than assumed. If a - // move took longer than the dwell the requirement allows, the pointer rested - // at an endpoint long enough for a compliant timer to commit, and any flip - // observed afterwards says nothing about hysteresis. - // - // Each move is one CDP round trip, so its duration is a property of the - // machine rather than of the canvas. A busy runner exceeds the allowance - // often enough that the probe has to expect it: the jitter is repeated, and - // the first sweep whose slowest move stays inside the allowance is the one - // read. Each sweep returns the pointer to where it started, 10 moves of +4 - // against 10 of -4, so a repeat re-probes the same bracketed edge. - const DWELL_ALLOWANCE_MS = 100; - const PROBE_SWEEPS = 3; - let observed: ActiveTargetTransition[] | undefined; - let slowest = Number.POSITIVE_INFINITY; - - for (let sweep = 0; sweep < PROBE_SWEEPS && observed === undefined; sweep++) { - const readTransitions = await driver.recordActiveTargetTransitions(); - const moveDurations: number[] = []; - for (let step = 0; step < 20; step++) { - const startedAt = Date.now(); - await driver.moveBy(0, step % 2 === 0 ? 4 : -4); - moveDurations.push(Date.now() - startedAt); - } - const log = await readTransitions(); - slowest = Math.max(...moveDurations); - test.info().annotations.push({ - type: `jitter-move-durations-ms-sweep-${sweep + 1}`, - description: JSON.stringify(moveDurations), - }); - if (slowest < DWELL_ALLOWANCE_MS) observed = log; + if (!edge.bracketed) { + await driver.cancel(); + test.skip( + true, + "the reverse search never found the edge, so a stable jitter cannot be told from a target that only ever advances" + ); + return; } + // The DWELL-AWARE probe, shared with the acceptance suite so both ask the + // question the same way. It steps to P-2 first and alternates by 4 (samples + // on genuinely opposite sides of the edge), records transitions from inside + // the page rather than sampling between moves, and repeats until a sweep is + // fast enough to be conclusive. + // + // The timing is what makes it valid rather than merely repeated: the + // requirement permits hysteresis as a >100ms dwell instead of a distance + // margin, and each move is a CDP round trip whose duration belongs to the + // machine. If a move outlasts that allowance the pointer rested long enough + // for a compliant timer to commit, and any flip afterwards says nothing. + const probe = await jitterAcrossEdge(driver); + const observed = probe.transitions; + const slowest = probe.slowestMoveMs; + const DWELL_ALLOWANCE_MS = probe.dwellAllowanceMs; + const PROBE_SWEEPS = probe.sweeps; await driver.cancel(); // An unmeasurable run is INCONCLUSIVE, not a defect. Failing here would diff --git a/e2e/tests/canvas/settle.test.ts b/e2e/tests/canvas/settle.test.ts new file mode 100644 index 0000000000..58f28108c1 --- /dev/null +++ b/e2e/tests/canvas/settle.test.ts @@ -0,0 +1,169 @@ +/** + * Unit tests for the settling reader every other probe in this suite is judged + * with. + * + * It earns its own tests because it occupies the position the auditing is done + * from: when it reports a stale value as settled, every assertion built on it + * fails or passes for a reason that has nothing to do with the canvas, and + * nothing else in the suite is standing far enough out to notice. + * + * No browser. They live under the Playwright runner because it is the only test + * runner this package has, matching `oscillation.test.ts`. + */ +import { expect, test } from "@playwright/test"; + +import { + DEFAULT_DWELL_ALLOWANCE_MS, + PERMITTED_DWELL_FLOOR_MS, + settledTarget, + settledValue, +} from "./driver"; + +/** + * A reader that behaves like a canvas whose hysteresis is a dwell TIMER: it + * keeps answering with the previous value for `dwellMs` after the pointer moved, + * then commits to the new one. + * + * This is a COMPLIANT canvas, not a broken one. The requirement permits + * hysteresis expressed as a dwell of up to {@link DEFAULT_DWELL_ALLOWANCE_MS} instead of + * a distance margin, so every assertion below is about the harness reading such + * a canvas correctly rather than about the canvas being right. + */ +function dwellingReader( + before: number, + after: number, + dwellMs: number +): () => Promise { + const movedAt = Date.now(); + return async () => (Date.now() - movedAt < dwellMs ? before : after); +} + +test("returns the value the reader commits to, not the one it is still lagging on", async () => { + // Two consecutive identical reads do not establish settlement: during a + // permitted dwell every read agrees, and all of them return the PRE-move + // value. Only stability observed across the whole allowance separates a + // canvas that has committed from one still entitled to lag. + expect( + await settledValue( + dwellingReader(1, 2, DEFAULT_DWELL_ALLOWANCE_MS / 2), + DEFAULT_DWELL_ALLOWANCE_MS + ) + ).toBe(2); +}); + +test("spends the whole allowance before calling an unchanged value settled", async () => { + // The cost is the point rather than a side effect: a canvas answering + // immediately and a canvas that has not yet changed its mind are + // indistinguishable until the interval it was permitted to lag for has + // passed. Any implementation that returns sooner is guessing. + const started = Date.now(); + expect(await settledValue(async () => 3, DEFAULT_DWELL_ALLOWANCE_MS)).toBe(3); + expect(Date.now() - started).toBeGreaterThanOrEqual( + DEFAULT_DWELL_ALLOWANCE_MS + ); +}); + +test("throws rather than returning a value from a reader that never holds still", async () => { + // A number handed back from a canvas still changing its mind is one no + // assertion downstream can qualify. Returning it silently would let an + // unstable canvas produce an ordinary-looking green, which is the failure + // this whole file exists to prevent. + let next = 0; + await expect( + settledValue( + async () => { + next += 1; + return next; + }, + DEFAULT_DWELL_ALLOWANCE_MS, + "test reading" + ) + ).rejects.toThrow(/test reading changed more than \d+ times/); +}); + +test("tolerates a reader that settles only after changing more than once", async () => { + // A canvas is entitled to a dwell, and to another if the first expiry moves + // the target somewhere that starts a second. A reader that accepted only one + // transition would return the intermediate value and call it settled. + const movedAt = Date.now(); + const step = DEFAULT_DWELL_ALLOWANCE_MS / 2; + const value = await settledValue(async () => { + const elapsed = Date.now() - movedAt; + if (elapsed < step) return 1; + if (elapsed < step * 2) return 2; + return 3; + }, DEFAULT_DWELL_ALLOWANCE_MS); + expect(value).toBe(3); +}); + +test("reads a canvas whose dwell sits ABOVE the permitted floor", async () => { + // The requirement is "a dwell of MORE than 100ms", so the floor is the + // smallest dwell a compliant canvas may use, not the largest. Waiting only + // the floor returns the pre-move value for every canvas that clears the bar + // it was told to clear — the more correct the implementation, the more + // reliably the harness misread it. + // + // This is the case that separates the two constants: with one shared value + // the settling wait equals the floor and this fails. + expect( + await settledValue( + dwellingReader(1, 2, PERMITTED_DWELL_FLOOR_MS + 50), + DEFAULT_DWELL_ALLOWANCE_MS + ) + ).toBe(2); +}); + +test("honours a dwell the driver declares to be longer than the default", async () => { + // The requirement sets no upper bound on a permitted dwell, so no global + // constant can be right for every canvas. A driver that dwells longer than + // the default says so, and the settling helper takes ITS figure — with a + // shared constant this returns the pre-move value. + // + // Trusting the driver here is safe because understating the dwell is + // self-punishing: readings come back stale and the suite fails. The jitter + // probe deliberately does NOT trust it, because that probe grades whether + // hysteresis exists at all. + const slow = DEFAULT_DWELL_ALLOWANCE_MS + 200; + const read = dwellingReader(1, 2, slow - 50); + + expect( + await settledTarget({ dwellAllowanceMs: slow, readActiveTarget: read }) + ).toBe(2); +}); + +test("takes the default for a driver that declares no dwell", async () => { + // The capability is optional, so a driver that declares nothing settles + // against the default rather than silently against a zero allowance. + const read = dwellingReader(1, 2, DEFAULT_DWELL_ALLOWANCE_MS / 2); + + expect(await settledTarget({ readActiveTarget: read })).toBe(2); +}); + +test("tolerates a change when the driver declares no dwell", async () => { + // The tolerance is a COUNT of changes, never a duration. A canvas whose + // hysteresis is a distance margin declares an allowance of zero, and it still + // re-renders asynchronously between two reads, so settling has to absorb a + // bounded number of changes at any allowance — including none. + let reads = 0; + const value = await settledValue(async () => { + reads += 1; + return reads <= 1 ? 1 : 2; + }, 0); + + expect(value).toBe(2); +}); + +test("accepts a reader that changes the permitted number of times and then holds", async () => { + // The count bounds CHANGES, so a reader that changes exactly the permitted + // number of times and then holds perfectly still has settled — asynchronous + // relayout produces precisely that shape. Settling therefore takes one + // OBSERVATION after the last permitted transition, since the value that + // follows the final change is the only one that can be confirmed stable. + let reads = 0; + const value = await settledValue(async () => { + reads += 1; + return reads <= 4 ? reads : 99; + }, 0); + + expect(value).toBe(99); +});