From 8aea92219e22d083a6d0ac8e2b53ddd435011599 Mon Sep 17 00:00:00 2001 From: Bora Shim Date: Wed, 14 Oct 2026 11:11:02 -0700 Subject: [PATCH] Fix custom-font ellipsis clipping on Android MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: This fixes two Android `Text` bugs. ## Problem 1: tail ellipsis clipped with custom fonts Android generates the tail ellipsis outside styled text spans, shaping it with `ReactTextView`'s own paint. That paint did not carry the base text attributes React Native measured with — typeface, font size, letter spacing, font feature settings, or shaping flags. For custom fonts the ellipsis glyph can be wider than the reserved space, overflowing the text bounds and losing its last dot under `overflow: hidden`. ## Problem 2: `fontVariant` ignored without `fontFamily` `fontVariant` (`small-caps`, `oldstyle-nums`, `lining-nums`, `tabular-nums`, `proportional-nums`, …) is applied through `CustomStyleSpan`, but `TextLayoutManager` only added that span when `fontFamily`, `fontWeight`, `fontStyle` or `fontVariationSettings` was also set. A `Text` (or nested fragment) that set only `fontVariant` rendered without the requested OpenType features. ## Fix - Add a `CustomStyleSpan` whenever `fontFeatureSettings` is set, in both span-building paths, so `fontVariant` works on its own. This changes the RNTester `fontVariants` example on Android. - Apply the resolved base `TextAttributeProps` to the view's paint before setting text, using the same `TextLayoutManager.updateTextPaint` that already builds the measurement paint. `updateTextPaint` now also applies letter spacing, font feature settings and shaping flags. A shared `resetTextPaint` clears the state when a view is recycled. - `PreparedLayout` now carries its `baseTextAttributes`, so selectable text (which still renders through `ReactTextView` with prepared layouts enabled) gets the correct paint. Changelog: [Android][Fixed] - Fix tail ellipsis overflowing or clipping on `Text` with a custom font [Android][Fixed] - Fix `fontVariant` being ignored on `Text` that doesn't also set `fontFamily`, `fontWeight` or `fontStyle` Reviewed By: Abbondanzo Differential Revision: D116515542 Signed-off-by: Bora Shim --- .../react/fabric/FabricUIManager.java | 3 +- .../react/views/text/PreparedLayout.kt | 1 + .../views/text/PreparedLayoutTextView.kt | 1 + .../react/views/text/ReactTextUpdate.kt | 1 + .../react/views/text/ReactTextView.java | 22 ++++ .../react/views/text/ReactTextViewManager.kt | 4 + .../react/views/text/TextAttributeProps.kt | 3 + .../react/views/text/TextLayoutManager.kt | 33 +++++- .../react/views/text/ReactTextViewTest.kt | 108 ++++++++++++++++++ ...xtLayoutManagerFontWeightAdjustmentTest.kt | 62 ++++++++++ 10 files changed, 232 insertions(+), 6 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 97de9935b8cf..3e58ab288c4a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -712,7 +712,8 @@ public PreparedLayout reusePreparedLayoutWithNewReactTags( preparedLayout.getVerticalOffset(), reactTags, preparedLayout.getTextBreakStrategy(), - preparedLayout.getJustificationMode()); + preparedLayout.getJustificationMode(), + preparedLayout.getBaseTextAttributes()); } @AnyThread diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayout.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayout.kt index 51836c884989..961d9e6e4722 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayout.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayout.kt @@ -22,4 +22,5 @@ internal class PreparedLayout( val reactTags: IntArray, val textBreakStrategy: Int, val justificationMode: Int, + val baseTextAttributes: TextAttributeProps, ) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayoutTextView.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayoutTextView.kt index 4202be214106..7caac8f5496e 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayoutTextView.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayoutTextView.kt @@ -502,6 +502,7 @@ internal class PreparedLayoutTextView(context: Context) : ViewGroup(context), Re reactTags, textBreakStrategy, justificationMode, + baseTextAttributes, ) } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.kt index 111febfc53ef..8ebc18deb436 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.kt @@ -16,6 +16,7 @@ internal class ReactTextUpdate( val textAlign: Int, val textBreakStrategy: Int, val justificationMode: Int, + val baseTextAttributes: TextAttributeProps? = null, ) { companion object { @JvmStatic diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java index 677eb472660b..d388d1d5f365 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java @@ -67,6 +67,7 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie // compile against an SDK older than 35 (see AndroidVersion). private static final @Nullable Method SET_USE_BOUNDS_FOR_WIDTH = resolveSetUseBoundsForWidth(); + private final float mDefaultTextSize; private int mNumberOfLines; private @Nullable TextUtils.TruncateAt mEllipsizeLocation; private boolean mAdjustsFontSizeToFit; @@ -85,6 +86,7 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie @SuppressWarnings("this-escape") public ReactTextView(Context context) { super(context); + mDefaultTextSize = getTextSize(); initView(); } @@ -165,6 +167,7 @@ private void initView() { // reset text setLayoutParams(EMPTY_LAYOUT_PARAMS); super.setText(null); + TextLayoutManager.resetTextPaint(getPaint(), mDefaultTextSize); applyTextAttributes(); // Call setters to ensure that any super setters are called @@ -384,6 +387,7 @@ public void setText(ReactTextUpdate update) { Linkify.addLinks((Spannable) spanned, mLinkifyMaskType); setMovementMethod(LinkMovementMethod.getInstance()); } + applyBaseTextAttributes(update.getBaseTextAttributes()); setText(spanned); int nextTextAlign = update.getTextAlign(); @@ -404,6 +408,24 @@ public void setText(ReactTextUpdate update) { } } + /** + * Android draws generated ellipses with the view's paint rather than a span, so it must match the + * base attributes TextLayoutManager measured with. + */ + private void applyBaseTextAttributes(@Nullable TextAttributeProps baseTextAttributes) { + if (baseTextAttributes == null) { + return; + } + + TextLayoutManager.resetTextPaint(getPaint(), mDefaultTextSize); + applyTextAttributes(); + TextLayoutManager.updateTextPaint( + getPaint(), + baseTextAttributes, + getContext().getAssets(), + ReactTypefaceUtils.getFontWeightAdjustment(getContext())); + } + @Override public int reactTagForTouch(float touchX, float touchY) { CharSequence text = getText(); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.kt index 92d781317a8d..8512fccba2f6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.kt @@ -201,6 +201,9 @@ public constructor( TextLayoutManager.getTextGravity(attributedString, spanned), textBreakStrategy, TextAttributeProps.getJustificationMode(props, currentJustificationMode), + TextAttributeProps.fromMapBuffer( + attributedString.getMapBuffer(TextLayoutManager.AS_KEY_BASE_ATTRIBUTES), + ), ) } @@ -230,6 +233,7 @@ public constructor( textAlign, preparedLayout.textBreakStrategy, preparedLayout.justificationMode, + preparedLayout.baseTextAttributes, ) } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.kt index 5c76e0838cc6..ca2d10ece398 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.kt @@ -82,6 +82,9 @@ public class TextAttributeProps private constructor() { private var lineHeightInput: Float = ReactConstants.UNSET.toFloat() private var letterSpacingInput: Float = Float.NaN + internal val isLetterSpacingSet: Boolean + get() = !letterSpacingInput.isNaN() + // `ReactConstants.UNSET` is -1, same as `LayoutDirection.UNDEFINED` (which is a hidden symbol) public var layoutDirection: Int = ReactConstants.UNSET private set diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.kt index d9c28ca5fa20..f3bac78200aa 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.kt @@ -371,6 +371,7 @@ internal object TextLayoutManager { if ( textAttributes.fontStyle != ReactConstants.UNSET || textAttributes.fontWeight != ReactConstants.UNSET || + textAttributes.fontFeatureSettings != null || textAttributes.fontVariationSettings != null || textAttributes.fontFamily != null ) { @@ -607,6 +608,7 @@ internal object TextLayoutManager { if ( fragment.props.fontStyle != ReactConstants.UNSET || fragment.props.fontWeight != ReactConstants.UNSET || + fragment.props.fontFeatureSettings != null || fragment.props.fontVariationSettings != null || fragment.props.fontFamily != null ) { @@ -1003,8 +1005,8 @@ internal object TextLayoutManager { * Sets attributes on the TextPaint, used for content outside the Spannable text, like for empty * strings, or newlines after the last trailing character */ - @VisibleForTesting - internal fun updateTextPaint( + @JvmStatic + fun updateTextPaint( paint: TextPaint, baseTextAttributes: TextAttributeProps, assets: AssetManager, @@ -1014,9 +1016,14 @@ internal object TextLayoutManager { paint.textSize = baseTextAttributes.fontSize.toFloat() } + if (baseTextAttributes.isLetterSpacingSet) { + paint.letterSpacing = baseTextAttributes.letterSpacing + } + if ( baseTextAttributes.fontStyle != ReactConstants.UNSET || baseTextAttributes.fontWeight != ReactConstants.UNSET || + baseTextAttributes.fontFeatureSettings != null || baseTextAttributes.fontVariationSettings != null || baseTextAttributes.fontFamily != null ) { @@ -1041,6 +1048,9 @@ internal object TextLayoutManager { paint.isFakeBoldText = missingStyle and Typeface.BOLD != 0 paint.textSkewX = if ((missingStyle and Typeface.ITALIC) != 0) -0.25f else 0f } + + paint.isSubpixelText = true + paint.isLinearText = true } else { val typeface = ReactTypefaceUtils.applyFontWeightAdjustment(null, fontWeightAdjustment) if (typeface != null) { @@ -1048,6 +1058,7 @@ internal object TextLayoutManager { } } + paint.fontFeatureSettings = baseTextAttributes.fontFeatureSettings ReactTypefaceUtils.applyFontVariationSettings(paint, baseTextAttributes.fontVariationSettings) } @@ -1061,12 +1072,23 @@ internal object TextLayoutManager { fontWeightAdjustment: Int, ): TextPaint { val paint = checkNotNull(textPaintInstance.get()) + resetTextPaint(paint, 12f) + updateTextPaint(paint, baseTextAttributes, assets, fontWeightAdjustment) + return paint + } + + /** Clears every attribute [updateTextPaint] may set, so a reused paint starts from defaults. */ + @JvmStatic + fun resetTextPaint(paint: TextPaint, textSize: Float) { paint.setTypeface(null) - paint.textSize = 12f + paint.textSize = textSize + paint.letterSpacing = 0f + paint.fontFeatureSettings = null paint.isFakeBoldText = false paint.textSkewX = 0f - updateTextPaint(paint, baseTextAttributes, assets, fontWeightAdjustment) - return paint + paint.isSubpixelText = false + paint.isLinearText = false + ReactTypefaceUtils.applyFontVariationSettings(paint, null) } private fun newPaintWithAttributes( @@ -1333,6 +1355,7 @@ internal object TextLayoutManager { reactTags, result.textBreakStrategy, result.justificationMode, + baseTextAttributes, ) } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/ReactTextViewTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/ReactTextViewTest.kt index 4deaf2daa543..21d03ca2d4f9 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/ReactTextViewTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/ReactTextViewTest.kt @@ -14,6 +14,8 @@ import android.graphics.Color import android.graphics.Paint import android.text.SpannableString import android.text.Spanned +import android.text.TextPaint +import android.text.TextUtils import android.text.style.ReplacementSpan import android.util.TypedValue import android.view.Gravity @@ -21,7 +23,10 @@ import android.view.View import android.view.ViewGroup import androidx.core.graphics.createBitmap import androidx.core.graphics.get +import com.facebook.react.bridge.JavaOnlyArray +import com.facebook.react.bridge.JavaOnlyMap import com.facebook.react.uimanager.DisplayMetricsHolder +import com.facebook.react.uimanager.ReactStylesDiffMap import com.facebook.react.views.text.internal.span.ReactAbsoluteSizeSpan import org.assertj.core.api.Assertions.assertThat import org.junit.After @@ -45,6 +50,86 @@ class ReactTextViewTest { DisplayMetricsHolder.setScreenDisplayMetrics(null) } + @Test + fun tailEllipsisUsesBaseTextAttributeMetrics() { + val attributes = + textAttributes( + "fontFamily", + "serif", + "fontSize", + 32.0, + "letterSpacing", + 1.5, + "fontVariant", + JavaOnlyArray.of("small-caps"), + ) + val expectedPaint = paintWithAttributes(attributes) + val view = TestReactTextView(RuntimeEnvironment.getApplication()) + view.setNumberOfLines(1) + view.setEllipsizeLocation(TextUtils.TruncateAt.END) + view.setText( + textUpdate(SpannableString("Search files in a very long directory name"), attributes) + ) + view.updateView() + view.measure( + View.MeasureSpec.makeMeasureSpec(40, View.MeasureSpec.EXACTLY), + View.MeasureSpec.makeMeasureSpec(60, View.MeasureSpec.EXACTLY), + ) + view.layout(0, 0, 40, 60) + + val paint = checkNotNull(view.layout).paint + assertThat(view.ellipsize).isEqualTo(TextUtils.TruncateAt.END) + assertThat(paint.typeface).isEqualTo(expectedPaint.typeface) + assertThat(paint.textSize).isEqualTo(expectedPaint.textSize) + assertThat(paint.letterSpacing).isEqualTo(expectedPaint.letterSpacing) + assertThat(paint.fontFeatureSettings).isEqualTo("'smcp'") + assertThat(paint.measureText("…")).isEqualTo(expectedPaint.measureText("…")) + } + + @Test + fun baseTextAttributesDoNotLeakAcrossUpdatesOrRecycling() { + val text = SpannableString("Text") + val view = TestReactTextView(RuntimeEnvironment.getApplication()) + val defaultTextSize = view.textSize + val custom = + textAttributes( + "fontFamily", + "serif", + "fontSize", + 32.0, + "letterSpacing", + 1.5, + "fontVariant", + JavaOnlyArray.of("small-caps"), + ) + + view.setText(textUpdate(text, custom)) + assertThat(view.paint.typeface).isNotNull + assertThat(view.textSize).isNotEqualTo(defaultTextSize) + + view.setText(textUpdate(text, textAttributes())) + assertDefaultPaint(view, defaultTextSize) + + view.setText(textUpdate(text, custom)) + view.recycleView() + assertDefaultPaint(view, defaultTextSize) + } + + @Test + fun baseTextAttributesPreserveViewFontSizeAndLetterSpacingWhenUnset() { + val view = TestReactTextView(RuntimeEnvironment.getApplication()) + view.setFontSize(32f) + view.letterSpacing = 1.5f + val expectedTextSize = view.textSize + val expectedLetterSpacing = view.letterSpacing + + view.setText(textUpdate(SpannableString("Text"), textAttributes("fontFamily", "serif"))) + + assertThat(view.paint.typeface).isNotNull + assertThat(view.textSize).isEqualTo(expectedTextSize) + assertThat(view.letterSpacing).isEqualTo(expectedLetterSpacing) + } + @Test fun drawsGlyphInkOutsideLineHeightWhenOverflowIsVisible() { val bitmap = drawReactTextViewWithOverflow(null) @@ -134,6 +219,29 @@ class ReactTextViewTest { private fun largestAbsoluteSizeSpan(text: Spanned): Int = text.getSpans(0, text.length, ReactAbsoluteSizeSpan::class.java).maxOfOrNull { it.size } ?: 0 + private fun textAttributes(vararg keysAndValues: Any): TextAttributeProps = + TextAttributeProps.fromReadableMap(ReactStylesDiffMap(JavaOnlyMap.of(*keysAndValues))) + + private fun paintWithAttributes(attributes: TextAttributeProps): TextPaint = + TextPaint(TextPaint.ANTI_ALIAS_FLAG).apply { + TextLayoutManager.updateTextPaint( + this, + attributes, + RuntimeEnvironment.getApplication().assets, + 0, + ) + } + + private fun textUpdate(text: Spanned, attributes: TextAttributeProps): ReactTextUpdate = + ReactTextUpdate(text, -1, Gravity.START, 0, 0, attributes) + + private fun assertDefaultPaint(view: ReactTextView, defaultTextSize: Float) { + assertThat(view.paint.typeface).isNull() + assertThat(view.textSize).isEqualTo(defaultTextSize) + assertThat(view.letterSpacing).isEqualTo(0f) + assertThat(view.paint.fontFeatureSettings).isNull() + } + private fun drawReactTextViewWithOverflow(overflow: String?): Bitmap { return drawReactTextViewWithOverflow(overflow, lineHeight = 24, viewHeight = 24, gravity = null) } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextLayoutManagerFontWeightAdjustmentTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextLayoutManagerFontWeightAdjustmentTest.kt index 7fadd9b556da..992b1e9b1cb5 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextLayoutManagerFontWeightAdjustmentTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextLayoutManagerFontWeightAdjustmentTest.kt @@ -12,6 +12,7 @@ import android.graphics.Typeface import android.os.Build import android.text.TextPaint import androidx.annotation.RequiresApi +import com.facebook.react.bridge.JavaOnlyArray import com.facebook.react.bridge.JavaOnlyMap import com.facebook.react.uimanager.DisplayMetricsHolder import com.facebook.react.uimanager.ReactStylesDiffMap @@ -94,6 +95,67 @@ class TextLayoutManagerFontWeightAdjustmentTest { assertThat(paint.fontVariationSettings).isNull() } + @Test + fun `text paint applies base attributes used to shape generated ellipses`() { + val textAttributes = + TextAttributeProps.fromReadableMap( + ReactStylesDiffMap( + JavaOnlyMap.of( + "fontFamily", + "serif", + "fontSize", + 32.0, + "letterSpacing", + 1.5, + "fontVariant", + JavaOnlyArray.of("small-caps"), + ), + ), + ) + + val paint = TextPaint() + TextLayoutManager.updateTextPaint( + paint, + textAttributes, + RuntimeEnvironment.getApplication().assets, + 0, + ) + + assertThat(paint.typeface).isNotNull + assertThat(paint.textSize).isEqualTo(textAttributes.fontSize.toFloat()) + assertThat(paint.letterSpacing).isEqualTo(textAttributes.letterSpacing) + assertThat(paint.fontFeatureSettings).isEqualTo("'smcp'") + + TextLayoutManager.resetTextPaint(paint, 12f) + + assertThat(paint.typeface).isNull() + assertThat(paint.textSize).isEqualTo(12f) + assertThat(paint.letterSpacing).isEqualTo(0f) + assertThat(paint.fontFeatureSettings).isNull() + } + + @Test + fun `text paint enables the shaping flags used by custom style spans`() { + val paint = mock() + val textAttributes = + TextAttributeProps.fromReadableMap( + ReactStylesDiffMap( + JavaOnlyMap.of("fontVariant", JavaOnlyArray.of("small-caps")), + ), + ) + + TextLayoutManager.updateTextPaint( + paint, + textAttributes, + RuntimeEnvironment.getApplication().assets, + 0, + ) + + verify(paint).setFontFeatureSettings("'smcp'") + verify(paint).setSubpixelText(true) + verify(paint).setLinearText(true) + } + @Test fun `custom style applies font variation settings after high level font properties`() { val paint = mock()