Uh oh!
There was an error while loading. Please reload this page.
iOS: Fixed the bug where a Backspace event was emitted when entering characters after clearing a text in TextInput by an empty string - #18627
Conversation
otusweb
commented
Apr 3, 2018
I tried this fix and it is working correctly in my project |
wack17s
commented
Apr 24, 2018
can this be merged? pls |
facebook-github-bot
commented
May 1, 2018
@hamaron I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project. |
hamaron
commented
May 1, 2018
@facebook-github-bot Yes this is still relevant. Based on the blame info that I can see, the other reviewer should be @shergin. shergin, can you take a look at this PR? |
kayzenkayzen
commented
May 3, 2018
Please Merge! |
GStipick
commented
May 3, 2018
The fix proposed by @hamaron worked for me as well. The bug that this PR is addressing is vital toward some of our teams code and the ability to update react-native. |
code-jenn-or
commented
May 3, 2018
This really needs to be addressed, creates very undesirable behavior in our app |
@facebook-github-bot Rebased the branch on the latest master branch. |
kayzenkayzen
commented
May 9, 2018
@hamaron please, fix checks! |
@kayzenkayzen Unfortunately, the tests had been broken when I rebased the branch on master, and the change I made is actually irrelevant to the test failures... |
kayzenkayzen
commented
May 11, 2018
@hamaron what does that mean? Is this pull request abandoned? if not, when it usually takes a fix to reach production? Do you know any hack for this issue? |
hamaron
commented
May 11, 2018
@kayzenkayzen Thought you wanted me to fix the test failures in CI. |
hamaron
commented
May 11, 2018
@shergin Sorry for bugging you, but it would be great if you could take a look at this bug fix. |
facebook-github-bot
commented
Jun 3, 2018
@hamaron I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project. |
@hamaron: the CI is finally fixed and passing on master. Can you rebase this PR to see the latest CI results? |
hamaron
commented
Jun 11, 2018
@Ashoat OK. I'll rebase the branch shortly. |
@TheSavior we're having a tough time finding a core member to champion the review, would like to tag you in to assist. This should be ready to go |
… a text in TextInput by an empty string
hamaron
commented
Jun 12, 2018
The branch was rebased and the tests became all green! :) |
elicwhite
commented
Jun 12, 2018
Adding @shergin because it is a text related iOS change. |
Ashoat
commented
Jun 15, 2018
@facebook-github-bot shipit |
facebook-github-bot
left a comment
There was a problem hiding this comment.
@Ashoat is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
…characters after clearing a text in TextInput by an empty string (#18627) Summary: The bug #18374 was caused by the loose condition to execute `stringByReplacingCharactersInRange` in the method `textInputShouldChangeTextInRange` . As a result, `findMismatch` wrongly returning `true` which ends up the Backspace event being fired in another `textInputShouldChangeTextInRange` call in `textInputDidChange`. <!-- Required: Write your motivation here. If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged. --> 1. Pass all the tests by `yarn run test` 2. Run the following code and type any text. (This code is brought from #18374. Thank you michalpetrov!!) And then verify that 'Backspace' events are not emitted after clearing text and entering any letters. ```javascript type Props = {}; type State = { text: string, keys: string }; export default class App extends Component<Props, State> { state = {text: '', keys: ''} render() { return ( <View style={styles.container}> <TextInput style={styles.textInput} value={this.state.text} onChangeText={this.onChangeText} onKeyPress={this.onKeyPress}/> <Button title="Clear" onPress={this.onClear}/> <Text>Text: {this.state.text}</Text> <Text>Keys: {this.state.keys}</Text> </View> ); } onChangeText = (text: string) => { this.setState({text}) } onKeyPress = ({ nativeEvent }: Object) => { this.setState({keys: this.state.keys + nativeEvent.key + ', '}) } onClear = () => { this.setState({text: '', keys: ''}) } } ``` <!-- Does this PR require a documentation change? Create a PR at https://github.com/facebook/react-native-website and add a link to it here. --> <!-- Required. Help reviewers and the release process by writing your own release notes. See below for an example. --> [IOS] [BUGFIX] [TextInput] - Fixed the bug where Backspace event was emitted when entering a character after clearing a text in TextInput by an empty string <!-- **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.** CATEGORY [----------] TYPE [ CLI ] [-------------] LOCATION [ DOCS ] [ BREAKING ] [-------------] [ GENERAL ] [ BUGFIX ] [ {Component} ] [ INTERNAL ] [ ENHANCEMENT ] [ {Filename} ] [ IOS ] [ FEATURE ] [ {Directory} ] |-----------| [ ANDROID ] [ MINOR ] [ {Framework} ] - | {Message} | [----------] [-------------] [-------------] |-----------| EXAMPLES: [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see --> Closes#18627 Differential Revision: D8436331 Pulled By: hramos fbshipit-source-id: ec75a6ca926061cbf7cb106db652f2b4a71c9a0c
…characters after clearing a text in TextInput by an empty string (react#18627) Summary: The bug react#18374 was caused by the loose condition to execute `stringByReplacingCharactersInRange` in the method `textInputShouldChangeTextInRange` . As a result, `findMismatch` wrongly returning `true` which ends up the Backspace event being fired in another `textInputShouldChangeTextInRange` call in `textInputDidChange`. <!-- Required: Write your motivation here. If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged. --> 1. Pass all the tests by `yarn run test` 2. Run the following code and type any text. (This code is brought from react#18374. Thank you michalpetrov!!) And then verify that 'Backspace' events are not emitted after clearing text and entering any letters. ```javascript type Props = {}; type State = { text: string, keys: string }; export default class App extends Component<Props, State> { state = {text: '', keys: ''} render() { return ( <View style={styles.container}> <TextInput style={styles.textInput} value={this.state.text} onChangeText={this.onChangeText} onKeyPress={this.onKeyPress}/> <Button title="Clear" onPress={this.onClear}/> <Text>Text: {this.state.text}</Text> <Text>Keys: {this.state.keys}</Text> </View> ); } onChangeText = (text: string) => { this.setState({text}) } onKeyPress = ({ nativeEvent }: Object) => { this.setState({keys: this.state.keys + nativeEvent.key + ', '}) } onClear = () => { this.setState({text: '', keys: ''}) } } ``` <!-- Does this PR require a documentation change? Create a PR at https://github.com/facebook/react-native-website and add a link to it here. --> <!-- Required. Help reviewers and the release process by writing your own release notes. See below for an example. --> [IOS] [BUGFIX] [TextInput] - Fixed the bug where Backspace event was emitted when entering a character after clearing a text in TextInput by an empty string <!-- **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.** CATEGORY [----------] TYPE [ CLI ] [-------------] LOCATION [ DOCS ] [ BREAKING ] [-------------] [ GENERAL ] [ BUGFIX ] [ {Component} ] [ INTERNAL ] [ ENHANCEMENT ] [ {Filename} ] [ IOS ] [ FEATURE ] [ {Directory} ] |-----------| [ ANDROID ] [ MINOR ] [ {Framework} ] - | {Message} | [----------] [-------------] [-------------] |-----------| EXAMPLES: [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see --> Closesreact#18627 Differential Revision: D8436331 Pulled By: hramos fbshipit-source-id: ec75a6ca926061cbf7cb106db652f2b4a71c9a0c
…characters after clearing a text in TextInput by an empty string (react#18627) Summary: The bug react#18374 was caused by the loose condition to execute `stringByReplacingCharactersInRange` in the method `textInputShouldChangeTextInRange` . As a result, `findMismatch` wrongly returning `true` which ends up the Backspace event being fired in another `textInputShouldChangeTextInRange` call in `textInputDidChange`. <!-- Required: Write your motivation here. If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged. --> 1. Pass all the tests by `yarn run test` 2. Run the following code and type any text. (This code is brought from react#18374. Thank you michalpetrov!!) And then verify that 'Backspace' events are not emitted after clearing text and entering any letters. ```javascript type Props = {}; type State = { text: string, keys: string }; export default class App extends Component<Props, State> { state = {text: '', keys: ''} render() { return ( <View style={styles.container}> <TextInput style={styles.textInput} value={this.state.text} onChangeText={this.onChangeText} onKeyPress={this.onKeyPress}/> <Button title="Clear" onPress={this.onClear}/> <Text>Text: {this.state.text}</Text> <Text>Keys: {this.state.keys}</Text> </View> ); } onChangeText = (text: string) => { this.setState({text}) } onKeyPress = ({ nativeEvent }: Object) => { this.setState({keys: this.state.keys + nativeEvent.key + ', '}) } onClear = () => { this.setState({text: '', keys: ''}) } } ``` <!-- Does this PR require a documentation change? Create a PR at https://github.com/facebook/react-native-website and add a link to it here. --> <!-- Required. Help reviewers and the release process by writing your own release notes. See below for an example. --> [IOS] [BUGFIX] [TextInput] - Fixed the bug where Backspace event was emitted when entering a character after clearing a text in TextInput by an empty string <!-- **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.** CATEGORY [----------] TYPE [ CLI ] [-------------] LOCATION [ DOCS ] [ BREAKING ] [-------------] [ GENERAL ] [ BUGFIX ] [ {Component} ] [ INTERNAL ] [ ENHANCEMENT ] [ {Filename} ] [ IOS ] [ FEATURE ] [ {Directory} ] |-----------| [ ANDROID ] [ MINOR ] [ {Framework} ] - | {Message} | [----------] [-------------] [-------------] |-----------| EXAMPLES: [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see --> Closesreact#18627 Differential Revision: D8436331 Pulled By: hramos fbshipit-source-id: ec75a6ca926061cbf7cb106db652f2b4a71c9a0c
…characters after clearing a text in TextInput by an empty string (react#18627) Summary: The bug react#18374 was caused by the loose condition to execute `stringByReplacingCharactersInRange` in the method `textInputShouldChangeTextInRange` . As a result, `findMismatch` wrongly returning `true` which ends up the Backspace event being fired in another `textInputShouldChangeTextInRange` call in `textInputDidChange`. <!-- Required: Write your motivation here. If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged. --> 1. Pass all the tests by `yarn run test` 2. Run the following code and type any text. (This code is brought from react#18374. Thank you michalpetrov!!) And then verify that 'Backspace' events are not emitted after clearing text and entering any letters. ```javascript type Props = {}; type State = { text: string, keys: string }; export default class App extends Component<Props, State> { state = {text: '', keys: ''} render() { return ( <View style={styles.container}> <TextInput style={styles.textInput} value={this.state.text} onChangeText={this.onChangeText} onKeyPress={this.onKeyPress}/> <Button title="Clear" onPress={this.onClear}/> <Text>Text: {this.state.text}</Text> <Text>Keys: {this.state.keys}</Text> </View> ); } onChangeText = (text: string) => { this.setState({text}) } onKeyPress = ({ nativeEvent }: Object) => { this.setState({keys: this.state.keys + nativeEvent.key + ', '}) } onClear = () => { this.setState({text: '', keys: ''}) } } ``` <!-- Does this PR require a documentation change? Create a PR at https://github.com/facebook/react-native-website and add a link to it here. --> <!-- Required. Help reviewers and the release process by writing your own release notes. See below for an example. --> [IOS] [BUGFIX] [TextInput] - Fixed the bug where Backspace event was emitted when entering a character after clearing a text in TextInput by an empty string <!-- **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.** CATEGORY [----------] TYPE [ CLI ] [-------------] LOCATION [ DOCS ] [ BREAKING ] [-------------] [ GENERAL ] [ BUGFIX ] [ {Component} ] [ INTERNAL ] [ ENHANCEMENT ] [ {Filename} ] [ IOS ] [ FEATURE ] [ {Directory} ] |-----------| [ ANDROID ] [ MINOR ] [ {Framework} ] - | {Message} | [----------] [-------------] [-------------] |-----------| EXAMPLES: [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see --> Closesreact#18627 Differential Revision: D8436331 Pulled By: hramos fbshipit-source-id: ec75a6ca926061cbf7cb106db652f2b4a71c9a0c
…characters after clearing a text in TextInput by an empty string (#18627) Summary: The bug #18374 was caused by the loose condition to execute `stringByReplacingCharactersInRange` in the method `textInputShouldChangeTextInRange` . As a result, `findMismatch` wrongly returning `true` which ends up the Backspace event being fired in another `textInputShouldChangeTextInRange` call in `textInputDidChange`. Closesreact/react-native#18627 Differential Revision: D8436331 Pulled By: hramos fbshipit-source-id: ec75a6ca926061cbf7cb106db652f2b4a71c9a0c
…characters after clearing a text in TextInput by an empty string (#18627) Summary: The bug #18374 was caused by the loose condition to execute `stringByReplacingCharactersInRange` in the method `textInputShouldChangeTextInRange` . As a result, `findMismatch` wrongly returning `true` which ends up the Backspace event being fired in another `textInputShouldChangeTextInRange` call in `textInputDidChange`. Closesreact/react-native#18627 Differential Revision: D8436331 Pulled By: hramos fbshipit-source-id: ec75a6ca926061cbf7cb106db652f2b4a71c9a0c
…characters after clearing a text in TextInput by an empty string (#18627) Summary: The bug #18374 was caused by the loose condition to execute `stringByReplacingCharactersInRange` in the method `textInputShouldChangeTextInRange` . As a result, `findMismatch` wrongly returning `true` which ends up the Backspace event being fired in another `textInputShouldChangeTextInRange` call in `textInputDidChange`. Closesreact/react-native#18627 Differential Revision: D8436331 Pulled By: hramos fbshipit-source-id: ec75a6ca926061cbf7cb106db652f2b4a71c9a0c
…characters after clearing a text in TextInput by an empty string (#18627) Summary: The bug #18374 was caused by the loose condition to execute `stringByReplacingCharactersInRange` in the method `textInputShouldChangeTextInRange` . As a result, `findMismatch` wrongly returning `true` which ends up the Backspace event being fired in another `textInputShouldChangeTextInRange` call in `textInputDidChange`. Closesreact/react-native#18627 Differential Revision: D8436331 Pulled By: hramos fbshipit-source-id: ec75a6ca926061cbf7cb106db652f2b4a71c9a0c
…characters after clearing a text in TextInput by an empty string (#18627) Summary: The bug #18374 was caused by the loose condition to execute `stringByReplacingCharactersInRange` in the method `textInputShouldChangeTextInRange` . As a result, `findMismatch` wrongly returning `true` which ends up the Backspace event being fired in another `textInputShouldChangeTextInRange` call in `textInputDidChange`. Closesreact/react-native#18627 Differential Revision: D8436331 Pulled By: hramos fbshipit-source-id: ec75a6ca926061cbf7cb106db652f2b4a71c9a0c
Summary: Fixes#21639 , seems we tried to fix this before, please see related `PR` like [D10392176](36507e4), #18627, but they don't solve it totally. [iOS] [Fixed] - Fix TextInput maxLength when insert characters at begin Pull Request resolved: #23472 Reviewed By: mmmulani Differential Revision: D14366406 Pulled By: ejanzer fbshipit-source-id: fc983810703997b48824f84f2f9198984afba9cd
Summary: Fixes#21639 , seems we tried to fix this before, please see related `PR` like [D10392176](36507e4), #18627, but they don't solve it totally. [iOS] [Fixed] - Fix TextInput maxLength when insert characters at begin Pull Request resolved: #23472 Reviewed By: mmmulani Differential Revision: D14366406 Pulled By: ejanzer fbshipit-source-id: fc983810703997b48824f84f2f9198984afba9cd
mtettmar
commented
Jun 29, 2023
Not sure if it's the same bug but I'm seeing the same behaviour in RN 0.72.0 - whatever key is pressed Backspace is also emitted. After upgrading to RN 0.72.0 we were seeing strange behaviour which led me to search and find the report this fix is based on. Have had to use the same workaround with the timestamp. |
…characters after clearing a text in TextInput by an empty string (#18627) Summary: The bug #18374 was caused by the loose condition to execute `stringByReplacingCharactersInRange` in the method `textInputShouldChangeTextInRange` . As a result, `findMismatch` wrongly returning `true` which ends up the Backspace event being fired in another `textInputShouldChangeTextInRange` call in `textInputDidChange`. Closesreact/react-native#18627 Differential Revision: D8436331 Pulled By: hramos fbshipit-source-id: ec75a6ca926061cbf7cb106db652f2b4a71c9a0c
The bug #18374 was caused by the loose condition to execute
stringByReplacingCharactersInRangein the methodtextInputShouldChangeTextInRange. As a result,findMismatchwrongly returningtruewhich ends up the Backspace event being fired in anothertextInputShouldChangeTextInRangecall intextInputDidChange.Test Plan
yarn run testand entering any letters.
Related PRs
Release Notes
[IOS] [BUGFIX] [TextInput] - Fixed the bug where Backspace event was emitted when entering a character after clearing a text in TextInput by an empty string