Skip to content

Commit

Permalink
Fix crash if set text and set selection at the same time. (#22723)
Browse files Browse the repository at this point in the history
Summary:
Since text and selection has dependency, handle text selection in
updateExtraData as well.

The root cause is due to setText is handled on extra data update but setSelection is handled on set property. And extra data update will be handled after all properties are handled. Since selection and text has dependency, move selection to extra data update as well.

Changelog:
----------
[Android] [Fixed] - Fix crash when set text and selection on textinput at the same time
Pull Request resolved: #22723

Differential Revision: D14783791

Pulled By: cpojer

fbshipit-source-id: a4065f3e151d23f4813d76e91d68362cfd4daaf4
  • Loading branch information
YuTeh Shen authored and facebook-github-bot committed Apr 4, 2019
1 parent ad708eb commit 84a1cac
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ public class ReactTextUpdate {
private final float mPaddingBottom;
private final int mTextAlign;
private final int mTextBreakStrategy;
private final int mSelectionStart;
private final int mSelectionEnd;
private final int mJustificationMode;

/**
Expand All @@ -51,7 +53,9 @@ public ReactTextUpdate(
paddingBottom,
textAlign,
Layout.BREAK_STRATEGY_HIGH_QUALITY,
Layout.JUSTIFICATION_MODE_NONE);
Layout.JUSTIFICATION_MODE_NONE,
-1,
-1);
}

public ReactTextUpdate(
Expand All @@ -65,6 +69,33 @@ public ReactTextUpdate(
int textAlign,
int textBreakStrategy,
int justificationMode) {
this(text,
jsEventCounter,
containsImages,
paddingStart,
paddingTop,
paddingEnd,
paddingBottom,
textAlign,
textBreakStrategy,
justificationMode,
-1,
-1);
}

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;
mJsEventCounter = jsEventCounter;
mContainsImages = containsImages;
Expand All @@ -74,6 +105,8 @@ public ReactTextUpdate(
mPaddingBottom = paddingBottom;
mTextAlign = textAlign;
mTextBreakStrategy = textBreakStrategy;
mSelectionStart = selectionStart;
mSelectionEnd = selectionEnd;
mJustificationMode = justificationMode;
}

Expand Down Expand Up @@ -116,4 +149,12 @@ public int getTextBreakStrategy() {
public int getJustificationMode() {
return mJustificationMode;
}

public int getSelectionStart() {
return mSelectionStart;
}

public int getSelectionEnd() {
return mSelectionEnd;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ public void updateExtraData(ReactEditText view, Object extraData) {
TextInlineImageSpan.possiblyUpdateInlineImageSpans(spannable, view);
}
view.maybeSetText(update);
if (update.getSelectionStart() != UNSET && update.getSelectionEnd() != UNSET)
view.setSelection(update.getSelectionStart(), update.getSelectionEnd());
}
}

Expand Down Expand Up @@ -274,17 +276,6 @@ public void setFontStyle(ReactEditText view, @Nullable String fontStyleString) {
}
}

@ReactProp(name = "selection")
public void setSelection(ReactEditText view, @Nullable ReadableMap selection) {
if (selection == null) {
return;
}

if (selection.hasKey("start") && selection.hasKey("end")) {
view.setSelection(selection.getInt("start"), selection.getInt("end"));
}
}

@ReactProp(name = "importantForAutofill")
public void setImportantForAutofill(ReactEditText view, @Nullable String value) {
int mode = View.IMPORTANT_FOR_AUTOFILL_AUTO;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import android.widget.EditText;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.JSApplicationIllegalArgumentException;
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.common.annotations.VisibleForTesting;
import com.facebook.react.uimanager.LayoutShadowNode;
import com.facebook.react.uimanager.NativeViewHierarchyOptimizer;
Expand Down Expand Up @@ -46,10 +47,13 @@ public class ReactTextInputShadowNode extends ReactBaseTextShadowNode

@VisibleForTesting public static final String PROP_TEXT = "text";
@VisibleForTesting public static final String PROP_PLACEHOLDER = "placeholder";
@VisibleForTesting public static final String PROP_SELECTION = "selection";

// Represents the {@code text} property only, not possible nested content.
private @Nullable String mText = null;
private @Nullable String mPlaceholder = null;
private int mSelectionStart = UNSET;
private int mSelectionEnd = UNSET;

public ReactTextInputShadowNode() {
mTextBreakStrategy = (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) ?
Expand Down Expand Up @@ -173,6 +177,19 @@ public void setPlaceholder(@Nullable String placeholder) {
return mPlaceholder;
}

@ReactProp(name = PROP_SELECTION)
public void setSelection(@Nullable ReadableMap selection) {
mSelectionStart = mSelectionEnd = UNSET;
if (selection == null)
return;

if (selection.hasKey("start") && selection.hasKey("end")) {
mSelectionStart = selection.getInt("start");
mSelectionEnd = selection.getInt("end");
markUpdated();
}
}

@Override
public void setTextBreakStrategy(@Nullable String textBreakStrategy) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
Expand Down Expand Up @@ -211,7 +228,9 @@ public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) {
getPadding(Spacing.BOTTOM),
mTextAlign,
mTextBreakStrategy,
mJustificationMode);
mJustificationMode,
mSelectionStart,
mSelectionEnd);
uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate);
}
}
Expand Down

0 comments on commit 84a1cac

Please sign in to comment.