Fix AttributedString comparison logic for TextInput state updates

Summary:
D37801394 (51f49ca) attempted to fix an issue of TextInput values being dropped when an uncontrolled component is restyled, and a defaultValue is present. I had missed quite a bit of functionality, where TextInput may have child Text elements, which the native side flattens into a single AttributedString. `lastNativeValue` includes a lossy version of the flattened string  produced from the child fragments, so sending it along with the children led to duplicating of the current input on each edit, and things blow up.

With some experimentation, I found that the text-loss behavior only happens on Fabric, and is triggered by a state update rather than my original assumption of the view manager command in the `useLayoutEffect` hook. `AndroidTextInputShadowNode` will compare the current and previous flattened strings, to intentionally allow the native value to drift from the React tree if the React tree hasn't changed. This `AttributedString` comparison includes layout metrics as of D20151505 (061f54e) meaning a restyle may cause a state update, and clear the text.

I do not have full understanding of the flow of state updates to layout, or the underlying issue that led to the equality check including layout information (since TextMeasurementCache seems to explicitly compare LayoutMetrics). D18894538 (254ebab) used a solution of sending a no-op state update to avoid updating text for placeholders, when the Attributed strings are equal (though as of now this code is never reached, since we return earlier on AttributedString equality). I co-opted this mechanism, to avoid sending text updates if the text content and attributes of the AttributedString has not changed, disregarding any layout information. This is how the comparison worked at the time of the diff.

I also updated the fragment hashing function to include layout metrics, since it was added to be part of the equality check, and is itself hashable.

Changelog:
[Android][Fixed] - Fix `AttributedString` comparison logic for TextInput state updates

Reviewed By: sammy-SC

Differential Revision: D37902643

fbshipit-source-id: c0f8e3112feb19bd0ee62b37bdadeb237a9f725e

ReactCommon/react/renderer/attributedstring/AttributedString.cpp

@@ -38,6 +38,11 @@ bool Fragment::operator==(const Fragment &rhs) const {
              rhs.parentShadowView.layoutMetrics);
 }
 
+bool Fragment::isContentEqual(const Fragment &rhs) const {
+  return std::tie(string, textAttributes) ==
+      std::tie(rhs.string, rhs.textAttributes);
+}
+
 bool Fragment::operator!=(const Fragment &rhs) const {
   return !(*this == rhs);
 }
@@ -126,6 +131,20 @@ bool AttributedString::operator!=(const AttributedString &rhs) const {
   return !(*this == rhs);
 }
 
+bool AttributedString::isContentEqual(const AttributedString &rhs) const {
+  if (fragments_.size() != rhs.fragments_.size()) {
+    return false;
+  }
+
+  for (auto i = 0; i < fragments_.size(); i++) {
+    if (!fragments_[i].isContentEqual(rhs.fragments_[i])) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 #pragma mark - DebugStringConvertible
 
 #if RN_DEBUG_STRING_CONVERTIBLE

ReactCommon/react/renderer/attributedstring/AttributedString.h

@@ -46,6 +46,12 @@ class AttributedString : public Sealable, public DebugStringConvertible {
      */
     bool isAttachment() const;
 
+    /*
+     * Returns whether the underlying text and attributes are equal,
+     * disregarding layout or other information.
+     */
+    bool isContentEqual(const Fragment &rhs) const;
+
     bool operator==(const Fragment &rhs) const;
     bool operator!=(const Fragment &rhs) const;
   };
@@ -96,6 +102,8 @@ class AttributedString : public Sealable, public DebugStringConvertible {
    */
   bool compareTextAttributesWithoutFrame(const AttributedString &rhs) const;
 
+  bool isContentEqual(const AttributedString &rhs) const;
+
   bool operator==(const AttributedString &rhs) const;
   bool operator!=(const AttributedString &rhs) const;
 
@@ -118,7 +126,11 @@ struct hash<facebook::react::AttributedString::Fragment> {
   size_t operator()(
       const facebook::react::AttributedString::Fragment &fragment) const {
     return folly::hash::hash_combine(
-        0, fragment.string, fragment.textAttributes, fragment.parentShadowView);
+        0,
+        fragment.string,
+        fragment.textAttributes,
+        fragment.parentShadowView,
+        fragment.parentShadowView.layoutMetrics);
   }
 };
 

ReactCommon/react/renderer/components/textinput/androidtextinput/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp

@@ -141,17 +141,17 @@ void AndroidTextInputShadowNode::updateStateIfNeeded() {
   auto defaultTextAttributes = TextAttributes::defaultTextAttributes();
   defaultTextAttributes.apply(getConcreteProps().textAttributes);
 
-  auto newEventCount =
-      (state.reactTreeAttributedString == reactTreeAttributedString
-           ? 0
-           : getConcreteProps().mostRecentEventCount);
-  auto newAttributedString = getMostRecentAttributedString();
-
   // Even if we're here and updating state, it may be only to update the layout
   // manager If that is the case, make sure we don't update text: pass in the
   // current attributedString unchanged, and pass in zero for the "event count"
   // so no changes are applied There's no way to prevent a state update from
   // flowing to Java, so we just ensure it's a noop in those cases.
+  auto newEventCount =
+      state.reactTreeAttributedString.isContentEqual(reactTreeAttributedString)
+      ? 0
+      : getConcreteProps().mostRecentEventCount;
+  auto newAttributedString = getMostRecentAttributedString();
+
   setStateData(AndroidTextInputState{
       newEventCount,
       newAttributedString,

packages/rn-tester/js/examples/TextInput/TextInputSharedExamples.js

@@ -72,6 +72,14 @@ const styles = StyleSheet.create({
     margin: 3,
     fontSize: 12,
   },
+  focusedUncontrolled: {
+    margin: -2,
+    borderWidth: 2,
+    borderColor: '#0a0a0a',
+    flex: 1,
+    fontSize: 13,
+    padding: 4,
+  },
 });
 
 class WithLabel extends React.Component<$FlowFixMeProps> {
@@ -477,6 +485,20 @@ class SelectionExample extends React.Component<
   }
 }
 
+function UncontrolledExample() {
+  const [isFocused, setIsFocused] = React.useState(false);
+
+  return (
+    <TextInput
+      defaultValue="Hello World!"
+      testID="uncontrolled-textinput"
+      style={isFocused ? styles.focusedUncontrolled : styles.default}
+      onFocus={() => setIsFocused(true)}
+      onBlur={() => setIsFocused(false)}
+    />
+  );
+}
+
 module.exports = ([
   {
     title: 'Auto-focus',
@@ -696,4 +718,11 @@ module.exports = ([
       );
     },
   },
+  {
+    title: 'Uncontrolled component with layout changes',
+    name: 'uncontrolledComponent',
+    render: function (): React.Node {
+      return <UncontrolledExample />;
+    },
+  },
 ]: Array<RNTesterModuleExample>);