Deduplicate textInputDid(Begin|End)Editing calls for multiline <TextInput> elements

[amgleitman]

Summary:

We recently found a bug where after editing a multiline <TextInput> with autoFocus={true} causes textInputDidBeginEditing to be called twice: once on accepting focus, and again when we start typing. If the text input already has text in it, the second call has a side effect of moving the cursor to the end, which is undesirable if the used moved the cursor somewhere to the middle.

This appears to only repro for multiline text inputs, which are backed by NSTextView instead of NSTextField. As of now, it's unknown why these behave differently, and since these events come from AppKit itself, we don't have as much control over them. Our solution is to add an extra bit of state so we can enforce that the effects of these calls don't get duplicated.

For added correctness, we add similar deduplication for textInputDidEndEditing to ensure that every DidBegin can be accompanied by exactly one DidEnd.

Test Plan:

Validated using the following test code (which was adapted from a previous test case involving programmatic editing of a <TextInput>'s content).

// Adapted from https://stackoverflow.com/questions/66525664/how-to-change-the-value-of-textinput-element-in-react-native

import React, {useState} from 'react';
import {
  Text,
  View,
  StyleSheet,
  TextInput,
  TouchableOpacity,
  FlatList,
  Switch,
} from 'react-native';

const App = () => {
  const [items, setItems] = useState<string[]>([]);
  const handleAddItem = (value: string) => {
    setItems([...items, value]);
  };
  return (
    <View>
      <AddItem addItem={handleAddItem} />
      <FlatList
        data={items}
        renderItem={({item}) => (
          <View style={[styles.card, colors.green]}>
            <Text>{item}</Text>
          </View>
        )}
      />
    </View>
  );
};

interface AddItemProps {
  addItem: (_: string) => void;
}

interface Selection {
  start?: number;
  end?: number;
}

const AddItem = ({addItem}: AddItemProps) => {
  const [replaceCount, setReplaceCount] = useState(0);
  const [selection, setSelection] = useState<Selection>({});
  const [showInput, setShowInput] = useState(true);
  const [text, setText] = useState('');

  const onChange = setText;

  return (
    <View>
      <View style={[styles.card, colors.blue]}>
        <Text style={styles.buttonText}>Header</Text>
      </View>
      <Switch value={showInput} onValueChange={() => setShowInput(p => !p)} />
      {showInput && (
        <TextInput
          multiline={true}
          clearTextOnFocus={true}
          autoFocus
          value={text}
          placeholder="Add Item..."
          style={[styles.card, colors.gray]}
          onChangeText={onChange}
          onSelectionChange={event => {
            const thisSelection = event.nativeEvent.selection;
            setSelection(thisSelection);
            console.log(`${thisSelection.start} - ${thisSelection.end}`);
          }}
          onFocus={() => console.log('onFocus')}
          onBlur={() => console.log('onBlur')}
        />
      )}
      <TouchableOpacity
        onPress={() => {
          const newReplaceCount = replaceCount + 1;
          setReplaceCount(newReplaceCount);

          const position = selection.start ?? 0;
          const prefix = text.substring(0, position);
          const suffix = text.substring(position);
          setText(`${prefix}(${newReplaceCount})${suffix}`);
        }}>
        <View style={[styles.card, colors.blue]}>
          <Text style={styles.buttonText}>Change Text</Text>
        </View>
      </TouchableOpacity>
      <TouchableOpacity
        onPress={() => {
          addItem(text);
          setText('');
        }}>
        <View style={[styles.card, colors.red]}>
          <Text style={styles.buttonText}>Add Item</Text>
        </View>
      </TouchableOpacity>
    </View>
  );
};

const styles = StyleSheet.create({
  buttonText: {
    fontWeight: 'bold',
  },
  card: {
    margin: 5,
    padding: 5,
    borderRadius: 5,
  },
});

const colors = StyleSheet.create({
  red: {backgroundColor: 'rgba(255,0,0,0.2)'},
  green: {backgroundColor: 'rgba(0,255,0,0.2)'},
  blue: {backgroundColor: 'rgba(0,0,255,0.2)'},
  gray: {backgroundColor: 'rgba(21,21,21,0.1)'},
});

export default App;

[Saadnajmi]

Similar comment as #2160:
Since this is touching iOS code too, could we check if the problem reproes on iOS, and whether it can be usptreamed? If not, could we limit to macOS only blocks via ifdefs? I'm not sure we need to do that but it helps a little in merges.

[amgleitman]

Similar comment as #2160: Since this is touching iOS code too, could we check if the problem reproes on iOS, and whether it can be usptreamed? If not, could we limit to macOS only blocks via ifdefs? I'm not sure we need to do that but it helps a little in merges.

This doesn't repro on iOS, so I have no problem adding #if TARGET_OS_OSX blocks.