From 56e51b54f76dd202e947999c2147da9e4dc28f8a Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 04:21:17 -0700 Subject: [PATCH 1/3] Minimize EditText Spans 1/9: Fix precedence (#36543) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36543 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. We cache the backing EditText span on text change to later measure. To measure outside of a TextInput we need to restore any spans we removed. Spans may overlap, so base attributes should be behind everything else. The logic here for dealing with precedence is incorrect, and we should instead accomplish this by twiddling with the `SPAN_PRIORITY` bits. Changelog: [Android][Fixed] - Minimize Spans 1/N: Fix precedence Differential Revision: https://internalfb.com/D44240779 fbshipit-source-id: fdd9081aa4e8a51a69ef18f7ae33227893e6563f --- .../react/views/textinput/ReactEditText.java | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 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 07c95b243db62e..837f79702f12ce 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 @@ -683,29 +683,18 @@ private void stripAttributeEquivalentSpans(SpannableStringBuilder sb) { } } - private void unstripAttributeEquivalentSpans( - SpannableStringBuilder workingText, Spannable originalText) { - // We must add spans back for Fabric to be able to measure, at lower precedence than any - // existing spans. Remove all spans, add the attributes, then re-add the spans over - workingText.append(originalText); + private void unstripAttributeEquivalentSpans(SpannableStringBuilder workingText) { + int spanFlags = Spannable.SPAN_INCLUSIVE_INCLUSIVE; - for (Object span : workingText.getSpans(0, workingText.length(), Object.class)) { - workingText.removeSpan(span); - } + // Set all bits for SPAN_PRIORITY so that this span has the highest possible priority + // (least precedence). This ensures the span is behind any overlapping spans. + spanFlags |= Spannable.SPAN_PRIORITY; workingText.setSpan( new ReactAbsoluteSizeSpan(mTextAttributes.getEffectiveFontSize()), 0, workingText.length(), - Spanned.SPAN_INCLUSIVE_INCLUSIVE); - - for (Object span : originalText.getSpans(0, originalText.length(), Object.class)) { - workingText.setSpan( - span, - originalText.getSpanStart(span), - originalText.getSpanEnd(span), - originalText.getSpanFlags(span)); - } + spanFlags); } private static boolean sameTextForSpan( @@ -1132,8 +1121,8 @@ private void updateCachedSpannable(boolean resetStyles) { // ... // - android.app.Activity.dispatchKeyEvent (Activity.java:3447) try { - Spannable text = (Spannable) currentText.subSequence(0, currentText.length()); - unstripAttributeEquivalentSpans(sb, text); + sb.append(currentText.subSequence(0, currentText.length())); + unstripAttributeEquivalentSpans(sb); } catch (IndexOutOfBoundsException e) { ReactSoftExceptionLogger.logSoftException(TAG, e); } From 3ead142519c4a9b5404c203c4777cc5ef84f5f10 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 04:21:17 -0700 Subject: [PATCH 2/3] Minimize EditText Spans 2/9: Make stripAttributeEquivalentSpans generic (#36546) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36546 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 generalizes `stripAttributeEquivalentSpans()` to allow plugging in different spans. Changelog: [Internal] Differential Revision: https://www.internalfb.com/diff/D44240781?entry_point=27 fbshipit-source-id: 196b46a6edd3f0b1ba5f712f5739ab3fea30bf17 --- .../react/views/textinput/ReactEditText.java | 54 ++++++++++++------- 1 file changed, 34 insertions(+), 20 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 837f79702f12ce..b9c4461395bcf4 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 @@ -585,9 +585,7 @@ public void maybeSetText(ReactTextUpdate reactTextUpdate) { new SpannableStringBuilder(reactTextUpdate.getText()); manageSpans(spannableStringBuilder, reactTextUpdate.mContainsMultipleFragments); - - // Mitigation for https://github.com/facebook/react-native/issues/35936 (S318090) - stripAttributeEquivalentSpans(spannableStringBuilder); + stripStyleEquivalentSpans(spannableStringBuilder); mContainsImages = reactTextUpdate.containsImages(); @@ -662,28 +660,44 @@ private void manageSpans( } } - private void stripAttributeEquivalentSpans(SpannableStringBuilder sb) { - // We have already set a font size on the EditText itself. We can safely remove sizing spans - // which are the same as the set font size, and not otherwise overlapped. - final int effectiveFontSize = mTextAttributes.getEffectiveFontSize(); - ReactAbsoluteSizeSpan[] spans = sb.getSpans(0, sb.length(), ReactAbsoluteSizeSpan.class); + // TODO: Replace with Predicate and lambdas once Java 8 builds in OSS + interface SpanPredicate { + boolean test(T span); + } + + /** + * Remove spans from the SpannableStringBuilder which can be represented by TextAppearance + * attributes on the underlying EditText. This works around instability on Samsung devices with + * the presence of spans https://github.com/facebook/react-native/issues/35936 (S318090) + */ + private void stripStyleEquivalentSpans(SpannableStringBuilder sb) { + stripSpansOfKind( + sb, + ReactAbsoluteSizeSpan.class, + new SpanPredicate() { + @Override + public boolean test(ReactAbsoluteSizeSpan span) { + return span.getSize() == mTextAttributes.getEffectiveFontSize(); + } + }); + } - outerLoop: - for (ReactAbsoluteSizeSpan span : spans) { - ReactAbsoluteSizeSpan[] overlappingSpans = - sb.getSpans(sb.getSpanStart(span), sb.getSpanEnd(span), ReactAbsoluteSizeSpan.class); + private void stripSpansOfKind( + SpannableStringBuilder sb, Class clazz, SpanPredicate shouldStrip) { + T[] spans = sb.getSpans(0, sb.length(), clazz); - for (ReactAbsoluteSizeSpan overlappingSpan : overlappingSpans) { - if (span.getSize() != effectiveFontSize) { - continue outerLoop; - } + for (T span : spans) { + if (shouldStrip.test(span)) { + sb.removeSpan(span); } - - sb.removeSpan(span); } } - private void unstripAttributeEquivalentSpans(SpannableStringBuilder workingText) { + /** + * Copy back styles represented as attributes to the underlying span, for later measurement + * outside the ReactEditText. + */ + private void restoreStyleEquivalentSpans(SpannableStringBuilder workingText) { int spanFlags = Spannable.SPAN_INCLUSIVE_INCLUSIVE; // Set all bits for SPAN_PRIORITY so that this span has the highest possible priority @@ -1122,7 +1136,7 @@ private void updateCachedSpannable(boolean resetStyles) { // - android.app.Activity.dispatchKeyEvent (Activity.java:3447) try { sb.append(currentText.subSequence(0, currentText.length())); - unstripAttributeEquivalentSpans(sb); + restoreStyleEquivalentSpans(sb); } catch (IndexOutOfBoundsException e) { ReactSoftExceptionLogger.logSoftException(TAG, e); } From 51157fc47cb915ba63aabad0b07533f6c405b29e Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 04:21:35 -0700 Subject: [PATCH 3/3] Minimize EditText Spans 3/9: ReactBackgroundColorSpan (#36547) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36547 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 adds `ReactBackgroundColorSpan` to the list of spans eligible to be stripped. Changelog: [Android][Fixed] - Minimize Spans 3/N: ReactBackgroundColorSpan Reviewed By: javache Differential Revision: D44240782 fbshipit-source-id: e60f95c020f39f17207db202286aed70d084102f --- .../react/views/textinput/ReactEditText.java | 28 +++++++++++++++---- .../view/ReactViewBackgroundManager.java | 5 ++++ 2 files changed, 28 insertions(+), 5 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 b9c4461395bcf4..aba83f2b289e8a 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 @@ -11,6 +11,7 @@ import static com.facebook.react.views.text.TextAttributeProps.UNSET; import android.content.Context; +import android.graphics.Color; import android.graphics.Rect; import android.graphics.Typeface; import android.graphics.drawable.Drawable; @@ -50,6 +51,7 @@ import com.facebook.react.views.text.CustomLineHeightSpan; import com.facebook.react.views.text.CustomStyleSpan; import com.facebook.react.views.text.ReactAbsoluteSizeSpan; +import com.facebook.react.views.text.ReactBackgroundColorSpan; import com.facebook.react.views.text.ReactSpan; import com.facebook.react.views.text.ReactTextUpdate; import com.facebook.react.views.text.ReactTypefaceUtils; @@ -680,6 +682,16 @@ public boolean test(ReactAbsoluteSizeSpan span) { return span.getSize() == mTextAttributes.getEffectiveFontSize(); } }); + + stripSpansOfKind( + sb, + ReactBackgroundColorSpan.class, + new SpanPredicate() { + @Override + public boolean test(ReactBackgroundColorSpan span) { + return span.getBackgroundColor() == mReactBackgroundManager.getBackgroundColor(); + } + }); } private void stripSpansOfKind( @@ -704,11 +716,17 @@ private void restoreStyleEquivalentSpans(SpannableStringBuilder workingText) { // (least precedence). This ensures the span is behind any overlapping spans. spanFlags |= Spannable.SPAN_PRIORITY; - workingText.setSpan( - new ReactAbsoluteSizeSpan(mTextAttributes.getEffectiveFontSize()), - 0, - workingText.length(), - spanFlags); + List spans = new ArrayList<>(); + spans.add(new ReactAbsoluteSizeSpan(mTextAttributes.getEffectiveFontSize())); + + int backgroundColor = mReactBackgroundManager.getBackgroundColor(); + if (backgroundColor != Color.TRANSPARENT) { + spans.add(new ReactBackgroundColorSpan(backgroundColor)); + } + + for (Object span : spans) { + workingText.setSpan(span, 0, workingText.length(), spanFlags); + } } private static boolean sameTextForSpan( diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundManager.java index 4a5fce52c3a9e5..9b479fa42214be 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundManager.java @@ -19,6 +19,7 @@ public class ReactViewBackgroundManager { private @Nullable ReactViewBackgroundDrawable mReactBackgroundDrawable; private View mView; + private int mColor = Color.TRANSPARENT; public ReactViewBackgroundManager(View view) { this.mView = view; @@ -56,6 +57,10 @@ public void setBackgroundColor(int color) { } } + public int getBackgroundColor() { + return mColor; + } + public void setBorderWidth(int position, float width) { getOrCreateReactViewBackground().setBorderWidth(position, width); }