Fix web autofill not picking up values after Windows Hello authentication - #184673
Fix web autofill not picking up values after Windows Hello authentication#184673rich22222 wants to merge 1 commit into
Conversation
github-actions
Bot
added
a: text input
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
9759788 to
eb113e9
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a 'change' event listener to the autofill form to handle asynchronous credential fills, such as those from Windows Hello, and includes a corresponding regression test. Feedback suggests modifying the event handler to process 'change' events even when the element is focused and recommends reusing the event listener instance to improve efficiency.
| void onAutofillEvent(DomEvent e) { | ||
| if (items[key] == null) { | ||
| throw StateError('AutofillInfo must have a valid uniqueIdentifier.'); | ||
| } else if (key != focusedElementId) { | ||
| // `input` events on the focused element are handled elsewhere. | ||
| final AutofillInfo autofillInfo = items[key]!.autofillInfo; | ||
| _handleChange(element, autofillInfo); | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation skips the event if the element is focused (key == focusedElementId). While input events on the focused element are typically handled by the main text editing strategy, the PR description notes that Windows Hello authentication may fire change instead of input. If the main strategy does not listen for change events, the focused element will miss the update. Consider allowing change events to be processed here even for the focused element to ensure the fix covers all fields in the autofill group.
| void onAutofillEvent(DomEvent e) { | |
| if (items[key] == null) { | |
| throw StateError('AutofillInfo must have a valid uniqueIdentifier.'); | |
| } else if (key != focusedElementId) { | |
| // `input` events on the focused element are handled elsewhere. | |
| final AutofillInfo autofillInfo = items[key]!.autofillInfo; | |
| _handleChange(element, autofillInfo); | |
| } | |
| } | |
| void onAutofillEvent(DomEvent e) { | |
| if (items[key] == null) { | |
| throw StateError('AutofillInfo must have a valid uniqueIdentifier.'); | |
| } else if (e.type == 'change' || key != focusedElementId) { | |
| // `input` events on the focused element are handled elsewhere. | |
| // We also handle `change` events here to catch async fills that | |
| // might not fire `input` (e.g. Windows Hello). | |
| final AutofillInfo autofillInfo = items[key]!.autofillInfo; | |
| _handleChange(element, autofillInfo); | |
| } | |
| } |
| subscriptions.add( | ||
| DomSubscription(element, 'input', createDomEventListener(onAutofillEvent)), | ||
| ); | ||
| // The 'change' event is more reliably fired after async credential | ||
| // fills (e.g., after Windows Hello authentication in Chrome). | ||
| // See https://github.com/flutter/flutter/issues/162066 | ||
| subscriptions.add( | ||
| DomSubscription( | ||
| element, | ||
| 'input', | ||
| createDomEventListener((DomEvent e) { | ||
| if (items[key] == null) { | ||
| throw StateError('AutofillInfo must have a valid uniqueIdentifier.'); | ||
| } else if (key != focusedElementId) { | ||
| // `input` events on the focused element are handled elsewhere. | ||
| final AutofillInfo autofillInfo = items[key]!.autofillInfo; | ||
| _handleChange(element, autofillInfo); | ||
| } | ||
| }), | ||
| ), | ||
| DomSubscription(element, 'change', createDomEventListener(onAutofillEvent)), | ||
| ); |
There was a problem hiding this comment.
You can avoid redundant calls to createDomEventListener by creating the listener once and reusing it for both subscriptions. This improves efficiency and readability.
| subscriptions.add( | |
| DomSubscription(element, 'input', createDomEventListener(onAutofillEvent)), | |
| ); | |
| // The 'change' event is more reliably fired after async credential | |
| // fills (e.g., after Windows Hello authentication in Chrome). | |
| // See https://github.com/flutter/flutter/issues/162066 | |
| subscriptions.add( | |
| DomSubscription( | |
| element, | |
| 'input', | |
| createDomEventListener((DomEvent e) { | |
| if (items[key] == null) { | |
| throw StateError('AutofillInfo must have a valid uniqueIdentifier.'); | |
| } else if (key != focusedElementId) { | |
| // `input` events on the focused element are handled elsewhere. | |
| final AutofillInfo autofillInfo = items[key]!.autofillInfo; | |
| _handleChange(element, autofillInfo); | |
| } | |
| }), | |
| ), | |
| DomSubscription(element, 'change', createDomEventListener(onAutofillEvent)), | |
| ); | |
| final DomEventListener listener = createDomEventListener(onAutofillEvent); | |
| subscriptions.add(DomSubscription(element, 'input', listener)); | |
| // The 'change' event is more reliably fired after async credential | |
| // fills (e.g., after Windows Hello authentication in Chrome). | |
| // See https://github.com/flutter/flutter/issues/162066 | |
| subscriptions.add(DomSubscription(element, 'change', listener)); |
References
- Optimize for readability: Code is read more often than it is written. (link)
…tion Listen for the 'change' DOM event in addition to 'input' on autofill form elements. When Chrome's password manager is protected by Windows Hello, the async biometric prompt causes credential fills that fire 'change' rather than 'input', so Flutter never received the updated values. The 'change' event is also handled for the focused element, since the main text editing strategy only listens for 'input' and would miss async credential fills on the active field. Fixes flutter#162066
eb113e9 to
2405465
Compare
|
I have a feeling the cause may be similar to what we fixed on iOS 26: #182024 On iOS, there's a similar autofill authentication popup, and it caused the input to lose focus temporarily and later gain focus, and Flutter didn't know how to handle that. Would you mind checking if the focused input is receiving a The fix I made in that PR was limited to iOS, but you may want to expand it to other platforms and see if that helps. Here's the relevant piece of code: |
flutter-zl
added
the
waiting for response
|
This pull request is being closed because it has not been updated in the last 21 days after a request for more information. If you are still working on this, please feel free to reopen it or file a new pull request. Thanks for your contribution. |
5 participants
Summary
Fixes #162066
Test plan
Pre-launch Checklist
Generated with Claude Code