Make Enter act while a person is driving the browser - #477
Open
kevin9327 wants to merge 1 commit into
Open
Conversation
`Input.dispatchKeyEvent` has two kinds of key-down. `rawKeyDown` is a key that produces no character: Chrome delivers it to the page and stops there, so a listener fires and nothing else happens. `keyDown` carries text, and that is what makes Chrome perform the key's own default action. Text was sent only for a printable character -- `event.key.length === 1` on the surface -- so Enter arrived as a `rawKeyDown`. Measured against Chromium 151 through this file, with the message the live screen actually builds: the form did not submit, a textarea took no new line, and a focused button was not pressed, while the page's own keydown listener saw every one of them. That is the sign-in at the end of almost every takeover. Enter now carries "\r" when the surface sent no text. One key rather than a list, measured the same way: Backspace, Delete, Tab, Home, End and the arrows all do what they mean as a rawKeyDown, and a single-line input still holds exactly what was typed. The fill-in is only on the way down, where a default action happens.
kevin9327
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso and
tylerslaton
as code owners
September 9, 2026 23:26
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A Bot hits a sign-in it cannot do itself and asks for help. A person takes the wheel, types their
email and password into the page — the characters appear — and presses Enter. Nothing happens. The
form does not submit. Nothing is refused, nothing is said, and the only way on is to find the button
and click it.
The same Enter does nothing in a text box (no new line) and nothing on a button somebody has tabbed
to. Every other key works, which is what makes it hard to believe: the typing lands, so the wheel
must be working.
Why
Input.dispatchKeyEventhas two kinds of key-down.rawKeyDownis a key that produces nocharacter: Chrome delivers the event to the page and stops there, so listeners fire and nothing else
happens.
keyDowncarries text, and that is what makes Chrome perform the key's own default action.screencast.tspicks between them by whether the surface sent text, and the surface sends text onlyfor a printable character:
"Enter".lengthis 5. So Enter went as arawKeyDown, forever.Measured, not reasoned
Driven through the shipped
startScreencastagainst Chromium 151 (playwright-core 1.62.1's ownbuild), with the message
live-screen.tsxactually constructs for a keydown —textpresent onlywhen
key.length === 1:The page's own
keydownlistener sawEnter/13in both runs. It was heard and not acted on.One key, and that was measured too
Every other editing key already does what it means as a
rawKeyDown, so filling text in for themwould be inventing an insertion. Same harness, same browser, caret and value read back after each
press in a textarea holding
hello world:Identical before and after the change.
The fix
A carriage return for Enter when the surface sent no text, in
screencast.tsrather than in thesurface, because this is a fact about the protocol and the file already holds the neighbouring one —
its
VIRTUAL_KEY_CODEScomment names Enter for the same reason.\rrather than\n: it is what a keyboard's Enter carries, and Chrome turns it into whatever thefield it lands in needs. The measurement above shows the single-line input left holding exactly what
was typed and no extra character.
Filled in only on the way down, where a default action happens. A key going up carries what it was
given and nothing more.
Tests
New:
agent-computer/tests/screencast.test.ts, 5 tests.screencast.tsimports Playwright for itstypes only and a type import is erased at run time, so these reach the translation with a fake CDP
session and no browser — they run anywhere the suite does.
origin/main(1c7bd92) with only the new file applied: 4 pass, 1 fail.The other four are the guard against over-correcting and pass before and after: a printable
character keeps its own text, Backspace / Delete / Tab / ArrowLeft / Home stay
rawKeyDownwith notext at all, Enter going up is a
keyUpcarrying nothing, and the virtual key code the surfaceoffered is still the one sent.
bun test agent-computer/tests— 236 pass, 15 skip, 20 fail. The same 20 fail onorigin/main(231 pass there): 11 in
shell.test.ts, which spawns/bin/bash, and 9 inworkspace.test.ts,which creates symlinks — neither is available on the Windows machine these were run on, and neither
is touched here.
Also run, clean:
bunx biome checkon both files,bun run typecheckinagent-computer.Note on the changelog
A person driving a browser behaves differently afterwards, so there is an entry. It goes at the top
of
## Unreleased, the line every open PR touching the changelog inserts at, so it will conflictwith any other that lands first. Happy to rebase.