EditableText does not request focus on autofill #97846
Conversation
flutter-dashboard
bot
added
a: text input
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
Looks like there are a bunch of failures. I think lots of things subtly depend on that requestFocus call.
|
|
||
| // This will show the keyboard for all selection changes on the | ||
| // EditableWidget, not just changes triggered by user gestures. | ||
| requestKeyboard(); |
There was a problem hiding this comment.
This reminds me, I recently wrote an unfinished PR where you and I discussed removing this requestKeyboard altogether: #93333 (comment)
I closed the PR without merging because I didn't think it was possible to do yet without further refactoring, so it's probably fine to still do requestKeyboard below in this PR. Just wanted to point out the relation.
There was a problem hiding this comment.
In that PR, removing this line caused a bunch of test failures, maybe similar to the test failures that are happening now for this PR.
There was a problem hiding this comment.
Yeah I feel requestKeyboard is usually just a side-effect of the user interaction (e.g. ) and shouldn't be bundled with _handleSelectionChanged
There was a problem hiding this comment.
on the original issue, the other textfield grabs the keyboard focus and the autocomplete failed to fill the passowrd field? can you explain more why a autocomplete cannot fill the unfocus textfield in this case?
There was a problem hiding this comment.
Sorry missed the comment. Discussed it offline but for the record this is another case of the race that happens when the platform text input plugin issues 2 consecutive changes. The autofill service first autofills the email field which is not focused by the time and immediately autofills the password field. The change in the email field arrives at the framework first and makes the email field the focus, which in turn triggers an setClient call, erasing the content in the password field in the platform text input plugin.
| await tester.pumpAndSettle(); | ||
|
|
||
| final EditableTextState state2 = tester.state<EditableTextState>(find.byWidget(editableText2)); | ||
| // Update editableText2, should not request focus. |
There was a problem hiding this comment.
Nit: Maybe say "Update editableText2 as if it were autofilled, should not request focus."
chunhtai
left a comment
chunhtai
left a comment
There was a problem hiding this comment.
The change looks fine, but i am curious how does this solve the issue.
|
@chunhtai before the patch when the unfocused username text field gets autofilled it triggers a focus change and resets the current content in the password field (as it calls setClient and what's already in the password field will be erased). |
4 participants
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. You must list at least one issue.
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.