TextInput controlled selection broken on both ios and android.

[Gutyn]

This issue is a continuation of the discussion:
dff490d#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:

https://github.com/Ginger-Labs/Input-bug

[chrisglein]

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
        />

[kelset]

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.

[fabOnReact]

My superficial understanding.

The below Java API is called from JavaScript

react-native/Libraries/Components/TextInput/TextInput.js

Line 1011 in a9971b4

viewCommands.setTextAndSelection(

react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java

Lines 213 to 214 in f8bcb10

	case SET_TEXT_AND_SELECTION:
	this.receiveCommand(reactEditText, "setTextAndSelection", args);

text can be updated from State, from Javascript, but we also expose separate api to update the Selection using nativeProps

react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java

Line 245 in f8bcb10

reactEditText.maybeSetTextFromJS(

maybeSetText updates the text

react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java

Line 473 in 9263eb5

public void maybeSetText(ReactTextUpdate reactTextUpdate) {

text is stored in a separate class ReactTextUpdate which includes his own attributes

react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java

Line 492 in 9263eb5

new SpannableStringBuilder(reactTextUpdate.getText());

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.

react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputShadowNode.java

Lines 232 to 249 in 9312313

	new ReactTextUpdate(
	spannedFromShadowNode(
	this,
	getText(),
	/* supportsInlineViews: */ false,
	/* nativeViewHierarchyOptimizer: */ null // only needed to support inline views
	),
	mMostRecentEventCount,
	mContainsImages,
	getPadding(Spacing.LEFT),
	getPadding(Spacing.TOP),
	getPadding(Spacing.RIGHT),
	getPadding(Spacing.BOTTOM),
	mTextAlign,
	mTextBreakStrategy,
	mJustificationMode,
	mSelectionStart,
	mSelectionEnd);

This is the ReactTextUpdate instance constructor

react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.java

Lines 115 to 128 in f8bcb10

	public ReactTextUpdate(
	Spannable text,
	int jsEventCounter,
	boolean containsImages,
	float paddingStart,
	float paddingTop,
	float paddingEnd,
	float paddingBottom,
	int textAlign,
	int textBreakStrategy,
	int justificationMode,
	int selectionStart,
	int selectionEnd) {
	mText = text;

I did some research on this working on #29070 (comment), but I ended up giving up as it was relatively complex functionality.

My main problem when working on #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

[JoshuaGross]

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.

027e8f9
e68f9bf
b861782

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.

[garrettm]

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.