Visitar URL original
fix(snapshot): omit uid for accessibility nodes without backendNodeId by twotnguyen · Pull Request #3187 · ChromeDevTools/chrome-devtools-mcp · GitHub
Skip to content

fix(snapshot): omit uid for accessibility nodes without backendNodeId - #3187

Merged
OrKoN merged 4 commits into
ChromeDevTools:mainfrom
twotnguyen:fix/snapshot-unique-uid-inline-text-box
Oct 16, 2026
Merged

OrKoN merged 4 commits into
ChromeDevTools:mainfrom
twotnguyen:fix/snapshot-unique-uid-inline-text-box

Conversation

@twotnguyen

@twotnguyen twotnguyen commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2960

Summary

When capturing a verbose accessibility snapshot, TextSnapshot assigned the exact same uid (1_undefined) to all nodes lacking a DOM backendNodeId (such as InlineTextBox or synthetic pseudo-element text nodes). Furthermore, because these nodes have no underlying DOM element, interaction tools cannot resolve them (getElementByUid() returns null and calls fail with Element with uid ... no longer exists on the page).

This PR addresses both issues:

  • Does not mint uids for accessibility nodes without a backendNodeId (id remains empty and is skipped in idToNode.set()).
  • Omits uid= in text rendering and id in JSON output within SnapshotFormatter when id is empty.
  • Prevents polluting uniqueBackendNodeIdToMcpId with undefined backendNodeId keys.

Testing

  • Unit tests in tests/TextSnapshot.test.ts using createMockAXNode and createMockPuppeteerPage() from tests/mocks.ts.
  • Unit test in tests/formatters/snapshotFormatter.test.ts verifying that uid= is omitted from toString() and id from toJSON() when node.id is empty.
  • Verified all 29 tests across tests/TextSnapshot.test.ts, tests/formatters/snapshotFormatter.test.ts, and tests/tools/snapshot.test.ts pass cleanly.
  • Verified npm run build and npm run format (ESLint + Prettier) pass with zero errors.

@google-cla

google-cla Bot commented Oct 16, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@OrKoN
OrKoN self-requested a review October 16, 2026 06:25

@OrKoN OrKoN left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please check the following:

  1. tests/mocks.ts / tests/TextSnapshot.test.ts: move the accessibility stub into createMockPuppeteerPage() (for example Object.defineProperty(page, 'accessibility', {value: sinon.createStubInstance(Accessibility)})) and the createAXNode helper into tests/mocks.ts as createMockAXNode. AGENTS.md asks for reusable mock factories to live in tests/mocks.ts and for mocks not to be tailored to one test file; the next snapshot unit test will need both again.

Nits

  1. tests/TextSnapshot.test.ts: also assert the two InlineTextBox uids are 1_3 and 1_5 (or at least that neither equals a paragraph uid) via snapshot.idToNode.get(...). notStrictEqual between the two only proves they differ from each other, not that they do not collide with a node that has a backendNodeId.

@twotnguyen

Copy link
Copy Markdown
Contributor Author

Moved the accessibility stub into createMockPuppeteerPage() in tests/mocks.ts and exported createMockAXNode from there. Updated tests/TextSnapshot.test.ts to use page.pptrPage.accessibility.snapshot directly and asserted exact expected uids (1_2, 1_4) along with role and name checks via snapshot.idToNode.get(...) to verify they don't collide with paragraph nodes. Re-pushed in commit b742b61.

@OrKoN
OrKoN self-requested a review October 16, 2026 09:41

@OrKoN OrKoN left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please check the following:

  1. src/TextSnapshot.ts (assignIds, else branch): do not mint a uid for a node without backendNodeId; leave id empty, skip idToNode.set(), and omit uid= in src/formatters/SnapshotFormatter.ts when id is empty. The current branch gives each such node a fresh ${snapshotId}_${n} that getElementByUid() cannot resolve (node.elementHandle() returns null for a node with no DOM node), so every tool call on it fails with Element with uid ... no longer exists on the page.; that is the second symptom in #2960, and the PR replaces one dead-end uid with one per node.

Nits

2 tests/mocks.ts: this PR and #3190 both introduce the MockPuppeteerPage type (accessibility here, mouse there) and both edit the Object.defineProperty/Object.assign block in createMockPuppeteerPage(). Whichever lands second needs a rebase that merges the two intersection fields into one type.

@twotnguyen

Copy link
Copy Markdown
Contributor Author

Good catch on the dead-end uids. Updated src/TextSnapshot.ts so nodes without backendNodeId leave id empty and skip idToNode.set(). Updated src/formatters/SnapshotFormatter.ts to omit uid= in text rendering and id in JSON output when id is empty. Added unit test in tests/formatters/snapshotFormatter.test.ts and updated tests/TextSnapshot.test.ts to assert that nodes without backendNodeId are excluded from idToNode and have empty id. Re-pushed in commit 1ee5c06.

@twotnguyen twotnguyen changed the title fix(snapshot): assign unique uids to accessibility nodes without backendNodeId fix(snapshot): omit uid for accessibility nodes without backendNodeId Oct 16, 2026
@OrKoN
OrKoN self-requested a review October 16, 2026 10:36
@OrKoN

OrKoN commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator

@twotnguyen please rebase

@twotnguyen
twotnguyen force-pushed the fix/snapshot-unique-uid-inline-text-box branch from 1ee5c06 to 494e3a4 Compare October 16, 2026 10:49
@twotnguyen

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main (382f441), merging the mouse and accessibility fields on MockPuppeteerPage in tests/mocks.ts.

@OrKoN
OrKoN enabled auto-merge October 16, 2026 10:51

@OrKoN OrKoN left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

there appear to be test failures

auto-merge was automatically disabled October 16, 2026 11:02

Head branch was pushed to by a user without write access

@twotnguyen

Copy link
Copy Markdown
Contributor Author

Updated tests/McpResponse.test.js.snapshot to reflect the omitted uid on InlineTextBox in McpResponse.test.ts. Re-pushed in commit 28679a9.

@OrKoN
OrKoN added this pull request to the merge queue Oct 16, 2026
Merged via the queue into ChromeDevTools:main with commit 9054351 Oct 16, 2026
14 checks passed
zjdCander pushed a commit to zjdCander/chrome-devtools-mcp that referenced this pull request Oct 19, 2026
## Summary

Add a **Pre-submit checklist** section to `AGENTS.md`. Each item comes
from a recent review finding or a landed bug fix in this repository.

The checklist has five groups:

- **Scope and PR description**: one change per PR, title and commit type
match, description lists every behavior change and matches the diff,
`npm run gen` and `npm run format` ran.
- **Design**: fix in the lowest layer that owns the data, fix upstream
(Puppeteer, devtools-frontend) first, one helper for repeated code,
options objects instead of positional `undefined` arguments, validation
in Zod schemas.
- **Correctness and edge cases**: no stale state after a failed call,
artifacts written before a throw, abandoned async continuations, mutex
guards, file removal races, `dispose()` releases every resource, secrets
never reach logs, path parameters validated even when they parse as
URLs.
- **Tests**: every new function has a test, no vacuous assertions,
`notCalled` targets the first call after validation, exact-argument
assertions, reusable stubs in `tests/mocks.ts`, bounded timeouts, no
second CDP session for event waits.
- **Documentation and output**: tool descriptions state when a parameter
is ignored, skills stay generic, one-sentence inline comments.

Sources: review comments on ChromeDevTools#2679, ChromeDevTools#2797, ChromeDevTools#2859, ChromeDevTools#2881, ChromeDevTools#2901, ChromeDevTools#3048,
ChromeDevTools#3082, ChromeDevTools#3109, ChromeDevTools#3120, ChromeDevTools#3132, ChromeDevTools#3134, ChromeDevTools#3139, ChromeDevTools#3148, ChromeDevTools#3156, ChromeDevTools#3158, ChromeDevTools#3161,
ChromeDevTools#3182, ChromeDevTools#3183, ChromeDevTools#3186, ChromeDevTools#3187, ChromeDevTools#3188, ChromeDevTools#3189, ChromeDevTools#3192, ChromeDevTools#3196, ChromeDevTools#3197, ChromeDevTools#3198,
ChromeDevTools#3203, ChromeDevTools#3204, and fixes ChromeDevTools#3150, ChromeDevTools#3160, ChromeDevTools#3185, ChromeDevTools#3202, ChromeDevTools#3207, ChromeDevTools#3208.

Existing content in `AGENTS.md` is unchanged.

Closes ChromeDevTools#3222
github-merge-queue Bot pushed a commit that referenced this pull request Oct 22, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.12.0](chrome-devtools-mcp-v1.11.0...chrome-devtools-mcp-v1.12.0)
(2026-10-22)


### 🎉 Features

* add --maxNavigationSaved flag to configure retained navigation history
([#3018](#3018))
([a580d13](a580d13))
* add Kimi Code plugin manifest
([#3029](#3029))
([cd00b64](cd00b64))
* Analyze heap snapshots without a browser connection
([#3156](#3156))
([78c8320](78c8320))
* **config:** add --watchConfig to reload config file changes
([#3248](#3248))
([b48561f](b48561f))
* **config:** introduce layered ServerConfig for live reload
([#3203](#3203))
([37f1fda](37f1fda))
* **config:** share ReadonlyServerConfig with McpContext and McpPage
([#3204](#3204))
([0813bf3](0813bf3))
* **config:** support reloading blockedUrlPattern and allowedUrlPattern
([#3268](#3268))
([cc850f5](cc850f5))
* **context:** release disabled feature state on config reload
([#3049](#3049))
([c0140d1](c0140d1))
* Paginate by default in heap snapshot tools
([#3201](#3201))
([fb8375b](fb8375b))
* Paginate detailed diff in compare_heapsnapshots
([#3195](#3195))
([4a9dd53](4a9dd53))
* run independent tool calls in parallel with a TaskRunner
([#3243](#3243))
([8a8d877](8a8d877))
* run page-scoped tools on different pages in parallel
([#3252](#3252))
([f8f18ac](f8f18ac))
* **server:** add McpServer.applyConfig() to reload configuration
([#3051](#3051))
([f874aeb](f874aeb))
* **slim:** support filepath in slim mode screenshot
([#3296](#3296))
([d399c4a](d399c4a))
* Sort objects for class by self size in compare_heapsnapshots
([#3205](#3205))
([f8a18c6](f8a18c6))
* **tools:** support updating ToolHandler definitions in place
([#3050](#3050))
([8064138](8064138))
* **wait_for:** support opt-in visible text matching
([#3212](#3212))
([eef92da](eef92da))


### 🛠️ Fixes

* allow paths below a filesystem-root workspace root
([#3277](#3277))
([296ccef](296ccef))
* block evaluate_script while a dialog is open
([#3196](#3196))
([545bcbb](545bcbb))
* bound take_snapshot wait and clear the geolocation override
([#3265](#3265))
([6282f54](6282f54))
* **browser:** redact WebSocket headers from connection logs
([#2901](#2901))
([3a2f91e](3a2f91e))
* capture thrown transform errors as Zod validation issues
([#3242](#3242))
([0f95c21](0f95c21))
* clear handled dialog in McpPage.getDialog()
([#3241](#3241))
([2ab93a3](2ab93a3)),
closes
[#2958](#2958)
* **cli:** disallow --slim in CLI start options
([#2987](#2987))
([#3113](#3113))
([ae3f61c](ae3f61c))
* **cli:** do not forward tool args to start
([#3114](#3114))
([fef1598](fef1598))
* **cli:** serialize object daemon arguments
([#2797](#2797))
([f73a440](f73a440))
* close the tab opened by a failed new_page
([#2928](#2928))
([c174677](c174677))
* **config:** accept native JSON objects for wsHeaders in config files
([#3262](#3262))
([371e6c2](371e6c2))
* **console:** describe Map, Set, function, DOM node, symbol and cyclic
arguments
([#3199](#3199))
([968e4e5](968e4e5))
* **console:** include the reason of unhandled promise rejections
([#3148](#3148))
([f7cdf73](f7cdf73))
* defer focus emulation until a page is explicitly targeted
([#3230](#3230))
([35abe35](35abe35))
* disable Puppeteer signal handlers for launched browsers
([#3238](#3238))
([3afec73](3afec73)),
closes
[#3154](#3154)
* discover third-party developer tools before tool lookup
([#3208](#3208))
([2777fa0](2777fa0)),
closes
[#2973](#2973)
* discover third-party developer tools in page tools
([#3207](#3207))
([cf33529](cf33529)),
closes
[#2980](#2980)
* drop InlineTextBox nodes from text snapshots
([#3202](#3202))
([f8c775a](f8c775a))
* **emulate:** keep previous viewport when setViewport fails
([#3150](#3150))
([19d883c](19d883c))
* exclude source maps from the npm package
([#3295](#3295))
([d575da4](d575da4))
* keep surrogate pairs and emoji whole when truncating titles, names and
bodies
([#3215](#3215))
([058adcf](058adcf))
* let resourceTypes match CORS preflights and prefetches
([#3217](#3217))
([e342c6e](e342c6e))
* **lighthouse:** explain unsupported URL schemes in navigation mode
([#3144](#3144))
([c6ad84a](c6ad84a))
* **lighthouse:** fail the audit when Lighthouse reports a runtime error
([#3188](#3188))
([53ea78d](53ea78d))
* **lighthouse:** restore the emulated viewport after a navigation audit
([#3189](#3189))
([c777ece](c777ece))
* **network:** list only earlier hops in a request's redirect chain
([#3198](#3198))
([4675940](4675940))
* **network:** remove unsupported websocket from list_network_requests
filter
([#2961](#2961))
([#3282](#3282))
([8b119d9](8b119d9))
* **network:** reset page network emulation before applying URL
restrictions
([#3312](#3312))
([f25f3df](f25f3df))
* **pages:** reject devtools:// URLs in navigate_page and new_page
([#2935](#2935))
([#3128](#3128))
([c94c03d](c94c03d))
* **pages:** report the actual page size after resize_page
([#2998](#2998))
([#3132](#3132))
([f003056](f003056))
* **pages:** revert reporting the page size after resize_page
([#3308](#3308))
([186ea66](186ea66)),
closes
[#3306](#3306)
* **pagination:** return all items when pageSize is omitted and pageIdx
is 0
([#2996](#2996))
([#3134](#3134))
([3ad9115](3ad9115))
* **performance:** populate structuredContent for traceInsight in
performance_analyze_insight
([#2981](#2981))
([#3115](#3115))
([c2c6cc4](c2c6cc4))
* preserve unread page fallback across snapshots and deduplicate page
disposal
([#3161](#3161))
([8604365](8604365))
* print the no-roots temp-directory warning under default flags
([#2927](#2927))
([0dd8ed9](0dd8ed9))
* reject empty, extra, and unknown parts in geolocation and viewport
transforms
([#3097](#3097))
([#3120](#3120))
([3ea31fb](3ea31fb))
* resolve error causes on the session that reported the exception
([#3220](#3220))
([b2d37e8](b2d37e8))
* resolve service worker IDs for console messages
([#2881](#2881))
([3804a9a](3804a9a))
* return message when page exposes no 3p developer tools
([#3197](#3197))
([663f3c3](663f3c3)),
closes
[#3004](#3004)
* **script:** surface non-Error thrown and rejected values in
evaluate_script
([#2951](#2951))
([#3127](#3127))
([b1e6e68](b1e6e68))
* **security:** validate filesystem paths that parse as http/ws URLs
([#3186](#3186))
([91e112d](91e112d))
* **snapshot:** omit uid for accessibility nodes without backendNodeId
([#3187](#3187))
([9054351](9054351))
* stop active screencast on McpPage.dispose()
([#3160](#3160))
([558af81](558af81))
* stop get_network_request waiting forever for an unread no-store body
([#3216](#3216))
([c556be1](c556be1))
* surface nested error causes and autoConnect hint on connection failure
([#3111](#3111))
([d7f1e78](d7f1e78))
* treat 0.0.0.0 and [::] browser endpoints as local
([#3321](#3321))
([7d5f7de](7d5f7de))
* treat IPv4-mapped IPv6 loopback as localhost
([#3255](#3255))
([6cfb34c](6cfb34c))
* truncate data: URLs in accessibility snapshots
([#3292](#3292))
([11e7d47](11e7d47))
* validate type_text submitKey before typing
([#3185](#3185))
([c8615d7](c8615d7))
* **wait_for:** find text in dynamically attached iframes
([#3210](#3210))
([5f84888](5f84888))
* warn when screenshot dimensions exceed 16384px limit
([#2952](#2952))
([#3291](#3291))
([10455b7](10455b7))


### 📄 Documentation

* add a pre-submit checklist to AGENTS.md
([#3236](#3236))
([e96ee6c](e96ee6c))
* clarify URL pattern port and hostname matching
([#3287](#3287))
([4723335](4723335))
* **config:** note that hot-reloaded URL restrictions keep open targets
attached
([#3314](#3314))
([47db2b8](47db2b8))
* correct CLI availability of extension tools
([#2852](#2852))
([a7280ee](a7280ee))
* explain how to install skills, including the CLI skill
([#3274](#3274))
([46eb546](46eb546))
* remove experimental status from CLI and promote it in README
([#3273](#3273))
([0ced16b](0ced16b))
* require a rebase only when the branch has merge conflicts
([#3239](#3239))
([d1143ac](d1143ac))
* **skills:** clarify Lighthouse score scale and contrast workflow
([#3006](#3006))
([#3218](#3218))
([bf8c6d9](bf8c6d9))


### ⚡ Performance

* **cli:** load a small third-party bundle in the chrome-devtools CLI
([#3200](#3200))
([552e1f8](552e1f8))
* load the Lighthouse bundle on first use
([#3169](#3169))
([798f03a](798f03a))


### 🏗️ Refactor

* **config:** isolate scripts and unit tests from home config discovery
([#3256](#3256))
([65e6b97](65e6b97))
* **config:** lock slim and pageIdRouting options when viaCli is set
([#3257](#3257))
([384d9aa](384d9aa))
* **config:** pass ReadonlyServerConfig to BrowserManager and add
OptionGate
([#3267](#3267))
([98302f2](98302f2))
* **config:** simplify watchConfig and start watcher inside connect
([#3259](#3259))
([878c5f6](878c5f6))
* extract errorResult() and routedPageId() in ToolHandler
([#3251](#3251))
([9a6b03a](9a6b03a))
* import types and values from a module in one statement
([#3303](#3303))
([f4eaf1f](f4eaf1f))
* **lighthouse:** move Lighthouse runner methods to McpPage and add
bundle types
([#3283](#3283))
([e1823a2](e1823a2))
* list optional properties last in inline object types
([#3302](#3302))
([dc929c0](dc929c0))
* register all tools upfront across modes
([#3163](#3163))
([58f8d1f](58f8d1f))
* **response:** pass live ReadonlyServerConfig to McpResponse
([#3258](#3258))
([6042490](6042490))
* **tools:** add explicit factory types to tool definitions
([#3284](#3284))
([4d1d7b2](4d1d7b2))
* use #private names instead of the private modifier
([#3301](#3301))
([2e13311](2e13311))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

Verbose take_snapshot gives every InlineTextBox the same uid, and that uid resolves to nothing

2 participants