Skip to content

[PM-34195]Apply readonly guards on autofill#20237

Merged
dan-livefront merged 6 commits intomainfrom
PM-34195-apply-readonly-guards-on-autofill
Apr 28, 2026
Merged

[PM-34195]Apply readonly guards on autofill#20237
dan-livefront merged 6 commits intomainfrom
PM-34195-apply-readonly-guards-on-autofill

Conversation

@dan-livefront
Copy link
Copy Markdown
Collaborator

@dan-livefront dan-livefront commented Apr 17, 2026

🎟️ Tracking

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

📔 Objective

Applying the same guards implemented on https://bitwarden.atlassian.net/browse/PM-33431 while cleaning up the isReadonlyOrDisabledFormFieldElement function.

@dan-livefront dan-livefront self-assigned this Apr 17, 2026
@dan-livefront dan-livefront added the ai-review Request a Claude code review label Apr 17, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 17, 2026

Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR extracts the private isReadonlyOrDisabledElement from AutofillOverlayContentService into a shared isReadonlyOrDisabledFormFieldElement utility and reuses it in InsertAutofillContentService.insertValueIntoField, adding the missing aria-readonly guard at the insertion call site (PM-34195). The refactor improves type safety by replacing casts with proper type guards (elementIsInputElement/elementIsTextAreaElement) before accessing readOnly, introduces an AutofillFieldReadonlyDisabledState Pick type for cached metadata, and includes targeted unit tests for both the new utility and the new aria-readonly skip behavior.

Behavior was traced across both call sites: the previous (elementCanBeFilled && element.disabled) skip for <select disabled> is preserved because getPropertyOrAttribute reads the disabled property first, and the aria-readonly check correctly uses checkString=true so only the literal string "true" triggers.

Code Review Details

No findings.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2026

Codecov Report

❌ Patch coverage is 69.23077% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.03%. Comparing base (c9005d6) to head (dc887e9).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ofill/services/autofill-overlay-content.service.ts 20.00% 0 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #20237      +/-   ##
==========================================
- Coverage   47.03%   47.03%   -0.01%     
==========================================
  Files        3921     3921              
  Lines      118499   118499              
  Branches    18130    18128       -2     
==========================================
- Hits        55737    55733       -4     
- Misses      58575    58579       +4     
  Partials     4187     4187              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dan-livefront dan-livefront requested a review from blackwood April 17, 2026 15:54
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 17, 2026

Logo
Checkmarx One – Scan Summary & Details4e2974fd-bdb0-46d9-ae9e-47e389862d2d

Great job! No new security vulnerabilities introduced in this pull request

@dan-livefront dan-livefront marked this pull request as ready for review April 17, 2026 16:09
@dan-livefront dan-livefront requested a review from a team as a code owner April 17, 2026 16:09
@sonarqubecloud
Copy link
Copy Markdown

@dan-livefront dan-livefront merged commit 902aa3a into main Apr 28, 2026
67 checks passed
@dan-livefront dan-livefront deleted the PM-34195-apply-readonly-guards-on-autofill branch April 28, 2026 17:16
@bw-ghapp
Copy link
Copy Markdown
Contributor

bw-ghapp Bot commented Apr 28, 2026

Changes in this PR impact the Autofill experience of the browser client

BIT has tested the core experience with these changes and the feature flag configuration used by vault.bitwarden.com.

✅ Fortunately, these BIT tests have passed! 🎉

@bw-ghapp
Copy link
Copy Markdown
Contributor

bw-ghapp Bot commented Apr 28, 2026

Changes in this PR impact the Autofill experience of the browser client

BIT has tested the core experience with these changes and all feature flags disabled.

✅ Fortunately, these BIT tests have passed! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants