Skip to content

Commit 0bae474

Browse files
JoshuaGrossfacebook-github-bot
authored andcommitted
TextInput: keep C++ state in-sync with updated AttributedStrings in Java
Summary: When the TextInput is updated on the Java side, make sure C++ State gets updated. We do this by making sure that the AttributedString data-structured in mirrored in Java and in C++. In practice, the AttributedString is copied into Java a few times during initialization, and then after then, 99% of the time Java is writing without receiving updates from C++. This means that we should optimize the Java-to-C++ update path most aggressively in the future. However, it turns out that for now, at least, we can't reuse NativeWritableMaps/NativeWritableArrays because they're consumed on the C++ side and can't be modified after that. This is a perf improvement for the future. This allows us the user to edit any fragments, and the changes will flow through C++ State. This also allows us to edit across multiple Fragments. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D18785960 fbshipit-source-id: 97b283ec411081eca4d2d7a4cce2b31b5e237c42
1 parent 2a46980 commit 0bae474

File tree

3 files changed

+158
-18
lines changed

3 files changed

+158
-18
lines changed

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
import android.text.Layout;
1313
import android.text.Spannable;
14+
import androidx.annotation.Nullable;
15+
import com.facebook.react.bridge.ReadableMap;
1416

1517
/**
1618
* Class that contains the data needed for a text update. Used by both <Text/> and <TextInput/>
@@ -31,6 +33,8 @@ public class ReactTextUpdate {
3133
private final int mSelectionEnd;
3234
private final int mJustificationMode;
3335

36+
public @Nullable ReadableMap mAttributedString = null;
37+
3438
/**
3539
* @deprecated Use a non-deprecated constructor for ReactTextUpdate instead. This one remains
3640
* because it's being used by a unit test that isn't currently open source.
@@ -135,6 +139,23 @@ public ReactTextUpdate(
135139
mJustificationMode = justificationMode;
136140
}
137141

142+
public static ReactTextUpdate buildReactTextUpdateFromState(
143+
Spannable text,
144+
int jsEventCounter,
145+
boolean containsImages,
146+
int textAlign,
147+
int textBreakStrategy,
148+
int justificationMode,
149+
ReadableMap attributedString) {
150+
151+
ReactTextUpdate textUpdate =
152+
new ReactTextUpdate(
153+
text, jsEventCounter, containsImages, textAlign, textBreakStrategy, justificationMode);
154+
155+
textUpdate.mAttributedString = attributedString;
156+
return textUpdate;
157+
}
158+
138159
public Spannable getText() {
139160
return mText;
140161
}

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

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,8 @@
3535
import androidx.core.view.AccessibilityDelegateCompat;
3636
import androidx.core.view.ViewCompat;
3737
import com.facebook.infer.annotation.Assertions;
38+
import com.facebook.react.bridge.JavaOnlyMap;
3839
import com.facebook.react.bridge.ReactContext;
39-
import com.facebook.react.bridge.WritableMap;
40-
import com.facebook.react.bridge.WritableNativeMap;
4140
import com.facebook.react.uimanager.StateWrapper;
4241
import com.facebook.react.uimanager.UIManagerModule;
4342
import com.facebook.react.views.text.ReactSpan;
@@ -72,8 +71,13 @@ public class ReactEditText extends EditText {
7271
private boolean mShouldAllowFocus;
7372
private int mDefaultGravityHorizontal;
7473
private int mDefaultGravityVertical;
74+
75+
/** A count of events sent to JS or C++. */
7576
protected int mNativeEventCount;
77+
78+
/** The most recent event number acked by JavaScript. Should only be updated from JS, not C++. */
7679
protected int mMostRecentEventCount;
80+
7781
private @Nullable ArrayList<TextWatcher> mListeners;
7882
private @Nullable TextWatcherDelegator mTextWatcherDelegator;
7983
private int mStagedInputType;
@@ -95,7 +99,11 @@ public class ReactEditText extends EditText {
9599

96100
private ReactViewBackgroundManager mReactBackgroundManager;
97101

102+
protected @Nullable JavaOnlyMap mAttributedString = null;
98103
protected @Nullable StateWrapper mStateWrapper = null;
104+
protected boolean mDisableTextDiffing = false;
105+
106+
protected boolean mIsSettingTextFromState = false;
99107

100108
private static final KeyListener sKeyListener = QwertyKeyListener.getInstanceForFullKeyboard();
101109

@@ -279,17 +287,7 @@ public void setContentSizeWatcher(ContentSizeWatcher contentSizeWatcher) {
279287
}
280288

281289
public void setMostRecentEventCount(int mostRecentEventCount) {
282-
if (mMostRecentEventCount == mostRecentEventCount) {
283-
return;
284-
}
285-
286290
mMostRecentEventCount = mostRecentEventCount;
287-
288-
if (mStateWrapper != null) {
289-
WritableMap map = new WritableNativeMap();
290-
map.putInt("mostRecentEventCount", mMostRecentEventCount);
291-
mStateWrapper.updateState(map);
292-
}
293291
}
294292

295293
public void setScrollWatcher(ScrollWatcher scrollWatcher) {
@@ -453,6 +451,18 @@ public int incrementAndGetEventCounter() {
453451
return ++mNativeEventCount;
454452
}
455453

454+
public void maybeSetTextFromJS(ReactTextUpdate reactTextUpdate) {
455+
mIsSettingTextFromJS = true;
456+
maybeSetText(reactTextUpdate);
457+
mIsSettingTextFromJS = false;
458+
}
459+
460+
public void maybeSetTextFromState(ReactTextUpdate reactTextUpdate) {
461+
mIsSettingTextFromState = true;
462+
maybeSetText(reactTextUpdate);
463+
mIsSettingTextFromState = false;
464+
}
465+
456466
// VisibleForTesting from {@link TextInputEventsTestCase}.
457467
public void maybeSetText(ReactTextUpdate reactTextUpdate) {
458468
if (isSecureText() && TextUtils.equals(getText(), reactTextUpdate.getText())) {
@@ -465,6 +475,10 @@ public void maybeSetText(ReactTextUpdate reactTextUpdate) {
465475
return;
466476
}
467477

478+
if (reactTextUpdate.mAttributedString != null) {
479+
mAttributedString = JavaOnlyMap.deepClone(reactTextUpdate.mAttributedString);
480+
}
481+
468482
// The current text gets replaced with the text received from JS. However, the spans on the
469483
// current text need to be adapted to the new text. Since TextView#setText() will remove or
470484
// reset some of these spans even if they are set directly, SpannableStringBuilder#replace() is
@@ -473,17 +487,24 @@ public void maybeSetText(ReactTextUpdate reactTextUpdate) {
473487
new SpannableStringBuilder(reactTextUpdate.getText());
474488
manageSpans(spannableStringBuilder);
475489
mContainsImages = reactTextUpdate.containsImages();
476-
mIsSettingTextFromJS = true;
490+
491+
// When we update text, we trigger onChangeText code that will
492+
// try to update state if the wrapper is available. Temporarily disable
493+
// to prevent an (asynchronous) infinite loop.
494+
mDisableTextDiffing = true;
477495

478496
// On some devices, when the text is cleared, buggy keyboards will not clear the composing
479497
// text so, we have to set text to null, which will clear the currently composing text.
480498
if (reactTextUpdate.getText().length() == 0) {
481499
setText(null);
482500
} else {
501+
// When we update text, we trigger onChangeText code that will
502+
// try to update state if the wrapper is available. Temporarily disable
503+
// to prevent an infinite loop.
483504
getText().replace(0, length(), spannableStringBuilder);
484505
}
506+
mDisableTextDiffing = false;
485507

486-
mIsSettingTextFromJS = false;
487508
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
488509
if (getBreakStrategy() != reactTextUpdate.getTextBreakStrategy()) {
489510
setBreakStrategy(reactTextUpdate.getTextBreakStrategy());

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

Lines changed: 102 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,17 @@
2828
import com.facebook.infer.annotation.Assertions;
2929
import com.facebook.react.bridge.Dynamic;
3030
import com.facebook.react.bridge.JSApplicationIllegalArgumentException;
31+
import com.facebook.react.bridge.JavaOnlyArray;
32+
import com.facebook.react.bridge.JavaOnlyMap;
3133
import com.facebook.react.bridge.ReactContext;
3234
import com.facebook.react.bridge.ReadableArray;
3335
import com.facebook.react.bridge.ReadableMap;
3436
import com.facebook.react.bridge.ReadableNativeMap;
3537
import com.facebook.react.bridge.ReadableType;
38+
import com.facebook.react.bridge.WritableArray;
39+
import com.facebook.react.bridge.WritableMap;
40+
import com.facebook.react.bridge.WritableNativeArray;
41+
import com.facebook.react.bridge.WritableNativeMap;
3642
import com.facebook.react.common.MapBuilder;
3743
import com.facebook.react.module.annotations.ReactModule;
3844
import com.facebook.react.uimanager.BaseViewManager;
@@ -225,7 +231,8 @@ public void receiveCommand(
225231

226232
// TODO: construct a ReactTextUpdate and use that with maybeSetText
227233
// instead of calling setText, etc directly - doing that will definitely cause bugs.
228-
reactEditText.maybeSetText(getReactTextUpdate(text, mostRecentEventCount, start, end));
234+
reactEditText.maybeSetTextFromJS(
235+
getReactTextUpdate(text, mostRecentEventCount, start, end));
229236
}
230237
break;
231238
}
@@ -257,7 +264,7 @@ public void updateExtraData(ReactEditText view, Object extraData) {
257264
Spannable spannable = update.getText();
258265
TextInlineImageSpan.possiblyUpdateInlineImageSpans(spannable, view);
259266
}
260-
view.maybeSetText(update);
267+
view.maybeSetTextFromState(update);
261268
if (update.getSelectionStart() != UNSET && update.getSelectionEnd() != UNSET)
262269
view.setSelection(update.getSelectionStart(), update.getSelectionEnd());
263270
}
@@ -842,6 +849,10 @@ public void beforeTextChanged(CharSequence s, int start, int count, int after) {
842849

843850
@Override
844851
public void onTextChanged(CharSequence s, int start, int before, int count) {
852+
if (mEditText.mDisableTextDiffing) {
853+
return;
854+
}
855+
845856
// Rearranging the text (i.e. changing between singleline and multiline attributes) can
846857
// also trigger onTextChanged, call the event in JS only when the text actually changed
847858
if (count == 0 && before == 0) {
@@ -856,6 +867,92 @@ public void onTextChanged(CharSequence s, int start, int before, int count) {
856867
return;
857868
}
858869

870+
// Fabric: update representation of AttributedString
871+
JavaOnlyMap attributedString = mEditText.mAttributedString;
872+
if (attributedString != null && attributedString.hasKey("fragments")) {
873+
String changedText = s.subSequence(start, start + count).toString();
874+
875+
String completeStr = attributedString.getString("string");
876+
String newCompleteStr =
877+
completeStr.substring(0, start)
878+
+ changedText
879+
+ (completeStr.length() > start + before
880+
? completeStr.substring(start + before)
881+
: "");
882+
attributedString.putString("string", newCompleteStr);
883+
884+
// Loop through all fragments and change them in-place
885+
JavaOnlyArray fragments = (JavaOnlyArray) attributedString.getArray("fragments");
886+
int positionInAttributedString = 0;
887+
boolean found = false;
888+
for (int i = 0; i < fragments.size() && !found; i++) {
889+
JavaOnlyMap fragment = (JavaOnlyMap) fragments.getMap(i);
890+
String fragmentStr = fragment.getString("string");
891+
int positionBefore = positionInAttributedString;
892+
positionInAttributedString += fragmentStr.length();
893+
if (positionInAttributedString < start) {
894+
continue;
895+
}
896+
897+
int relativePosition = start - positionBefore;
898+
found = true;
899+
900+
// Does the change span multiple Fragments?
901+
// If so, we put any new text entirely in the first
902+
// Fragment that we edit. For example, if you select two words
903+
// across Fragment boundaries, "one | two", and replace them with a
904+
// character "x", the first Fragment will replace "one " with "x", and the
905+
// second Fragment will replace "two" with an empty string.
906+
int remaining = fragmentStr.length() - relativePosition;
907+
908+
String newString =
909+
fragmentStr.substring(0, relativePosition)
910+
+ changedText
911+
+ (fragmentStr.substring(relativePosition + Math.min(before, remaining)));
912+
fragment.putString("string", newString);
913+
914+
// If we're changing 10 characters (before=10) and remaining=3,
915+
// we want to remove 3 characters from this fragment (`Math.min(before, remaining)`)
916+
// and 7 from the next Fragment (`before = 10 - 3`)
917+
if (remaining < before) {
918+
changedText = "";
919+
start += remaining;
920+
before = before - remaining;
921+
found = false;
922+
}
923+
}
924+
}
925+
926+
// Fabric: communicate to C++ layer that text has changed
927+
// We need to call `incrementAndGetEventCounter` here explicitly because this
928+
// update may race with other updates.
929+
// TODO: currently WritableNativeMaps/WritableNativeArrays cannot be reused so
930+
// we must recreate these data structures every time. It would be nice to have a
931+
// reusable data-structure to use for TextInput because constructing these and copying
932+
// on every keystroke is very expensive.
933+
if (mEditText.mStateWrapper != null && attributedString != null) {
934+
WritableMap map = new WritableNativeMap();
935+
WritableMap newAttributedString = new WritableNativeMap();
936+
937+
WritableArray fragments = new WritableNativeArray();
938+
939+
for (int i = 0; i < attributedString.getArray("fragments").size(); i++) {
940+
ReadableMap readableFragment = attributedString.getArray("fragments").getMap(i);
941+
WritableMap fragment = new WritableNativeMap();
942+
fragment.putDouble("reactTag", readableFragment.getInt("reactTag"));
943+
fragment.putString("string", readableFragment.getString("string"));
944+
fragments.pushMap(fragment);
945+
}
946+
947+
newAttributedString.putString("string", attributedString.getString("string"));
948+
newAttributedString.putArray("fragments", fragments);
949+
950+
map.putInt("mostRecentEventCount", mEditText.incrementAndGetEventCounter());
951+
map.putMap("textChanged", newAttributedString);
952+
953+
mEditText.mStateWrapper.updateState(map);
954+
}
955+
859956
// The event that contains the event counter and updates it must be sent first.
860957
// TODO: t7936714 merge these events
861958
mEventDispatcher.dispatchEvent(
@@ -1116,12 +1213,13 @@ public Object updateState(
11161213

11171214
view.mStateWrapper = stateWrapper;
11181215

1119-
return new ReactTextUpdate(
1216+
return ReactTextUpdate.buildReactTextUpdateFromState(
11201217
spanned,
11211218
state.getInt("mostRecentEventCount"),
11221219
false, // TODO add this into local Data
11231220
textViewProps.getTextAlign(),
11241221
textBreakStrategy,
1125-
justificationMode);
1222+
justificationMode,
1223+
attributedString);
11261224
}
11271225
}

0 commit comments

Comments
 (0)