Skip to content

Commit 35d6357

Browse files
DavertMikclaude
andcommitted
refactor(locator): find ARIA checkables in Locator, not in each helper
Locator.checkable already knows how to find a control by its label. It just hard-coded the tag. Headless component libraries express the same semantics with a role, so generalise the tag test to a tag-or-role test and skip aria-hidden elements. self::input[@type='checkbox' or @type='radio'] or @ROLE='checkbox' or @ROLE='radio' or @ROLE='switch' This replaces the three per-helper implementations added in #5704 — a getByRole loop in Playwright, a role-scoped ::-p-aria loop in Puppeteer, and a hoisted aria/ lookup plus keepCheckable filter in WebDriver — with one XPath predicate that all three helpers inherit, since they all call Locator.checkable.byText. Puppeteer's and WebDriver's ARIA fallbacks return to their original position after the XPath. An XPath predicate is role-scoped by construction, so there is no precision trade-off to manage: a heading sharing the label text cannot match, and the per-helper filtering that guarded against it is no longer needed. The aria-hidden guard is what skips the hidden mirror input that libraries render and point <label for> at, so the visible control is resolved instead. Behaviour is unchanged for native controls: on existing fixtures the generated XPath returns an identical node set. Verified against real radix-ui@1.6.7 and @base-ui/react@1.8.0 components: checkOption and seeCheckboxIsChecked work for Checkbox, Switch, Radio Group and Checkbox Group on both libraries, standalone and inside a form. Playwright 53 passing, Puppeteer 49 passing, WebDriver 41 passing (3 pending), unit 815 + 81 locator, lint clean. Net -25 lines of library code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcwzSXPnfaig8nBZD2Vxfi
1 parent cfc9545 commit 35d6357

5 files changed

Lines changed: 67 additions & 49 deletions

File tree

lib/helper/Playwright.js

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ let defaultSelectorEnginesInitialized = false
5050
const popupStore = new Popup()
5151
const consoleLogStore = new Console()
5252
const availableBrowsers = ['chromium', 'webkit', 'firefox', 'electron']
53-
const checkableRoles = ['checkbox', 'radio', 'switch']
5453

5554
import { setRestartStrategy, restartsSession, restartsContext, restartsBrowser } from './extras/PlaywrightRestartOpts.js'
5655
import { createValueEngine, createDisabledEngine } from './extras/PlaywrightPropEngine.js'
@@ -4389,17 +4388,6 @@ async function findCheckable(locator, context) {
43894388
return findElements.call(this, contextEl, matchedLocator)
43904389
}
43914390

