From 74701fc641dbb93265ae854d0b3754bd96cf4caa Mon Sep 17 00:00:00 2001 From: Serena <94026305+serenakeyitan@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:08:56 -0700 Subject: [PATCH] The dismissal test waits for the frame, not the popup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dismissal spares the drag and the artifact pill` failed about one run in four, always on its own first assertion: `precondition: a word click did not open the composer`. The frame decides whether a click dismisses or acts, from its own `shellUiOpen` copy of the state — and the shell only sends that in an effect, after React has already unmounted the card. The preceding test closes the composer with the X button and this one clicks a word a few milliseconds later; when that mousedown lands in the gap, the frame still believes something is open, reads the click as a dismissal, and posts `tdoc:cleared` instead of a word selection. `openCount()` was watching the shell's half of a two-part state change. Mirror `shellUiOpen` onto `` as `data-tdoc-ui-open`, next to `data-tdoc-selecting`, `data-tdoc-editing` and `data-tdoc-interaction-mode` which are already there, and gate the test on the frame's own copy instead of on a timeout. Also: step 2 never clicked the pill. `boundingBox()` is already in main-frame coordinates for an element inside an iframe, and the test added the doc-frame offset to it a second time, putting the click 48px low, on prose. Prose dismisses too, so the assertion passed without exercising the path it is named after. Co-Authored-By: Claude Opus 5 --- server/frame-probe.js | 12 +++++++++++- test/browser-editing.test.js | 19 +++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/server/frame-probe.js b/server/frame-probe.js index 12194771..337367c3 100644 --- a/server/frame-probe.js +++ b/server/frame-probe.js @@ -27,6 +27,16 @@ // While something is open, the next click anywhere in the document only // dismisses it — it must not open a different comment or start a new one. var shellUiOpen = false; + // Mirrored onto alongside the other frame state. The shell unmounts a + // card at commit and only tells us one effect + one postMessage later, so + // "the popup is gone from the shell" and "the frame will act on the next + // click again" are two different moments. Anything waiting on the second one + // has to be able to see it. + function setShellUiOpen(open) { + shellUiOpen = open; + if (open) document.documentElement.setAttribute('data-tdoc-ui-open', ''); + else document.documentElement.removeAttribute('data-tdoc-ui-open'); + } var swallowClick = false; var dismissDownX = 0, dismissDownY = 0; var COMMENT_ICON_PATH = 'M2 2H12A10 10 0 1 1 2 12V2Z'; @@ -1064,7 +1074,7 @@ } else if (d.type === 'tdoc:theme') applyTheme(d.theme); else if (d.type === 'tdoc:mode') setInteractionMode(d.mode); - else if (d.type === 'tdoc:uiOpen') shellUiOpen = !!d.open; + else if (d.type === 'tdoc:uiOpen') setShellUiOpen(!!d.open); else if (d.type === 'tdoc:editFormat') formatEdit(d.command, d.value); else if (d.type === 'tdoc:editRestore') { var restoreRoot = findEditRoot(); diff --git a/test/browser-editing.test.js b/test/browser-editing.test.js index 47713862..88d9819f 100644 --- a/test/browser-editing.test.js +++ b/test/browser-editing.test.js @@ -288,6 +288,14 @@ async function chooseMode(page, label) { }; }); const openCount = () => page.locator('.tdoc-popup, .tdoc-margin-comment.active').count(); + // The frame, not the shell, decides whether a click dismisses or acts, and + // it decides from its own copy of the open state — which arrives one + // effect and one postMessage after the shell has already mounted or + // unmounted the card. openCount() is the shell's half of that; a click + // sent between the two halves is read as a dismissal and never opens the + // composer, which is what made this test fail one run in four. + const frameKnows = (open) => frame.waitForFunction( + (want) => document.documentElement.hasAttribute('data-tdoc-ui-open') === want, open); const clickAt = async (point) => { await page.mouse.click(box.x + point.x, box.y + point.y); await page.waitForTimeout(250); @@ -296,8 +304,10 @@ async function chooseMode(page, label) { // 1. A drag while something is open is a selection, not a dismissal. // Clearing on mousedown unmounted the focused composer, and losing // that focus wiped the selection mid-drag. + await frameKnows(false); await clickAt(geometry.first); assert(await openCount() > 0, 'precondition: a word click did not open the composer'); + await frameKnows(true); await page.mouse.move(box.x + geometry.drag.x1, box.y + geometry.drag.y); await page.mouse.down(); await page.mouse.move(box.x + geometry.drag.x2, box.y + geometry.drag.y, { steps: 12 }); @@ -305,13 +315,14 @@ async function chooseMode(page, label) { await page.waitForTimeout(400); assert(await openCount() > 0, 'a drag while something was open reported no selection'); await page.locator('.tdoc-popup button.x').click().catch(() => {}); - await page.waitForTimeout(200); + await frameKnows(false); // 2. The artifact pill is our own UI, but while a card is open it is // outside that card like anything else: it dismisses, it does not // open an element comment. await clickAt(geometry.first); assert(await openCount() > 0, 'precondition: composer did not reopen'); + await frameKnows(true); // Dispatch the hover inside the frame: Playwright's own hover() checks // actionability against the top document, where the open composer sits // over the artifact and the check never settles. @@ -323,9 +334,13 @@ async function chooseMode(page, label) { })); }); await frame.locator('.tdoc-comment-pill').waitFor({ state: 'visible' }); + // boundingBox() is already in main-frame coordinates for an element inside + // an iframe. Adding the doc-frame offset a second time put this click 48px + // below the pill, on plain prose — which also dismisses, so the assertion + // passed without ever touching the pill it is named after. const pill = await frame.locator('.tdoc-comment-pill').boundingBox(); assert(pill, 'hovering the artifact did not show the comment pill'); - await page.mouse.click(box.x + pill.x + pill.width / 2, box.y + pill.y + pill.height / 2); + await page.mouse.click(pill.x + pill.width / 2, pill.y + pill.height / 2); await page.waitForTimeout(400); assert(await openCount() === 0, 'clicking the artifact pill opened a comment instead of dismissing');