[PM-29014] Update BitwardenPasswordField semantics to support accessibility for visible passwords

[SaintPatrck]

🎟️ Tracking

PM-29014
Relates to #6212

📔 Objective

Improve accessibility support in BitwardenPasswordField by ensuring Talkback can read the password aloud when the user chooses to make it visible, while explicitly marking the content as sensitive data.

Behavioral changes:

• When the "show password" toggle is active, Talkback will now read the actual password text instead of silence or obfuscated characters. This behavior occurs regardless of the system-wide "Speak Passwords" setting due to the custom visual transformation usage.
• The field is semantically marked as containing sensitive data.

Specific changes:

• Apply a semantics modifier to the internal TextField in BitwardenPasswordField.
• Set contentDescription to the actual text value only when showPassword is true; otherwise, it defaults to an empty string.
• Set the isSensitiveData semantic property to true.

📸 Screenshots

---

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[github-actions]

Checkmarx One – Scan Summary & Details – 82744e52-e682-4da1-8a1f-f745ac7b5af3

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

[claude]

Claude finished @SaintPatrck's task —— View job

---

Code Review Complete

I've reviewed the accessibility changes to BitwardenPasswordField. Overall, this is a well-implemented enhancement with excellent code quality and documentation. Here's my detailed analysis:

---

🎯 Overall Assessment

👍 Great addition - This change addresses an important accessibility gap for Talkback users who need to verify passwords when they're visible.

---

Security Analysis

⚠️ Critical Discussion Point: System Setting Override

Lines 239-241 of BitwardenPasswordField.kt: The comment correctly identifies that this overrides the user's "Speak Passwords" system setting.

Concern: Users who have disabled "Speak Passwords" for privacy/security reasons will still have passwords spoken aloud when visible. This could be problematic in shared environments (offices, public spaces) and violates the principle of least surprise.

Questions:

• ❓ Is this the intended behavior? Should we respect the user's "Speak Passwords" preference?
• ❓ Have you considered adding a Bitwarden-specific setting to control this behavior?
• 💭 Could this create a security concern where users expect passwords to remain silent based on their system settings?

👍 Correct Use of isSensitiveData

Line 245: Setting isSensitiveData = true is appropriate and follows Android best practices. This ensures proper handling by accessibility services and excludes content from screenshots on supported Android versions.

---

Accessibility Implementation

👍 Technically Sound Solution

Lines 235-244: The implementation correctly solves the VisualTransformation issue. The logic after the fix in commit da5776363 is correct:

contentDescription = textFieldValue.text
    .takeIf { showPassword }
    .orEmpty()

• ✅ When showPassword = true: Talkback reads the password
• ✅ When showPassword = false: Content description is empty
• ✅ Proper use of Kotlin idioms

The initial commit incorrectly used takeUnless { !showPassword } (double negation), but this was properly fixed.

---

Code Style & Documentation

👍 Exemplary Inline Comments

Lines 235-241: The comments follow the project's style guidelines perfectly:

• ✅ Proper formatting (//, capitalization, punctuation)
• ✅ Clearly explain both the "what" and the "why"
• ✅ Include important NOTE about system setting override

This is excellent documentation that future developers will appreciate.

---

Architecture & Best Practices

👍 Follows Compose Best Practices

• ✅ Uses .semantics { } modifier correctly
• ✅ Good modifier chain ordering (semantics before other modifiers)
• ✅ Consistent with :ui module patterns (reusable component)
• ✅ No coupling to business logic

🤔 Performance Consideration

Line 242: textFieldValue.text is read inside the semantics block, causing recalculation on every text change.

Analysis: This is likely acceptable for password fields (typically shorter text), and Compose should handle this efficiently. Verdict: Minor concern, but likely fine in practice.

---

Testing & Future Improvements

🌱 Consider Adding Tests

No test file currently exists for BitwardenPasswordField. Consider adding accessibility tests to verify:

• Semantics properties are set correctly for both showPassword states
• contentDescription matches text value when visible
• isSensitiveData is always true

💭 Consistency Question

Should this pattern also apply to other sensitive fields with custom visual transformations (PIN inputs, seed phrases, etc.)?

🎨 Potential Enhancement

For some character sets or special characters, Talkback's pronunciation might be unclear. Future consideration: Could we provide phonetic hints or context like "Password visible: [text]"?

---

Summary & Recommendations

Category	Status	Notes
Security	⚠️ Needs Discussion	System setting override should be validated
Accessibility	👍 Correct	Implementation is technically sound
Code Style	👍 Excellent	Comments are exemplary
Architecture	👍 Good	Follows project patterns
Testing	🌱 Future Work	Consider adding accessibility tests

---

Recommended Actions

Before merge:

1. ❓ Clarify & confirm that overriding "Speak Passwords" is the desired behavior from UX/security perspective
2. 📝 Consider documenting this behavior in user-facing accessibility documentation

Future improvements:

1. 🌱 Add unit tests for semantics properties
2. 🌱 Consider adding an in-app setting for Talkback password reading
3. 🌱 Evaluate applying similar patterns to other sensitive fields

---

Final Verdict

This is a well-implemented accessibility enhancement with excellent code quality and documentation. The code itself is ready from a technical perspective. The primary consideration is the behavioral decision to override system settings - if that's been validated by product/UX, this is good to merge. 👍

---

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.38%. Comparing base (2c01abd) to head (da57763).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6222   +/-   ##
=======================================
  Coverage   85.38%   85.38%           
=======================================
  Files         755      755           
  Lines       54109    54112    +3     
  Branches     7795     7797    +2     
=======================================
+ Hits        46203    46206    +3     
  Misses       5195     5195           
  Partials     2711     2711           

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[SaintPatrck]

Holding for product review & approval. CC @maxkpower