Skip to content

fix(aria): annotate aria-hidden elements in AI snapshots - #42268

Merged
Dmitry Gozman (dgozman) merged 1 commit into
microsoft:mainfrom
triemerge:fix/aria-hidden-annotation
Aug 20, 2026
Merged

fix(aria): annotate aria-hidden elements in AI snapshots#42268
Dmitry Gozman (dgozman) merged 1 commit into
microsoft:mainfrom
triemerge:fix/aria-hidden-annotation

Conversation

@triemerge

@triemergeAditya Kumar Gupta (triemerge) commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Rationale

The AI-mode snapshot (mode: "ai") intentionally includes aria-hidden elements because they are visually visible, but nothing distinguishes them from ARIA-visible content. When a test fails and error-context.md is generated, agents (and humans) see content that getByRole and ariaSnapshot() will never find, with no indication why.

This adds an [aria-hidden] annotation following the same bracket pattern as [checked], [disabled], [expanded] etc. Elements included only through visual visibility (not ARIA visibility) now render with [aria-hidden] in the key. Default-mode snapshots are completely unaffected since they exclude these elements entirely.

Fixes#42223


const childAriaNode = visible ? toAriaNode(element, options, nameSourceElements) : null;
if (childAriaNode && !isElementVisibleForAria)
childAriaNode.props['aria-hidden'] = 'true';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It annotates every node in a hidden subtree, not just the boundary. This will bloat snapshots in the scenario like the one in the issue.

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.

+1. Let's also add a test that covers this, to make sure we only annotate the top element in the hidden subtree.

@triemergeAditya Kumar Gupta (triemerge)Aug 19, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed, checks the element's own attribute now.

@triemergeAditya Kumar Gupta (triemerge)Aug 19, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Let's also add a test that covers this, to make sure we only annotate the top element in the hidden subtree.

added


// AI snapshot includes aria-hidden elements but annotates them.
const aiSnapshot = await snapshotForAI(page);
expect(aiSnapshot).toContain('[aria-hidden]');

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 assert the exact snapshot with toContainYaml like other tests here.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done.


const childAriaNode = visible ? toAriaNode(element, options, nameSourceElements) : null;
if (childAriaNode && !isElementVisibleForAria)
childAriaNode.props['aria-hidden'] = 'true';

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.

+1. Let's also add a test that covers this, to make sure we only annotate the top element in the hidden subtree.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

21 flaky⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:294 › screencast › should capture navigation `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:736 › screencast › should work with video+trace `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/chromium/chromium.spec.ts:373 › should produce network events, routing, and annotations for Service Worker `@realtime-time-library-chromium-linux`
⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@realtime-time-library-chromium-linux`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@realtime-time-library-chromium-linux`
⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@chromium-ubuntu-22.04-node22`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@chromium-ubuntu-22.04-node22`
⚠️ [firefox-library] › library/global-fetch.spec.ts:293 › should return security details from response `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-library] › library/har.spec.ts:639 › should have security details `@firefox-ubuntu-22.04-node20`
⚠️ [webkit-library] › library/global-fetch.spec.ts:293 › should return security details from response `@webkit-ubuntu-22.04-node20`
⚠️ [webkit-library] › library/har.spec.ts:639 › should have security details `@webkit-ubuntu-22.04-node20`
⚠️ [webkit-page] › page/page-set-input-files.spec.ts:38 › should upload a folder `@webkit-ubuntu-22.04-node20`

51215 passed, 1233 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

3 failed
❌ [chrome] › mcp/config.spec.ts:203 › chromiumSandbox › --sandbox enables the sandbox @mcp-ubuntu-latest-chrome
❌ [firefox] › mcp/annotate.spec.ts:386 › should cancel browser_annotate when the MCP request is aborted @mcp-windows-latest-firefox
❌ [firefox] › mcp/cli-core.spec.ts:57 › click link @mcp-windows-latest-firefox

8120 passed, 1341 skipped


Merge workflow run.

@dgozmanDmitry Gozman (dgozman) 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.

Looks good to me, thank you.

Sign up for freeto 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.

[Bug]: error-context.md Page snapshot includes content excluded by ariaSnapshot()

3 participants

@triemerge@yury-s@dgozman