diff --git a/CHANGELOG.md b/CHANGELOG.md index 73e29559b..cde4540e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,17 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A half-ticked box is no longer described to a Bot as ticked + +The snapshot a Bot reads before it acts on a page says whether each box is ticked, and Playwright +writes that as `[checked]` for one that is and `[checked=mixed]` for one that is neither — which is +what the "select all" above a partly-ticked list carries. The parser treated any value other than +the string `false` as ticked, and `mixed` is one, so a half-ticked box was reported as done. A Bot +asked to select everything read it as already selected, clicked nothing, and said the rows were +chosen when most of them were not. `mixed` is now reported as not ticked, which is both the true +half of a yes-or-no answer and the one that gets the right action: clicking a half-ticked box ticks +it. An ordinary tick and an ordinary empty box are unchanged. + ### A Bot cannot end its turn by asking a person nothing `ask_person` is how a Bot stops and puts something to a person instead of guessing, and a call with diff --git a/agent-computer/src/aria-snapshot.ts b/agent-computer/src/aria-snapshot.ts index 9086edd73..3c2856c7d 100644 --- a/agent-computer/src/aria-snapshot.ts +++ b/agent-computer/src/aria-snapshot.ts @@ -184,8 +184,15 @@ function toElement( // Playwright emits `[checked]` only when something is checked, so absence is ambiguous on its own: a // Bot cannot tell an unticked box from a control that does not tick. Reported as false for the roles // that can be checked, and left off entirely for the ones that cannot. + // + // `[checked=mixed]` is the third state and the one spelling Playwright gives the flag a value for: + // `aria-checked="mixed"`, which is what the box above a partly-ticked list carries. It is not + // ticked, and a Bot told it was left the rows underneath unselected and reported the job done. The + // contract this fills in is `checked?: boolean`, so "neither" has to be said as false — which is + // also the answer that gets the right action, since clicking a mixed box ticks it. if (descriptor.flags.has("checked")) { - element.checked = descriptor.flags.get("checked") !== "false"; + const state = descriptor.flags.get("checked"); + element.checked = state !== "mixed" && state !== "false"; } else if (CHECKABLE_ROLES.has(descriptor.role)) { element.checked = false; } diff --git a/agent-computer/tests/aria-snapshot.test.ts b/agent-computer/tests/aria-snapshot.test.ts index d9b77fdea..50dd87043 100644 --- a/agent-computer/tests/aria-snapshot.test.ts +++ b/agent-computer/tests/aria-snapshot.test.ts @@ -105,6 +105,51 @@ describe("parseAriaSnapshot, against captured output", () => { expect(byName.get("Submit order")).not.toHaveProperty("checked"); }); + /** + * A half-ticked "select all", captured the same way the fixture above was. + * + * `ariaSnapshot({ mode: "ai" })` against a fieldset of three boxes in Chromium 151, with the first + * one's `indeterminate` set — which is what a box above a partly-ticked list carries. Playwright + * writes that as `[checked=mixed]`: a value, where an ordinary tick is the bare `[checked]`. It is + * the only flag in this output that carries one, which is the whole of the bug. + * + * Captured rather than written, for the reason the note at the top of this file gives. Guessed at, + * the entry loses the `generic` wrapper each box sits inside and the `text` node beside it, and the + * guess would pass while the shipped parser walked something else. + */ + const MIXED = `- group "Toppings" [ref=e2]: + - generic [ref=e4]: + - checkbox "Select all" [checked=mixed] [ref=e5] + - text: Select all + - generic [ref=e6]: + - checkbox "Bacon" [checked] [ref=e7] + - text: Bacon + - generic [ref=e8]: + - checkbox "Extra Cheese" [ref=e9] + - text: Extra Cheese`; + + test("the mixed fixture is genuinely valid YAML", () => { + expect(() => Bun.YAML.parse(MIXED)).not.toThrow(); + }); + + test("a half-ticked box is not reported as ticked", () => { + const byName = new Map( + parseAriaSnapshot(MIXED).elements.map((e) => [e.name, e]), + ); + // Not checked, so a Bot asked to tick it clicks it. Told it was already checked, it left the + // rows underneath unselected and said they were done. + expect(byName.get("Select all")?.checked).toBe(false); + // And the two beside it are unchanged, so this is not a swap. + expect(byName.get("Bacon")?.checked).toBe(true); + expect(byName.get("Extra Cheese")?.checked).toBe(false); + }); + + test("a half-ticked box is still a control a Bot can act on", () => { + const [first] = parseAriaSnapshot(MIXED).elements; + // The ref is what a click needs, and it sits after the flag Playwright gave a value to. + expect(first).toMatchObject({ ref: "e5", role: "checkbox" }); + }); + test("empty and unparseable input produce no elements rather than throwing", () => { expect(parseAriaSnapshot("").elements).toEqual([]); expect(parseAriaSnapshot("\t- [[[ not yaml").elements).toEqual([]);