Skip to content

Commit 087c0de

Browse files
DavertMikclaude
andcommitted
perf(locator): guard aria-labelledby branch with attr-existence predicate
The wide clickable / labelContains field XPath includes: .//*[@aria-labelledby = //*[@id][normalize-space(string(.)) = X]/@id] That predicate forces every element to evaluate the inner //*[@id] subquery, which is O(N²) on any non-trivial document for pure-JS XPath engines (xpath npm: 7641ms on a 2k-line page; fontoxpath: 7057ms on the same branch). Browser engines optimize via join-pushdown. Adding [@aria-labelledby] as a left-to-right filter predicate first cuts the slow comparison to only elements that actually have the attribute: .//*[@aria-labelledby][@aria-labelledby = //*[@id][...]/@id] 7641ms → 52ms (147×). Semantics identical: in XPath, [A][B] and [A and B] produce the same result-set, but predicates are evaluated left-to-right, so the cheap attr-existence check filters out the bulk first. This is a single-character XPath change — codeceptq goes from 9000ms → 325ms on test/data/github.html with no special-case code. Reverted the per-strategy reimplementation in lib/command/query.js (back to using Locator.clickable.wide / Locator.field.byText directly). Added two unit tests for the aria-labelledby branch in Locator.clickable.wide (positive + negative). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0d6d0f2 commit 087c0de

3 files changed

Lines changed: 54 additions & 108 deletions

File tree

lib/command/query.js

Lines changed: 25 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -14,42 +14,38 @@ export default async function query(locator, context, options = {}) {
1414
return
1515
}
1616

17-
const { doc, source } = htmlToDoc(html)
18-
19-
let xpathBranches
20-
let contextBranches = null
17+
let xpathExpr
18+
let contextExpr = null
2119
try {
22-
xpathBranches = buildXPaths(locator, options, doc)
23-
if (context) contextBranches = buildXPaths(context, {}, doc)
20+
xpathExpr = buildXPath(locator, options)
21+
if (context) contextExpr = buildXPath(context, {})
2422
} catch (err) {
2523
console.error(`codeceptq: cannot build XPath: ${err.message}`)
2624
process.exitCode = 2
2725
return
2826
}
2927

30-
const xpathExpr = xpathBranches.join(' | ')
31-
const contextExpr = contextBranches ? contextBranches.join(' | ') : null
28+
const { doc, source } = htmlToDoc(html)
3229

3330
let nodes
3431
try {
35-
const contexts = contextBranches ? collectNodes(contextBranches, doc) : [doc]
36-
const seen = new Set()
37-
nodes = []
38-
for (const ctx of contexts) {
39-
for (const branch of xpathBranches) {
40-
for (const m of toArray(xpath.select(branch, ctx))) {
32+
if (contextExpr) {
33+
const ctxNodes = toArray(xpath.select(contextExpr, doc))
34+
const seen = new Set()
35+
nodes = []
36+
for (const ctx of ctxNodes) {
37+
for (const m of toArray(xpath.select(xpathExpr, ctx))) {
4138
if (!seen.has(m)) {
4239
seen.add(m)
4340
nodes.push(m)
4441
}
4542
}
4643
}
44+
} else {
45+
nodes = toArray(xpath.select(xpathExpr, doc))
4746
}
48-
// XPath unions return nodes in document order. We evaluate branches
49-
// separately so re-sort by source position to match that contract.
50-
nodes.sort((a, b) => (a.__startOffset ?? 0) - (b.__startOffset ?? 0))
5147
} catch (err) {
52-
console.error(`codeceptq: XPath evaluation failed: ${err.message}`)
48+
console.error(`codeceptq: XPath evaluation failed for "${xpathExpr}": ${err.message}`)
5349
process.exitCode = 2
5450
return
5551
}
@@ -101,100 +97,23 @@ export default async function query(locator, context, options = {}) {
10197
if (nodes.length === 0) process.exitCode = 1
10298
}
10399

104-
// Returns the array of XPath branches for a given input + options.
105-
// The semantic locators (--click/--field/--checkable) bake doc-wide subqueries
106-
// (label[@for] resolution, id-by-visible-text) into literal values so the
107-
// evaluator sees flat predicates. Without this, xpath npm re-evaluates each
108-
// inner //path per outer node — O(N²) on any non-trivial document.
109-
function buildXPaths(input, options, doc) {
110-
const lit = xpathLocator.literal(input)
111-
112-
if (options.field) return fieldByText(input, doc)
113-
if (options.click || options.clickable) return clickableWide(input, doc)
114-
if (options.checkable) return checkableByText(input, doc)
100+
function buildXPath(input, options) {
101+
const literal = xpathLocator.literal(input)
102+
if (options.field) return Locator.field.byText(literal)
103+
if (options.click || options.clickable) return Locator.clickable.wide(literal)
104+
if (options.checkable) return Locator.checkable.byText(literal)
115105
if (options.select) {
116-
return [Locator.select.byVisibleText(lit).replace(/\.\/(option|optgroup)/g, './/$1')]
106+
return Locator.select.byVisibleText(literal).replace(/\.\/(option|optgroup)/g, './/$1')
117107
}
118108

119-
if (options.xpath) return [new Locator({ xpath: input }).toXPath()]
120-
if (options.css) return [new Locator({ css: input }).toXPath()]
109+
if (options.xpath) return new Locator({ xpath: input }).toXPath()
110+
if (options.css) return new Locator({ css: input }).toXPath()
121111

122112
const loc = new Locator(input)
123-
if (loc.type === 'fuzzy') return [...clickableWide(input, doc), ...fieldByText(input, doc)]
124-
return [loc.toXPath()]
125-
}
126-
127-
function clickableWide(text, doc) {
128-
const lit = xpathLocator.literal(text)
129-
const labelledIds = idsByVisibleText(doc, text)
130-
const ariaLabelledBy = labelledIds.length ? `.//*[${anyAttrEquals('@aria-labelledby', labelledIds)}]` : null
131-
132-
return [
133-
`.//a[./@href][((contains(normalize-space(string(.)), ${lit})) or .//img[contains(./@alt, ${lit})])]`,
134-
`.//input[./@type = 'submit' or ./@type = 'image' or ./@type = 'button'][contains(./@value, ${lit})]`,
135-
`.//input[./@type = 'image'][contains(./@alt, ${lit})]`,
136-
`.//button[contains(normalize-space(string(.)), ${lit})]`,
137-
`.//label[contains(normalize-space(string(.)), ${lit})]`,
138-
`.//input[./@type = 'submit' or ./@type = 'image' or ./@type = 'button'][./@name = ${lit}]`,
139-
`.//button[./@name = ${lit}]`,
140-
`.//*[@aria-label = ${lit}]`,
141-
`.//*[@title = ${lit}]`,
142-
ariaLabelledBy,
143-
`.//*[@role='button'][normalize-space(.)=${lit}]`,
144-
`.//*[@role='tab' or @role='link' or @role='menuitem' or @role='menuitemcheckbox' or @role='menuitemradio' or @role='option' or @role='treeitem'][contains(normalize-space(string(.)), ${lit})]`,
145-
].filter(Boolean)
146-
}
147-
148-
function fieldByText(text, doc) {
149-
const lit = xpathLocator.literal(text)
150-
const fieldGuard = `[self::input | self::textarea | self::select][not(./@type = 'submit' or ./@type = 'image' or ./@type = 'hidden')]`
151-
152-
const labelFors = labelForsByContainsText(doc, text)
153-
const idMatch = labelFors.length ? ` or ${anyAttrEquals('./@id', labelFors)}` : ''
154-
155-
return [
156-
`.//*${fieldGuard}[((./@name = ${lit}) or ./@placeholder = ${lit}${idMatch})]`,
157-
`.//label[contains(normalize-space(string(.)), ${lit})]//.//*${fieldGuard}`,
158-
]
159-
}
160-
161-
function checkableByText(text, doc) {
162-
const lit = xpathLocator.literal(text)
163-
const labelFors = labelForsByContainsText(doc, text)
164-
const idMatch = labelFors.length ? `${anyAttrEquals('@id', labelFors)} or ` : ''
165-
166-
return [
167-
`.//input[@type = 'checkbox' or @type = 'radio'][${idMatch}@placeholder = ${lit}]`,
168-
`.//label[contains(normalize-space(string(.)), ${lit})]//input[@type = 'radio' or @type = 'checkbox']`,
169-
]
170-
}
171-
172-
function idsByVisibleText(doc, text) {
173-
const lit = xpathLocator.literal(text)
174-
return toArray(xpath.select(`//*[@id][normalize-space(string(.)) = ${lit}]/@id`, doc)).map(a => a.value || '')
175-
}
176-
177-
function labelForsByContainsText(doc, text) {
178-
const lit = xpathLocator.literal(text)
179-
return toArray(xpath.select(`//label[@for][contains(normalize-space(string(.)), ${lit})]/@for`, doc)).map(a => a.value || '')
180-
}
181-
182-
function anyAttrEquals(lhs, values) {
183-
return values.map(v => `${lhs} = ${xpathLocator.literal(v)}`).join(' or ')
184-
}
185-
186-
function collectNodes(branches, ctx) {
187-
const seen = new Set()
188-
const out = []
189-
for (const expr of branches) {
190-
for (const n of toArray(xpath.select(expr, ctx))) {
191-
if (!seen.has(n)) {
192-
seen.add(n)
193-
out.push(n)
194-
}
195-
}
113+
if (loc.type === 'fuzzy') {
114+
return xpathLocator.combine([Locator.clickable.wide(literal), Locator.field.byText(literal)])
196115
}
197-
return out
116+
return loc.toXPath()
198117
}
199118

200119
function htmlToDoc(html) {

lib/locator.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ Locator.clickable = {
589589
`.//button[./@name = ${literal}]`,
590590
`.//*[@aria-label = ${literal}]`,
591591
`.//*[@title = ${literal}]`,
592-
`.//*[@aria-labelledby = //*[@id][normalize-space(string(.)) = ${literal}]/@id ]`,
592+
`.//*[@aria-labelledby][@aria-labelledby = //*[@id][normalize-space(string(.)) = ${literal}]/@id]`,
593593
`.//*[@role='button'][normalize-space(.)=${literal}]`,
594594
`.//*[@role='tab' or @role='link' or @role='menuitem' or @role='menuitemcheckbox' or @role='menuitemradio' or @role='option' or @role='treeitem'][contains(normalize-space(string(.)), ${literal})]`,
595595
]),
@@ -632,7 +632,7 @@ Locator.field = {
632632
`.//label[contains(normalize-space(string(.)), ${literal})]//.//*[self::input | self::textarea | self::select][not(./@type = 'submit' or ./@type = 'image' or ./@type = 'hidden')]`,
633633
`.//*[@aria-label = ${literal}]`,
634634
`.//*[@title = ${literal}]`,
635-
`.//*[@aria-labelledby = //*[@id][normalize-space(string(.)) = ${literal}]/@id ]`,
635+
`.//*[@aria-labelledby][@aria-labelledby = //*[@id][normalize-space(string(.)) = ${literal}]/@id]`,
636636
]),
637637

638638
/**

test/unit/locator_test.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -766,6 +766,33 @@ describe('Locator', () => {
766766
expect(tabMatches[0].getAttribute('data-tab')).to.eql('description')
767767
})
768768

769+
it('matches an element labelled via aria-labelledby pointing at text', () => {
770+
const xml = `<root>
771+
<button id="open" aria-labelledby="lbl"></button>
772+
<span id="lbl">Open dialog</span>
773+
<button>Other</button>
774+
</root>`
775+
const xdoc = new DOMParser().parseFromString(xml, 'text/xml')
776+
const root = xpath.select1('//root', xdoc)
777+
const xp = Locator.clickable.wide("'Open dialog'")
778+
const nodes = xpath.select(xp, root)
779+
const buttons = nodes.filter(n => n.tagName === 'button' && n.getAttribute('id') === 'open')
780+
expect(buttons).to.have.length(1, xp)
781+
})
782+
783+
it('does not match elements without aria-labelledby when no other branch hits', () => {
784+
const xml = `<root>
785+
<span id="lbl">Some text</span>
786+
<button>Unrelated</button>
787+
</root>`
788+
const xdoc = new DOMParser().parseFromString(xml, 'text/xml')
789+
const root = xpath.select1('//root', xdoc)
790+
const xp = Locator.clickable.wide("'Some text'")
791+
const nodes = xpath.select(xp, root)
792+
// No buttons or links match; only the <span> via .//label[contains...] won't fire either.
793+
expect(nodes.filter(n => n.tagName === 'button')).to.have.length(0, xp)
794+
})
795+
769796
it('matches role="menuitem" by text', () => {
770797
const menuXml = `<root>
771798
<ul role="menu">

0 commit comments

Comments
 (0)