Skip to content
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

Fix multiline TextInput crash when inserting/removing lots of text #29307

Closed
wants to merge 1 commit into from

Conversation

tido64
Copy link
Collaborator

@tido64 tido64 commented Jul 8, 2020

Summary

Multiline TextInput can crash when really long texts are inserted and removed quickly. This is caused by the fact that -[NSAttributedString string] doesn't really return a copy, and may mutate the string while it is being used by convertIdToFollyDynamic. See microsoft#489 (comment) for more details on the issue.

This issue was originally reported in microsoft#486 and was fixed in microsoft#489.

Changelog

[iOS] [Fixed] - Fix multiline TextInput crash when inserting/removing lots of text

Test Plan

  1. Open RNTester > TextInput
  2. Search for a multiline example
  3. Copy some large text and paste it into the text input view
  4. Remove some (or all) text
  5. Repeat steps 3-4

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner labels Jul 8, 2020
@tido64 tido64 requested review from shergin, zhongwuzw and elicwhite July 8, 2020 20:22
@react-native-bot react-native-bot added Bug Platform: iOS iOS applications. labels Jul 8, 2020
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: e206e34

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 6,764,990 0
android hermes armeabi-v7a 6,427,925 0
android hermes x86 7,152,623 0
android hermes x86_64 7,042,550 0
android jsc arm64-v8a 8,936,316 0
android jsc armeabi-v7a 8,591,663 0
android jsc x86 8,767,077 0
android jsc x86_64 9,342,614 0

Base commit: e206e34

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@JoshuaGross has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@shergin
Copy link
Contributor

shergin commented Jul 17, 2020

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @tido64 in 15dda0a.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Jul 17, 2020
@tido64 tido64 deleted the fix-multiline-textinput-crash branch July 17, 2020 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants