[iOS] Fix view removal process for AutofillContextAction.cancel

[koji-1009]

The removeFromSuperview in the hideTextInput method was triggering the save password, so I changed it to cleanUpViewHierarchy.

fix flutter/flutter#145681

sample app code

https://github.com/koji-1009/autofill_cancel_issue

before

before.mp4

after

after.mp4

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[cbracken]

Based on the comment here my assumption is that we should be setting clearText:YES in this case.

@LongCatIsLooong can you confirm?

[LongCatIsLooong]

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?

[LongCatIsLooong]

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?)

[koji-1009]

In the apps I put in the description,

1. push from screen1 to screen2
2. focus on TextField (password)
3. pop from screen2 to screen1

then MethodChannel is called as follows.

TextInput.setClient
TextInput.setEditableSizeAndTransform
TextInput.setMarkedTextRect
TextInput.setStyle
TextInput.setEditingState
TextInput.show
TextInput.requestAutofill
TextInput.setCaretRect
TextInput.clearClient
TextInput.hide
TextInput.finishAutofillContext

---

I noticed that in TextInput.finishAutofillContext the auto fill process is expected to be performed, but is actually performed in TextInput.hide. When either _activeView or _inputHider was removeFromSuperview, the process would run.

removeFromSuperView has been added at #23776, but if it is not needed I will add a commit to remove it.

[LongCatIsLooong]

Thanks for the explanation!

@chunhtai do you remember why the active field is removed from the view hierarchy when the keyboard is dismissed? iOS autofill will try to save the password if the active field is a password field so we should probably keep the field in the view hierarchy if possible so iOS doesn't think it's the best time to show the "save password" prompt.

[cbracken]

+1 to keeping it in the view hierarchy unless there's a very good reason not to. That's also likely the path of least surprise for Apple APIs. In non-Flutter apps, it'd be pretty unexpected to add and remove text fields based on focus.

If I were to make a guess as to why we do this, it's likely that it's so that we can prune it in and out of the tree as the user focuses and unfocuses each of the Flutter text fields in the app. Keeping it in the tree only as necessary avoids issues like keeping track of position and whether it's onscreen or offscreen during scrolls etc.

[chunhtai]

A quick digging lead me to my previous comment
flutter/flutter#71885 (comment)

We want to hide NativeField because both FlutterField and NativeField will produce accessibility node in voiceover. We want the FlutterField to be the only accessibility node in voice over because it contains all the information about rect, label, value.

My guess is I was trying to avoid the UITextinput from receiving a11y focus. if we keep it attached, we need to make sure the UITextInput won't receive a11y focus when swiping left or right

[cbracken]

Sounds like next step is for @koji-1009 to add a little detail as to why this is necessary (based on the discussion here) in comment.

@LongCatIsLooong once that is done, any objection to this landing?

[koji-1009]

I added a comment. 8f1fe67

[cbracken]

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.

[koji-1009]

@cbracken
Thanks for the review. I'm just going on vacation and will try to add the test code.

[koji-1009]

The results of the test run are shown here.
flutter/flutter#160653 (comment)

[cbracken]

Thanks for adding the test!

[cbracken]

[cbracken]

Oh wait. Just noticed this is open against the flutter/engine repo.

@koji-1009 in late December, we merged the engine repo into the flutter/flutter repo. Would you mind re-opening this patch against that repo, link to this one in the description, and I can re-approve over there.

Apologies for the confusion!

[koji-1009]

@cbracken
I have updated flutter/flutter#160653 from draft to open! Thanks for the review!