Android TextInput: Fix updating of style props (#22994)

Summary:
For certain style props, each time any style prop changed, the previous version of the style would remain. For example, if you passed `"underline"` for `textDecorationLine` on a `TextInput` and then later passed `undefined` for `textDecorationLine`, the underline would remain.

We solved this problem before in de586bfa18. The fix was to use `manageSpans` to remove the old spans we added before adding the new spans. However, that fix hardcoded the list of spans to remove. Every span type that was introduced since that commit is affected by this bug:
  - CustomLetterSpacingSpan
  - CustomLineHeightSpan
  - CustomTextTransformSpan
  - ShadowStyleSpan
  - StrikethroughSpan
  - UnderlineSpan
  - TextInlineImageSpan

The reason this bug was reintroduced is `ReactBaseTextShadowNode` is responsible for adding spans and `ReactEditText` is responsible for removing spans. These classes fell out of sync.

This fix attempts a more robust solution. Every span that React Native adds to text now implements the `ReactSpan` interface. `manageSpans` deletes all spans that React Native adds by targeting the ones that implement `ReactSpan`. `ReactBaseTextShadowNode.SetSpanOperation` has been updated so that it's a compiler error to add a span that doesn't implement `ReactSpan`.
Pull Request resolved: https://github.com/facebook/react-native/pull/22994

Differential Revision: D13727580

Pulled By: mdvacca

fbshipit-source-id: 07b2eb08832efafb6c2806aa3329f0e9466b09fb
This commit is contained in:
Adam Comella
2019-01-18 11:31:24 -08:00
committed by Facebook Github Bot
parent 3144299b5a
commit 1f912b9f31
16 changed files with 127 additions and 44 deletions

View File

@@ -23,7 +23,7 @@ import com.facebook.infer.annotation.Assertions;
* spans affecting font size.
*/
@TargetApi(Build.VERSION_CODES.LOLLIPOP)
public class CustomLetterSpacingSpan extends MetricAffectingSpan {
public class CustomLetterSpacingSpan extends MetricAffectingSpan implements ReactSpan {
private final float mLetterSpacing;

View File

@@ -14,7 +14,7 @@ import android.text.style.LineHeightSpan;
* We use a custom {@link LineHeightSpan}, because `lineSpacingExtra` is broken. Details here:
* https://github.com/facebook/react-native/issues/7546
*/
public class CustomLineHeightSpan implements LineHeightSpan {
public class CustomLineHeightSpan implements LineHeightSpan, ReactSpan {
private final int mHeight;
CustomLineHeightSpan(float height) {

View File

@@ -15,7 +15,7 @@ import android.graphics.Typeface;
import android.text.TextPaint;
import android.text.style.MetricAffectingSpan;
public class CustomStyleSpan extends MetricAffectingSpan {
public class CustomStyleSpan extends MetricAffectingSpan implements ReactSpan {
/**
* A {@link MetricAffectingSpan} that allows to change the style of the displayed font.

View File

@@ -12,7 +12,7 @@ import android.graphics.Paint;
import android.text.style.ReplacementSpan;
import java.text.BreakIterator;
public class CustomTextTransformSpan extends ReplacementSpan {
public class CustomTextTransformSpan extends ReplacementSpan implements ReactSpan {
/**
* A {@link ReplacementSpan} that allows declarative changing of text casing.

View File

@@ -0,0 +1,19 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
package com.facebook.react.views.text;
import android.text.style.AbsoluteSizeSpan;
/*
* Wraps {@link AbsoluteSizeSpan} as a {@link ReactSpan}.
*/
public class ReactAbsoluteSizeSpan extends AbsoluteSizeSpan implements ReactSpan {
public ReactAbsoluteSizeSpan(int size) {
super(size);
}
}

View File

@@ -0,0 +1,19 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
package com.facebook.react.views.text;
import android.text.style.BackgroundColorSpan;
/*
* Wraps {@link BackgroundColorSpan} as a {@link ReactSpan}.
*/
public class ReactBackgroundColorSpan extends BackgroundColorSpan implements ReactSpan {
public ReactBackgroundColorSpan(int color) {
super(color);
}
}

View File

@@ -12,11 +12,6 @@ import android.os.Build;
import android.text.Layout;
import android.text.Spannable;
import android.text.SpannableStringBuilder;
import android.text.style.AbsoluteSizeSpan;
import android.text.style.BackgroundColorSpan;
import android.text.style.ForegroundColorSpan;
import android.text.style.StrikethroughSpan;
import android.text.style.UnderlineSpan;
import android.view.Gravity;
import com.facebook.react.bridge.JSApplicationIllegalArgumentException;
import com.facebook.react.bridge.ReadableMap;
@@ -24,8 +19,6 @@ import com.facebook.react.uimanager.IllegalViewOperationException;
import com.facebook.react.uimanager.LayoutShadowNode;
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.ReactShadowNode;
import com.facebook.react.uimanager.UIManagerModule;
import com.facebook.react.uimanager.ViewDefaults;
import com.facebook.react.uimanager.ViewProps;
import com.facebook.react.uimanager.annotations.ReactProp;
import com.facebook.yoga.YogaDirection;
@@ -60,9 +53,9 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
private static class SetSpanOperation {
protected int start, end;
protected Object what;
protected ReactSpan what;
SetSpanOperation(int start, int end, Object what) {
SetSpanOperation(int start, int end, ReactSpan what) {
this.start = start;
this.end = end;
this.what = what;
@@ -122,12 +115,12 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
int end = sb.length();
if (end >= start) {
if (textShadowNode.mIsColorSet) {
ops.add(new SetSpanOperation(start, end, new ForegroundColorSpan(textShadowNode.mColor)));
ops.add(new SetSpanOperation(start, end, new ReactForegroundColorSpan(textShadowNode.mColor)));
}
if (textShadowNode.mIsBackgroundColorSet) {
ops.add(
new SetSpanOperation(
start, end, new BackgroundColorSpan(textShadowNode.mBackgroundColor)));
start, end, new ReactBackgroundColorSpan(textShadowNode.mBackgroundColor)));
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
float effectiveLetterSpacing = textAttributes.getEffectiveLetterSpacing();
@@ -143,7 +136,7 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
if (// `getEffectiveFontSize` always returns a value so don't need to check for anything like
// `Float.NaN`.
parentTextAttributes == null || parentTextAttributes.getEffectiveFontSize() != effectiveFontSize) {
ops.add(new SetSpanOperation(start, end, new AbsoluteSizeSpan(effectiveFontSize)));
ops.add(new SetSpanOperation(start, end, new ReactAbsoluteSizeSpan(effectiveFontSize)));
}
if (textShadowNode.mFontStyle != UNSET
|| textShadowNode.mFontWeight != UNSET
@@ -159,10 +152,10 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
textShadowNode.getThemedContext().getAssets())));
}
if (textShadowNode.mIsUnderlineTextDecorationSet) {
ops.add(new SetSpanOperation(start, end, new UnderlineSpan()));
ops.add(new SetSpanOperation(start, end, new ReactUnderlineSpan()));
}
if (textShadowNode.mIsLineThroughTextDecorationSet) {
ops.add(new SetSpanOperation(start, end, new StrikethroughSpan()));
ops.add(new SetSpanOperation(start, end, new ReactStrikethroughSpan()));
}
if (
(

View File

@@ -0,0 +1,19 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
package com.facebook.react.views.text;
import android.text.style.ForegroundColorSpan;
/*
* Wraps {@link ForegroundColorSpan} as a {@link ReactSpan}.
*/
public class ReactForegroundColorSpan extends ForegroundColorSpan implements ReactSpan {
public ReactForegroundColorSpan(int color) {
super(color);
}
}

View File

@@ -0,0 +1,15 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
package com.facebook.react.views.text;
/*
* Enables us to distinguish between spans that were added by React Native and spans that were
* added by something else. All spans that React Native adds should implement this interface.
*/
public interface ReactSpan {
}

View File

@@ -0,0 +1,16 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
package com.facebook.react.views.text;
import android.text.style.StrikethroughSpan;
/*
* Wraps {@link StrikethroughSpan} as a {@link ReactSpan}.
*/
public class ReactStrikethroughSpan extends StrikethroughSpan implements ReactSpan {
}

View File

@@ -11,7 +11,7 @@ package com.facebook.react.views.text;
* Instances of this class are used to place reactTag information of nested text react nodes
* into spannable text rendered by single {@link TextView}
*/
public class ReactTagSpan {
public class ReactTagSpan implements ReactSpan {
private final int mReactTag;

View File

@@ -0,0 +1,16 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
package com.facebook.react.views.text;
import android.text.style.UnderlineSpan;
/*
* Wraps {@link UnderlineSpan} as a {@link ReactSpan}.
*/
public class ReactUnderlineSpan extends UnderlineSpan implements ReactSpan {
}

View File

@@ -11,7 +11,7 @@ package com.facebook.react.views.text;
import android.text.TextPaint;
import android.text.style.CharacterStyle;
public class ShadowStyleSpan extends CharacterStyle {
public class ShadowStyleSpan extends CharacterStyle implements ReactSpan {
private final float mDx, mDy, mRadius;
private final int mColor;

View File

@@ -18,7 +18,7 @@
/**
* Base class for inline image spans.
*/
public abstract class TextInlineImageSpan extends ReplacementSpan {
public abstract class TextInlineImageSpan extends ReplacementSpan implements ReactSpan {
/**
* For TextInlineImageSpan we need to update the Span to know that the window is attached and

View File

@@ -18,11 +18,6 @@ import android.text.SpannableStringBuilder;
import android.text.Spanned;
import android.text.StaticLayout;
import android.text.TextPaint;
import android.text.style.AbsoluteSizeSpan;
import android.text.style.BackgroundColorSpan;
import android.text.style.ForegroundColorSpan;
import android.text.style.StrikethroughSpan;
import android.text.style.UnderlineSpan;
import android.util.LruCache;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.bridge.ReadableArray;
@@ -88,12 +83,12 @@ public class TextLayoutManager {
int end = sb.length();
if (end >= start) {
if (textAttributes.mIsColorSet) {
ops.add(new SetSpanOperation(start, end, new ForegroundColorSpan(textAttributes.mColor)));
ops.add(new SetSpanOperation(start, end, new ReactForegroundColorSpan(textAttributes.mColor)));
}
if (textAttributes.mIsBackgroundColorSet) {
ops.add(
new SetSpanOperation(
start, end, new BackgroundColorSpan(textAttributes.mBackgroundColor)));
start, end, new ReactBackgroundColorSpan(textAttributes.mBackgroundColor)));
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
if (!Float.isNaN(textAttributes.mLetterSpacing)) {
@@ -105,7 +100,7 @@ public class TextLayoutManager {
}
ops.add(
new SetSpanOperation(
start, end, new AbsoluteSizeSpan(textAttributes.mFontSize)));
start, end, new ReactAbsoluteSizeSpan(textAttributes.mFontSize)));
if (textAttributes.mFontStyle != UNSET
|| textAttributes.mFontWeight != UNSET
|| textAttributes.mFontFamily != null) {
@@ -120,10 +115,10 @@ public class TextLayoutManager {
context.getAssets())));
}
if (textAttributes.mIsUnderlineTextDecorationSet) {
ops.add(new SetSpanOperation(start, end, new UnderlineSpan()));
ops.add(new SetSpanOperation(start, end, new ReactUnderlineSpan()));
}
if (textAttributes.mIsLineThroughTextDecorationSet) {
ops.add(new SetSpanOperation(start, end, new StrikethroughSpan()));
ops.add(new SetSpanOperation(start, end, new ReactStrikethroughSpan()));
}
if (textAttributes.mTextShadowOffsetDx != 0 || textAttributes.mTextShadowOffsetDy != 0) {
ops.add(
@@ -324,9 +319,9 @@ public class TextLayoutManager {
private static class SetSpanOperation {
protected int start, end;
protected Object what;
protected ReactSpan what;
SetSpanOperation(int start, int end, Object what) {
SetSpanOperation(int start, int end, ReactSpan what) {
this.start = start;
this.end = end;
this.what = what;

View File

@@ -20,9 +20,6 @@ import android.text.TextWatcher;
import android.text.TextUtils;
import android.text.method.KeyListener;
import android.text.method.QwertyKeyListener;
import android.text.style.AbsoluteSizeSpan;
import android.text.style.BackgroundColorSpan;
import android.text.style.ForegroundColorSpan;
import android.util.TypedValue;
import android.view.Gravity;
import android.view.KeyEvent;
@@ -34,10 +31,8 @@ import android.view.inputmethod.InputMethodManager;
import android.widget.EditText;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.UIManagerModule;
import com.facebook.react.views.text.CustomStyleSpan;
import com.facebook.react.views.text.ReactTagSpan;
import com.facebook.react.views.text.ReactSpan;
import com.facebook.react.views.text.ReactTextUpdate;
import com.facebook.react.views.text.TextAttributes;
import com.facebook.react.views.text.TextInlineImageSpan;
@@ -402,11 +397,7 @@ public class ReactEditText extends EditText {
Object[] spans = getText().getSpans(0, length(), Object.class);
for (int spanIdx = 0; spanIdx < spans.length; spanIdx++) {
// Remove all styling spans we might have previously set
if (ForegroundColorSpan.class.isInstance(spans[spanIdx]) ||
BackgroundColorSpan.class.isInstance(spans[spanIdx]) ||
AbsoluteSizeSpan.class.isInstance(spans[spanIdx]) ||
CustomStyleSpan.class.isInstance(spans[spanIdx]) ||
ReactTagSpan.class.isInstance(spans[spanIdx])) {
if (spans[spanIdx] instanceof ReactSpan) {
getText().removeSpan(spans[spanIdx]);
}