diff --git a/browse/PLAN-snapshot-dropdown-interactive.md b/browse/PLAN-snapshot-dropdown-interactive.md deleted file mode 100644 index 75356911..00000000 --- a/browse/PLAN-snapshot-dropdown-interactive.md +++ /dev/null @@ -1,102 +0,0 @@ -# Plan: Snapshot Dropdown/Autocomplete Interactive Element Detection - -## Problem - -`snapshot -i` misses dropdown/autocomplete items on modern web apps. These elements: -1. Are often `
`/`
  • ` with click handlers but no semantic ARIA roles -2. Live inside dynamically-created portals/popovers (floating containers) -3. Don't appear in Playwright's accessibility tree (`ariaSnapshot()`) - -The `-C` flag (cursor-interactive scan) was designed for this but: -- Requires separate flag — agents using `-i` don't get it automatically -- Skips elements that HAVE an ARIA role (even if the ARIA tree missed them) -- Doesn't prioritize popover/portal containers where dropdown items live - -## Root Cause - -Playwright's `ariaSnapshot()` builds from the browser's accessibility tree. Dynamically-rendered popovers (React portals, Radix Popover, etc.) may not be in the accessibility tree if: -- The component doesn't set ARIA roles -- The portal renders outside the scoped `body` locator's subtree timing -- The browser hasn't updated the accessibility tree yet after DOM mutation - -## Changes - -### 1. Auto-enable cursor-interactive scan with `-i` flag - -**File:** `browse/src/snapshot.ts` - -When `-i` (interactive) is passed, automatically include the cursor-interactive scan. This means agents always see clickable non-ARIA elements when they ask for interactive elements. - -The `-C` flag remains as a standalone option for non-interactive snapshots. - -``` -if (opts.interactive) { - opts.cursorInteractive = true; -} -``` - -### 2. Add popover/portal priority scanning - -**File:** `browse/src/snapshot.ts` (inside cursor-interactive evaluate block) - -Before the general cursor:pointer scan, specifically scan for visible floating containers (popovers, dropdowns, menus) and include ALL their direct children as interactive: - -Detection heuristics for floating containers: -- `position: fixed` or `position: absolute` with `z-index >= 10` -- Has `role="listbox"`, `role="menu"`, `role="dialog"`, `role="tooltip"`, `[data-radix-popper-content-wrapper]`, `[data-floating-ui-portal]`, etc. -- Appeared recently in the DOM (not in initial page load) -- Is visible (`offsetParent !== null` or `position: fixed`) - -For each floating container, include child elements that: -- Have text content -- Are visible -- Have cursor:pointer OR onclick OR role="option" OR role="menuitem" -- Tag with reason `popover-child` for clarity - -### 3. Remove the `hasRole` skip in cursor-interactive scan - -**File:** `browse/src/snapshot.ts` - -Currently: `if (hasRole) continue;` — skips any element with an ARIA role, assuming the ARIA tree already captured it. - -Problem: if the ARIA tree MISSED the element (timing, portal, bad DOM structure), it falls through both systems. - -Fix: Only skip if the element's role is in `INTERACTIVE_ROLES` AND it was actually captured in the main refMap. Otherwise include it. - -Since we can't easily check the refMap from inside `page.evaluate()`, the simpler fix: remove the `hasRole` skip entirely for elements inside detected floating containers. For elements outside floating containers, keep the `hasRole` skip as-is (to avoid duplicates in normal page content). - -### 4. Add dropdown test fixture and tests - -**File:** `browse/test/fixtures/dropdown.html` - -HTML page with: -- A combobox input that shows a dropdown on focus/type -- Dropdown items as `
    ` with click handlers (no ARIA roles) -- Dropdown items as `
  • ` with `role="option"` -- A React-portal-style container (`position: fixed`, high z-index) - -**File:** `browse/test/snapshot.test.ts` - -New test cases: -- `snapshot -i` on dropdown page finds dropdown items via cursor scan -- `snapshot -i` on dropdown page includes popover-child elements -- `@c` refs from dropdown scan are clickable -- Elements inside floating containers with ARIA roles are captured even when ARIA tree misses them - -## Rollout Risk - -**Low.** The `-C` scan is additive — it only adds `@c` refs, never removes `@e` refs. The change to auto-enable it with `-i` increases output size but agents already handle mixed ref types. - -**One concern:** The `-C` scan queries ALL elements (`document.querySelectorAll('*')`) which can be slow on heavy pages. For the popover-specific scan, we limit to elements inside detected floating containers, which is fast (small subtree). - -## Testing - -```bash -cd /data/gstack/browse && bun test snapshot -``` - -## Files Changed - -1. `browse/src/snapshot.ts` — auto-enable -C with -i, popover scanning, remove hasRole skip in floating containers -2. `browse/test/fixtures/dropdown.html` — new test fixture -3. `browse/test/snapshot.test.ts` — new dropdown/popover test cases diff --git a/browse/src/snapshot.ts b/browse/src/snapshot.ts index 5581fe6e..840cd686 100644 --- a/browse/src/snapshot.ts +++ b/browse/src/snapshot.ts @@ -233,12 +233,7 @@ export async function handleSnapshot( output.push(outputLine); } - // ─── Cursor-interactive scan (-C, or auto with -i) ──────── - // Auto-enable cursor scan when interactive mode is on — agents asking for - // interactive elements should always see clickable non-ARIA items too. - if (opts.interactive && !opts.cursorInteractive) { - opts.cursorInteractive = true; - } + // ─── Cursor-interactive scan (-C) ───────────────────────── if (opts.cursorInteractive) { try { const cursorElements = await target.evaluate(() => { @@ -261,37 +256,9 @@ export async function handleSnapshot( const hasTabindex = el.hasAttribute('tabindex') && parseInt(el.getAttribute('tabindex')!, 10) >= 0; const hasRole = el.hasAttribute('role'); - // Check if element is inside a floating container (portal/popover/dropdown) - const isInFloating = (() => { - let parent: Element | null = el; - while (parent && parent !== document.documentElement) { - const pStyle = getComputedStyle(parent); - const isFloating = (pStyle.position === 'fixed' || pStyle.position === 'absolute') && - parseInt(pStyle.zIndex || '0', 10) >= 10; - const hasPortalAttr = parent.hasAttribute('data-floating-ui-portal') || - parent.hasAttribute('data-radix-popper-content-wrapper') || - parent.hasAttribute('data-radix-portal') || - parent.hasAttribute('data-popper-placement') || - parent.getAttribute('role') === 'listbox' || - parent.getAttribute('role') === 'menu'; - if (isFloating || hasPortalAttr) return true; - parent = parent.parentElement; - } - return false; - })(); - - if (!hasCursorPointer && !hasOnclick && !hasTabindex) { - // For elements inside floating containers, also check for role="option"/"menuitem" - if (isInFloating && hasRole) { - const role = el.getAttribute('role'); - if (role !== 'option' && role !== 'menuitem' && role !== 'menuitemcheckbox' && role !== 'menuitemradio') continue; - } else { - continue; - } - } - // Skip elements with ARIA roles UNLESS they're inside a floating container - // (floating container items may be missed by the accessibility tree) - if (hasRole && !isInFloating) continue; + if (!hasCursorPointer && !hasOnclick && !hasTabindex) continue; + // Skip if it has an ARIA role (likely already captured) + if (hasRole) continue; // Build deterministic nth-child CSS path const parts: string[] = []; @@ -308,11 +275,9 @@ export async function handleSnapshot( const text = (el as HTMLElement).innerText?.trim().slice(0, 80) || el.tagName.toLowerCase(); const reasons: string[] = []; - if (isInFloating) reasons.push('popover-child'); if (hasCursorPointer) reasons.push('cursor:pointer'); if (hasOnclick) reasons.push('onclick'); if (hasTabindex) reasons.push(`tabindex=${el.getAttribute('tabindex')}`); - if (hasRole) reasons.push(`role=${el.getAttribute('role')}`); results.push({ selector, text, reason: reasons.join(', ') }); } diff --git a/browse/test/fixtures/dropdown.html b/browse/test/fixtures/dropdown.html deleted file mode 100644 index 7919bceb..00000000 --- a/browse/test/fixtures/dropdown.html +++ /dev/null @@ -1,61 +0,0 @@ - - - - - Test Page - Dropdown/Autocomplete - - - -

    Dropdown Test

    - -
    - -
    - - - - - - - Normal Link - - - - diff --git a/browse/test/snapshot.test.ts b/browse/test/snapshot.test.ts index bcbf8cd7..db5e8004 100644 --- a/browse/test/snapshot.test.ts +++ b/browse/test/snapshot.test.ts @@ -386,77 +386,6 @@ describe('Cursor-interactive', () => { // And cursor-interactive section expect(result).toContain('cursor-interactive'); }); - - test('snapshot -i alone also includes cursor-interactive elements', async () => { - await handleWriteCommand('goto', [baseUrl + '/cursor-interactive.html'], bm); - const result = await handleMetaCommand('snapshot', ['-i'], bm, shutdown); - // -i now auto-enables -C - expect(result).toContain('[button]'); - expect(result).toContain('[link]'); - expect(result).toContain('cursor-interactive'); - expect(result).toContain('@c'); - }); -}); - -// ─── Dropdown/Popover Detection ───────────────────────────────── - -describe('Dropdown/popover detection', () => { - test('snapshot -i auto-enables cursor scan and finds dropdown items', async () => { - await handleWriteCommand('goto', [baseUrl + '/dropdown.html'], bm); - const result = await handleMetaCommand('snapshot', ['-i'], bm, shutdown); - // Should find standard interactive elements - expect(result).toContain('[button]'); - expect(result).toContain('[link]'); - expect(result).toContain('[textbox]'); - // Should also find cursor-interactive dropdown items - expect(result).toContain('cursor-interactive'); - expect(result).toContain('@c'); - expect(result).toContain('Alice Johnson'); - expect(result).toContain('Bob Smith'); - }); - - test('dropdown items in floating container are tagged as popover-child', async () => { - await handleWriteCommand('goto', [baseUrl + '/dropdown.html'], bm); - const result = await handleMetaCommand('snapshot', ['-i'], bm, shutdown); - expect(result).toContain('popover-child'); - }); - - test('dropdown items with role="option" in portal are captured', async () => { - await handleWriteCommand('goto', [baseUrl + '/dropdown.html'], bm); - const result = await handleMetaCommand('snapshot', ['-i'], bm, shutdown); - // Dave Wilson has role="option" — should be captured even though it has a role - expect(result).toContain('Dave Wilson'); - }); - - test('static text in dropdown without interactivity is NOT captured', async () => { - await handleWriteCommand('goto', [baseUrl + '/dropdown.html'], bm); - const result = await handleMetaCommand('snapshot', ['-i'], bm, shutdown); - // "No results? Try a different search." has no cursor:pointer, no onclick, no tabindex - expect(result).not.toContain('No results'); - }); - - test('@c ref from dropdown is clickable', async () => { - await handleWriteCommand('goto', [baseUrl + '/dropdown.html'], bm); - const snap = await handleMetaCommand('snapshot', ['-i'], bm, shutdown); - // Find a @c ref for Alice - const aliceLine = snap.split('\n').find(l => l.includes('@c') && l.includes('Alice')); - if (aliceLine) { - const refMatch = aliceLine.match(/@(c\d+)/); - if (refMatch) { - const result = await handleWriteCommand('click', [`@${refMatch[1]}`], bm); - expect(result).toContain('Clicked'); - } - } - }); - - test('snapshot -C still works standalone without -i', async () => { - await handleWriteCommand('goto', [baseUrl + '/dropdown.html'], bm); - const result = await handleMetaCommand('snapshot', ['-C'], bm, shutdown); - expect(result).toContain('cursor-interactive'); - expect(result).toContain('Alice Johnson'); - // Without -i, should include non-interactive ARIA elements too - expect(result).toContain('[heading]'); - }); }); // ─── Snapshot Error Paths ───────────────────────────────────────