Skip to content

fix(ui): disarm the inline confirm on every exit - #323

Merged
argszero merged 1 commit into
mainfrom
fix/inline-confirm-disarms-on-every-exit
Sep 29, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/inline-confirm-disarms-on-every-exit

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

confirmInline(btn, onConfirm, text) (ui/js/app.js) has exactly three ways out of the state it puts a button into ("确认删除?"): the second click, the 3-second timeout, and Escape. Their shared teardown was wired to one exit out of three, so two of them left something behind.

Related Issue

None — found during a read of the inline-confirmation component.

Changes

  • ui/js/app.js — confirmInline now routes all three exits through one disarm(): clear the timer, delete dataset.confirm, drop .confirming, unregister the document-level keydown listener, and put the button's own markup back.
  • src/state_gate.rs — new gate the_inline_confirm_disarms_on_every_exit (six rules, all derived from confirmInline's own arming writes — no snapshot, no hand-written roster) plus four companions: a derived-roster positive control, scanner self-tests, a six-mutant table where each mutant reddens exactly the rule it targets, and the closure rule on the live tree.
  • ui/README.md — the inline-component contract gains the same statement as a sub-bullet, naming the gate and its scope (this is how the neighbouring contracts are documented).
  • ui/index.html — cache-bust for app.js (20260928-2 → 20260929-1).

The two exits

  • Second click cleared dataset.confirm and .confirming but never restored the label. On a FAILED delete no call site re-renders, so the button keeps reading the confirmation prompt although it is not armed any more; clicking it again captures that prompt as the "original" label, so from then on even the timeout restores the prompt — the button never shows its own label again for the rest of the session.
  • 3-second timeout restored the label but did not unregister the keydown listener, and neither did the click exit, so every confirmed/timed-out confirmation left one dead closure behind, pinning its button. A later Escape fired the whole pile, each rewriting the innerHTML of a button that is long gone. Measured on the pre-fix tree: listeners at rest 1 → 2 → 3.

Provenance is drift, not a trade-off: the unregister call was written (document.removeEventListener("keydown", esc)) and attached to the Escape branch alone, while the label restore lived in a revert() the other two exits could not reach. Both were introduced by 990cd5a (#41); the contract in ui/README.md has said "3 秒无操作或 Esc 还原,再次点击执行" ever since, so only the code disagreed.

Arming and confirming are two separate calls, so the second one has no handle on the first one's closure; the pre-arm label and the listener are therefore recorded on the node (next to the existing _confirmT), which is what lets the teardown reach them from either call.

Tests

  • cargo test — 426 passed, 0 failed (421 before this PR: +5 gate tests)
  • cargo fmt --check clean
  • cargo clippy --all-targets -- -D warnings clean

New tests: the five R117 tests in src/state_gate.rs (the gate itself, the derived roster, the rule teeth, the live-tree closure rule and the scanner self-tests).

Beyond the suite, a jsdom probe over the real ui/ drove the actual delete button (10 legs × 2 languages × 7 trees = 140 checks, misdeclared=0): the pre-fix tree is red on the four disposal legs plus the stale-listener leg, the fixed tree is green on all ten, and four competing fixes ("restore the label only", "unregister only", "drop the Escape exit", "re-render at the failing call site") are each rejected by their own legs.

Checklist

  • Branch name follows the convention (fix/…)
  • Commit message uses Conventional Commits (fix(ui): …)
  • Single responsibility, minimal change

`confirmInline(btn, onConfirm, text)` in `ui/js/app.js` turns a button into
its own confirmation ("确认删除?") and has exactly three ways out of that
state: the second click, the 3-second timeout, and Escape. Their shared
teardown was wired to one exit out of three:

- the second click cleared `dataset.confirm` and `.confirming` but never
  restored the label. On a FAILED delete no call site re-renders, so the
  button keeps reading the confirmation prompt although it is not armed any
  more; clicking it again then captures that prompt as the "original" label,
  so from that point on even the timeout restores the prompt -- the button
  never shows its own label again for the rest of the session.
- the timeout exit restored the label but did not unregister the
  document-level keydown listener, and neither did the click exit, so each
  confirmed/timed-out confirmation left one dead closure behind, pinning its
  button. A later Escape fired the whole pile, each rewriting the
  `innerHTML` of a button that is long gone (measured on the pre-fix tree:
  1 -> 2 -> 3 listeners).

Provenance is drift, not a trade-off: the unregister call was written --
`document.removeEventListener("keydown", esc)` -- and attached to the Escape
branch alone, while the label restore lived in a `revert()` that the other
two exits could not reach. Both were introduced by 990cd5a (#41) and the
contract in `ui/README.md:90` has said "3 秒无操作或 Esc 还原,再次点击执行"
ever since, so only the code disagreed.

Both exits now route through one `disarm()`: clear the timer, delete the
flag, drop `.confirming`, unregister the keydown listener, and put the
button's own markup back. Because arming and confirming are two separate
calls, the second one has no handle on the first one's closure, so the
pre-arm label and the listener are recorded on the node (next to the
existing `_confirmT`) and the teardown can reach them from either call.

Gate: `src/state_gate.rs` gains six rules, all derived from `confirmInline`'s
own arming writes (never from a snapshot) -- the teardown is one block-bodied
closure and it is the one that unregisters; the click, timeout and Escape
exits each reach it; the listener removed is the one registered; and the
markup restored is the button's own, not the confirmation prompt. Four
companion tests pin the roster (derived, with a positive control), the
extractors, a six-mutant table in which each mutant reddens exactly the rule
it targets, and the closure rule on the live tree. Scope is lexical: it
proves the three exits share one disposal, not what the screen shows (this
repository's CI has no JS runner). `ui/README.md` gains the same contract as
a sub-bullet of the inline-component section, naming the gate and its scope,
which is how the neighbouring contracts are documented.

Verification:
- jsdom probe over the real `ui/` (10 legs x 2 languages x 7 trees = 140
  checks, `misdeclared=0`): pre-fix tree red on D1/D2/D3/D4/C1, fixed tree
  green on all, and four competing fixes ("restore the label only",
  "unregister only", "drop the Escape exit", "re-render at the failing call
  site") each rejected by their own legs.
- `cargo test` 426 passed, 0 failed (421 before: +5 gate tests).
- `cargo fmt --check` clean; `cargo clippy --all-targets -- -D warnings`
  clean.
- `ui/index.html` cache-bust for `app.js` bumped per convention
  (`20260928-2` -> `20260929-1`), since the file's bytes changed.
@argszero

Copy link
Copy Markdown
Owner Author

Self-review (committer; allow_self_merge is on, so GitHub will not let me approve my own PR — recording the review as a comment instead).

Scope check: the diff is four files — the fix (ui/js/app.js), the gate (src/state_gate.rs), the contract note (ui/README.md) and the cache-bust (ui/index.html). No unrelated edits.

Fix check: all three exits (second click, timeout, Escape) now call one disarm(); the pre-arm label and the registered listener are stored on the node, so the confirming call can dispose of what the arming call set up. The Escape listener stays a named function expression, and the happy path is untouched.

Gate check: six rules, each derived from confirmInline's own arming writes rather than a snapshot, with a derived roster plus a positive control, a six-mutant table in which each mutant reddens exactly its own rule, and a live-tree leg that injects a second disposal closure and requires rule 1 to go red. Its scope is lexical and says so — the on-screen half was measured by a jsdom probe over the real ui/ (140 checks across 7 trees, misdeclared=0, pre-fix red on the five axis legs, four competing fixes rejected).

Verification: cargo test 426 passed / 0 failed (421 before, +5); cargo fmt --check clean; cargo clippy --all-targets -- -D warnings clean; CI green on both jobs.

@argszero
argszero merged commit aab71ba into main Sep 29, 2026
2 checks passed
@argszero
argszero deleted the fix/inline-confirm-disarms-on-every-exit branch September 29, 2026 00:31
@argszero argszero mentioned this pull request Sep 30, 2026
12 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant