From 87611d41432d47964b9859dfbf636946a2628a69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Fo=C5=99t?= Date: Tue, 25 Apr 2023 16:36:33 +0200 Subject: [PATCH 01/10] Fix test e2e script (#37081) --- scripts/test-manual-e2e.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/test-manual-e2e.sh b/scripts/test-manual-e2e.sh index 80b66b4ecd1..e1426510e9d 100755 --- a/scripts/test-manual-e2e.sh +++ b/scripts/test-manual-e2e.sh @@ -113,22 +113,22 @@ init_template_app(){ success "Preparing version $PACKAGE_VERSION" - npm pack - TIMESTAMP=$(date +%s) PACKAGE=$(pwd)/react-native-$PACKAGE_VERSION-$TIMESTAMP.tgz - success "Package bundled ($PACKAGE)" - - mv "$(pwd)/react-native-$PACKAGE_VERSION.tgz" "$PACKAGE" node scripts/set-rn-template-version.js "file:$PACKAGE" success "React Native version changed in the template" + npm pack + success "Package bundled ($PACKAGE)" + + mv "$(pwd)/react-native-$PACKAGE_VERSION.tgz" "$PACKAGE" + project_name="RNTestProject" cd /tmp/ || exit rm -rf "$project_name" - node "$repo_root/cli.js" init "$project_name" --template "$repo_root" + node "$repo_root/cli.js" init "$project_name" --template "$PACKAGE" info "Double checking the versions in package.json are correct:" grep "\"react-native\": \".*react-native-$PACKAGE_VERSION-$TIMESTAMP.tgz\"" "/tmp/${project_name}/package.json" || error "Incorrect version number in /tmp/${project_name}/package.json" From ccb9366575d24f0dfae62fddd888b70b9a71d93d Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Thu, 9 Feb 2023 09:56:05 -0800 Subject: [PATCH 02/10] 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 7e00b87edce..eed878fa5b1 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 @@ -551,7 +551,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(); @@ -626,7 +626,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(); @@ -647,6 +647,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, @@ -1061,7 +1086,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 11755c1d69cb533749e5f9308b5e812ddfe5178e Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 05:24:09 -0700 Subject: [PATCH 03/10] 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 eed878fa5b1..2d9e8d04530 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 @@ -647,29 +647,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( @@ -1086,8 +1075,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 485b8ef1566af169d0da8bcb489105cdb1f92f46 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 05:24:09 -0700 Subject: [PATCH 04/10] 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 2d9e8d04530..fef3b9f6dce 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 @@ -549,9 +549,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(); @@ -626,28 +624,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 @@ -1076,7 +1090,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 3cdcbe7e72ef2e744d499eb77067e82dfc8de96d Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 05:24:09 -0700 Subject: [PATCH 05/10] 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 fef3b9f6dce..95e111cf70a 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; @@ -644,6 +646,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( @@ -668,11 +680,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 c89b4e3ad0c..f59c3800720 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; @@ -50,6 +51,10 @@ public class ReactViewBackgroundManager { } } + public int getBackgroundColor() { + return mColor; + } + public void setBorderWidth(int position, float width) { getOrCreateReactViewBackground().setBorderWidth(position, width); } From b164055130de04162a092d3b28a62b1579a55f8d Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 05:24:09 -0700 Subject: [PATCH 06/10] 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 95e111cf70a..92462c41b08 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; @@ -656,6 +657,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( @@ -682,6 +693,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 54cc78aa0557b396aa4b441586c6b5b001dc5f13 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 12:31:22 -0700 Subject: [PATCH 07/10] 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 92462c41b08..7bb6e0f8e05 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; @@ -667,6 +670,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( @@ -700,6 +723,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 df6cc09407b..49e90cb2b64 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; @@ -903,6 +904,20 @@ public class ReactTextInputManager extends BaseViewManager Date: Fri, 24 Mar 2023 12:31:22 -0700 Subject: [PATCH 08/10] 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 7bb6e0f8e05..7aa3ec7bbdc 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 @@ -690,6 +690,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( @@ -731,6 +743,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); } @@ -1078,7 +1097,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 3b07e7e9be5febbabef9d668c9f0607ca45c00ed Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 12:31:22 -0700 Subject: [PATCH 09/10] 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 7aa3ec7bbdc..b77dfd4bab9 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 @@ -726,33 +726,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 e89f919b92c766549db3e201ea540bce6ac5a076 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 Mar 2023 12:31:22 -0700 Subject: [PATCH 10/10] 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 b77dfd4bab9..9b38c072c64 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 @@ -482,6 +483,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; @@ -493,6 +502,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}. @@ -702,6 +722,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( @@ -759,6 +792,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 49e90cb2b64..03a53ac7747 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 @@ -68,6 +68,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