Skip to content

fix(locator): restrict loose field branches to field-like elements - #5703

Open
DavertMik wants to merge 6 commits into
4.xfrom
fix/field-locator-loose-branches
Open

fix(locator): restrict loose field branches to field-like elements#5703
DavertMik wants to merge 6 commits into
4.xfrom
fix/field-locator-loose-branches

Conversation

@DavertMik

@DavertMik DavertMik commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Problem

Locator.field.labelContains ended with three branches that had no tag restriction at all:

`.//*[@aria-label = ${literal}]`,
`.//*[@title = ${literal}]`,
`.//*[@aria-labelledby][@aria-labelledby = //*[@id][normalize-space(string(.)) = ${literal}]/@id]`,

.//* matches any element, so a wrapper carrying the accessible name was returned by findFields
alongside the control it wraps.

The mechanism is worth spelling out, because it explains why "just reorder the branches" would not
have worked: xpathLocator.combine joins the branches with |, and an XPath union is evaluated in
document order
, not branch order. A labelled wrapper therefore always precedes the field inside it,
and selectElement takes els[0].

Two measured consequences:

  • Base UI Slider. Slider.Root renders <div role="group" aria-labelledby="…-label"> and the
    real <input type="range"> inside carries the same aria-labelledby. findFields returned
    [div, input], so fillField / clearField failed with
    Element is not an <input>, <textarea>, <select> or [contenteditable].
  • Tabs. I.selectOption('Settings tabs', 'Password') matched the role="tablist" container via
    @aria-label and failed with Element is not a <select> element instead of a clean not-found.

In both cases the real field was in the result set — it just lost the race to a container.

Change

lib/locator.js — the three loose branches now require a field tag or a form-control ARIA role:

self::input or self::textarea or self::select
or @role = 'textbox' or @role = 'searchbox' or @role = 'combobox'
or @role = 'spinbutton' or @role = 'slider' or @role = 'listbox'
or @role = 'checkbox' or @role = 'radio' or @role = 'switch'
or @contenteditable = 'true'

Why the allowlist and not "prefer innermost"

The plan offered two options. The allowlist (a) removes the container from the node-set outright, so
document order stops mattering. Option (b) — drop any match that contains another match — would have
to fight the union's document ordering and would change the result order of every currently-passing
findFields call, including the custom combobox/listbox cases below. The allowlist changes only which
element types may match, which is the property actually being asserted.

Two entries in that list are load-bearing for existing tests, not speculative:

  • listboxcustom_select.php has a standalone div[role=listbox][aria-labelledby]
    ("Favorite Color") that selectOption must resolve, plus a combobox + listbox pair sharing one
    aria-labelledby where both matches are relied on.
  • checkboxrole_elements.php has span[role=checkbox][aria-label] widgets that the WebDriver
    helper reaches through findFields (proceedSeeCheckbox). A first revision of this PR omitted the
    checkable roles and turned that test red in CI; radio and switch are included with it so the
    ARIA form-control family stays complete. option is deliberately not included — no fixture
    carries an accessible name on a role=option, and options are reached through their listbox.

labelEquals and byText were left alone — they have no loose branch to restrict.
Locator.clickable.wide has the same three lines and is deliberately untouched: clicking an element
by its accessible name is intended behaviour there, and the tablist regression in clickable was
already handled separately by clickable.self's narrowest-match guard.

Regression audit

Every aria-label / aria-labelledby / title in test/data/ was checked: each one reached by a
field action sits on an input, a role=combobox|textbox|listbox widget or a role=checkbox widget;
the remaining ones are on <a> / <svg> elements only reachable by click. The role-locator and
"aria selectors without role locators" suites pass unchanged — they are the guard that the allowlist
is not too tight.

