Skip to content

fix: forward boxes option in aria snapshots - #3356

Open
Mihail Galay (GalayM) wants to merge 4 commits into
microsoft:mainfrom
GalayM:fix/aria-snapshot-boxes
Open

Mihail Galay (GalayM) wants to merge 4 commits into
microsoft:mainfrom
GalayM:fix/aria-snapshot-boxes

Conversation

@GalayM

@GalayM Mihail Galay (GalayM) commented Sep 12, 2026

Copy link
Copy Markdown

Fixes #3355.

Boxes = true is currently ignored by both Page.AriaSnapshotAsync() and Locator.AriaSnapshotAsync(). Forward the option to the driver so snapshots include the documented bounding boxes.

The tests cover both methods in Default and Ai modes, check exact geometry, and verify that omitted or false options leave boxes out, including omission after a call with Boxes = true.

Also move ResolveNpmExecutable above the instance methods to fix the existing SA1204 failure blocking the code-style check. Its implementation is unchanged.

Tested on Windows / .NET 8:

  • All four new cases fail before the fix.
  • All 12 PageAriaSnapshotTests pass in Chromium, Firefox, and WebKit after the fix.
  • dotnet build ./src --no-restore and dotnet format ./src/ --verify-no-changes --no-restore pass.

Copilot AI lite review requested due to automatic review settings September 12, 2026 21:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The fix is small and targeted (forwards the missing option) with solid regression tests, with only minor test-assertion refinements suggested.

Pull request overview

This PR fixes a bug where Boxes = true was ignored by Page.AriaSnapshotAsync() and Locator.AriaSnapshotAsync() by forwarding the option to the driver, and adds regression coverage to ensure bounding boxes appear when requested.

Changes:

  • Forward options.Boxes as boxes in the ariaSnapshot driver request for both Page and Locator snapshots.
  • Add new tests covering Boxes behavior for both Page and Locator in Default and Ai modes.
File summaries
File Description
src/Playwright/Core/Page.cs Forwards Boxes option to the ariaSnapshot request payload.
src/Playwright/Core/Locator.cs Forwards Boxes option to the ariaSnapshot request payload for locators.
src/Playwright.Tests/PageAriaSnapshotTests.cs Adds regression tests ensuring bounding boxes appear only when Boxes = true for both APIs and modes.
Review details

Suppressed comments (1)

src/Playwright.Tests/PageAriaSnapshotTests.cs:59

  • The final assertion repeats the earlier Boxes = false check, so it doesn’t verify the behavior when Boxes is omitted after a Boxes = true call. Consider changing this assertion to call AriaSnapshotAsync without Boxes to ensure omission still produces a snapshot without boxes (and that the option isn’t sticky).
        Assert.AreEqual(snapshot, await locator.AriaSnapshotAsync(new() { Mode = mode, Boxes = false }));

        var snapshotWithBoxes = await locator.AriaSnapshotAsync(new() { Mode = mode, Boxes = true });
        StringAssert.Contains("[box=20,30,100,40]", snapshotWithBoxes);
        Assert.AreEqual(snapshot, await locator.AriaSnapshotAsync(new() { Mode = mode, Boxes = false }));
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.


var snapshotWithBoxes = await Page.AriaSnapshotAsync(new() { Mode = mode, Boxes = true });
StringAssert.Contains("[box=20,30,100,40]", snapshotWithBoxes);
Assert.AreEqual(snapshot, await Page.AriaSnapshotAsync(new() { Mode = mode, Boxes = false }));
@GalayM

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

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.

[Bug]: AriaSnapshotAsync ignores Boxes in Page and Locator

2 participants