fix: resolve unnamed element refs matching multiple elements (#573)
* fix: resolve unnamed element refs matching multiple elements (#500) When a page has one unnamed button among several named buttons, clicking its ref fails with "matched N elements" because the locator `getByRole('button')` matches all buttons on the page. Normalize unnamed interactive elements to `name: ""` so the selector becomes `getByRole('button', { name: "", exact: true })` which matches only buttons with empty accessible names. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: remove dead code branch in buildSelector Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: make RefMap.name required string, remove dead code branches Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: hyunjinee <leehj0110@kakao.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
hyunjinee
parent
79d8dfe34c
commit
7238b7da4c
+1
-1
@@ -929,7 +929,7 @@ async function handleSnapshot(
|
||||
});
|
||||
|
||||
// Simplify refs for output (just role and name)
|
||||
const simpleRefs: Record<string, { role: string; name?: string }> = {};
|
||||
const simpleRefs: Record<string, { role: string; name: string }> = {};
|
||||
for (const [ref, data] of Object.entries(refs)) {
|
||||
simpleRefs[ref] = { role: data.role, name: data.name };
|
||||
}
|
||||
|
||||
@@ -154,6 +154,30 @@ describe('BrowserManager', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('unnamed-button ref uniqueness', () => {
|
||||
it('should click the correct unnamed button among named buttons', async () => {
|
||||
const page = browser.getPage();
|
||||
// 1 unnamed button among 2 named buttons
|
||||
await page.setContent(`
|
||||
<html><body>
|
||||
<button>OK</button>
|
||||
<button onclick="document.title='unnamed'"></button>
|
||||
<button>Cancel</button>
|
||||
</body></html>
|
||||
`);
|
||||
|
||||
const snapshot = await browser.getSnapshot();
|
||||
const refs = snapshot.refs;
|
||||
const unnamedRefs = Object.entries(refs).filter(([, v]) => v.role === 'button' && !v.name);
|
||||
expect(unnamedRefs.length).toBe(1);
|
||||
|
||||
const [refId] = unnamedRefs[0];
|
||||
await executeCommand({ id: 'test', action: 'click', selector: `@${refId}` }, browser);
|
||||
const title = await page.title();
|
||||
expect(title).toBe('unnamed');
|
||||
});
|
||||
});
|
||||
|
||||
describe('cursor-ref selector uniqueness', () => {
|
||||
it('should produce unique selectors for repeated DOM structures', async () => {
|
||||
const page = browser.getPage();
|
||||
|
||||
+4
-6
@@ -226,12 +226,10 @@ export class BrowserManager {
|
||||
}
|
||||
|
||||
// Build locator with exact: true to avoid substring matches
|
||||
let locator: Locator;
|
||||
if (refData.name) {
|
||||
locator = page.getByRole(refData.role as any, { name: refData.name, exact: true });
|
||||
} else {
|
||||
locator = page.getByRole(refData.role as any);
|
||||
}
|
||||
let locator: Locator = page.getByRole(refData.role as any, {
|
||||
name: refData.name,
|
||||
exact: true,
|
||||
});
|
||||
|
||||
// If an nth index is stored (for disambiguation), use it
|
||||
if (refData.nth !== undefined) {
|
||||
|
||||
+16
-16
@@ -23,7 +23,7 @@ export interface RefMap {
|
||||
[ref: string]: {
|
||||
selector: string;
|
||||
role: string;
|
||||
name?: string;
|
||||
name: string;
|
||||
/** Index for disambiguation when multiple elements have same role+name */
|
||||
nth?: number;
|
||||
};
|
||||
@@ -130,12 +130,9 @@ const STRUCTURAL_ROLES = new Set([
|
||||
/**
|
||||
* Build a selector string for storing in ref map
|
||||
*/
|
||||
function buildSelector(role: string, name?: string): string {
|
||||
if (name) {
|
||||
const escapedName = JSON.stringify(name);
|
||||
return `getByRole('${role}', { name: ${escapedName}, exact: true })`;
|
||||
}
|
||||
return `getByRole('${role}')`;
|
||||
function buildSelector(role: string, name: string): string {
|
||||
const escapedName = JSON.stringify(name);
|
||||
return `getByRole('${role}', { name: ${escapedName}, exact: true })`;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -293,7 +290,7 @@ export async function getEnhancedSnapshot(
|
||||
const cursorElements = await findCursorInteractiveElements(page, options.selector);
|
||||
|
||||
// Filter out elements whose text is already captured in the snapshot
|
||||
const existingTexts = new Set(Object.values(refs).map((r) => r.name?.toLowerCase()));
|
||||
const existingTexts = new Set(Object.values(refs).map((r) => r.name.toLowerCase()));
|
||||
// Also extract quoted strings from the ARIA tree for broader dedup
|
||||
for (const m of enhancedTree.matchAll(/"([^"]+)"/g)) {
|
||||
existingTexts.add(m[1].toLowerCase());
|
||||
@@ -404,12 +401,13 @@ function processAriaTree(ariaTree: string, refs: RefMap, options: SnapshotOption
|
||||
|
||||
if (INTERACTIVE_ROLES.has(roleLower)) {
|
||||
const ref = nextRef();
|
||||
const nth = tracker.getNextIndex(roleLower, name);
|
||||
tracker.trackRef(roleLower, name, ref);
|
||||
const resolvedName = name ?? '';
|
||||
const nth = tracker.getNextIndex(roleLower, resolvedName);
|
||||
tracker.trackRef(roleLower, resolvedName, ref);
|
||||
refs[ref] = {
|
||||
selector: buildSelector(roleLower, name),
|
||||
selector: buildSelector(roleLower, resolvedName),
|
||||
role: roleLower,
|
||||
name,
|
||||
name: resolvedName,
|
||||
nth, // Always store nth, we'll use it for duplicates
|
||||
};
|
||||
|
||||
@@ -531,13 +529,15 @@ function processLine(
|
||||
|
||||
if (shouldHaveRef) {
|
||||
const ref = nextRef();
|
||||
const nth = tracker.getNextIndex(roleLower, name);
|
||||
tracker.trackRef(roleLower, name, ref);
|
||||
// Normalize to "" so unnamed elements get exact-match selectors
|
||||
const resolvedName = isInteractive ? (name ?? '') : name!;
|
||||
const nth = tracker.getNextIndex(roleLower, resolvedName);
|
||||
tracker.trackRef(roleLower, resolvedName, ref);
|
||||
|
||||
refs[ref] = {
|
||||
selector: buildSelector(roleLower, name),
|
||||
selector: buildSelector(roleLower, resolvedName),
|
||||
role: roleLower,
|
||||
name,
|
||||
name: resolvedName,
|
||||
nth, // Always store nth, we'll clean up non-duplicates later
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user