Postulo/postulo

Fork 0

Escape sometimes does not close a cell editor, in a full browser run #161

New issue

Closed

opened 2026-09-10 04:53:36 +00:00 by tiagoagueda · 2 comments

tiagoagueda commented

2026-09-10 04:53:36 +00:00

Owner

Observation

tests/e2e/test_editable_cells.py::test_escape_abandons_and_keeps_the_old_value failed once
in a full browser run and passed on the next full run and in isolation.

expect(page.locator("[data-cell-editor]")).to_have_count(0)
E  unexpected value "1"

Escape did not close the editor within Playwright's retry window. One failure in three full
runs; nothing reproducible yet.

Why it is worth an issue rather than a shrug

A test that fails one run in three is worse than a test that fails every time. It trains
whoever sees it to re-run rather than to look, and the next genuine failure of this file
gets the same treatment. This project already learnt that lesson twice: the sign-in limiter
handing a 429 to whichever test happened to run past the allowance, and the column-width
saves tearing the live server down mid-request. Both had the same shape — passes alone,
fails in company — and both turned out to be real.

Worth being careful about

The helper does not wait for the editor to settle. open_the_editor clicks the cell and
returns; the fill() that follows auto-waits for the input, so the field exists, but nothing
waits for app.js to have finished with it. If Escape can arrive before the handler is
listening, the keypress goes nowhere and the editor stays — which is exactly the observed
failure.

That is a hypothesis, not a diagnosis. Adding a wait would probably make the symptom go
away without anyone knowing whether the bug was in the test or in the page, and that
matters here: if a real person can press Escape a beat too early and have it ignored, the
fix belongs in app.js, not in the test.

So: reproduce first. Run the file in a loop, or the full browser suite repeatedly, and find
out whether Escape is being missed or the editor is being reopened. Only then decide which
side the fix belongs on.

Classification

Bug, 0.4.0. Not a regression from any one change — the cell editor arrived in #135 and this
was seen while working on #56.

## Observation `tests/e2e/test_editable_cells.py::test_escape_abandons_and_keeps_the_old_value` failed once in a full browser run and passed on the next full run and in isolation. ``` expect(page.locator("[data-cell-editor]")).to_have_count(0) E unexpected value "1" ``` Escape did not close the editor within Playwright's retry window. One failure in three full runs; nothing reproducible yet. ## Why it is worth an issue rather than a shrug A test that fails one run in three is worse than a test that fails every time. It trains whoever sees it to re-run rather than to look, and the next genuine failure of this file gets the same treatment. This project already learnt that lesson twice: the sign-in limiter handing a `429` to whichever test happened to run past the allowance, and the column-width saves tearing the live server down mid-request. Both had the same shape — passes alone, fails in company — and both turned out to be real. ## Worth being careful about **The helper does not wait for the editor to settle.** `open_the_editor` clicks the cell and returns; the `fill()` that follows auto-waits for the input, so the field exists, but nothing waits for `app.js` to have finished with it. If Escape can arrive before the handler is listening, the keypress goes nowhere and the editor stays — which is exactly the observed failure. That is a hypothesis, not a diagnosis. **Adding a wait would probably make the symptom go away without anyone knowing whether the bug was in the test or in the page**, and that matters here: if a real person can press Escape a beat too early and have it ignored, the fix belongs in `app.js`, not in the test. So: reproduce first. Run the file in a loop, or the full browser suite repeatedly, and find out whether Escape is being missed or the editor is being reopened. Only then decide which side the fix belongs on. ## Classification Bug, 0.4.0. Not a regression from any one change — the cell editor arrived in #135 and this was seen while working on #56.

tiagoagueda added the

tier

label

2026-09-12 10:01:59 +00:00

tiagoagueda commented

2026-09-12 11:20:34 +00:00

Author

Owner

Not reproduced in fifteen runs, and the stated hypothesis is wrong

Tried, because this issue asks for that before anything is changed:

tests/e2e/test_editable_cells.py on its own, twelve times — twelve passes.
The full browser suite three times — 90 passed, 90 passed, 90 passed.

So nothing is being fixed here. What the attempt did settle is what it is not.

The hypothesis in this issue cannot be the cause

