From 7238b7da4ca62a19e47d1d80027963868e742473 Mon Sep 17 00:00:00 2001 From: "jin.2" Date: Mon, 2 Mar 2026 00:42:05 +0900 Subject: [PATCH] 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 * refactor: remove dead code branch in buildSelector Co-Authored-By: Claude Opus 4.6 * refactor: make RefMap.name required string, remove dead code branches Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: hyunjinee Co-authored-by: Claude Opus 4.6 --- src/actions.ts | 2 +- src/browser.test.ts | 24 ++++++++++++++++++++++++ src/browser.ts | 10 ++++------ src/snapshot.ts | 32 ++++++++++++++++---------------- 4 files changed, 45 insertions(+), 23 deletions(-) diff --git a/src/actions.ts b/src/actions.ts index 5b5b06f..1e36f94 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -929,7 +929,7 @@ async function handleSnapshot( }); // Simplify refs for output (just role and name) - const simpleRefs: Record = {}; + const simpleRefs: Record = {}; for (const [ref, data] of Object.entries(refs)) { simpleRefs[ref] = { role: data.role, name: data.name }; } diff --git a/src/browser.test.ts b/src/browser.test.ts index 48bf3ed..8338d71 100644 --- a/src/browser.test.ts +++ b/src/browser.test.ts @@ -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(` + + + + + + `); + + 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(); diff --git a/src/browser.ts b/src/browser.ts index c32aea3..73fe0d0 100644 --- a/src/browser.ts +++ b/src/browser.ts @@ -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) { diff --git a/src/snapshot.ts b/src/snapshot.ts index 40a205f..6e6c774 100644 --- a/src/snapshot.ts +++ b/src/snapshot.ts @@ -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 };