-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[iOS] Fix view removal process for AutofillContextAction.cancel
#57209
base: main
Are you sure you want to change the base?
Conversation
@@ -2675,8 +2675,7 @@ - (void)hideTextInput { | |||
[self removeEnableFlutterTextInputViewAccessibilityTimer]; | |||
_activeView.accessibilityEnabled = NO; | |||
[_activeView resignFirstResponder]; | |||
[_activeView removeFromSuperview]; | |||
[_inputHider removeFromSuperview]; | |||
[self cleanUpViewHierarchy:YES clearText:NO delayRemoval:NO]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the comment here my assumption is that we should be setting clearText:YES
in this case.
@LongCatIsLooong can you confirm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been a while since I looked at this. Is there a quick explanation of what is supposed to happen here? I think this will be called when the framework dismisses the software keyboard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not making a lot sense to me right now that cleanUpViewHierarchy
needs to be called when the framework dismisses the keyboard. A bit explanation as to why the bug is happening (is the framework dismissing the keyboard before the autofill group cleans up the view hierarchy?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @koji-1009 thanks for your contribution. To avoid a future regression, please add a test. If you need suggestions on how to write it or where it should be wired up, please let us know and we'll be glad to point you in the right direction! Take a look at shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm for examples.
@cbracken |
The
removeFromSuperview
in thehideTextInput
method was triggering the save password, so I changed it tocleanUpViewHierarchy
.fix flutter/flutter#145681
sample app code
https://github.com/koji-1009/autofill_cancel_issue
before
before.mp4
after
after.mp4
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.