React-native: TextInput controlled selection broken on both ios and android.
This issue is a continuation of the discussion:
https://github.com/facebook/react-native/commit/dff490d140010913d3209a2f3e987914b9c4eee4#commitcomment-39332764
The link to the sample project that demonstrates the issues:
https://github.com/Ginger-Labs/Input-bug
Description
Controlled selection seems to be broken on both ios and android, to demonstrate the issues I created a sample project (find the link above).
React Native version:
System:
OS: macOS 10.15.4
CPU: (8) x64 Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz
Memory: 1.55 GB / 16.00 GB
Shell: 5.7.1 - /bin/zsh
Binaries:
Node: 10.14.2 - /usr/local/bin/node
Yarn: 1.13.0 - /usr/local/bin/yarn
npm: 6.14.5 - ~/.npm-global/bin/npm
Watchman: 4.9.0 - /usr/local/bin/watchman
SDKs:
iOS SDK:
Platforms: iOS 13.5, DriverKit 19.0, macOS 10.15, tvOS 13.4, watchOS 6.2
Android SDK:
API Levels: 23, 24, 25, 26, 27, 28, 29
Build Tools: 26.0.3, 28.0.3, 29.0.0, 30.0.0
System Images: android-28 | Google APIs Intel x86 Atom, android-28 | Google Play Intel x86 Atom, android-29 | Google APIs Intel x86 Atom, android-29 | Google Play Intel x86 Atom_64
Android NDK: 21.1.6352462
IDEs:
Android Studio: 3.6 AI-192.7142.36.36.6392135
Xcode: 11.5/11E608c - /usr/bin/xcodebuild
npmPackages:
react: ~16.9.0 => 16.9.0
react-native: ~0.62.2 => 0.62.2
npmGlobalPackages:
create-react-native-app: 3.4.0
react-native-app-id: 0.0.5
react-native-cli: 2.0.1
Steps To Reproduce
The reproduction steps are in the sample project's ReadMe file.
For simplicity purposes, I will post them here as well:
SIM - iPhone 11 (13.4.1):
1) Click on the "Click Me" Button, set the cursor in the middle, add some text, click on the button again: Notice the text is set to needed one but the selection is not 10
2) Input text: "Hello world", move cursor in between words, click on "@":
Expected: "Hello @Mihailworld" with the cursor at 13
Actual: "Hello @Mihailworld" with the cursor at the end of the whole string.
SIM - nexus 6P API28
1) Add text, click on enter (new line).
Expected: The text stays and a new line is created with "-" in front.
Actual: The first line becomes empty and the second line "-".
2) Press enter twice and you will get a:
Exception in native call java.lang.IndexOutOfBoundsException: setSpan (6 ... 6) ends beyond length 3
3) Press "@" twice and observe the same bug above.
Expected Results
I expect the TextInput to work as intended (unless I am missing something conceptual).
Snack, code example, screenshot, or link to a repository:
Gutyn
All 13 comments
The text string is being programmatically replaced. The expectation stated here is that the cursor is stable as a result of how TextInput.selection is being managed. Relevant bits of code here:
function replace(previous, range, chars) {
const text = previous.slice(0, range.start) + chars + previous.slice(range.end)
const sel = range.start + chars.length
return {
text,
selection: {start: sel, end: sel}
}
}
onSelectionChange = evt => {
console.log('onSelectionChange: ', evt.nativeEvent)
let nextSelection = evt.nativeEvent.selection
if (!this.inputEvent) {
return this.setState({selection: nextSelection})
}
const {previousText, text, range} = this.inputEvent
let nextText = replace(this.state.text, range, text).text
delete this.inputEvent
function replaceInputWith(chars) {
const result = replace(previousText, range, chars)
nextText = result.text
nextSelection = result.selection
}
switch (text) {
case '@': {
replaceInputWith('@Mihail')
break
}
case '\n': {
replaceInputWith('\n- ')
break
}
}
this.setState({
text: nextText,
selection: nextSelection
})
}
<TextInput
selection={this.state.selection}
value={this.state.text}
placeholder={'Say Something'}
onSelectionChange={this.onSelectionChange}
onChange={this.onChange}
onChangeText={this.onChangeText}
onTextInput={this.onTextInput}
onKeyPress={this.onKeyPress}
multiline
autoFocus
/>
chrisglein
on 9 Jun 2020
cc @TheSavior I think this is relevant to your commit rewriting TextInput to hooks.
As far as I am aware, there is no available commit on master that would fix it via a cherry pick in a release - but I may be wrong.
kelset
on 14 Jul 2020
My superficial understanding.
The below Java API is called from JavaScript
text can be updated from State, from Javascript, but we also expose separate api to update the Selection using nativeProps
maybeSetText updates the text
text is stored in a separate class ReactTextUpdate which includes his own attributes
The mText state (the string we receive from Javascript) is stored in instance of class ReactTextUpdate attributes mText. This is where it is first set.
This is the ReactTextUpdate instance constructor
I did some research on this working on https://github.com/facebook/react-native/pull/29070#issuecomment-650127577, but I ended up giving up as it was relatively complex functionality.
My main problem when working on https://github.com/facebook/react-native/pull/29070 was that length() would be different then actual lenght of the string, causing duplicated letters.
Thanks a lot
I wish you a good day
Fabrizio Bertoglio
fabriziobertoglio1987
on 14 Jul 2020
It looks like there could be some relevant commits missing from the 0.63 branch? Forgive me if I'm wrong, maybe some of these are already included but it doesn't look like it to me.
https://github.com/facebook/react-native/commit/027e8f9b1600c931aa4b826b905a67969733c6ea
https://github.com/facebook/react-native/commit/e68f9bf76846dbbc767ec4dbcabc9d83adb5f0dd
https://github.com/facebook/react-native/commit/b861782562080bf9c91afcac0f823d96088c2d8d
There were a lot of changes made to TextInput up through those diffs, roughly, so if some of them aren't included in the branch I would expect some TextInput issues.
JoshuaGross
on 15 Jul 2020
Here’s a rough outline of our hack workarounds:
- never send text/selection via props in render
- create logic to try to track what you expect the native side to be showing, and if they differ, in render/update (depending on plaintext/attributed) send the text/selection values you want
- on the JS side coalesce selection + text changes, they need to happen together to have proper logic for changing the string/selection in custom ways
- when setting selection on Android via setNativeProps, have to re-set it back to null after a 1ms delay, or it'll get stuck
- various other minor hacks (for example, attributed text needs different logic than plaintext)
It's pretty involved, has a bunch of hacks, and doesn't work all the time. If I was gonna do it again, I'd probably just fork the native code...
If anyone who is working on fixing this bug in RN core would like to chat about my experience with it or possible solutions, I’d love to, anytime.
garrettm
on 15 Jul 2020
on Android onTextInput receives
{
nativeEvent: {
previousText: ""
}
}
when the cursor is moved between the 2 words like this
Large Text[CURSOR]
After
Large [CURSOR]Text
caused from this line. previousText is calculated here with .substring(start, before), but before has value of 0 so substring(6, 6 + 0) returns empty string.
If I add the below text { start: 0, before: 10}
If I move middle of the word { start: 0, before: 10}
If I insert from the previous cursor position the a character { start: 6, before: 0}
onTextInput receives mPreviousText = empty string
This issue is not very clear to me especially the example attached.. I unsubscribed from this issue and may not follow-up. Sorry
Additionally:
1) I click on Click me button, the state text value is set and onTextInput is not called
2) this.inputEvent is set inside onTextInput callback
3) additional conditions change the execution of the program.. Are this essential to reproduce the bug?
All this conditions are not relevant for reproducing this bug/issue and they are considerable amount of work for the OpenSource contributor that need to delete existing JavaScript logic to reproduce the issue with a minimal reproducible example
Minimal
The more code there is to go through, the less likely people can find your problem. Streamline your example in one of two ways:
Restart from scratch. Create a new program, adding in only what is needed to see the problem. Use simple, descriptive names for functions and variables – don’t copy the names you’re using in your existing code.
Divide and conquer. If you’re not sure what the source of the problem is, start removing code a bit at a time until the problem disappears – then add the last part back.
Reproducible
Eliminate any issues that aren't relevant to the problem. If your question isn’t about a compiler error, ensure that there are no compile-time errors. Use a program such as JSLint to validate interpreted languages. Validate any HTML or XML.
Sorry, Thanks a lot. Best Regards. Fabrizio Bertoglio
fabriziobertoglio1987
on 15 Jul 2020
@garrettm @fabriziobertoglio1987 I would expect a variety of bugs until (at least) the three commits I mentioned above are merged into release.
JoshuaGross
on 18 Jul 2020
Is it expected that these three commits get into 63.2?
garrettm
on 21 Jul 2020
@TheSavior @kelset controlled selection is broken on both platforms since the component has been rewritten to hooks. Is there anything can be done to fix it as it's critical for some cases?
todorone
on 23 Sep 2020
I thought the commits mentioned by Joshua were cherry picked in 0.63.2. Please submit a new repro if that still happens with latest. We are also working our way towards a 0.63.3 release which should have more fixes.
kelset
on 23 Sep 2020
For the record, the controlled selection is still broken in 0.63.3.
focux
on 6 Oct 2020
We're on 0.63.2 and still having issues too
garrettm
on 6 Oct 2020
Have the same issue, unfortunately. I'd like to wonder if there is any update or suggestion of how to solve it?
Thanks!
velidan
on 9 Dec 2020
Related issues
arunabha
·
3Comments
ghost
·
3Comments
anchetaWern
·
3Comments
vikeri
·
3Comments
phongyewtong
·
3Comments
Most helpful comment
It looks like there could be some relevant commits missing from the 0.63 branch? Forgive me if I'm wrong, maybe some of these are already included but it doesn't look like it to me.
https://github.com/facebook/react-native/commit/027e8f9b1600c931aa4b826b905a67969733c6ea
https://github.com/facebook/react-native/commit/e68f9bf76846dbbc767ec4dbcabc9d83adb5f0dd
https://github.com/facebook/react-native/commit/b861782562080bf9c91afcac0f823d96088c2d8d
There were a lot of changes made to TextInput up through those diffs, roughly, so if some of them aren't included in the branch I would expect some TextInput issues.