Skip to content

fix(interaction): route fill and press typing into the ref's CDP session - #488

Open
wcy12378 wants to merge 2 commits into
Tencent:mainfrom
wcy12378:fix/438-oopif-input-routing
Open

wcy12378 wants to merge 2 commits into
Tencent:mainfrom
wcy12378:fix/438-oopif-input-routing

Conversation

@wcy12378

Copy link
Copy Markdown
Contributor

Fixes #438

Problem

Refs for cross-frame (OOPIF) nodes carry cdpSessionId, and DOM.focus already resolves through that child session (cdpRunnerForTarget/sendToTarget). But the typing that follows — Input.dispatchKeyEvent/Input.insertText in fill, and the key events in press — went through the tab-level deps.cdp.send, which always hits the root session. Text landed in the main frame while the OOPIF field stayed empty.

Mouse events are unaffected: Input.dispatchMouseEvent is viewport-relative and reaches OOPIF content through the composited widget, which is why only the keyboard path is broken.

Change

  • fill: the three typing calls (caret-positioning rawKeyDown/keyUp and Input.insertText) now go through the same nodeCdp runner as DOM.focus.
  • press: key events go through a keyCdp runner — the node's child session when a ref/selector resolved, the root session otherwise (ref-less press keeps its current behavior).

cdpRunnerForTarget already falls back to the tab-level send when the node has no session, so non-OOPIF behavior is identical.

Validation

Windows x64, Node 24, pnpm 10.17.0 (apps/extension):

  • pnpm test src/tools/__tests__/interaction.test.ts → 129 passed (127 pre-existing + 2 new)
  • New routes fill typing into the ref's OOPIF child session: a ref with cdpSessionId — every Input.* call recorded on the child session via sendToTarget, zero on the root
  • New routes press key events into the ref's OOPIF child session: all three dispatchKeyEvent phases (rawKeyDown/char/keyUp) on the child session, none on the root
  • Red check: reverting only interaction.ts makes exactly the two new tests fail (typing lands on the root session), confirming they exercise the bug
  • pnpm compile (tsc --noEmit) clean

wcy12378 and others added 2 commits October 24, 2026 17:17
Fixes the 'Frontend lint, typecheck, tests, build' CI failure: biome
check rejected the hand-formatted Page.getLayoutMetrics mock entries
in interaction.test.ts and the line-wrapped keyCdp ternary in
interaction.ts. Reformatted both files with biome so 'pnpm lint'
passes.
@wcy12378

Copy link
Copy Markdown
Contributor Author

CI failure root cause: biome check rejected the hand-formatted code in two spots — the one-line Page.getLayoutMetrics mock entries in interaction.test.ts (needed multi-line) and the line-wrapped keyCdp ternary in interaction.ts (needed single-line). pnpm lint failed before typecheck/tests could run.

Fix: reformatted both files with biome (biome check --write), commit fc725c9. No logic changes. Verified locally: biome check clean, extension tsc --noEmit, i18n/vom/extension vitest suites all green, wxt build succeeds.

@iuyo5678

Copy link
Copy Markdown
Collaborator

Thanks for working on this and following up on the CI fixes. I reviewed fc725c9 and would like to hold off on merging until we have a browser-level reproduction of the original issue and address the regressions below.

The assumption that sending keyboard input to the root CDP session necessarily types into the main frame does not match the Chromium behavior I checked. Both Input.dispatchKeyEvent and Input.insertText resolve the focused render widget when invoked on the root frame, so they can reach a focused OOPIF. See Chromium's InputHandler implementation.

I compared the actual interaction handlers from main (b63f551) and main with this patch, using a local cross-site iframe in Chrome 155 on macOS:

  • Filling, appending text, and pressing Enter inside the OOPIF already worked on main and continued to work with the patch.
  • When Tab moved focus out of the iframe, main delivered keyup to the newly focused element. With this patch, keyup was sent back to the original iframe instead.
  • When Escape caused the parent to remove the iframe, main completed successfully. With this patch, the subsequent keyup targeted the destroyed child session and returned cdp_failed / input_outcome_unknown with "Session with given id not found." I used hold_ms: 200 to make the teardown timing reproducible. The close action had already happened, so this turns a successful interaction into a reported failure.

The new unit tests verify which session receives the calls, but do not establish that the original browser interaction fails or that the change fixes it. They also keep the child session and focus stable throughout the key sequence, which misses these cases.

Could you provide a minimal test page, browser/OS version, and steps that fail on main and pass with this change? If the failure depends on background tabs or a particular browser version, please include that setup. My checks do not rule out an environment-specific issue, but the current evidence does not support changing the routing globally.

Once we have that reproduction, we can assess a targeted fix and add browser regressions for focus leaving an iframe and an iframe being removed during a key press. For now, I don't think this version is ready to merge.

This branch has not been deployed

No deployments
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.

bsk fill / bsk press ignore the OOPIF session and type into the parent frame

2 participants