Tests

  • test/unit/locator_test.js — new Locator.field.labelContains block pinning the XPath: the slider
    wrapper is dropped and the input resolved; ul[role=tablist][aria-label] and
    div[role=group][title] match nothing; div[role=textbox][aria-label],
    span[role=checkbox][aria-label] and input[aria-label] still match; a combobox + listbox sharing
    one aria-labelledby still yields both.
  • test/data/app/view/form/field_containers.php — new fixture: a Base-UI-shaped
    div[role=group][aria-labelledby] wrapping input[type=range][aria-labelledby], a
    div[role=textbox][aria-labelledby], and a ul[role=tablist][aria-label].
  • test/helper/webapi.jsseeInField('Volume', '30') / dontSeeInField('Volume', '70') prove the
    input wins over the wrapper; seeInField('Nickname', 'Bob') proves a custom widget is still
    reachable; selectOption('Settings tabs', …) asserts a clean not-found rather than … <select> ….

Read-only assertions were used for the slider on purpose — Playwright's fill() handles
input[type=range] but Puppeteer and WebDriver send keystrokes, which do not set a range value.
grabValueFrom is not used either: the WebDriver helper resolves it through _locate, not
findFields, so it has never accepted a fuzzy field name.

Local runs (serially, own fixture server): WebDriver 390 passing / 0 failing, Playwright
435 passing / 3 failing (all three are pre-existing external-network tests — a seeTraffic
assertion against codecept.io and two third-party makeApiRequest calls), unit 777 passing,
lint clean.

Not fixed here — follow-up

Puppeteer's findFields has a second loose path this PR does not touch: after the XPath
strategies it falls back to page.$$('::-p-aria(<name>)'), which resolves
ul[role=tablist][aria-label="Settings tabs"] by accessible name and still produces
Element is not <select>. Restricting that means changing helper code, which is outside this change's
scope, so the tablist integration test carries an isHelper('Puppeteer') skip with the reason stated
inline, and the gap is left for a follow-up. The unit test still pins the locator-level behaviour for
all helpers, and the slider fix works on Puppeteer today (verified locally).

Overlap with #5701

PR #5701 (fix/fill-field-combobox, open, targets 4.x) edits the same functions: it adds
not(./@aria-hidden = 'true') to the tag-restricted branches of labelEquals, labelContains,
byName and byText. That work is complementary — it filters Base UI's hidden mirror inputs, this one
restricts which element types the loose branches may match — but the edits sit on adjacent lines of
labelContains, so a textual conflict is expected when the second of the two merges. Both changes
must be kept
; nothing here reverts or reflows those aria-hidden guards.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TcwzSXPnfaig8nBZD2Vxfi

DavertMik and others added 2 commits September 8, 2026 20:46
`Locator.field.labelContains` ended with three branches matching `.//*`
with no tag restriction, so any container carrying `@aria-label`,
`@title` or `@aria-labelledby` was returned by `findFields`.

`xpathLocator.combine` joins branches with `|`, and a union is evaluated
in document order, so a labelled wrapper always preceded the control it
wraps. A Base UI slider (`div[role=group][aria-labelledby]` around
`input[type=range]` with the same `aria-labelledby`) resolved to the
`div`, and `selectOption` on a `ul[role=tablist][aria-label]` reported
"Element is not a <select> element" instead of a clean not-found.

Scope the three branches to a field tag or an editable ARIA role, which
keeps every custom `role=combobox` / `role=textbox` / `role=listbox`
widget reachable by accessible name while dropping plain containers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcwzSXPnfaig8nBZD2Vxfi
`span[role=checkbox][aria-label]` widgets are resolved through
`findFields` by the WebDriver helper, so the allowlist must cover the
checkable ARIA form controls too.

Also drop `grabValueFrom` from the new wrapper test: the WebDriver helper
resolves that action through `_locate`, not `findFields`, so it never
accepted a fuzzy field name. `seeInField` / `dontSeeInField` assert the
same thing across all three helpers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcwzSXPnfaig8nBZD2Vxfi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant