Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

iOS Text Editing Infinite Loop #20160

Merged
merged 22 commits into from
Oct 2, 2020

Conversation

justinmc
Copy link
Contributor

@justinmc justinmc commented Jul 30, 2020

Description

Programmatically modifying text while it is being input causes an infinite loop on iOS, but not on Android.

For example, this is reproduced when setting an onChanged handler and adding a character to the text. See the linked issue for more details.

The reason why Android doesn't cause an infinite loop is that Android batches text changes that come very quickly. This PR solves the problem on iOS by performing a similar batching by debouncing. eliminating the ACK response that the engine was sending back to the framework.

Related Issues

Closes flutter/flutter#61282

Further work in this area:
#21534
flutter/flutter#66864

Tests

I added the following tests:

  • Updated tests that depended on the ACK response.

Breaking Change

Not a breaking change.

@justinmc justinmc self-assigned this Jul 30, 2020
@justinmc justinmc marked this pull request as draft July 30, 2020 21:24
@auto-assign auto-assign bot requested a review from liyuqian July 30, 2020 21:27
@justinmc justinmc force-pushed the ios-range-overflow-infinite-loop branch from ee5d2b0 to 630710e Compare July 30, 2020 21:37
@justinmc justinmc force-pushed the ios-range-overflow-infinite-loop branch from 630710e to a979911 Compare July 30, 2020 23:56
@justinmc justinmc force-pushed the ios-range-overflow-infinite-loop branch from 4cbce78 to 44c766a Compare September 14, 2020 19:07
@justinmc justinmc marked this pull request as ready for review September 14, 2020 19:08
@auto-assign auto-assign bot requested a review from gaaclarke September 14, 2020 19:09
@@ -420,6 +420,7 @@ @implementation FlutterTextInputView {
int _textInputClient;
const char* _selectionAffinity;
FlutterTextRange* _selectedTextRange;
NSDictionary* _latestState;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We reuse FlutterTextInputView so the cache needs to be erased every time the connection changes/resets. Also this needs to be properly deallocated I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright got the deallocation in 9949623. For erasing the state each time the connection changes, would that just be in resetAllClientIds?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FlutterTextInputView.setClientId I think?

@LongCatIsLooong
Copy link
Contributor

Is possible to fix this by holding off the framework -> engine TextEditingValue update until onChange is called? Or there're other manifestations of this bug (other than changing the text editing value in onChange)?

@justinmc
Copy link
Contributor Author

@LongCatIsLooong Good thinking. I just tried playing around with that idea and it didn't seem to work, though (messing with the calls around here).

The reason it doesn't work seems to be that the problem is more with the engine sending updates to the framework too fast. Here's an example:

  • Framework receives "a" from the engine, changes it to "a0", and sends it back to the engine.
  • Framework receives "ab" from the engine before the "a0" ack because the typing was so fast. It changes it to "ab0" and sends that to the engine.
  • Framework receives the engine's ack of its first message, "a0". Comparing that to its latest value, "ab0", it concludes that it needs to call onChanged and another zero is added and sent to the engine: "a00".
  • Framework receives the engine's ack of its second message, "ab0", calls onChanged and sends the engine "ab00".
  • Infinite loop where messages alternate between "a000..." and "ab000..." with increasing zeroes.

Maybe this could be fixed in the framework with a similar sort of batching that is being done in the engine in this PR, or a more sophisticated queue that waits for ack responses from the engine?

@justinmc justinmc merged commit 69cbb2b into flutter:master Oct 2, 2020
@justinmc justinmc deleted the ios-range-overflow-infinite-loop branch October 2, 2020 20:51
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 2, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 2, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 3, 2020
mehmetf pushed a commit that referenced this pull request Oct 7, 2020
Fixes an infinite loop by eliminating an unnecessary engine/framework message.
gspencergoog pushed a commit to gspencergoog/engine that referenced this pull request Oct 20, 2020
Fixes an infinite loop by eliminating an unnecessary engine/framework message.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Easy to trigger TextField onChange infinite loop on iOS
3 participants