ReviewOS

stacks/very-happy-dom

isChecked and getByRole({checked}) ignore aria-checked, so custom checkboxes always read unchecked

#1616
Closed glennmichael123 opened this 8 days ago ·
8 days ago

Problem

Checked state is read only from the DOM checked property, so an element that carries it through ARIA reports unchecked.

document.body.innerHTML = `
  <div role="checkbox" aria-checked="true" id="c"></div>
  <div role="switch"   aria-checked="true" id="s"></div>
  <div role="radio"    aria-checked="true" id="r"></div>`

await page.locator('#c').isChecked()   // false   expected true
await page.locator('#s').isChecked()   // false   expected true
await page.locator('#r').isChecked()   // false   expected true

await page.getByRole('checkbox', { checked: true }).count()   // 0   expected 1

Three places read it the same way:

  • Locator.isChecked() — this._one().checked === true
  • BrowserPage.isChecked(selector) — the same
  • byRole's checked option — (element.checked === true) !== options.checked

and toBeChecked() (#1603) is built on the first, so it inherits it.

Why it matters

This is the shape of every design system's checkbox. A native <input type=checkbox> cannot be styled far enough for most component libraries, so they render a div, span or button with role="checkbox" and drive aria-checked — which is exactly the contract that makes it a checkbox to a screen reader, and the contract Playwright reads.

The failure is quiet in the direction that matters. toBeChecked() on a custom checkbox that is checked fails, which is loud and fixable. But not.toBeChecked() passes unconditionally, and so does getByRole('checkbox', { checked: false }) — so a test asserting "the box starts unticked" passes whether or not it does, and keeps passing after the bug that leaves it ticked.

check() and uncheck() have the same root: they decide whether to click by reading element.checked, so on a custom checkbox they always click, meaning check() on an already-checked control toggles it off. That is the opposite of what check() promises.

Proposed scope

One definition of "checked", used by all of them:

checked(element):
  if 'checked' in element              -> element.checked === true
  aria = element.getAttribute('aria-checked')
  if aria === 'true'                   -> true
  if aria === 'false'                  -> false
  if aria === 'mixed'                  -> 'mixed'
  otherwise                            -> undefined   (not a checkable thing)
  • Only for elements whose role is checkbox, radio, switch, menuitemcheckbox or menuitemradio — computeRole() already answers that, and src/aria/roles.ts is where this belongs so byRole and the state readers cannot diverge.
  • The native property wins where both exist, since that is the live state.
  • aria-checked="mixed" is a third value. isChecked() returns a boolean, so mixed has to be false there; toBeChecked({ checked: 'mixed' }) is how Playwright exposes it and is the honest place for it.
  • check() / uncheck() must decide from the shared predicate, otherwise they keep toggling custom controls the wrong way.
  • An element that is not checkable at all should stay false for isChecked(), not become undefined, so existing callers are unaffected.

While here: getByRole's selected and expanded options read element.selected and aria-expanded respectively, so selected has the same gap for aria-selected on a role="option" or role="tab".

Acceptance

  • The reproduction above reports true three times and a count of 1.
  • aria-checked="false" reports false, and a missing attribute on a role="checkbox" also reports false.
  • not.toBeChecked() fails against aria-checked="true" — the assertion that used to pass unconditionally.
  • check() on a custom checkbox already reporting checked does not click it, asserted with a click listener.
  • A native <input type=checkbox checked> is unchanged.
  • aria-selected on role="option" is matched by getByRole('option', { selected: true }).

Found while sweeping the remaining Playwright gaps after #1607.

Sign in to comment on this issue.