4392-
for (const exact of [true, false]) {
4393-
for (const role of checkableRoles) {
4394-
try {
4395-
const roleEls = await contextEl.getByRole(role, { name: matchedLocator.value, exact }).all()
4396-
if (roleEls.length) return roleEls
4397-
} catch (err) {
4398-
// getByRole not supported or failed
4399-
}
4400-
}
4401-
}
4402-
44034391
const literal = xpathLocator.literal(matchedLocator.value)
44044392
let els = await findElements.call(this, contextEl, Locator.checkable.byText(literal))
44054393
if (els.length) {

lib/helper/Puppeteer.js

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ function wrapError(e) {
6464
let perfTiming
6565
const popupStore = new Popup()
6666
const consoleLogStore = new Console()
67-
const checkableRoles = ['checkbox', 'radio', 'switch']
6867

6968
/**
7069
* ## Configuration
@@ -3196,19 +3195,8 @@ async function findCheckable(locator, context) {
31963195
return findElements.call(this, contextEl, matchedLocator)
31973196
}
31983197

3199-
// Try ARIA selector for accessible name
3200-
let els
3201-
for (const role of checkableRoles) {
3202-
try {
3203-
els = await contextEl.$$(`::-p-aria([name="${matchedLocator.value}"][role="${role}"])`)
3204-
if (els.length) return els
3205-
} catch (err) {
3206-
// ARIA selector not supported or failed
3207-
}
3208-
}
3209-
32103198
const literal = xpathLocator.literal(matchedLocator.value)
3211-
els = await findElements.call(this, contextEl, Locator.checkable.byText(literal))
3199+
let els = await findElements.call(this, contextEl, Locator.checkable.byText(literal))
32123200
if (els.length) {
32133201
return els
32143202
}
@@ -3217,6 +3205,14 @@ async function findCheckable(locator, context) {
32173205
return els
32183206
}
32193207

3208+
// Try ARIA selector for accessible name
3209+
try {
3210+
els = await contextEl.$$(`::-p-aria(${matchedLocator.value})`)
3211+
if (els.length) return els
3212+
} catch (err) {
3213+
// ARIA selector not supported or failed
3214+
}
3215+
32203216
return findElements.call(this, contextEl, matchedLocator.value)
32213217
}
32223218

lib/helper/WebDriver.js

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3248,38 +3248,24 @@ async function findCheckable(locator, locateFn) {
32483248
if (locator.isRole()) return locateFn(locator, true)
32493249
if (!locator.isFuzzy()) return locateFn(locator, true)
32503250

3251-
// Try ARIA selector for accessible name
3252-
try {
3253-
els = await keepCheckable.call(this, await locateFn(`aria/${locator.value}`))
3254-
if (els.length) return els
3255-
} catch (e) {
3256-
// ARIA selector not supported or failed
3257-
}
3258-
32593251
const literal = xpathLocator.literal(locator.value)
32603252
els = await locateFn(Locator.checkable.byText(literal))
32613253
if (els.length) return els
32623254

32633255
els = await locateFn(Locator.checkable.byName(literal))
32643256
if (els.length) return els
32653257

3258+
// Try ARIA selector for accessible name
3259+
try {
3260+
els = await locateFn(`aria/${locator.value}`)
3261+
if (els.length) return els
3262+
} catch (e) {
3263+
// ARIA selector not supported or failed
3264+
}
3265+
32663266
return await locateFn(locator.value) // by css or xpath
32673267
}
32683268

3269-
async function keepCheckable(els) {
3270-
if (!els || !els.length) return []
3271-
3272-
const checkable = await this.browser.execute(function () {
3273-
return Array.prototype.slice.call(arguments).map(function (el) {
3274-
if (!el) return false
3275-
const role = el.getAttribute('role')
3276-
if (role) return ['checkbox', 'radio', 'switch'].indexOf(role) > -1
3277-
return el.tagName === 'INPUT' && (el.type === 'checkbox' || el.type === 'radio')
3278-
})
3279-
}, ...els)
3280-
3281-
return els.filter((el, index) => checkable[index])
3282-
}
32833269

32843270
function withStrictLocator(locator) {
32853271
locator = new Locator(locator)

lib/locator.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -649,15 +649,20 @@ Locator.field = {
649649
]),
650650
}
651651

652+
const checkable = `self::input[@type = 'checkbox' or @type = 'radio'] or @role = 'checkbox' or @role = 'radio' or @role = 'switch'`
653+
const visibleCheckable = `.//*[${checkable}][not(@aria-hidden = 'true')]`
654+
652655
Locator.checkable = {
653656
/**
654657
* @param {string} literal
655658
* @returns {string}
656659
*/
657660
byText: literal =>
658661
xpathLocator.combine([
659-
`.//input[@type = 'checkbox' or @type = 'radio'][(@id = //label[@for][contains(normalize-space(string(.)), ${literal})]/@for) or @placeholder = ${literal}]`,
660-
`.//label[contains(normalize-space(string(.)), ${literal})]//input[@type = 'radio' or @type = 'checkbox']`,
662+
`${visibleCheckable}[(@id = //label[@for][contains(normalize-space(string(.)), ${literal})]/@for) or @placeholder = ${literal}]`,
663+
`.//label[contains(normalize-space(string(.)), ${literal})]//*[${checkable}][not(@aria-hidden = 'true')]`,
664+
`${visibleCheckable}[@aria-labelledby = //*[@id][contains(normalize-space(string(.)), ${literal})]/@id]`,
665+
`${visibleCheckable}[@aria-label = ${literal}]`,
661666
]),
662667

663668
/**

test/unit/locator_test.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,4 +808,47 @@ describe('Locator', () => {
808808
expect(items[0].getAttribute('id')).to.eql('rename')
809809
})
810810
})
811+
812+
describe('Locator.checkable.byText', () => {
813+
const select = (xml, literal) => {
814+
const doc = new DOMParser().parseFromString(xml, 'text/xml')
815+
return xpath.select(Locator.checkable.byText(literal), xpath.select1('//root', doc))
816+
}
817+
818+
it('matches a native input labelled by label[for]', () => {
819+
const nodes = select('<root><input type="checkbox" id="a"/><label for="a">I Agree</label></root>', "'I Agree'")
820+
expect(nodes).to.have.length(1)
821+
expect(nodes[0].getAttribute('id')).to.eql('a')
822+
})
823+
824+
it('matches a role=checkbox labelled by label[for]', () => {
825+
const nodes = select('<root><button role="checkbox" id="a"></button><label for="a">Accept terms</label></root>', "'Accept terms'")
826+
expect(nodes).to.have.length(1)
827+
expect(nodes[0].tagName).to.eql('button')
828+
})
829+
830+
it('matches a role=switch labelled by aria-labelledby', () => {
831+
const nodes = select('<root><span role="switch" aria-labelledby="l"></span><label id="l">Airplane mode</label></root>', "'Airplane mode'")
832+
expect(nodes).to.have.length(1)
833+
expect(nodes[0].getAttribute('role')).to.eql('switch')
834+
})
835+
836+
it('matches a role=radio named by aria-label', () => {
837+
const nodes = select('<root><span role="radio" aria-label="Compact"></span></root>', "'Compact'")
838+
expect(nodes).to.have.length(1)
839+
expect(nodes[0].getAttribute('role')).to.eql('radio')
840+
})
841+
842+
it('resolves the visible control, not the aria-hidden input the label points at', () => {
843+
const xml =
844+
'<root>' +
845+
'<span role="checkbox" aria-labelledby="l" id="visible"></span>' +
846+
'<input type="checkbox" id="mirror" aria-hidden="true"/>' +
847+
'<label for="mirror" id="l">Accept terms</label>' +
848+
'</root>'
849+
const nodes = select(xml, "'Accept terms'")
850+
expect(nodes).to.have.length(1)
851+
expect(nodes[0].getAttribute('id')).to.eql('visible')
852+
})
853+
})
811854
})

0 commit comments

Comments
 (0)