From b65ba6063d3336faca96719c8c7fef45ee277d4f Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 11:25:20 -0700 Subject: [PATCH 1/3] Minimize EditText Spans 5/N: Strikethrough and Underline Summary: This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior. This change makes us apply strikethrough and underline as paint flags to the underlying EditText, instead of just the spans. We then opt ReactUnderlineSpan and ReactStrikethroughSpan into being strippable. This does actually create visual behavior changes, where child text will inherit any underline or strikethrough of the root EditText (including if the child specifies `textDecorationLine: "none"`. The new behavior is consistent with both iOS and web though, so it seems like more of a bugfix than a regression. Changelog: [Android][Fixed] - Minimize Spans 5/N: Strikethrough and Underline Differential Revision: https://www.internalfb.com/diff/D44240778?entry_point=27 fbshipit-source-id: 6a2b9714425e1d3738ded5bbe66bab7500e7bc99 --- .../react/views/textinput/ReactEditText.java | 31 +++++++++++++++++++ .../textinput/ReactTextInputManager.java | 15 +++++++++ 2 files changed, 46 insertions(+) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 7b9e84a97b7526..3b31642f354345 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -12,6 +12,7 @@ import android.content.Context; import android.graphics.Color; +import android.graphics.Paint; import android.graphics.Rect; import android.graphics.Typeface; import android.graphics.drawable.Drawable; @@ -54,8 +55,10 @@ import com.facebook.react.views.text.ReactBackgroundColorSpan; import com.facebook.react.views.text.ReactForegroundColorSpan; import com.facebook.react.views.text.ReactSpan; +import com.facebook.react.views.text.ReactStrikethroughSpan; import com.facebook.react.views.text.ReactTextUpdate; import com.facebook.react.views.text.ReactTypefaceUtils; +import com.facebook.react.views.text.ReactUnderlineSpan; import com.facebook.react.views.text.TextAttributes; import com.facebook.react.views.text.TextInlineImageSpan; import com.facebook.react.views.text.TextLayoutManager; @@ -703,6 +706,26 @@ public boolean test(ReactForegroundColorSpan span) { return span.getForegroundColor() == getCurrentTextColor(); } }); + + stripSpansOfKind( + sb, + ReactStrikethroughSpan.class, + new SpanPredicate() { + @Override + public boolean test(ReactStrikethroughSpan span) { + return (getPaintFlags() & Paint.STRIKE_THRU_TEXT_FLAG) != 0; + } + }); + + stripSpansOfKind( + sb, + ReactUnderlineSpan.class, + new SpanPredicate() { + @Override + public boolean test(ReactUnderlineSpan span) { + return (getPaintFlags() & Paint.UNDERLINE_TEXT_FLAG) != 0; + } + }); } private void stripSpansOfKind( @@ -736,6 +759,14 @@ private void restoreStyleEquivalentSpans(SpannableStringBuilder workingText) { spans.add(new ReactBackgroundColorSpan(backgroundColor)); } + if ((getPaintFlags() & Paint.STRIKE_THRU_TEXT_FLAG) != 0) { + spans.add(new ReactStrikethroughSpan()); + } + + if ((getPaintFlags() & Paint.UNDERLINE_TEXT_FLAG) != 0) { + spans.add(new ReactUnderlineSpan()); + } + for (Object span : spans) { workingText.setSpan(span, 0, workingText.length(), spanFlags); } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java index f3c10a6dc92c36..82ca31fc247150 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java @@ -13,6 +13,7 @@ import android.content.res.ColorStateList; import android.graphics.BlendMode; import android.graphics.BlendModeColorFilter; +import android.graphics.Paint; import android.graphics.PorterDuff; import android.graphics.drawable.Drawable; import android.os.Build; @@ -935,6 +936,20 @@ public void setAutoFocus(ReactEditText view, boolean autoFocus) { view.setAutoFocus(autoFocus); } + @ReactProp(name = ViewProps.TEXT_DECORATION_LINE) + public void setTextDecorationLine(ReactEditText view, @Nullable String textDecorationLineString) { + view.setPaintFlags( + view.getPaintFlags() & ~(Paint.STRIKE_THRU_TEXT_FLAG | Paint.UNDERLINE_TEXT_FLAG)); + + for (String token : textDecorationLineString.split(" ")) { + if (token.equals("underline")) { + view.setPaintFlags(view.getPaintFlags() | Paint.UNDERLINE_TEXT_FLAG); + } else if (token.equals("line-through")) { + view.setPaintFlags(view.getPaintFlags() | Paint.STRIKE_THRU_TEXT_FLAG); + } + } + } + @ReactPropGroup( names = { ViewProps.BORDER_WIDTH, From 9d1667d10c3c1aad5b1ae6249a05eb03e90ac750 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 11:25:20 -0700 Subject: [PATCH 2/3] Minimize EditText Spans 6/N: letterSpacing Summary: This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior. This change lets us set `letterSpacing` on the EditText instead of using our custom span. Changelog: [Android][Fixed] - Minimize EditText Spans 6/N: letterSpacing Differential Revision: https://internalfb.com/D44240777 fbshipit-source-id: 53ecfa3f46df695ad8917099528d61efbafccf5a --- .../views/text/CustomLetterSpacingSpan.java | 4 ++++ .../react/views/textinput/ReactEditText.java | 23 ++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java index 3b9cf58e33d3a1..d537cd5dccc101 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java @@ -37,6 +37,10 @@ public void updateMeasureState(TextPaint paint) { apply(paint); } + public float getSpacing() { + return mLetterSpacing; + } + private void apply(TextPaint paint) { if (!Float.isNaN(mLetterSpacing)) { paint.setLetterSpacing(mLetterSpacing); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 3b31642f354345..041821112b8656 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -726,6 +726,18 @@ public boolean test(ReactUnderlineSpan span) { return (getPaintFlags() & Paint.UNDERLINE_TEXT_FLAG) != 0; } }); + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { + stripSpansOfKind( + sb, + CustomLetterSpacingSpan.class, + new SpanPredicate() { + @Override + public boolean test(CustomLetterSpacingSpan span) { + return span.getSpacing() == mTextAttributes.getEffectiveLetterSpacing(); + } + }); + } } private void stripSpansOfKind( @@ -767,6 +779,13 @@ private void restoreStyleEquivalentSpans(SpannableStringBuilder workingText) { spans.add(new ReactUnderlineSpan()); } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { + float effectiveLetterSpacing = mTextAttributes.getEffectiveLetterSpacing(); + if (!Float.isNaN(effectiveLetterSpacing)) { + spans.add(new CustomLetterSpacingSpan(effectiveLetterSpacing)); + } + } + for (Object span : spans) { workingText.setSpan(span, 0, workingText.length(), spanFlags); } @@ -1124,7 +1143,9 @@ protected void applyTextAttributes() { float effectiveLetterSpacing = mTextAttributes.getEffectiveLetterSpacing(); if (!Float.isNaN(effectiveLetterSpacing)) { - setLetterSpacing(effectiveLetterSpacing); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { + setLetterSpacing(effectiveLetterSpacing); + } } } From a2841ac96e4f71feff7794a2eee22ebb8ce6353e Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 11:25:45 -0700 Subject: [PATCH 3/3] Minimize EditText Spans 7/9: Avoid temp list (#36576) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36576 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior. This change addresses some minor CR feedback and removes the temporary list of spans in favor of applying them directly. Changelog: [Internal] Reviewed By: javache Differential Revision: D44295190 fbshipit-source-id: 058e753fbf7ea782062c2516e3227d392c28a0d7 --- .../react/views/textinput/ReactEditText.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 041821112b8656..a1b10ae1a4af7f 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -762,33 +762,39 @@ private void restoreStyleEquivalentSpans(SpannableStringBuilder workingText) { // (least precedence). This ensures the span is behind any overlapping spans. spanFlags |= Spannable.SPAN_PRIORITY; - List spans = new ArrayList<>(); - spans.add(new ReactAbsoluteSizeSpan(mTextAttributes.getEffectiveFontSize())); - spans.add(new ReactForegroundColorSpan(getCurrentTextColor())); + workingText.setSpan( + new ReactAbsoluteSizeSpan(mTextAttributes.getEffectiveFontSize()), + 0, + workingText.length(), + spanFlags); + + workingText.setSpan( + new ReactForegroundColorSpan(getCurrentTextColor()), 0, workingText.length(), spanFlags); int backgroundColor = mReactBackgroundManager.getBackgroundColor(); if (backgroundColor != Color.TRANSPARENT) { - spans.add(new ReactBackgroundColorSpan(backgroundColor)); + workingText.setSpan( + new ReactBackgroundColorSpan(backgroundColor), 0, workingText.length(), spanFlags); } if ((getPaintFlags() & Paint.STRIKE_THRU_TEXT_FLAG) != 0) { - spans.add(new ReactStrikethroughSpan()); + workingText.setSpan(new ReactStrikethroughSpan(), 0, workingText.length(), spanFlags); } if ((getPaintFlags() & Paint.UNDERLINE_TEXT_FLAG) != 0) { - spans.add(new ReactUnderlineSpan()); + workingText.setSpan(new ReactUnderlineSpan(), 0, workingText.length(), spanFlags); } if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { float effectiveLetterSpacing = mTextAttributes.getEffectiveLetterSpacing(); if (!Float.isNaN(effectiveLetterSpacing)) { - spans.add(new CustomLetterSpacingSpan(effectiveLetterSpacing)); + workingText.setSpan( + new CustomLetterSpacingSpan(effectiveLetterSpacing), + 0, + workingText.length(), + spanFlags); } } - - for (Object span : spans) { - workingText.setSpan(span, 0, workingText.length(), spanFlags); - } } private static boolean sameTextForSpan(