ReviewOS

stacks/very-happy-dom

An element collapsed to height:0 is treated as visible, so getByRole matches inside a closed drawer

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

Problem

An element collapsed to nothing is treated as visible, so getByRole matches inside it and isVisible() agrees.

document.head.innerHTML = '<style>.collapsed { height: 0; overflow: hidden }</style>'
document.body.innerHTML = '<div class="collapsed"><button>Hidden action</button></div>'

await page.getByRole('button').count()      // 1     expected 0
await page.getByRole('button').isVisible()  // true  expected false

// and, since #1604, the action goes through:
await page.getByRole('button').click()      // clicks a button nobody can reach

isRendered() in src/aria/visibility.ts checks display, visibility and the hidden attribute up the ancestor chain, and nothing about size. Playwright's definition is "non-empty bounding box and not visibility: hidden", so the box is half of it.

Why it matters

height: 0; overflow: hidden is how a collapsed accordion, a closed disclosure, an inactive tab panel and a slide-out drawer are built when the author wants a transition — display: none cannot be animated, so anything that animates open uses this instead. All of that markup is reachable today.

This is the #1601 failure in a different mechanism. Two matches make strict mode throw, and .first() — the fix that suggests itself — binds to the collapsed copy because it comes first in document order. Where only the collapsed copy exists, the query resolves and the assertion passes against markup no user can reach. Since #1604 the action auto-waits and then succeeds, so a click lands on a button inside a closed drawer and the test reports success.

Proposed scope

The naive fix is wrong and would be much worse than the bug. There is no layout pass, so an element nobody sized reports 0 × 0 — a plain <button>Save</button> with no CSS has an empty box, asserted in test/box-metrics-cascade.test.ts ("an element nobody sized still reports zero"). Requiring a non-empty box would make almost every unstyled element invisible and empty out every getBy* query in the suite.

So 0 here means "unknown", not "collapsed", and the two have to be told apart. They can be: a collapse is an explicit declaration, and resolveProperty() against the cascade can say whether one exists.

  • Treat an element as not rendered when a width or height is declared and resolves to zero — inline, or from a stylesheet rule, which #1600 already routes through collectCascade.
  • Walk ancestors for the same, since the collapse is normally on the container rather than the control.
  • An element with no declaration is left exactly as it is today: unknown size, assumed rendered.
  • overflow: hidden need not be required. A height: 0 container clips its content in the cases that matter, and demanding both would miss max-height: 0, which is the other common spelling.
  • Keep the two predicates separate, as #1601 established: this belongs in isRendered, so isExposedToAria inherits it and page.isVisible follows.

Worth stating in the doc comment that this is a narrower rule than Playwright's, and why — a reader who knows Playwright's definition will otherwise assume the bounding box is being consulted in general.

Acceptance

  • The reproduction above reports a count of 0 and isVisible() false.
  • height: 0 on an ancestor hides a descendant that declares no size of its own.
  • max-height: 0 is treated the same.
  • An element with no size declaration is still visible — the guard against the naive fix, and the test that matters most here.
  • An explicit width: 0 with a non-zero height is hidden.
  • The existing suite is unchanged: no getBy* query anywhere starts returning fewer elements.
  • includeHidden: true still finds them.

Found while sweeping the remaining Playwright gaps after #1607.

Sign in to comment on this issue.