app: a variable row's own delete button can survive a sibling row's blur
Round 3 of adversarial review, reproduced live: type an uncommitted edit into one Variables row, then click a DIFFERENT row's own delete button without blurring the first. The click's mousedown blurs the edited field, which commits and triggers a full non-keyed redraw of the whole list (inspector.js draw() -> node.replaceChildren()) -- destroying the delete button mid-gesture, so the click that started on it never arrives. No error, nothing removed, no sign anything happened. Fixed with the same mousedown+preventDefault idiom this session's own autocomplete.js already uses for the same reason: stop the browser's default focus-move (and the blur/commit/redraw it would trigger) before it can pull the target out from under its own pending click. Confirmed with a Playwright test reproducing the exact sequence.
This commit is contained in:
+22
-2
@@ -16,6 +16,27 @@ const RATINGS = [
|
||||
{ value: "disliked", label: "Disliked" },
|
||||
];
|
||||
|
||||
/**
|
||||
* A "✕" that removes something the moment it is pressed, not the moment a
|
||||
* browser would normally call it clicked.
|
||||
*
|
||||
* Between a text field's blur-commit and its own click, a browser blurs the
|
||||
* field FIRST -- and a commit here triggers a full, non-keyed redraw of this
|
||||
* whole list (draw() -> node.replaceChildren()). Wire this to `onClick` and
|
||||
* pressing ✕ right after typing in ANOTHER row's field destroys this exact
|
||||
* button mid-gesture: the field's blur/commit/redraw runs before the click
|
||||
* the mousedown started ever reaches it, so the press does nothing and says
|
||||
* nothing. `mousedown` fires first and `preventDefault()` on it stops the
|
||||
* browser's own focus-move (and so the blur it would otherwise cause) before
|
||||
* this handler's own redraw gets the chance to pull the rug out from under
|
||||
* itself.
|
||||
*/
|
||||
function removeButton(onRemove) {
|
||||
const btn = composer.button.sm({ label: "✕", tone: "danger" });
|
||||
btn.node.addEventListener("mousedown", (e) => { e.preventDefault(); onRemove(); });
|
||||
return btn;
|
||||
}
|
||||
|
||||
export function createInspector({ project, onRename } = {}) {
|
||||
const host = composer.region.stack({ gap: "sm", fill: true });
|
||||
let tab = "scene";
|
||||
@@ -168,8 +189,7 @@ export function createInspector({ project, onRename } = {}) {
|
||||
onCommit: (value) => project.setVariables(project.variables.map(
|
||||
(x, j) => (j === i ? { ...x, value } : x))),
|
||||
}),
|
||||
composer.button.sm({ label: "✕", tone: "danger",
|
||||
onClick: () => project.setVariables(project.variables.filter((_, j) => j !== i)) }),
|
||||
removeButton(() => project.setVariables(project.variables.filter((_, j) => j !== i))),
|
||||
] }),
|
||||
})),
|
||||
composer.button.md({ label: "+ Add variable",
|
||||
|
||||
@@ -147,3 +147,33 @@ test("each Generate draws a fresh seed for shortcut expansion, not a fixed hash
|
||||
expect(seeds.every((s) => s > 0)).toBe(true);
|
||||
expect(new Set(seeds).size, "the same seed was sent twice in a row").toBeGreaterThan(1);
|
||||
});
|
||||
|
||||
test("removing a variable works even mid-edit on a different row", async ({ page }) => {
|
||||
// A real race: row0's blur-commit triggers a full, non-keyed redraw of
|
||||
// this whole list (inspector.js's draw() -> node.replaceChildren()) --
|
||||
// wired to a plain click, pressing row1's ✕ right after typing in row0
|
||||
// (without ever blurring it first) destroys row1's own button mid-
|
||||
// gesture, so the click that started on it never arrives and nothing is
|
||||
// removed. mousedown+preventDefault on the ✕ (inspector.js's
|
||||
// removeButton()) is what this proves still works.
|
||||
await app(page);
|
||||
await page.getByRole("tab", { name: "Project" }).click();
|
||||
|
||||
const add = page.getByRole("button", { name: "+ Add variable" });
|
||||
await add.click();
|
||||
await page.locator('input[placeholder="name"]').last().fill("row0");
|
||||
await add.click();
|
||||
await page.locator('input[placeholder="name"]').last().fill("row1");
|
||||
|
||||
const rowCount = await page.locator('input[placeholder="name"]').count();
|
||||
// Type into row0's name field WITHOUT blurring it, then press row1's own
|
||||
// ✕ (the last one -- rows are appended, never reordered) in one motion --
|
||||
// the exact sequence the race needs.
|
||||
const row0Name = page.locator('input[placeholder="name"]').nth(rowCount - 2);
|
||||
await row0Name.pressSequentially("-edited", { delay: 20 });
|
||||
const removeButtons = page.getByRole("button", { name: "✕" });
|
||||
await removeButtons.last().click();
|
||||
|
||||
await expect(page.locator('input[placeholder="name"]')).toHaveCount(rowCount - 1);
|
||||
await expect(page.locator("body")).not.toContainText("$row1");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user