It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

@googlebot I signed it!

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Hey I am very new to contributing , I have read documentations but I am still confused that how to proceed

Looks like this change is causing some tests to fail: https://cirrus-ci.com/task/4806069081866240. This can either mean you're breaking existing functionality or the tests need to be updated. Can you please take a look at those?

Also, in order to accept this PR you will have to write a test to ensure that we never regress and accidentally re-introduce the bug into our code base. Can you please add some tests?

/cc @justinmc

Yeah I just forgot to put this into WIP.

@cv692001 I'm not sure if you're asking in relation to this PR, but if you're just asking about contributing in general, feel free to reach out to me via the contact info in my Github profile.

@goderbauer @justinmc I have a question about using the tester to simulate the composing input.

I wrote something like this:

final EditableTextState state =
  tester.state<EditableTextState>(find.byType(EditableText));

state.updateEditingValue(const TextEditingValue(text: '111 1'));
expect(state.currentTextEditingValue.composing, TextRange.empty);

state.updateEditingValue(const TextEditingValue(text: '111 1', composing: TextRange(start: 2, end: 4)));
expect(state.currentTextEditingValue.composing, const TextRange(start: 2, end: 4));

state.updateEditingValue(const TextEditingValue(text: '111 11', composing: TextRange(start: 2, end: 5)));
expect(state.currentTextEditingValue.composing, const TextRange(start: 2, end: 5));

When I tried to add more text with composing like '111 11' (6 characters) it ends up nothing happened. How can I achieve the effect just like the real device's? (Like the screenshot below)

Forget about it. I forgot that I run this on stable, now it works on the new one.

I worry that this is a problem too:

Maybe we shouldn't count composing characters as part of the character limit? Would a native Chinese speaker expect the behavior in the screenshot, or should the character count be zero until the composing characters are committed?

CC @LongCatIsLooong

We should watch out for this causing any undesirable behavior in any existing Flutter apps. For example, with this PR, it's now possible to get an invalid value from a TextField using textFieldController.text. That may cause some Flutter apps to receive invalid values when they previously could not, if they're not checking the valid state of the form.

@justinmc I'll see if I can add a test to ensure it won't happened.

IMO we shouldn't specialize LengthLimitingTextInputFormatter. The fix should be applied to the implementation of that formatter if possible.

@LongCatIsLooong That make sense. But last night I was thinking about this, should we prevent all formatters update while composing?

Other input formatters (even third party ones) may have this issue too, @AlexV525 did you by any chance look into why it crashes?

Basically when the composing editing value is updated by the formatter, the composing TextRange's end is more than the length of the texts.

Let's go back to the crashing screenshot. Such as 12345 (with 5 composing TextRange(start: 3, end: 4)), then type 6, the value was updated to 12346 (with 6 composing). Since then the composing might be TextRange(start: 3, end: 5) which is more than the actual length. At this time, call delete and the substring is failed, and crashed.

After a few hours of investigation, I found that it's hard to land this fix directly on the LengthLimitingTextInputFormatter. Here is a strange behavior I saw:

1. Input aaaa, with the TextField displayed a a a.
2. Input another 'a', now the value in _formatAndSetValue becomes a, which means it's been truncated or updated from the method _handleTextInputInvocation.
3. Continuing input multiple a, then delete when it's over-sized, crashed again.

If we only release the constraint inside the LengthLimitingTextInputFormatter, value won't be truncated when the composing is end and I see no events related.

Maybe it's something with keyboard behavior and the only thing we can do is what we currently have done.

It's done.

Now the changes are both landed on LengthLimitingTextInputFormatter and the _formatAndSetValue, to keep composing status unified.

We should watch out for this causing any undesirable behavior in any existing Flutter apps. For example, with this PR, it's now possible to get an invalid value from a TextField using textFieldController.text. That may cause some Flutter apps to receive invalid values when they previously could not, if they're not checking the valid state of the form.

The last thing is about this concern. For me, technically, it should be the current value whether it's composing or not. I added a test about form validation when the value composing. Although it can return false if the composing value cannot pass the validator, we definitely should keep our eyes on related behaviors.

I've updated the description for behavior changes.

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

Oops, seems run into a CLA issue, can you check this out? @justinmc

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

(knock knock...)