fix: detect custom clickable elements in take_snapshot#452
Merged
shivammittal274 merged 1 commit intomainfrom Mar 16, 2026
Merged
fix: detect custom clickable elements in take_snapshot#452shivammittal274 merged 1 commit intomainfrom
shivammittal274 merged 1 commit intomainfrom
Conversation
take_snapshot only used the AX tree, which misses custom components (cursor:pointer divs, onclick handlers, etc.) that lack ARIA roles. These elements appeared as role="generic" and were invisible to the agent. Changes: - Merge findCursorInteractiveElements into snapshot() so take_snapshot catches cursor:pointer, onclick, and tabindex elements - Add DisclosureTriangle to INTERACTIVE_ROLES for <summary> elements - Use aria-label as text fallback in cursor detection for icon-only buttons - Fix dedup bug in enhancedSnapshot that was silently dropping all cursor-detected elements by checking against all AX node IDs instead of only already-included output IDs
Contributor
Greptile SummaryThis PR extends
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[take_snapshot / enhancedSnapshot called] --> B[Fetch AX Tree via CDP]
B --> C{AX nodes empty?}
C -- Yes --> D[Return empty string]
C -- No --> E[Build interactive/enhanced tree lines]
E --> F[findCursorInteractiveElements via JS injection]
F --> G{Cursor elements found?}
G -- No --> J[Return tree lines]
G -- Yes --> H[Parse rendered line IDs into includedIds set]
H --> I[Append non-duplicate cursor elements as 'clickable']
I --> J
F -- Error --> J
Prompt To Fix All With AIThis is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/browser/browser.ts
Line: 395-413
Comment:
**Duplicated cursor-detection merging logic**
The cursor-element merging block in `snapshot()` (lines 395–413) is nearly identical to the one in `enhancedSnapshot()` (lines 462–490), differing only in the output format string and error handling. Consider extracting a shared helper (e.g., `mergeCursorElements(lines, session, formatFn)`) to avoid maintaining the same dedup logic in two places. This would reduce the risk of future divergence between the two code paths.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: d4e0a30 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
take_snapshotso the agent sees custom components — upvote buttons, dropdown triggers, clickable cards, etc.DisclosureTriangletoINTERACTIVE_ROLESso<summary>elements are capturedaria-labelas text fallback in cursor detection for icon-only buttons (e.g. close ✕)enhancedSnapshotthat silently dropped all cursor-detected elementsBefore → After
take_snapshoton a page with custom components:Before (16/31 elements):
After (28/31 elements — adds cursor-detected elements):
Test plan
DisclosureTrianglecaptures<summary>elementsaria-labelfallback captures icon-only buttons