The helper does not wait for the editor to settle. […] If Escape can arrive before the
handler is listening, the keypress goes nowhere and the editor stays

The handler is not attached to the editor. It is a single document-level keydown
listener, registered once when app.js is evaluated:

document.addEventListener("keydown", function (event) {
  if (event.key !== "Escape") return;
  var editor = event.target.closest ? event.target.closest("[data-cell-editor]") : null;
  ...

It is therefore listening long before any cell can be opened, and there is no window in
which an editor exists but its Escape handler does not. The reason this issue gave for
wanting a wait in the test does not hold, which matters because that wait was the obvious
thing to reach for.

Two more things ruled out

The key reaches the right place. The test uses field.press("Escape"), and Playwright
focuses the element before pressing, so event.target is the input inside the editor and
closest("[data-cell-editor]") matches. It is not a focus race.

The right element is clicked. editor.querySelector("[hx-get]") could in principle find
the wrong control, but partials/table/cell.html puts exactly one hx-get inside the form
— the Cancel button. The Save button is a plain submit and the hx-post is on the form.

What is left, and why it points the other way

What remains between the keypress and the assertion is the htmx GET that Cancel fires.
If the live server is slow — which is what a full run has that an isolated one does not —
the editor is still in the page when Playwright's retry window expires, and the failure
reads exactly as reported: "unexpected value 1".

If that is it, the fix is on the test side after all, but not the fix this issue
anticipated. Lengthening a timeout would hide a slow response; asserting on the outcome
that actually matters — the cell showing its old value again — waits for the same round trip
while saying what it is waiting for. Worth noting the two assertions after the failing one
already do exactly that.

Suggested next step

Fifteen clean runs is not proof of anything for a fault seen once in three, so this is not
evidence it has gone. Either:

leave it open with the above recorded, and look again the next time it fails — the
diagnosis is now three steps further on than it was, which is most of the value; or
instrument rather than re-run: log the time between the Escape and the swap in that test,
so the next failure says whether the request was slow or never went.

Re-running more is the option with the worst ratio of time to information.

One thing that changed under this

app.js gained two dragenter listeners in #174, in the board and widget drag blocks. They
share the file and nothing else with this — no keydown, no cell editor — but the runs above
were made after that landed, so they describe the current file rather than the one this issue
was opened against.

## Not reproduced in fifteen runs, and the stated hypothesis is wrong Tried, because this issue asks for that before anything is changed: - `tests/e2e/test_editable_cells.py` on its own, **twelve times** — twelve passes. - The **full browser suite three times** — 90 passed, 90 passed, 90 passed. So nothing is being fixed here. What the attempt did settle is what it is *not*. ### The hypothesis in this issue cannot be the cause > **The helper does not wait for the editor to settle.** […] If Escape can arrive before the > handler is listening, the keypress goes nowhere and the editor stays The handler is not attached to the editor. It is a **single `document`-level `keydown` listener, registered once when `app.js` is evaluated**: document.addEventListener("keydown", function (event) { if (event.key !== "Escape") return; var editor = event.target.closest ? event.target.closest("[data-cell-editor]") : null; ... It is therefore listening long before any cell can be opened, and there is no window in which an editor exists but its Escape handler does not. The reason this issue gave for wanting a wait in the test does not hold, which matters because that wait was the obvious thing to reach for. ### Two more things ruled out **The key reaches the right place.** The test uses `field.press("Escape")`, and Playwright focuses the element before pressing, so `event.target` is the input inside the editor and `closest("[data-cell-editor]")` matches. It is not a focus race. **The right element is clicked.** `editor.querySelector("[hx-get]")` could in principle find the wrong control, but `partials/table/cell.html` puts exactly one `hx-get` inside the form — the Cancel button. The Save button is a plain submit and the `hx-post` is on the form. ### What is left, and why it points the other way What remains between the keypress and the assertion is the **htmx `GET` that Cancel fires**. If the live server is slow — which is what a full run has that an isolated one does not — the editor is still in the page when Playwright's retry window expires, and the failure reads exactly as reported: *"unexpected value 1"*. If that is it, the fix is on the test side after all, but **not the fix this issue anticipated**. Lengthening a timeout would hide a slow response; asserting on the outcome that actually matters — the cell showing its old value again — waits for the same round trip while saying what it is waiting for. Worth noting the two assertions after the failing one already do exactly that. ### Suggested next step Fifteen clean runs is not proof of anything for a fault seen once in three, so this is not evidence it has gone. Either: - leave it open with the above recorded, and look again the next time it fails — the diagnosis is now three steps further on than it was, which is most of the value; or - instrument rather than re-run: log the time between the Escape and the swap in that test, so the next failure says whether the request was slow or never went. Re-running more is the option with the worst ratio of time to information. ### One thing that changed under this `app.js` gained two `dragenter` listeners in #174, in the board and widget drag blocks. They share the file and nothing else with this — no `keydown`, no cell editor — but the runs above were made after that landed, so they describe the current file rather than the one this issue was opened against.

tiagoagueda added the

bug

label

2026-09-12 11:20:35 +00:00

tiagoagueda referenced this issue from a commit

2026-09-12 16:23:16 +00:00

Close a cell editor on Escape however quickly it is pressed

tiagoagueda closed this issue

2026-09-12 16:23:16 +00:00

tiagoagueda commented

2026-09-12 16:23:44 +00:00

Author

Owner

Fixed in `5e778aef6` -- and the comment above was wrong

The earlier comment ruled out this issue's hypothesis, and the hypothesis was right in
substance. It was wrong only in which handler:

The handler is not attached to the editor. It is a single document-level keydown
listener, registered once when app.js is evaluated.

True, and beside the point. What that listener does is click the editor's Cancel
button, and that button's hx-get is wired by htmx only when the swap that brought the
editor settles -- defaultSettleDelay, 20 ms after the swap, in the vendored htmx
2.0.10. htmx:afterSwap fires at once, and app.js puts the caret in the input right then.
So for 20 ms the editor is visible, focused, and deaf to Escape: the keypress is heard, the
click is made, and nothing is listening to it. A hand is never that quick. A browser test
is, one run in three.

The fix

The keydown handler has htmx process the editor before clicking Cancel. Processing is
idempotent -- htmx skips an element it has already initialised -- so this wires the button
when the settle has not yet, and does nothing when it has.

Reproduced, deterministically, before fixing

Fifteen clean re-runs settled nothing, as the comment above said. What did: a test that
widens the settle window to two seconds (htmx.config.defaultSettleDelay through
add_init_script) and presses Escape inside it. On the old code it failed every time,
at exactly the assertion this issue reports; on the fix it passes. It stays in the suite as
the regression test. The original fast test is untouched -- a fast client is what found this.

Fifteen runs of the file and the full browser suite since, all green.

## Fixed in `5e778aef6` -- and the comment above was wrong The earlier comment ruled out this issue's hypothesis, and the hypothesis was right in substance. It was wrong only in *which* handler: > The handler is not attached to the editor. It is a single `document`-level `keydown` > listener, registered once when `app.js` is evaluated. True, and beside the point. What that listener *does* is click the editor's **Cancel** button, and that button's `hx-get` is wired by htmx only when the swap that brought the editor **settles** -- `defaultSettleDelay`, 20 ms after the swap, in the vendored htmx 2.0.10. `htmx:afterSwap` fires at once, and `app.js` puts the caret in the input right then. So for 20 ms the editor is visible, focused, and deaf to Escape: the keypress is heard, the click is made, and nothing is listening to it. A hand is never that quick. A browser test is, one run in three. ### The fix The `keydown` handler has htmx process the editor before clicking Cancel. Processing is idempotent -- htmx skips an element it has already initialised -- so this wires the button when the settle has not yet, and does nothing when it has. ### Reproduced, deterministically, before fixing Fifteen clean re-runs settled nothing, as the comment above said. What did: a test that widens the settle window to two seconds (`htmx.config.defaultSettleDelay` through `add_init_script`) and presses Escape inside it. On the old code it failed **every time**, at exactly the assertion this issue reports; on the fix it passes. It stays in the suite as the regression test. The original fast test is untouched -- a fast client is what found this. Fifteen runs of the file and the full browser suite since, all green.