Skip to content

[PM-39612] fix: resolve autofill container-hint redirect to autofillable child#7146

Merged
aj-rosado merged 5 commits into
mainfrom
PM-39612/fix-autofill-container-hint-redirect
Jul 9, 2026
Merged

[PM-39612] fix: resolve autofill container-hint redirect to autofillable child#7146
aj-rosado merged 5 commits into
mainfrom
PM-39612/fix-autofill-container-hint-redirect

Conversation

@aj-rosado

Copy link
Copy Markdown
Contributor

🎟️ Tracking

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

📔 Objective

Some Android apps wrap their login EditText fields inside a container view (e.g., Material TextInputLayout). The container carries the semantic autofill hint (USERNAME, PASSWORD) but has autofillType = NONE (0), meaning it cannot receive a fill value. The inner EditText has autofillType = TEXT (1) but no hint, so heuristics never classify it. As a result, autofill silently failed — the fill dataset was built against the container's autofillId with autofillType = 0, which the framework dropped at fill time.

Changes:

  • ViewNodeExtensions.kt: When a container node carries a semantic hint but has autofillType = NONE, findFirstAutofillableChild() locates the first descendant with autofillType = TEXT. The child's autofillId and autofillType are used in AutofillView.Data so the fill reaches the actual input field.
  • AutofillParserImpl.kt: Added claimedAutofillIds tracking during traversal to prevent a child node from being added a second time after the container has already redirected to it.

@aj-rosado aj-rosado added the ai-review Request a Claude code review label Jul 7, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug labels Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the autofill container-hint redirect fix across ViewNodeExtensions.kt (redirect to first autofillable child when a hint-bearing container has AUTOFILL_TYPE_NONE) and AutofillParserImpl.kt (claimed-ID tracking to prevent duplicate views after redirect). Traced the traversal dedup logic for direct-child, nested-grandchild, and no-autofillable-child fallback paths, plus the ignoreAutofillIds filtering that keeps a redirected child from being marked ignored. The change is well-scoped, correctly ordered (self claims before children are filtered), and covered by targeted unit tests for each path.

Code Review Details

No blocking findings.

The two existing reviewer threads (the view property extraction in ViewNodeExtensions.kt and the filter-branch simplification suggestion in AutofillParserImpl.kt) are style/preference discussions already in progress; no additional inline comments were added to avoid duplication.

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.73684% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.44%. Comparing base (05b7e27) to head (48f48dd).

Files with missing lines Patch % Lines
...bitwarden/data/autofill/util/ViewNodeExtensions.kt 90.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7146      +/-   ##
==========================================
- Coverage   86.49%   86.44%   -0.06%     
==========================================
  Files         908      912       +4     
  Lines       65230    65516     +286     
  Branches     9600     9610      +10     
==========================================
+ Hits        56422    56634     +212     
- Misses       5365     5437      +72     
- Partials     3443     3445       +2     
Flag Coverage Δ
app-data 17.42% <94.73%> (+0.11%) ⬆️
app-ui-auth-tools 18.62% <0.00%> (-0.01%) ⬇️
app-ui-platform 16.61% <0.00%> (+0.03%) ⬆️
app-ui-vault 28.04% <0.00%> (-0.10%) ⬇️
authenticator 6.17% <0.00%> (+0.01%) ⬆️
lib-core-network-bridge 4.05% <0.00%> (+<0.01%) ⬆️
lib-data-ui 1.14% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aj-rosado
aj-rosado marked this pull request as ready for review July 7, 2026 15:15
@aj-rosado
aj-rosado requested review from a team and david-livefront as code owners July 7, 2026 15:15
@aj-rosado aj-rosado added t:misc Change Type - ¯\_(ツ)_/¯ and removed t:misc Change Type - ¯\_(ツ)_/¯ labels Jul 8, 2026
autofillHint = this.supportedAutofillHint,
autofillHint = hint,
)
return view

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.

Why did we add the view property, can we leave it as a return?

if (view !is AutofillView.Unused) {
claimedAutofillIds.add(id)
}
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.

What do you think of this?

val id = view.data.autofillId
if (id in claimedAutofillIds) {
    false
} else if (view !is AutofillView.Unused) {
    claimedAutofillIds.add(id)
    true
} else {
    true
}

@aj-rosado
aj-rosado requested a review from david-livefront July 9, 2026 16:55
@aj-rosado
aj-rosado added this pull request to the merge queue Jul 9, 2026
@aj-rosado

Copy link
Copy Markdown
Contributor Author

Thank you @david-livefront

Merged via the queue into main with commit 7a88c4b Jul 9, 2026
23 checks passed
@aj-rosado
aj-rosado deleted the PM-39612/fix-autofill-container-hint-redirect branch July 9, 2026 17:26
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 app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants