From 2ddef5690299fcbe1f60cec3b4962490674d3727 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Thu, 9 Feb 2023 09:56:05 -0800 Subject: [PATCH 01/11] Fix measurement of uncontrolled TextInput after edit Summary: D42721684 (https://github.com/facebook/react-native/commit/be69c8b5a77ae60cced1b2af64e48b90d9955be5) left a pretty bad bug when using Fabric for Android. I missed that in Fabric specifically, on edit we will cache the Spannable backing the EditText for use in future measurement. Because we've stripped the sizing spans, Spannable measurement has incorrect font size, and the TextInput size will change (collapsing) after the first edit. This effectively breaks any uncontrolled TextInput which does not have explicit dimensions set. Changelog: [Android][Fixed] - Fix measurement of uncontrolled TextInput after edit Reviewed By: sammy-SC Differential Revision: D43158407 fbshipit-source-id: 51602eab06c9a50e2b60ef0ed87bdb4df025e51e --- .../react/views/textinput/ReactEditText.java | 32 +++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 77ab6bb365b..4549d1075ac 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -557,7 +557,7 @@ public class ReactEditText extends AppCompatEditText manageSpans(spannableStringBuilder, reactTextUpdate.mContainsMultipleFragments); // Mitigation for https://github.com/facebook/react-native/issues/35936 (S318090) - stripAbsoluteSizeSpans(spannableStringBuilder); + stripAtributeEquivalentSpans(spannableStringBuilder); mContainsImages = reactTextUpdate.containsImages(); @@ -632,7 +632,7 @@ public class ReactEditText extends AppCompatEditText } } - private void stripAbsoluteSizeSpans(SpannableStringBuilder sb) { + private void stripAtributeEquivalentSpans(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(); @@ -653,6 +653,31 @@ public class ReactEditText extends AppCompatEditText } } + 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); + + for (Object span : workingText.getSpans(0, workingText.length(), Object.class)) { + workingText.removeSpan(span); + } + + 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)); + } + } + private static boolean sameTextForSpan( final Editable oldText, final SpannableStringBuilder newText, @@ -1072,7 +1097,8 @@ public class ReactEditText extends AppCompatEditText // ... // - android.app.Activity.dispatchKeyEvent (Activity.java:3447) try { - sb.append(currentText.subSequence(0, currentText.length())); + Spannable text = (Spannable) currentText.subSequence(0, currentText.length()); + unstripAttributeEquivalentSpans(sb, text); } catch (IndexOutOfBoundsException e) { ReactSoftExceptionLogger.logSoftException(TAG, e); } From 6d8868b2d8aa5ba2d5c90620ce355acbdabe5052 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 05:24:09 -0700 Subject: [PATCH 02/11] 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 Reviewed By: javache Differential Revision: D44240779 fbshipit-source-id: f731b353587888faad946b8cf1e868095cdeced3 --- .../react/views/textinput/ReactEditText.java | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 4549d1075ac..8692009c9f5 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -653,29 +653,18 @@ public class ReactEditText extends AppCompatEditText } } - 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( @@ -1097,8 +1086,8 @@ public class ReactEditText extends AppCompatEditText // ... // - 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 ff991aa15b62ac0ff5efbba7af006350318f8f07 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 05:24:09 -0700 Subject: [PATCH 03/11] 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] Reviewed By: rshest Differential Revision: D44240781 fbshipit-source-id: 89005266020f216368e9ad9ce382699bd8db85a8 # Conflicts: # ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java --- .../react/views/textinput/ReactEditText.java | 54 ++++++++++++------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 8692009c9f5..0d0add3b5e8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -555,9 +555,7 @@ public class ReactEditText extends AppCompatEditText new SpannableStringBuilder(reactTextUpdate.getText()); manageSpans(spannableStringBuilder, reactTextUpdate.mContainsMultipleFragments); - - // Mitigation for https://github.com/facebook/react-native/issues/35936 (S318090) - stripAtributeEquivalentSpans(spannableStringBuilder); + stripStyleEquivalentSpans(spannableStringBuilder); mContainsImages = reactTextUpdate.containsImages(); @@ -632,28 +630,44 @@ public class ReactEditText extends AppCompatEditText } } - private void stripAtributeEquivalentSpans(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); + } - outerLoop: - for (ReactAbsoluteSizeSpan span : spans) { - ReactAbsoluteSizeSpan[] overlappingSpans = - sb.getSpans(sb.getSpanStart(span), sb.getSpanEnd(span), ReactAbsoluteSizeSpan.class); + /** + * 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(); + } + }); + } - for (ReactAbsoluteSizeSpan overlappingSpan : overlappingSpans) { - if (span.getSize() != effectiveFontSize) { - continue outerLoop; - } + private void stripSpansOfKind( + SpannableStringBuilder sb, Class clazz, SpanPredicate shouldStrip) { + T[] spans = sb.getSpans(0, sb.length(), clazz); + + 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 @@ -1087,7 +1101,7 @@ public class ReactEditText extends AppCompatEditText // - 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 62d992019e238cfadb2328b14e6967e6f6d8d722 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 05:24:09 -0700 Subject: [PATCH 04/11] 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: 2ded1a1687a41cf6d5f83e89ffadd2d932089969 --- .../react/views/textinput/ReactEditText.java | 28 +++++++++++++++---- .../view/ReactViewBackgroundManager.java | 5 ++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 0d0add3b5e8..9f78449f561 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -11,6 +11,7 @@ import static com.facebook.react.uimanager.UIManagerHelper.getReactContext; 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.CustomLetterSpacingSpan; 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; @@ -650,6 +652,16 @@ public class ReactEditText extends AppCompatEditText 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( @@ -674,11 +686,17 @@ public class ReactEditText extends AppCompatEditText // (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/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundManager.java index 4a5fce52c3a..9b479fa4221 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewBackgroundManager.java +++ b/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 class ReactViewBackgroundManager { } } + public int getBackgroundColor() { + return mColor; + } + public void setBorderWidth(int position, float width) { getOrCreateReactViewBackground().setBorderWidth(position, width); } From 205623cbe8a90fb4ab8d8035ae1e71c52fd87829 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 05:24:09 -0700 Subject: [PATCH 05/11] Minimize EditText Spans 4/9: ReactForegroundColorSpan (#36545) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36545 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 ReactForegroundColorSpan to the list of spans eligible to be stripped. Changelog: [Android][Fixed] - Minimize Spans 4/N: ReactForegroundColorSpan Reviewed By: javache Differential Revision: D44240780 fbshipit-source-id: d86939cc2d7ed9116a4167026c7d48928fc51757 --- .../react/views/textinput/ReactEditText.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 9f78449f561..3869110194a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -52,6 +52,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.ReactForegroundColorSpan; import com.facebook.react.views.text.ReactSpan; import com.facebook.react.views.text.ReactTextUpdate; import com.facebook.react.views.text.ReactTypefaceUtils; @@ -662,6 +663,16 @@ public class ReactEditText extends AppCompatEditText return span.getBackgroundColor() == mReactBackgroundManager.getBackgroundColor(); } }); + + stripSpansOfKind( + sb, + ReactForegroundColorSpan.class, + new SpanPredicate() { + @Override + public boolean test(ReactForegroundColorSpan span) { + return span.getForegroundColor() == getCurrentTextColor(); + } + }); } private void stripSpansOfKind( @@ -688,6 +699,7 @@ public class ReactEditText extends AppCompatEditText List spans = new ArrayList<>(); spans.add(new ReactAbsoluteSizeSpan(mTextAttributes.getEffectiveFontSize())); + spans.add(new ReactForegroundColorSpan(getCurrentTextColor())); int backgroundColor = mReactBackgroundManager.getBackgroundColor(); if (backgroundColor != Color.TRANSPARENT) { From f1c33b9e154cb31cb8d86bc2d7547574c3953abd Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 12:31:22 -0700 Subject: [PATCH 06/11] Minimize EditText Spans 5/9: Strikethrough and Underline (#36544) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36544 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 Reviewed By: rshest Differential Revision: D44240778 fbshipit-source-id: d564dfc0121057a5e3b09bb71b8f5662e28be17e --- .../react/views/textinput/ReactEditText.java | 31 +++++++++++++++++++ .../textinput/ReactTextInputManager.java | 15 +++++++++ 2 files changed, 46 insertions(+) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 3869110194a..d5ca89667c5 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -12,6 +12,7 @@ import static com.facebook.react.views.text.TextAttributeProps.UNSET; 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.ReactAbsoluteSizeSpan; 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; @@ -673,6 +676,26 @@ public class ReactEditText extends AppCompatEditText 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( @@ -706,6 +729,14 @@ public class ReactEditText extends AppCompatEditText 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/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java index f3134f9cb1d..252e45bdebb 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java @@ -13,6 +13,7 @@ import android.content.Context; 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; @@ -912,6 +913,20 @@ public class ReactTextInputManager extends BaseViewManager Date: Fri, 24 Mar 2023 12:31:22 -0700 Subject: [PATCH 07/11] Minimize EditText Spans 6/9: letterSpacing (#36548) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36548 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 Reviewed By: rshest Differential Revision: D44240777 fbshipit-source-id: 9bd10c3261257037d8cacf37971011aaa94d1a77 --- .../views/text/CustomLetterSpacingSpan.java | 4 ++++ .../react/views/textinput/ReactEditText.java | 23 ++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java index 3b9cf58e33d..d537cd5dccc 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java @@ -37,6 +37,10 @@ public class CustomLetterSpacingSpan extends MetricAffectingSpan implements Reac apply(paint); } + public float getSpacing() { + return mLetterSpacing; + } + private void apply(TextPaint paint) { if (!Float.isNaN(mLetterSpacing)) { paint.setLetterSpacing(mLetterSpacing); diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index d5ca89667c5..8a03fb36eec 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -696,6 +696,18 @@ public class ReactEditText extends AppCompatEditText 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( @@ -737,6 +749,13 @@ public class ReactEditText extends AppCompatEditText 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); } @@ -1089,7 +1108,9 @@ public class ReactEditText extends AppCompatEditText float effectiveLetterSpacing = mTextAttributes.getEffectiveLetterSpacing(); if (!Float.isNaN(effectiveLetterSpacing)) { - setLetterSpacing(effectiveLetterSpacing); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { + setLetterSpacing(effectiveLetterSpacing); + } } } From 471cc514a84362a56fa534ec3a3cb9e8a2872897 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 12:31:22 -0700 Subject: [PATCH 08/11] 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: bd784e2c514301d45d0bacd8ee6de5c512fc565c --- .../react/views/textinput/ReactEditText.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 8a03fb36eec..26ebb2e7cb4 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -732,33 +732,39 @@ public class ReactEditText extends AppCompatEditText // (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( From 322f721c4a68f1a16cfe5c6d3219e7c6fc0d7274 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 12:31:22 -0700 Subject: [PATCH 09/11] Minimize EditText Spans 8/9: CustomStyleSpan (#36577) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36577 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 allows us to strip CustomStyleSpan. We already set all but `fontVariant` on the underlying EditText, so we just need to route that through as well. Note that because this span is non-parcelable, it is seemingly not subject to the buggy behavior on Samsung devices of infinitely cloning the spans, but non-parcelable spans have different issues on the devices (they disappear), so moving `fontVariant` to the top-level makes sense here. Changelog: [Android][Fixed] - Minimize EditText Spans 8/N: CustomStyleSpan Reviewed By: javache Differential Revision: D44297384 fbshipit-source-id: ed4c000e961dd456a2a8f4397e27c23a87defb6e --- .../react/views/text/CustomStyleSpan.java | 4 ++ .../react/views/textinput/ReactEditText.java | 49 +++++++++++++++++++ .../textinput/ReactTextInputManager.java | 6 +++ 3 files changed, 59 insertions(+) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomStyleSpan.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomStyleSpan.java index b249126cf95..7866390bfa0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomStyleSpan.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomStyleSpan.java @@ -71,6 +71,10 @@ public class CustomStyleSpan extends MetricAffectingSpan implements ReactSpan { return mFontFamily; } + public @Nullable String getFontFeatureSettings() { + return mFeatureSettings; + } + private static void apply( Paint paint, int style, diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 26ebb2e7cb4..f8987fb8089 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -65,6 +65,7 @@ import com.facebook.react.views.text.TextLayoutManager; import com.facebook.react.views.view.ReactViewBackgroundManager; import java.util.ArrayList; import java.util.List; +import java.util.Objects; /** * A wrapper around the EditText that lets us better control what happens when an EditText gets @@ -488,6 +489,14 @@ public class ReactEditText extends AppCompatEditText } } + @Override + public void setFontFeatureSettings(String fontFeatureSettings) { + if (!Objects.equals(fontFeatureSettings, getFontFeatureSettings())) { + super.setFontFeatureSettings(fontFeatureSettings); + mTypefaceDirty = true; + } + } + public void maybeUpdateTypeface() { if (!mTypefaceDirty) { return; @@ -499,6 +508,17 @@ public class ReactEditText extends AppCompatEditText ReactTypefaceUtils.applyStyles( getTypeface(), mFontStyle, mFontWeight, mFontFamily, getContext().getAssets()); setTypeface(newTypeface); + + // Match behavior of CustomStyleSpan and enable SUBPIXEL_TEXT_FLAG when setting anything + // nonstandard + if (mFontStyle != UNSET + || mFontWeight != UNSET + || mFontFamily != null + || getFontFeatureSettings() != null) { + setPaintFlags(getPaintFlags() | Paint.SUBPIXEL_TEXT_FLAG); + } else { + setPaintFlags(getPaintFlags() & (~Paint.SUBPIXEL_TEXT_FLAG)); + } } // VisibleForTesting from {@link TextInputEventsTestCase}. @@ -708,6 +728,19 @@ public class ReactEditText extends AppCompatEditText } }); } + + stripSpansOfKind( + sb, + CustomStyleSpan.class, + new SpanPredicate() { + @Override + public boolean test(CustomStyleSpan span) { + return span.getStyle() == mFontStyle + && Objects.equals(span.getFontFamily(), mFontFamily) + && span.getWeight() == mFontWeight + && Objects.equals(span.getFontFeatureSettings(), getFontFeatureSettings()); + } + }); } private void stripSpansOfKind( @@ -765,6 +798,22 @@ public class ReactEditText extends AppCompatEditText spanFlags); } } + + if (mFontStyle != UNSET + || mFontWeight != UNSET + || mFontFamily != null + || getFontFeatureSettings() != null) { + workingText.setSpan( + new CustomStyleSpan( + mFontStyle, + mFontWeight, + getFontFeatureSettings(), + mFontFamily, + getContext().getAssets()), + 0, + workingText.length(), + spanFlags); + } } private static boolean sameTextForSpan( diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java index 252e45bdebb..d62251bc319 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java @@ -67,6 +67,7 @@ import com.facebook.react.views.text.DefaultStyleValuesUtil; import com.facebook.react.views.text.ReactBaseTextShadowNode; import com.facebook.react.views.text.ReactTextUpdate; import com.facebook.react.views.text.ReactTextViewManagerCallback; +import com.facebook.react.views.text.ReactTypefaceUtils; import com.facebook.react.views.text.TextAttributeProps; import com.facebook.react.views.text.TextInlineImageSpan; import com.facebook.react.views.text.TextLayoutManager; @@ -397,6 +398,11 @@ public class ReactTextInputManager extends BaseViewManager Date: Tue, 16 Aug 2022 12:21:52 -0700 Subject: [PATCH 10/11] Support MapBuffer in TextInput State Summary: Support MapBuffer in TextInput State. Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D38546271 fbshipit-source-id: 93e7fd79d9d8473dd646410f3047fcfafa8516f1 --- .../text/TextLayoutManagerMapBuffer.java | 5 ++ .../com/facebook/react/views/textinput/BUCK | 5 +- .../textinput/ReactTextInputManager.java | 51 +++++++++++++++++++ .../renderer/components/text/ParagraphState.h | 9 ++++ .../renderer/components/text/conversions.h | 10 +--- .../AndroidTextInputState.cpp | 22 ++++++++ .../androidtextinput/AndroidTextInputState.h | 4 +- 7 files changed, 93 insertions(+), 13 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java index ecbf0d45489..d86cf9a095a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java @@ -106,6 +106,11 @@ public class TextLayoutManagerMapBuffer { MapBuffer fragment = fragments.getMapBuffer((short) 0); MapBuffer textAttributes = fragment.getMapBuffer(FR_KEY_TEXT_ATTRIBUTES); + + if (!textAttributes.contains(TextAttributeProps.TA_KEY_LAYOUT_DIRECTION)) { + return false; + } + return TextAttributeProps.getLayoutDirection( textAttributes.getString(TextAttributeProps.TA_KEY_LAYOUT_DIRECTION)) == LayoutDirection.RTL; diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/BUCK b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/BUCK index c3930446837..ef99a74bf37 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/BUCK @@ -9,7 +9,8 @@ rn_android_library( "pfh:ReactNative_CommonInfrastructurePlaceholder", "supermodule:xplat/default/public.react_native.infra", ], - language = "JAVA", + language = "KOTLIN", + pure_kotlin = False, required_for_source_only_abi = True, visibility = [ "PUBLIC", @@ -29,7 +30,9 @@ rn_android_library( react_native_target("java/com/facebook/react/views/imagehelper:imagehelper"), react_native_target("java/com/facebook/react/views/scroll:scroll"), react_native_target("java/com/facebook/react/views/text:text"), + react_native_target("java/com/facebook/react/common/mapbuffer:mapbuffer"), react_native_target("java/com/facebook/react/views/view:view"), + react_native_target("java/com/facebook/react/config:config"), ], exported_deps = [ react_native_dep("third-party/android/androidx:appcompat"), diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java index d62251bc319..99025ee3ccc 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java @@ -45,6 +45,8 @@ import com.facebook.react.bridge.ReadableType; import com.facebook.react.bridge.WritableMap; import com.facebook.react.bridge.WritableNativeMap; import com.facebook.react.common.MapBuilder; +import com.facebook.react.common.mapbuffer.MapBuffer; +import com.facebook.react.config.ReactFeatureFlags; import com.facebook.react.module.annotations.ReactModule; import com.facebook.react.uimanager.BaseViewManager; import com.facebook.react.uimanager.FabricViewStateManager; @@ -71,6 +73,7 @@ import com.facebook.react.views.text.ReactTypefaceUtils; import com.facebook.react.views.text.TextAttributeProps; import com.facebook.react.views.text.TextInlineImageSpan; import com.facebook.react.views.text.TextLayoutManager; +import com.facebook.react.views.text.TextLayoutManagerMapBuffer; import com.facebook.react.views.text.TextTransform; import com.facebook.yoga.YogaConstants; import java.lang.reflect.Field; @@ -85,6 +88,12 @@ public class ReactTextInputManager extends BaseViewManager 1; + + int textBreakStrategy = + TextAttributeProps.getTextBreakStrategy( + paragraphAttributes.getString(TextLayoutManagerMapBuffer.PA_KEY_TEXT_BREAK_STRATEGY)); + + return ReactTextUpdate.buildReactTextUpdateFromState( + spanned, + state.getInt(TX_STATE_KEY_MOST_RECENT_EVENT_COUNT), + TextAttributeProps.getTextAlignment( + props, TextLayoutManagerMapBuffer.isRTL(attributedString)), + textBreakStrategy, + TextAttributeProps.getJustificationMode(props), + containsMultipleFragments); + } } diff --git a/ReactCommon/react/renderer/components/text/ParagraphState.h b/ReactCommon/react/renderer/components/text/ParagraphState.h index 76dcbcf1cee..24a8e1258e8 100644 --- a/ReactCommon/react/renderer/components/text/ParagraphState.h +++ b/ReactCommon/react/renderer/components/text/ParagraphState.h @@ -20,6 +20,15 @@ namespace facebook { namespace react { +#ifdef ANDROID +// constants for Text State serialization +constexpr static MapBuffer::Key TX_STATE_KEY_ATTRIBUTED_STRING = 0; +constexpr static MapBuffer::Key TX_STATE_KEY_PARAGRAPH_ATTRIBUTES = 1; +// Used for TextInput only +constexpr static MapBuffer::Key TX_STATE_KEY_HASH = 2; +constexpr static MapBuffer::Key TX_STATE_KEY_MOST_RECENT_EVENT_COUNT = 3; +#endif + /* * State for component. * Represents what to render and how to render. diff --git a/ReactCommon/react/renderer/components/text/conversions.h b/ReactCommon/react/renderer/components/text/conversions.h index 712b8f2fde2..a4a90aa37c9 100644 --- a/ReactCommon/react/renderer/components/text/conversions.h +++ b/ReactCommon/react/renderer/components/text/conversions.h @@ -26,21 +26,13 @@ inline folly::dynamic toDynamic(ParagraphState const ¶graphState) { return newState; } -// constants for Text State serialization -constexpr static MapBuffer::Key TX_STATE_KEY_ATTRIBUTED_STRING = 0; -constexpr static MapBuffer::Key TX_STATE_KEY_PARAGRAPH_ATTRIBUTES = 1; -// Used for TextInput -constexpr static MapBuffer::Key TX_STATE_KEY_HASH = 2; -constexpr static MapBuffer::Key TX_STATE_KEY_MOST_RECENT_EVENT_COUNT = 3; - inline MapBuffer toMapBuffer(ParagraphState const ¶graphState) { auto builder = MapBufferBuilder(); auto attStringMapBuffer = toMapBuffer(paragraphState.attributedString); builder.putMapBuffer(TX_STATE_KEY_ATTRIBUTED_STRING, attStringMapBuffer); auto paMapBuffer = toMapBuffer(paragraphState.paragraphAttributes); builder.putMapBuffer(TX_STATE_KEY_PARAGRAPH_ATTRIBUTES, paMapBuffer); - // TODO: Used for TextInput - builder.putInt(TX_STATE_KEY_HASH, 1234); + builder.putInt(TX_STATE_KEY_HASH, attStringMapBuffer.getInt(AS_KEY_HASH)); return builder.build(); } #endif diff --git a/ReactCommon/react/renderer/components/textinput/androidtextinput/react/renderer/components/androidtextinput/AndroidTextInputState.cpp b/ReactCommon/react/renderer/components/textinput/androidtextinput/react/renderer/components/androidtextinput/AndroidTextInputState.cpp index a2deb59db33..c57e8b7970f 100644 --- a/ReactCommon/react/renderer/components/textinput/androidtextinput/react/renderer/components/androidtextinput/AndroidTextInputState.cpp +++ b/ReactCommon/react/renderer/components/textinput/androidtextinput/react/renderer/components/androidtextinput/AndroidTextInputState.cpp @@ -10,6 +10,11 @@ #include #include +#ifdef ANDROID +#include +#include +#endif + #include namespace facebook { @@ -89,6 +94,23 @@ folly::dynamic AndroidTextInputState::getDynamic() const { } return newState; } + +MapBuffer AndroidTextInputState::getMapBuffer() const { + auto builder = MapBufferBuilder(); + // See comment in getDynamic block. + if (cachedAttributedStringId == 0) { + builder.putInt(TX_STATE_KEY_MOST_RECENT_EVENT_COUNT, mostRecentEventCount); + + auto attStringMapBuffer = toMapBuffer(attributedString); + builder.putMapBuffer(TX_STATE_KEY_ATTRIBUTED_STRING, attStringMapBuffer); + auto paMapBuffer = toMapBuffer(paragraphAttributes); + builder.putMapBuffer(TX_STATE_KEY_PARAGRAPH_ATTRIBUTES, paMapBuffer); + + builder.putInt(TX_STATE_KEY_HASH, attStringMapBuffer.getInt(AS_KEY_HASH)); + } + return builder.build(); +} + #endif } // namespace react diff --git a/ReactCommon/react/renderer/components/textinput/androidtextinput/react/renderer/components/androidtextinput/AndroidTextInputState.h b/ReactCommon/react/renderer/components/textinput/androidtextinput/react/renderer/components/androidtextinput/AndroidTextInputState.h index 4dc50ac9410..f46d6829e1a 100644 --- a/ReactCommon/react/renderer/components/textinput/androidtextinput/react/renderer/components/androidtextinput/AndroidTextInputState.h +++ b/ReactCommon/react/renderer/components/textinput/androidtextinput/react/renderer/components/androidtextinput/AndroidTextInputState.h @@ -94,9 +94,7 @@ class AndroidTextInputState final { AndroidTextInputState const &previousState, folly::dynamic const &data); folly::dynamic getDynamic() const; - MapBuffer getMapBuffer() const { - return MapBufferBuilder::EMPTY(); - }; + MapBuffer getMapBuffer() const; }; } // namespace react From 5339e95cb95cb9f8361b537c8a5a6bea7ceaaf50 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 23:43:32 -0700 Subject: [PATCH 11/11] Mimimize EditText Spans 9/9: Remove addSpansForMeasurement() (#36575) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36575 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. D23670779 addedd a previous mechanism to add spans for measurement caching, like we needed to do as part of this change. It is called in more specific cases (e.g. when there is a text hint but no text), but it edits the live EditText spannable instead of the cache copy, and does not handle nested text at all. We are already adding spans back to the input after this, behind everything else, and can replace it with the code we have been adding. Changelog: [Android][Fixed] - Mimimize EditText Spans 9/9: Remove `addSpansForMeasurement()` Reviewed By: javache Differential Revision: D44298159 fbshipit-source-id: 1af44a39de7550b7e66e45db9ebc3523ae9ff002 # Conflicts: # ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java --- .../react/views/text/ReactTextUpdate.java | 6 +- .../react/views/textinput/ReactEditText.java | 126 +++--------------- .../textinput/ReactTextInputManager.java | 12 +- 3 files changed, 24 insertions(+), 120 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.java index f1591a5fe35..82c9f5d75ac 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.java @@ -31,8 +31,6 @@ public class ReactTextUpdate { private final int mSelectionEnd; private final int mJustificationMode; - public boolean mContainsMultipleFragments; - /** * @deprecated Use a non-deprecated constructor for ReactTextUpdate instead. This one remains * because it's being used by a unit test that isn't currently open source. @@ -142,13 +140,11 @@ public class ReactTextUpdate { int jsEventCounter, int textAlign, int textBreakStrategy, - int justificationMode, - boolean containsMultipleFragments) { + int justificationMode) { ReactTextUpdate reactTextUpdate = new ReactTextUpdate( text, jsEventCounter, false, textAlign, textBreakStrategy, justificationMode); - reactTextUpdate.mContainsMultipleFragments = containsMultipleFragments; return reactTextUpdate; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index f8987fb8089..b078c8f87b4 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -64,7 +64,6 @@ import com.facebook.react.views.text.TextInlineImageSpan; import com.facebook.react.views.text.TextLayoutManager; import com.facebook.react.views.view.ReactViewBackgroundManager; import java.util.ArrayList; -import java.util.List; import java.util.Objects; /** @@ -89,7 +88,6 @@ public class ReactEditText extends AppCompatEditText // *TextChanged events should be triggered. This is less expensive than removing the text // listeners and adding them back again after the text change is completed. protected boolean mIsSettingTextFromJS; - protected boolean mIsSettingTextFromCacheUpdate = false; private int mDefaultGravityHorizontal; private int mDefaultGravityVertical; @@ -370,7 +368,7 @@ public class ReactEditText extends AppCompatEditText } super.onSelectionChanged(selStart, selEnd); - if (!mIsSettingTextFromCacheUpdate && mSelectionWatcher != null && hasFocus()) { + if (mSelectionWatcher != null && hasFocus()) { mSelectionWatcher.onSelectionChanged(selStart, selEnd); } } @@ -580,7 +578,7 @@ public class ReactEditText extends AppCompatEditText SpannableStringBuilder spannableStringBuilder = new SpannableStringBuilder(reactTextUpdate.getText()); - manageSpans(spannableStringBuilder, reactTextUpdate.mContainsMultipleFragments); + manageSpans(spannableStringBuilder); stripStyleEquivalentSpans(spannableStringBuilder); mContainsImages = reactTextUpdate.containsImages(); @@ -609,7 +607,7 @@ public class ReactEditText extends AppCompatEditText } // Update cached spans (in Fabric only). - updateCachedSpannable(false); + updateCachedSpannable(); } /** @@ -618,8 +616,7 @@ public class ReactEditText extends AppCompatEditText * will adapt to the new text, hence why {@link SpannableStringBuilder#replace} never removes * them. */ - private void manageSpans( - SpannableStringBuilder spannableStringBuilder, boolean skipAddSpansForMeasurements) { + private void manageSpans(SpannableStringBuilder spannableStringBuilder) { Object[] spans = getText().getSpans(0, length(), Object.class); for (int spanIdx = 0; spanIdx < spans.length; spanIdx++) { Object span = spans[spanIdx]; @@ -647,13 +644,6 @@ public class ReactEditText extends AppCompatEditText spannableStringBuilder.setSpan(span, spanStart, spanEnd, spanFlags); } } - - // In Fabric only, apply necessary styles to entire span - // If the Spannable was constructed from multiple fragments, we don't apply any spans that could - // impact the whole Spannable, because that would override "local" styles per-fragment - if (!skipAddSpansForMeasurements) { - addSpansForMeasurement(getText()); - } } // TODO: Replace with Predicate and lambdas once Java 8 builds in OSS @@ -755,10 +745,10 @@ public class ReactEditText extends AppCompatEditText } /** - * Copy back styles represented as attributes to the underlying span, for later measurement - * outside the ReactEditText. + * Copy styles represented as attributes to the underlying span, for later measurement or other + * usage outside the ReactEditText. */ - private void restoreStyleEquivalentSpans(SpannableStringBuilder workingText) { + private void addSpansFromStyleAttributes(SpannableStringBuilder workingText) { int spanFlags = Spannable.SPAN_INCLUSIVE_INCLUSIVE; // Set all bits for SPAN_PRIORITY so that this span has the highest possible priority @@ -814,6 +804,11 @@ public class ReactEditText extends AppCompatEditText workingText.length(), spanFlags); } + + float lineHeight = mTextAttributes.getEffectiveLineHeight(); + if (!Float.isNaN(lineHeight)) { + workingText.setSpan(new CustomLineHeightSpan(lineHeight), 0, workingText.length(), spanFlags); + } } private static boolean sameTextForSpan( @@ -832,73 +827,6 @@ public class ReactEditText extends AppCompatEditText return true; } - // This is hacked in for Fabric. When we delete non-Fabric code, we might be able to simplify or - // clean this up a bit. - private void addSpansForMeasurement(Spannable spannable) { - if (!mFabricViewStateManager.hasStateWrapper()) { - return; - } - - boolean originalDisableTextDiffing = mDisableTextDiffing; - mDisableTextDiffing = true; - - int start = 0; - int end = spannable.length(); - - // Remove duplicate spans we might add here - Object[] spans = spannable.getSpans(0, length(), Object.class); - for (Object span : spans) { - int spanFlags = spannable.getSpanFlags(span); - boolean isInclusive = - (spanFlags & Spanned.SPAN_INCLUSIVE_INCLUSIVE) == Spanned.SPAN_INCLUSIVE_INCLUSIVE - || (spanFlags & Spanned.SPAN_INCLUSIVE_EXCLUSIVE) == Spanned.SPAN_INCLUSIVE_EXCLUSIVE; - if (isInclusive - && span instanceof ReactSpan - && spannable.getSpanStart(span) == start - && spannable.getSpanEnd(span) == end) { - spannable.removeSpan(span); - } - } - - List ops = new ArrayList<>(); - - if (!Float.isNaN(mTextAttributes.getLetterSpacing())) { - ops.add( - new TextLayoutManager.SetSpanOperation( - start, end, new CustomLetterSpacingSpan(mTextAttributes.getLetterSpacing()))); - } - ops.add( - new TextLayoutManager.SetSpanOperation( - start, end, new ReactAbsoluteSizeSpan((int) mTextAttributes.getEffectiveFontSize()))); - if (mFontStyle != UNSET || mFontWeight != UNSET || mFontFamily != null) { - ops.add( - new TextLayoutManager.SetSpanOperation( - start, - end, - new CustomStyleSpan( - mFontStyle, - mFontWeight, - null, // TODO: do we need to support FontFeatureSettings / fontVariant? - mFontFamily, - getReactContext(ReactEditText.this).getAssets()))); - } - if (!Float.isNaN(mTextAttributes.getEffectiveLineHeight())) { - ops.add( - new TextLayoutManager.SetSpanOperation( - start, end, new CustomLineHeightSpan(mTextAttributes.getEffectiveLineHeight()))); - } - - int priority = 0; - for (TextLayoutManager.SetSpanOperation op : ops) { - // Actual order of calling {@code execute} does NOT matter, - // but the {@code priority} DOES matter. - op.execute(spannable, priority); - priority++; - } - - mDisableTextDiffing = originalDisableTextDiffing; - } - protected boolean showSoftKeyboard() { return mInputMethodManager.showSoftInput(this, 0); } @@ -1180,7 +1108,7 @@ public class ReactEditText extends AppCompatEditText * TextLayoutManager.java with some very minor modifications. There's some duplication between * here and TextLayoutManager, so there might be an opportunity for refactor. */ - private void updateCachedSpannable(boolean resetStyles) { + private void updateCachedSpannable() { // Noops in non-Fabric if (mFabricViewStateManager == null || !mFabricViewStateManager.hasStateWrapper()) { return; @@ -1190,12 +1118,6 @@ public class ReactEditText extends AppCompatEditText return; } - if (resetStyles) { - mIsSettingTextFromCacheUpdate = true; - addSpansForMeasurement(getText()); - mIsSettingTextFromCacheUpdate = false; - } - Editable currentText = getText(); boolean haveText = currentText != null && currentText.length() > 0; @@ -1238,7 +1160,6 @@ public class ReactEditText extends AppCompatEditText // - android.app.Activity.dispatchKeyEvent (Activity.java:3447) try { sb.append(currentText.subSequence(0, currentText.length())); - restoreStyleEquivalentSpans(sb); } catch (IndexOutOfBoundsException e) { ReactSoftExceptionLogger.logSoftException(TAG, e); } @@ -1254,11 +1175,9 @@ public class ReactEditText extends AppCompatEditText // Measure something so we have correct height, even if there's no string. sb.append("I"); } - - // Make sure that all text styles are applied when we're measurable the hint or "blank" text - addSpansForMeasurement(sb); } + addSpansFromStyleAttributes(sb); TextLayoutManager.setCachedSpannabledForTag(getId(), sb); } @@ -1273,7 +1192,7 @@ public class ReactEditText extends AppCompatEditText private class TextWatcherDelegator implements TextWatcher { @Override public void beforeTextChanged(CharSequence s, int start, int count, int after) { - if (!mIsSettingTextFromCacheUpdate && !mIsSettingTextFromJS && mListeners != null) { + if (!mIsSettingTextFromJS && mListeners != null) { for (TextWatcher listener : mListeners) { listener.beforeTextChanged(s, start, count, after); } @@ -1287,23 +1206,20 @@ public class ReactEditText extends AppCompatEditText TAG, "onTextChanged[" + getId() + "]: " + s + " " + start + " " + before + " " + count); } - if (!mIsSettingTextFromCacheUpdate) { - if (!mIsSettingTextFromJS && mListeners != null) { - for (TextWatcher listener : mListeners) { - listener.onTextChanged(s, start, before, count); - } + if (!mIsSettingTextFromJS && mListeners != null) { + for (TextWatcher listener : mListeners) { + listener.onTextChanged(s, start, before, count); } - - updateCachedSpannable( - !mIsSettingTextFromJS && !mIsSettingTextFromState && start == 0 && before == 0); } + updateCachedSpannable(); + onContentSizeChange(); } @Override public void afterTextChanged(Editable s) { - if (!mIsSettingTextFromCacheUpdate && !mIsSettingTextFromJS && mListeners != null) { + if (!mIsSettingTextFromJS && mListeners != null) { for (TextWatcher listener : mListeners) { listener.afterTextChanged(s); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java index 99025ee3ccc..d34961afda5 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java @@ -1339,9 +1339,6 @@ public class ReactTextInputManager extends BaseViewManager 1; - int textBreakStrategy = TextAttributeProps.getTextBreakStrategy(paragraphAttributes.getString("textBreakStrategy")); @@ -1350,8 +1347,7 @@ public class ReactTextInputManager extends BaseViewManager 1; - int textBreakStrategy = TextAttributeProps.getTextBreakStrategy( paragraphAttributes.getString(TextLayoutManagerMapBuffer.PA_KEY_TEXT_BREAK_STRATEGY)); @@ -1385,7 +1378,6 @@ public class ReactTextInputManager extends BaseViewManager