Skip to content

Conversation

@seanmcguire12
Copy link
Member

@seanmcguire12 seanmcguire12 commented Dec 4, 2025

why

  • using page.evaluate() with long running injected scripts is error prone

what changed

  • this PR changes getAccessibilityTree() to use CDP to find scrollable elements instead of finding them with injected JS

test plan


Summary by cubic

Switch scrollable-node detection to CDP to remove error-prone injected scripts, improving reliability of AX tree generation and role decoration.

  • Refactors
    • Build scrollable element set via CDP DOM tree (records isScrollable; includes html fallback).
    • Remove injected-JS path and findScrollableElementIds; decorateRoles now uses backend IDs.
    • Update types with isScrollable on DOMNode and scrollableBackendIds in maps.

Written for commit 2551c7f. Summary will update automatically on new commits.

@changeset-bot
Copy link

changeset-bot bot commented Dec 4, 2025

🦋 Changeset detected

Latest commit: 2551c7f

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 4, 2025

Greptile Overview

Greptile Summary

This PR replaces the JavaScript-based scrollable element detection with CDP's native isScrollable property from DOM.getDocument. The previous approach used page.evaluate() to run injected JavaScript that iterated over all DOM elements, checked computed styles, and tested scrollability - which was error-prone for long-running scripts. The new approach leverages CDP's DOM tree which already provides scrollability information via the isScrollable property.

Key changes:

  • Removed findScrollableElementIds() function that used window.getScrollableElementXpaths() via page evaluation
  • Added scrollableBackendIds collection during the DFS walk in buildBackendIdMaps()
  • Added RootWebArea exclusion to prevent the document root from being marked as scrollable
  • Preserved behavior of always treating <html> elements as scrollable
  • Minor code style improvement: refactored comma expression to explicit braces

Confidence Score: 4/5

  • This PR is safe to merge with low risk - it simplifies scrollable detection by using native CDP data instead of injected JS.
  • The change replaces a complex JS injection approach with CDP's native isScrollable property, which is more reliable. The logic correctly preserves existing behavior (HTML elements always scrollable, RootWebArea excluded). The type changes are properly aligned. Minor deduction for behavioral difference: the old approach tested actual scrollability via canElementScroll(), while CDP's isScrollable may report different results in edge cases.
  • lib/a11y/utils.ts - main logic change, verify CDP's isScrollable behavior matches previous JS-based detection in edge cases

Important Files Changed

File Analysis

Filename Score Overview
lib/a11y/utils.ts 4/5 Replaces page.evaluate()-based scrollable detection with CDP's isScrollable property, removes findScrollableElementIds function, adds RootWebArea exclusion and HTML element auto-inclusion. Cleaner code style for iframe node assignment.
types/context.ts 5/5 Adds isScrollable property to DOMNode type and scrollableBackendIds to BackendIdMaps type to support CDP-based scrollable detection.
.changeset/open-birds-pull.md 5/5 Standard changeset file documenting this as a patch release for using CDP to find scrollable nodes.
examples/external_clients/aisdk.ts 5/5 Minor whitespace cleanup - removed extra blank line.

Sequence Diagram

sequenceDiagram
    participant GF as getAccessibilityTree()
    participant BBM as buildBackendIdMaps()
    participant CDP as Chrome DevTools Protocol
    participant DR as decorateRoles()

    GF->>BBM: Build DOM maps
    BBM->>CDP: DOM.getDocument(depth: -1, pierce: true)
    CDP-->>BBM: DOM tree with isScrollable property
    BBM->>BBM: DFS walk: collect tagNameMap, xpathMap, scrollableBackendIds
    Note over BBM: Check node.isScrollable === true
    Note over BBM: Also add HTML elements as scrollable
    BBM-->>GF: {tagNameMap, xpathMap, scrollableBackendIds}
    GF->>CDP: Accessibility.getFullAXTree
    CDP-->>GF: AX nodes
    GF->>DR: decorateRoles(nodes, scrollableBackendIds)
    DR->>DR: Skip RootWebArea from scrollable decoration
    DR->>DR: Prepend "scrollable" to matching node roles
    DR-->>GF: Decorated AccessibilityNodes
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@seanmcguire12 seanmcguire12 merged commit f26333e into v2 Dec 4, 2025
15 of 16 checks passed
@github-actions github-actions bot mentioned this pull request Dec 5, 2025
seanmcguire12 pushed a commit that referenced this pull request Dec 5, 2025
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to v2, this PR will
be updated.


