Skip to content

add closed shadow dom test#488

Merged
audreyality merged 2 commits into
mainfrom
autofill/pm-35399/add-closed-shadow-dom-test
May 18, 2026
Merged

add closed shadow dom test#488
audreyality merged 2 commits into
mainfrom
autofill/pm-35399/add-closed-shadow-dom-test

Conversation

@audreyality

Copy link
Copy Markdown
Member

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-35399

📔 Objective

Introduce a test that autofills inputs in closed shadow DOMs.

This test only runs the first Autofill scenario because the field within the shadow root cannot be focused by Playwright.

@audreyality audreyality requested a review from a team as a code owner May 8, 2026 16:15
@audreyality audreyality requested a review from jprusik May 8, 2026 16:15
@audreyality

This comment was marked as resolved.

@audreyality audreyality added the ai-review-vnext Request a Claude code review using the vNext workflow label May 8, 2026
@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR adds a new test scenario for autofill into inputs inside closed shadow DOMs at /forms/login/shadow-root-inputs-closed, introducing a verifyAccessor: "value" | "text" field on FillProperties so the spec can read filled values from non-input mirror elements via toHaveText. The autofill spec correctly waits for attached rather than visible for text-mode mirrors (which have no rendered size until populated), and the page entry skips the three test scenarios (InlineMenuAutofill, NewCredentialsNotification, PasswordUpdateNotification) that require focusing the input — a documented Playwright limitation for closed shadow roots. The trailing-slash fix on the existing shadow-root-inputs/ cipher URL is a nice catch — without it, the new shadow-root-inputs-closed page would also match that cipher under UriMatchType.StartsWith.

Code Review Details

No issues found. The change is well-scoped, internally consistent (testPages and pageCiphers stay 1:1), and the JSDoc on verifyAccessor plus the inline comments at the trailing-slash fix and the wait-state branch make the reasoning self-evident.

Comment thread constants/test-pages.ts
Comment on lines +243 to +244
TestNames.NewCredentialsNotification,
TestNames.PasswordUpdateNotification,

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.

Playwright can still use navigational keyboard controls (Tab, Space, Enter) in order to navigate into an inline menu and execute a presumed selection (followed by a form submit via Enter). I have a PoC kicking around somewhere, I'll see if I can find it...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The issue here isn't navigating the overlay. It's navigating into the username/password boxes in the first place. Keyboard controls can still work for that; what I wonder about is whether it's too brittle for the test case, since it depends on the tab order of the page's controls.

@audreyality audreyality merged commit 384a6e3 into main May 18, 2026
29 checks passed
@audreyality audreyality deleted the autofill/pm-35399/add-closed-shadow-dom-test branch May 18, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-vnext Request a Claude code review using the vNext workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants