Read a half-ticked box as not ticked, not as ticked - #475
Open
kevin9327 wants to merge 2 commits into
Open
Conversation
Playwright renders `aria-checked="mixed"` as `[checked=mixed]` and an ordinary tick as the bare `[checked]` -- the one flag it gives a value to. `toElement` treated any value but the string "false" as checked, so the box above a partly-ticked list arrived in the snapshot as `checked: true`. A Bot asked to select everything therefore read the "select all" as already done, clicked nothing, and reported rows as chosen that were not. The contract is `checked?: boolean`, so mixed is reported as false: the true half of a yes-or-no answer, and the one that produces the right action, since clicking a half-ticked box ticks it.
kevin9327
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso and
tylerslaton
as code owners
September 9, 2026 23:05
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The note at the top of this file says why: an invented entry loses the
`generic` wrapper each box sits inside and the `text` node beside it, so a
guess can pass while the shipped parser walks something else. Taken from
`ariaSnapshot({ mode: "ai" })` against a fieldset of three boxes in
Chromium 151, the first with `indeterminate` set.
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.
Ask a Bot to tick every row on a page whose "select all" box is half-ticked — some rows already
selected, some not — and it clicks nothing and tells the person the rows are selected. They are not.
The same happens on any tri-state checkbox: a consent tree, a permissions matrix, a partly expanded
filter list.
Why
The snapshot a Bot reads before it acts carries
checkedper control. Here is whatariaSnapshot({ mode: "ai" })actually produces, captured against Chromium 151 (playwright-core1.62.1's own build) for a fieldset of three boxes with the first one's
indeterminateset:An ordinary tick is the bare
[checked]; the third state is[checked=mixed]— the only flag inthat output carrying a value. Playwright's renderer says the same thing in two lines:
toElementread the flag like this:"false"is a spelling Playwright does not emit."mixed", the one it does, is not"false", so ahalf-ticked box arrived as
checked: trueand the Bot went on from a snapshot that said the job wasdone.
The fix
The defensive
"false"branch stays, since removing it would be a second change in one line.Why
falseand not "leave it off": the published contract ischecked?: boolean(
server/src/computer/schema.ts), and for a checkable role this file always fills it in on purpose —its own comment says absence would otherwise be ambiguous between an unticked box and a control that
does not tick.
falseis the true half of a yes-or-no answer about a box that is not ticked, and itis also the answer that produces the right action, because clicking a half-ticked box ticks it.
Measured
bun test agent-computer/tests/aria-snapshot.test.tsorigin/main(1c7bd92) with only the three new tests applied: 26 pass, 1 fail.The fixture is the captured output above rather than a hand-written entry, for the reason the note at
the top of that file gives: a guess loses the
genericwrapper each box sits inside and thetextnode beside it, and would pass while the shipped parser walked something else.
The other two new tests are the guard against over-correcting and pass before and after: the fixture
is valid YAML, and a half-ticked box is still a control with a usable
ref— its flag carries a valueand
refcomes after it, which is the parse that would break if this were done with a pattern. Theexisting case, an ordinary
[checked]and an ordinary empty box, is asserted in the same test and isunchanged.
bun test agent-computer/tests— 234 pass, 15 skip, 20 fail. The same 20 fail onorigin/main(231 pass there, so the three new ones are the whole difference): 11 in
shell.test.ts, which spawns/bin/bash, and 9 inworkspace.test.ts, which creates symlinks. Neither is available on the Windowsmachine these were run on and neither is touched here.
Also run, clean:
bunx biome checkon both changed files, andbun run typecheckinagent-computer.Note on the changelog
A Bot behaves differently afterwards, so there is an entry. It goes at the top of
## Unreleased,which is the line every open PR touching the changelog inserts at, so it will conflict with any other
that lands first. Happy to rebase.