# Releases
## @browserbasehq/[email protected]

### Patch Changes

- [#1275](#1275)
[`a372b3c`](a372b3c)
Thanks [@miguelg719](https://github.com/miguelg719)! - Remove process
exit on signal handler

- [#1143](#1143)
[`fc06d40`](fc06d40)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - add logger
param to external aisdk client

- [#1137](#1137)
[`2dbac99`](2dbac99)
Thanks [@miguelg719](https://github.com/miguelg719)! - Add haiku 4.5
computer use support

- [#1116](#1116)
[`b419fc3`](b419fc3)
Thanks [@tkattkat](https://github.com/tkattkat)! - patch stagehand agent
api support

- [#1362](#1362)
[`f26333e`](f26333e)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - use CDP to
find scrollable nodes instead of injected JS

- [#1125](#1125)
[`cbff109`](cbff109)
Thanks [@tkattkat](https://github.com/tkattkat)! - update cua agents key
& system prompt handling

- [#1363](#1363)
[`223e158`](223e158)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - add
causedBy to StagehandDefaultError

- [#1123](#1123)
[`f426ba5`](f426ba5)
Thanks [@tkattkat](https://github.com/tkattkat)! - Add pageUrl &
timestamp to agent actions

- [#1365](#1365)
[`2f71b02`](2f71b02)
Thanks [@seanmcguire12](https://github.com/seanmcguire12)! - export
getAccessibilityTree()

- [#1366](#1366)
[`e098b0d`](e098b0d)
Thanks [@miguelg719](https://github.com/miguelg719)! - Update finding
scrollable nodes using CDP

## @browserbasehq/[email protected]

### Patch Changes

- Updated dependencies
\[[`a372b3c`](a372b3c),
[`fc06d40`](fc06d40),
[`2dbac99`](2dbac99),
[`b419fc3`](b419fc3),
[`f26333e`](f26333e),
[`cbff109`](cbff109),
[`223e158`](223e158),
[`f426ba5`](f426ba5),
[`2f71b02`](2f71b02),
[`e098b0d`](e098b0d)]:
    -   @browserbasehq/[email protected]

## @browserbasehq/[email protected]

### Patch Changes

- Updated dependencies
\[[`a372b3c`](a372b3c),
[`fc06d40`](fc06d40),
[`2dbac99`](2dbac99),
[`b419fc3`](b419fc3),
[`f26333e`](f26333e),
[`cbff109`](cbff109),
[`223e158`](223e158),
[`f426ba5`](f426ba5),
[`2f71b02`](2f71b02),
[`e098b0d`](e098b0d)]:
    -   @browserbasehq/[email protected]


<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Publish Stagehand 2.5.3 and bump evals (1.1.3) and examples (1.0.12).
Improves stability, CDP-based scrolling, and adds better logging and
error context.

- **New Features**
  - Haiku 4.5 computer-use support.
  - Export getAccessibilityTree().
  - Logger param for the external AISDK client.
  - Page URL and timestamp on agent actions.
  - causedBy on StagehandDefaultError for richer error context.

- **Bug Fixes**
  - Detect scrollable nodes via CDP (removed injected JS).
  - Refined CDP scrollable node detection.
  - Do not exit the process on signal handler.
  - Patch agent API support and update CUA key/system prompt handling.

<sup>Written for commit 6b62062.
Summary will update automatically on new commits.</sup>

<!-- End of auto-generated description by cubic. -->

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants