Skip to content

fix(web-integration): prevent dropped characters when input re-renders mid-type#2518

Open
quanru wants to merge 3 commits into
mainfrom
fix/web-clear-then-type-race
Open

fix(web-integration): prevent dropped characters when input re-renders mid-type#2518
quanru wants to merge 3 commits into
mainfrom
fix/web-clear-then-type-race

Conversation

@quanru
Copy link
Copy Markdown
Collaborator

@quanru quanru commented May 22, 2026

Summary

typeText in replace mode (the default for the Input action) calls clearInput and then immediately runs keyboard.type. clearInput fires a synthetic `input` event with `value === ''`, which many controlled components react to by re-rendering — sometimes replacing the input element entirely. If that replacement lands after `clearInput` returns but during `keyboard.type`, the first few characters land on the now-detached old element and are silently dropped.

This bug reproduces against real React/Vue/etc. controlled inputs and is independent of the `keyboardTypeDelay` knob: increasing the delay only widens the race window, it does not close it. Users currently work around it by passing `mode: 'typeOnly'`, which skips `clearInput` and therefore the race.

Fix

  • Add `Page.waitForDomQuiet({ quietMs, timeoutMs })` to `packages/web-integration/src/puppeteer/base-page.ts`. It uses a `MutationObserver` on `document.body` to wait until the DOM has been mutation-free for `quietMs` (default 100ms), bounded by `timeoutMs` (default 500ms).
  • Declare `waitForDomQuiet` as an optional method on `AbstractWebPage` (`packages/web-integration/src/web-page.ts`) so it is type-safe to call from shared code.
  • In `createWebInputPrimitives`' `typeText` (replace branch), call `await page.waitForDomQuiet?.()` between `clearInput` and `keyboard.type`. Async re-renders triggered by clearing are now absorbed before the first character is typed.

The new method is gated behind `?.` so non-puppeteer/playwright `AbstractWebPage` implementations (e.g. static page) are unaffected.

Regression test

`packages/web-integration/tests/unit-test/puppeteer/input-clear-then-type-race.test.ts` drives a real puppeteer browser against a page that replaces the input element ~250ms after the clear-triggered `input` event — squarely inside the race window. Two cases:

  • Raw `clearInput` + `keyboard.type` still drops characters (`Hello → llo`). This pins down that the bug is real and that the fix doesn't silently hide it on the low-level path.
  • `Input` action (replace mode) preserves all characters (`Hello → Hello`), because it now flows through `waitForDomQuiet`.

Test plan

  • `pnpm run lint`
  • `npx vitest --run tests/unit-test/puppeteer/input-clear-then-type-race.test.ts tests/unit-test/web-actions-navigation.test.ts tests/unit-test/yaml/input-mode-typeonly.test.ts tests/unit-test/yaml/input-mode-typeonly-e2e.test.ts` (4 files, 9 tests, all pass)
  • Manually verify against the original React form where the bug was first observed

…s mid-type

In `replace` mode, `typeText` calls `clearInput` and then immediately
runs `keyboard.type`. `clearInput` fires a synthetic `input` event with
`value === ''`, which many controlled components react to by
re-rendering and replacing the input element. If that replacement lands
AFTER `clearInput` returns but DURING `keyboard.type`, the first
characters land on the now-detached old element and are lost.

Add `Page.waitForDomQuiet({ quietMs, timeoutMs })` that uses a
`MutationObserver` to wait until the DOM has been mutation-free for
`quietMs` (default 100ms), bounded by `timeoutMs` (default 500ms).
Call it in the replace branch of `typeText` between `clearInput` and
`keyboard.type` so async re-renders are absorbed before typing starts.

Adds a deterministic puppeteer-driven regression test that simulates
the re-render pattern and demonstrates the bug on the raw path while
verifying the fix on the production `Input` action path.
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 22, 2026

Deploying midscene with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1d4fd7f
Status: ✅  Deploy successful!
Preview URL: https://cf26bd97.midscene.pages.dev
Branch Preview URL: https://fix-web-clear-then-type-race.midscene.pages.dev

View logs

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3945686707

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +480 to +484
obs.observe(document.body, {
childList: true,
subtree: true,
attributes: true,
characterData: true,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Scope DOM-quiet wait to the input subtree

waitForDomQuiet observes document.body with subtree/attributes/characterData, so any unrelated mutations anywhere on the page keep resetting the quiet timer until the 500ms hard timeout. Because typeText now calls this on every replace-mode input (web-page.ts), dynamic pages with ticking counters/spinners can incur an extra ~500ms per field and may hit higher-level action timeouts even when the target input is stable. Limit observation to the target element (or a narrow ancestor) so unrelated DOM churn does not throttle typing.

Useful? React with 👍 / 👎.

quanru added 2 commits May 22, 2026 14:47
…tion

The initial `settleTimer = setTimeout(done, q)` caused the
MutationObserver to disconnect after `quietMs` even when nothing had
mutated yet. Frameworks that re-render later than `quietMs` (e.g. via
`setTimeout` or async effects) were missed, so the typed characters
still landed on the about-to-be-replaced element.

Drop the initial settleTimer: only resolve on (a) `quietMs` of stillness
after at least one observed mutation, or (b) the `timeoutMs` cap. The
no-mutation path now waits the full `timeoutMs` before returning, which
is the unavoidable cost of catching late re-renders.
The default WDA `/wda/keys` path pushes the whole string in a single
request, so XCUITest fires keystrokes back-to-back with ~30-50ms gaps.
If the input's onChange handler blocks for longer than that gap (RN
re-render, predictive bar, autocorrect), every keystroke that lands
inside the blocking window is dropped, producing contiguous gaps such
as "Al is amazing" arriving as "Al mazing".

Send characters one at a time with an inter-key delay so the gap
exceeds the typical app reaction window. The new `keyboardTypeDelay`
option on `IOSDeviceInputOpt` defaults to 80ms and can be set to 0 to
restore the legacy one-shot behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant