-
-
Notifications
You must be signed in to change notification settings - Fork 15
Improve browser selector stability #123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -124,51 +124,112 @@ const accessibilityTreeScript = () => (function () { | |
| return (el.textContent?.trim().replace(/\s+/g, ' ') || '').slice(0, 100); | ||
| } | ||
|
|
||
| function getSelectorAccessibleName(el: HTMLElement, role: string | null): string { | ||
| const ariaLabel = el.getAttribute('aria-label'); | ||
| if (ariaLabel) return ariaLabel.trim(); | ||
|
|
||
| const labelledBy = el.getAttribute('aria-labelledby'); | ||
| if (labelledBy) { | ||
| const texts = labelledBy.split(/\s+/) | ||
| .map(id => document.getElementById(id)?.textContent?.trim() || '') | ||
| .filter(Boolean); | ||
| if (texts.length > 0) return texts.join(' ').slice(0, 100); | ||
| } | ||
|
|
||
| const tag = el.tagName.toLowerCase(); | ||
| if (tag === 'img' || (tag === 'input' && el.getAttribute('type') === 'image')) { | ||
| const alt = el.getAttribute('alt'); | ||
| if (alt !== null) return alt.trim(); | ||
| } | ||
|
|
||
| if (['input', 'select', 'textarea'].includes(tag)) { | ||
| const id = el.getAttribute('id'); | ||
| if (id) { | ||
| const label = document.querySelector(`label[for="${CSS.escape(id)}"]`); | ||
| if (label) return label.textContent?.trim() || ''; | ||
| } | ||
| const parentLabel = el.closest('label'); | ||
| if (parentLabel) { | ||
| const clone = parentLabel.cloneNode(true) as HTMLElement; | ||
| clone.querySelectorAll('input,select,textarea').forEach(n => n.remove()); | ||
| const lt = clone.textContent?.trim(); | ||
| if (lt) return lt; | ||
| } | ||
| return ''; | ||
| } | ||
|
|
||
| if (role && CONTAINER_ROLES.has(role)) return ''; | ||
| return (el.textContent?.trim().replace(/\s+/g, ' ') || '').slice(0, 100); | ||
| } | ||
|
|
||
| function isLikelyGeneratedId(id: string): boolean { | ||
| return /^[0-9a-f]{8,}$/i.test(id) || | ||
| /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(id) || | ||
| /(?:^|[-_:])\d{8,}(?:$|[-_:])/.test(id) || | ||
| /^(?:ember|react-select|radix|headlessui|mui|mantine|chakra|auto|generated)[-_:]?\d+/i.test(id); | ||
| } | ||
|
|
||
| function uniqueCssSelector(selector: string): string | null { | ||
| return document.querySelectorAll(selector).length === 1 ? selector : null; | ||
| } | ||
|
|
||
| function getSelector(element: HTMLElement): string { | ||
| const tag = element.tagName.toLowerCase(); | ||
|
|
||
| const text = element.textContent?.trim().replace(/\s+/g, ' '); | ||
| if (text && text.length > 0 && text.length <= 50) { | ||
| const sameTagElements = document.querySelectorAll(tag); | ||
| let matchCount = 0; | ||
| sameTagElements.forEach(el => { if (el.textContent?.includes(text)) matchCount++; }); | ||
| if (matchCount === 1) return `${tag}*=${text}`; | ||
| for (const attr of ['data-testid', 'data-test', 'data-qa']) { | ||
| const value = element.getAttribute(attr); | ||
| if (!value) continue; | ||
| const selector = uniqueCssSelector(`[${attr}="${CSS.escape(value)}"]`); | ||
| if (selector) return selector; | ||
| } | ||
|
|
||
| const ariaLabel = element.getAttribute('aria-label'); | ||
| if (ariaLabel && ariaLabel.length <= 80) return `aria/${ariaLabel}`; | ||
|
|
||
| const testId = element.getAttribute('data-testid'); | ||
| if (testId) { | ||
| const sel = `[data-testid="${CSS.escape(testId)}"]`; | ||
| if (document.querySelectorAll(sel).length === 1) return sel; | ||
| const role = getRole(element); | ||
| const accessibleName = getSelectorAccessibleName(element, role); | ||
| if (role && accessibleName && accessibleName.length <= 80) { | ||
| let matchCount = 0; | ||
| document.querySelectorAll('*').forEach(el => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: Each invocation of getSelector() for an element with a viable accessible name scans all DOM elements (line 190), calling getRole(), isVisible(), and getSelectorAccessibleName() on every one. On a 10K-element page with 100 elements hitting this branch, that's 1M iterations, each non-trivial. The interactable-elements script (line 202) correctly scopes this to interactableSelectors. At minimum, this should scope to elements that the walker actually assigns roles to — e.g. '[role], [aria-label], [aria-labelledby], a[href], button, input, select, textarea, h1-h6, img, nav, main, header, footer, aside, dialog, form, section'. Ideally, precompute role + accessibleName → count in a single pass during the tree walk and do O(1) lookups here instead. |
||
| const htmlEl = el as HTMLElement; | ||
| const candidateRole = getRole(htmlEl); | ||
| if (isVisible(htmlEl) && candidateRole && getSelectorAccessibleName(htmlEl, candidateRole) === accessibleName) matchCount++; | ||
| }); | ||
| if (matchCount === 1) return `aria/${accessibleName}`; | ||
| } | ||
|
Comment on lines
+188
to
196
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The interactable-elements script narrows the scan to |
||
|
|
||
| if (element.id) return `#${CSS.escape(element.id)}`; | ||
| if (element.id && !isLikelyGeneratedId(element.id)) return `#${CSS.escape(element.id)}`; | ||
|
|
||
| const nameAttr = element.getAttribute('name'); | ||
| if (nameAttr) { | ||
| const sel = `${tag}[name="${CSS.escape(nameAttr)}"]`; | ||
| if (document.querySelectorAll(sel).length === 1) return sel; | ||
| const selector = uniqueCssSelector(sel); | ||
| if (selector) return selector; | ||
| } | ||
|
|
||
| const placeholder = element.getAttribute('placeholder'); | ||
| if (placeholder) { | ||
| const sel = `${tag}[placeholder="${CSS.escape(placeholder)}"]`; | ||
| const selector = uniqueCssSelector(sel); | ||
| if (selector) return selector; | ||
| } | ||
|
|
||
| if (element.className && typeof element.className === 'string') { | ||
| const classes = element.className.trim().split(/\s+/).filter(Boolean); | ||
| for (const cls of classes) { | ||
| const sel = `${tag}.${CSS.escape(cls)}`; | ||
| if (document.querySelectorAll(sel).length === 1) return sel; | ||
| const selector = uniqueCssSelector(sel); | ||
| if (selector) return selector; | ||
| } | ||
| if (classes.length >= 2) { | ||
| const sel = `${tag}${classes.slice(0, 2).map(c => `.${CSS.escape(c)}`).join('')}`; | ||
| if (document.querySelectorAll(sel).length === 1) return sel; | ||
| const selector = uniqueCssSelector(sel); | ||
| if (selector) return selector; | ||
| } | ||
| } | ||
|
|
||
| let current: HTMLElement | null = element; | ||
| const path: string[] = []; | ||
| while (current && current !== document.documentElement) { | ||
| let seg = current.tagName.toLowerCase(); | ||
| if (current.id) { path.unshift(`#${CSS.escape(current.id)}`); break; } | ||
| if (current.id && !isLikelyGeneratedId(current.id)) { path.unshift(`#${CSS.escape(current.id)}`); break; } | ||
| const parent = current.parentElement; | ||
| if (parent) { | ||
| const siblings = Array.from(parent.children).filter(c => c.tagName === current!.tagName); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,60 +108,142 @@ const elementsScript = (includeBounds: boolean) => (function () { | |
| return (el.textContent?.trim().replace(/\s+/g, ' ') || '').slice(0, 100); | ||
| } | ||
|
|
||
| function getRole(el: HTMLElement): string | null { | ||
| const explicit = el.getAttribute('role'); | ||
| if (explicit) return explicit.split(' ')[0]; | ||
|
|
||
| const tag = el.tagName.toLowerCase(); | ||
| switch (tag) { | ||
| case 'button': return 'button'; | ||
| case 'a': return el.hasAttribute('href') ? 'link' : null; | ||
| case 'input': { | ||
| const type = (el.getAttribute('type') || 'text').toLowerCase(); | ||
| if (type === 'hidden') return null; | ||
| if (type === 'checkbox' || type === 'radio') return type; | ||
| if (type === 'range') return 'slider'; | ||
| if (type === 'search') return 'searchbox'; | ||
| if (type === 'number') return 'spinbutton'; | ||
| if (['submit', 'reset', 'button', 'image'].includes(type)) return 'button'; | ||
| return 'textbox'; | ||
| } | ||
| case 'select': return 'combobox'; | ||
| case 'textarea': return 'textbox'; | ||
| } | ||
|
|
||
| if ((el as HTMLElement & { contentEditable: string }).contentEditable === 'true') return 'textbox'; | ||
| return null; | ||
| } | ||
|
|
||
| function getSelectorAccessibleName(el: HTMLElement): string { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Same getAccessibleName / getSelectorAccessibleName duplication as the accessibility-tree script getAccessibleName (L60–109) covers 8 fallback layers. getSelectorAccessibleName (L137–172) stops at textContent, skipping placeholder and title. The behavioral split is intentional (placeholder/title selectors are unstable), but the structural duplication carries the same maintenance risk as in the sibling script. Same suggestion: rename to getStableSelectorName() with a comment explaining the deliberate omission. |
||
| const ariaLabel = el.getAttribute('aria-label'); | ||
| if (ariaLabel) return ariaLabel.trim(); | ||
|
|
||
| const labelledBy = el.getAttribute('aria-labelledby'); | ||
| if (labelledBy) { | ||
| const texts = labelledBy.split(/\s+/) | ||
| .map(id => document.getElementById(id)?.textContent?.trim() || '') | ||
| .filter(Boolean); | ||
| if (texts.length > 0) return texts.join(' ').slice(0, 100); | ||
| } | ||
|
|
||
| const tag = el.tagName.toLowerCase(); | ||
| if (tag === 'img' || (tag === 'input' && el.getAttribute('type') === 'image')) { | ||
| const alt = el.getAttribute('alt'); | ||
| if (alt !== null) return alt.trim(); | ||
| } | ||
|
|
||
| if (['input', 'select', 'textarea'].includes(tag)) { | ||
| const id = el.getAttribute('id'); | ||
| if (id) { | ||
| const label = document.querySelector(`label[for="${CSS.escape(id)}"]`); | ||
| if (label) return label.textContent?.trim() || ''; | ||
| } | ||
| const parentLabel = el.closest('label'); | ||
| if (parentLabel) { | ||
| const clone = parentLabel.cloneNode(true) as HTMLElement; | ||
| clone.querySelectorAll('input,select,textarea').forEach(n => n.remove()); | ||
| const lt = clone.textContent?.trim(); | ||
| if (lt) return lt; | ||
| } | ||
| return ''; | ||
| } | ||
|
|
||
| return (el.textContent?.trim().replace(/\s+/g, ' ') || '').slice(0, 100); | ||
| } | ||
|
|
||
| function isLikelyGeneratedId(id: string): boolean { | ||
| return /^[0-9a-f]{8,}$/i.test(id) || | ||
| /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(id) || | ||
| /(?:^|[-_:])\d{8,}(?:$|[-_:])/.test(id) || | ||
| /^(?:ember|react-select|radix|headlessui|mui|mantine|chakra|auto|generated)[-_:]?\d+/i.test(id); | ||
| } | ||
|
|
||
| function uniqueCssSelector(selector: string): string | null { | ||
| return document.querySelectorAll(selector).length === 1 ? selector : null; | ||
| } | ||
|
|
||
| function getSelector(element: HTMLElement): string { | ||
| const tag = element.tagName.toLowerCase(); | ||
|
|
||
| // 1. tag*=Text — best per WebdriverIO docs | ||
| const text = element.textContent?.trim().replace(/\s+/g, ' '); | ||
| if (text && text.length > 0 && text.length <= 50) { | ||
| const sameTagElements = document.querySelectorAll(tag); | ||
| let matchCount = 0; | ||
| sameTagElements.forEach(el => { | ||
| if (el.textContent?.includes(text)) matchCount++; | ||
| }); | ||
| if (matchCount === 1) return `${tag}*=${text}`; | ||
| // 1. Explicit test hooks | ||
| for (const attr of ['data-testid', 'data-test', 'data-qa']) { | ||
| const value = element.getAttribute(attr); | ||
| if (!value) continue; | ||
| const selector = uniqueCssSelector(`[${attr}="${CSS.escape(value)}"]`); | ||
| if (selector) return selector; | ||
| } | ||
|
|
||
| // 2. aria/label | ||
| const ariaLabel = element.getAttribute('aria-label'); | ||
| if (ariaLabel && ariaLabel.length <= 80) return `aria/${ariaLabel}`; | ||
|
|
||
| // 3. data-testid | ||
| const testId = element.getAttribute('data-testid'); | ||
| if (testId) { | ||
| const sel = `[data-testid="${CSS.escape(testId)}"]`; | ||
| if (document.querySelectorAll(sel).length === 1) return sel; | ||
| // 2. Accessible name. WebdriverIO's aria/ selector is name-based, so only | ||
| // emit it when the accessible-name candidate is unique among interactables. | ||
| const role = getRole(element); | ||
| const accessibleName = getSelectorAccessibleName(element); | ||
| if (role && accessibleName && accessibleName.length <= 80) { | ||
| let matchCount = 0; | ||
| document.querySelectorAll(interactableSelectors).forEach(el => { | ||
| const htmlEl = el as HTMLElement; | ||
| if (isVisible(htmlEl) && getRole(htmlEl) && getSelectorAccessibleName(htmlEl) === accessibleName) matchCount++; | ||
| }); | ||
| if (matchCount === 1) return `aria/${accessibleName}`; | ||
| } | ||
|
|
||
| // 4. #id | ||
| if (element.id) return `#${CSS.escape(element.id)}`; | ||
| // 3. Stable #id | ||
| if (element.id && !isLikelyGeneratedId(element.id)) return `#${CSS.escape(element.id)}`; | ||
|
|
||
| // 5. [name] — form elements | ||
| // 4. Stable input attributes | ||
| const nameAttr = element.getAttribute('name'); | ||
| if (nameAttr) { | ||
| const sel = `${tag}[name="${CSS.escape(nameAttr)}"]`; | ||
| if (document.querySelectorAll(sel).length === 1) return sel; | ||
| const selector = uniqueCssSelector(sel); | ||
| if (selector) return selector; | ||
| } | ||
|
|
||
| const placeholder = element.getAttribute('placeholder'); | ||
| if (placeholder) { | ||
| const sel = `${tag}[placeholder="${CSS.escape(placeholder)}"]`; | ||
| const selector = uniqueCssSelector(sel); | ||
| if (selector) return selector; | ||
| } | ||
|
|
||
| // 6. tag.class — try each class individually, then first-two combination | ||
| // 5. Scoped CSS fallback — try classes before structural paths | ||
| if (element.className && typeof element.className === 'string') { | ||
| const classes = element.className.trim().split(/\s+/).filter(Boolean); | ||
| for (const cls of classes) { | ||
| const sel = `${tag}.${CSS.escape(cls)}`; | ||
| if (document.querySelectorAll(sel).length === 1) return sel; | ||
| const selector = uniqueCssSelector(sel); | ||
| if (selector) return selector; | ||
| } | ||
| if (classes.length >= 2) { | ||
| const sel = `${tag}${classes.slice(0, 2).map(c => `.${CSS.escape(c)}`).join('')}`; | ||
| if (document.querySelectorAll(sel).length === 1) return sel; | ||
| const selector = uniqueCssSelector(sel); | ||
| if (selector) return selector; | ||
| } | ||
| } | ||
|
|
||
| // 7. CSS path fallback | ||
| let current: HTMLElement | null = element; | ||
| const path: string[] = []; | ||
| while (current && current !== document.documentElement) { | ||
| let seg = current.tagName.toLowerCase(); | ||
| if (current.id) { | ||
| if (current.id && !isLikelyGeneratedId(current.id)) { | ||
| path.unshift(`#${CSS.escape(current.id)}`); | ||
| break; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion:
getAccessibleNameandgetSelectorAccessibleNameare near-duplicates with subtle behavioral differencesThe only differences: getSelectorAccessibleName omits placeholder and title fallbacks and adds a CONTAINER_ROLES guard. These omissions are correct for selector generation — placeholder and title produce brittle selectors — but the intent is obscured by having two 30-line functions that are ~80% identical.
If someone patches getAccessibleName in the future without realizing getSelectorAccessibleName exists, selector names will silently diverge from the reported name field.