Skip to content

Commit ababa4d

Browse files
Bora Shimmeta-codesync[bot]
authored andcommitted
Fix custom-font ellipsis clipping on Android (#59088)
Summary: Pull Request resolved: #59088 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 fbshipit-source-id: 814c63d28e03552f10f48faf863d801d60db021c
1 parent 3d6a381 commit ababa4d

10 files changed

Lines changed: 232 additions & 6 deletions

File tree

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,8 @@ public PreparedLayout reusePreparedLayoutWithNewReactTags(
712712
preparedLayout.getVerticalOffset(),
713713
reactTags,
714714
preparedLayout.getTextBreakStrategy(),
715-
preparedLayout.getJustificationMode());
715+
preparedLayout.getJustificationMode(),
716+
preparedLayout.getBaseTextAttributes());
716717
}
717718

718719
@AnyThread

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayout.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@ internal class PreparedLayout(
2222
val reactTags: IntArray,
2323
val textBreakStrategy: Int,
2424
val justificationMode: Int,
25+
val baseTextAttributes: TextAttributeProps,
2526
)

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayoutTextView.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,7 @@ internal class PreparedLayoutTextView(context: Context) : ViewGroup(context), Re
502502
reactTags,
503503
textBreakStrategy,
504504
justificationMode,
505+
baseTextAttributes,
505506
)
506507
}
507508
}

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ internal class ReactTextUpdate(
1616
val textAlign: Int,
1717
val textBreakStrategy: Int,
1818
val justificationMode: Int,
19+
val baseTextAttributes: TextAttributeProps? = null,
1920
) {
2021
companion object {
2122
@JvmStatic

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie
6767
// compile against an SDK older than 35 (see AndroidVersion).
6868
private static final @Nullable Method SET_USE_BOUNDS_FOR_WIDTH = resolveSetUseBoundsForWidth();
6969

70+
private final float mDefaultTextSize;
7071
private int mNumberOfLines;
7172
private @Nullable TextUtils.TruncateAt mEllipsizeLocation;
7273
private boolean mAdjustsFontSizeToFit;
@@ -85,6 +86,7 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie
8586
@SuppressWarnings("this-escape")
8687
public ReactTextView(Context context) {
8788
super(context);
89+
mDefaultTextSize = getTextSize();
8890
initView();
8991
}
9092

@@ -165,6 +167,7 @@ private void initView() {
165167
// reset text
166168
setLayoutParams(EMPTY_LAYOUT_PARAMS);
167169
super.setText(null);
170+
TextLayoutManager.resetTextPaint(getPaint(), mDefaultTextSize);
168171
applyTextAttributes();
169172

170173
// Call setters to ensure that any super setters are called
@@ -384,6 +387,7 @@ public void setText(ReactTextUpdate update) {
384387
Linkify.addLinks((Spannable) spanned, mLinkifyMaskType);
385388
setMovementMethod(LinkMovementMethod.getInstance());
386389
}
390+
applyBaseTextAttributes(update.getBaseTextAttributes());
387391
setText(spanned);
388392

389393
int nextTextAlign = update.getTextAlign();
@@ -404,6 +408,24 @@ public void setText(ReactTextUpdate update) {
404408
}
405409
}
406410

411+
/**
412+
* Android draws generated ellipses with the view's paint rather than a span, so it must match the
413+
* base attributes TextLayoutManager measured with.
414+
*/
415+
private void applyBaseTextAttributes(@Nullable TextAttributeProps baseTextAttributes) {
416+
if (baseTextAttributes == null) {
417+
return;
418+
}
419+
420+
TextLayoutManager.resetTextPaint(getPaint(), mDefaultTextSize);
421+
applyTextAttributes();
422+
TextLayoutManager.updateTextPaint(
423+
getPaint(),
424+
baseTextAttributes,
425+
getContext().getAssets(),
426+
ReactTypefaceUtils.getFontWeightAdjustment(getContext()));
427+
}
428+
407429
@Override
408430
public int reactTagForTouch(float touchX, float touchY) {
409431
CharSequence text = getText();

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,9 @@ public constructor(
201201
TextLayoutManager.getTextGravity(attributedString, spanned),
202202
textBreakStrategy,
203203
TextAttributeProps.getJustificationMode(props, currentJustificationMode),
204+
TextAttributeProps.fromMapBuffer(
205+
attributedString.getMapBuffer(TextLayoutManager.AS_KEY_BASE_ATTRIBUTES),
206+
),
204207
)
205208
}
206209

@@ -230,6 +233,7 @@ public constructor(
230233
textAlign,
231234
preparedLayout.textBreakStrategy,
232235
preparedLayout.justificationMode,
236+
preparedLayout.baseTextAttributes,
233237
)
234238
}
235239

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ public class TextAttributeProps private constructor() {
8282
private var lineHeightInput: Float = ReactConstants.UNSET.toFloat()
8383
private var letterSpacingInput: Float = Float.NaN
8484

85+
internal val isLetterSpacingSet: Boolean
86+
get() = !letterSpacingInput.isNaN()
87+
8588
// `ReactConstants.UNSET` is -1, same as `LayoutDirection.UNDEFINED` (which is a hidden symbol)
8689
public var layoutDirection: Int = ReactConstants.UNSET
8790
private set

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.kt‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,7 @@ internal object TextLayoutManager {
371371
if (
372372
textAttributes.fontStyle != ReactConstants.UNSET ||
373373
textAttributes.fontWeight != ReactConstants.UNSET ||
374+
textAttributes.fontFeatureSettings != null ||
374375
textAttributes.fontVariationSettings != null ||
375376
textAttributes.fontFamily != null
376377
) {
@@ -607,6 +608,7 @@ internal object TextLayoutManager {
607608
if (
608609
fragment.props.fontStyle != ReactConstants.UNSET ||
609610
fragment.props.fontWeight != ReactConstants.UNSET ||
611+
fragment.props.fontFeatureSettings != null ||
610612
fragment.props.fontVariationSettings != null ||
611613
fragment.props.fontFamily != null
612614
) {
@@ -1003,8 +1005,8 @@ internal object TextLayoutManager {
10031005
* Sets attributes on the TextPaint, used for content outside the Spannable text, like for empty
10041006
* strings, or newlines after the last trailing character
10051007
*/
1006-
@VisibleForTesting
1007-
internal fun updateTextPaint(
1008+
@JvmStatic
1009+
fun updateTextPaint(
10081010
paint: TextPaint,
10091011
baseTextAttributes: TextAttributeProps,
10101012
assets: AssetManager,
@@ -1014,9 +1016,14 @@ internal object TextLayoutManager {
10141016
paint.textSize = baseTextAttributes.fontSize.toFloat()
10151017
}
10161018

1019+
if (baseTextAttributes.isLetterSpacingSet) {
1020+
paint.letterSpacing = baseTextAttributes.letterSpacing
1021+
}
1022+
10171023
if (
10181024
baseTextAttributes.fontStyle != ReactConstants.UNSET ||
10191025
baseTextAttributes.fontWeight != ReactConstants.UNSET ||
1026+
baseTextAttributes.fontFeatureSettings != null ||
10201027
baseTextAttributes.fontVariationSettings != null ||
10211028
baseTextAttributes.fontFamily != null
10221029
) {
@@ -1041,13 +1048,17 @@ internal object TextLayoutManager {
10411048
paint.isFakeBoldText = missingStyle and Typeface.BOLD != 0
10421049
paint.textSkewX = if ((missingStyle and Typeface.ITALIC) != 0) -0.25f else 0f
10431050
}
1051+
1052+
paint.isSubpixelText = true
1053+
paint.isLinearText = true
10441054
} else {
10451055
val typeface = ReactTypefaceUtils.applyFontWeightAdjustment(null, fontWeightAdjustment)
10461056
if (typeface != null) {
10471057
paint.setTypeface(typeface)
10481058
}
10491059
}
10501060

1061+
paint.fontFeatureSettings = baseTextAttributes.fontFeatureSettings
10511062
ReactTypefaceUtils.applyFontVariationSettings(paint, baseTextAttributes.fontVariationSettings)
10521063
}
10531064

@@ -1061,12 +1072,23 @@ internal object TextLayoutManager {
10611072
fontWeightAdjustment: Int,
10621073
): TextPaint {
10631074
val paint = checkNotNull(textPaintInstance.get())
1075+
resetTextPaint(paint, 12f)
1076+
updateTextPaint(paint, baseTextAttributes, assets, fontWeightAdjustment)
1077+
return paint
1078+
}
1079+
1080+
/** Clears every attribute [updateTextPaint] may set, so a reused paint starts from defaults. */
1081+
@JvmStatic
1082+
fun resetTextPaint(paint: TextPaint, textSize: Float) {
10641083
paint.setTypeface(null)
1065-
paint.textSize = 12f
1084+
paint.textSize = textSize
1085+
paint.letterSpacing = 0f
1086+
paint.fontFeatureSettings = null
10661087
paint.isFakeBoldText = false
10671088
paint.textSkewX = 0f
1068-
updateTextPaint(paint, baseTextAttributes, assets, fontWeightAdjustment)
1069-
return paint
1089+
paint.isSubpixelText = false
1090+
paint.isLinearText = false
1091+
ReactTypefaceUtils.applyFontVariationSettings(paint, null)
10701092
}
10711093

10721094
private fun newPaintWithAttributes(
@@ -1333,6 +1355,7 @@ internal object TextLayoutManager {
13331355
reactTags,
13341356
result.textBreakStrategy,
13351357
result.justificationMode,
1358+
baseTextAttributes,
13361359
)
13371360
}
13381361

‎packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/ReactTextViewTest.kt‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,19 @@ import android.graphics.Color
1414
import android.graphics.Paint
1515
import android.text.SpannableString
1616
import android.text.Spanned
17+
import android.text.TextPaint
18+
import android.text.TextUtils
1719
import android.text.style.ReplacementSpan
1820
import android.util.TypedValue
1921
import android.view.Gravity
2022
import android.view.View
2123
import android.view.ViewGroup
2224
import androidx.core.graphics.createBitmap
2325
import androidx.core.graphics.get
26+
import com.facebook.react.bridge.JavaOnlyArray
27+
import com.facebook.react.bridge.JavaOnlyMap
2428
import com.facebook.react.uimanager.DisplayMetricsHolder
29+
import com.facebook.react.uimanager.ReactStylesDiffMap
2530
import com.facebook.react.views.text.internal.span.ReactAbsoluteSizeSpan
2631
import org.assertj.core.api.Assertions.assertThat
2732
import org.junit.After
@@ -45,6 +50,86 @@ class ReactTextViewTest {
4550
DisplayMetricsHolder.setScreenDisplayMetrics(null)
4651
}
4752

53+
@Test
54+
fun tailEllipsisUsesBaseTextAttributeMetrics() {
55+
val attributes =
56+
textAttributes(
57+
"fontFamily",
58+
"serif",
59+
"fontSize",
60+
32.0,
61+
"letterSpacing",
62+
1.5,
63+
"fontVariant",
64+
JavaOnlyArray.of("small-caps"),
65+
)
66+
val expectedPaint = paintWithAttributes(attributes)
67+
val view = TestReactTextView(RuntimeEnvironment.getApplication())
68+
view.setNumberOfLines(1)
69+
view.setEllipsizeLocation(TextUtils.TruncateAt.END)
70+
view.setText(
71+
textUpdate(SpannableString("Search files in a very long directory name"), attributes)
72+
)
73+
view.updateView()
74+
view.measure(
75+
View.MeasureSpec.makeMeasureSpec(40, View.MeasureSpec.EXACTLY),
76+
View.MeasureSpec.makeMeasureSpec(60, View.MeasureSpec.EXACTLY),
77+
)
78+
view.layout(0, 0, 40, 60)
79+
80+
val paint = checkNotNull(view.layout).paint
81+
assertThat(view.ellipsize).isEqualTo(TextUtils.TruncateAt.END)
82+
assertThat(paint.typeface).isEqualTo(expectedPaint.typeface)
83+
assertThat(paint.textSize).isEqualTo(expectedPaint.textSize)
84+
assertThat(paint.letterSpacing).isEqualTo(expectedPaint.letterSpacing)
85+
assertThat(paint.fontFeatureSettings).isEqualTo("'smcp'")
86+
assertThat(paint.measureText("…")).isEqualTo(expectedPaint.measureText("…"))
87+
}
88+
89+
@Test
90+
fun baseTextAttributesDoNotLeakAcrossUpdatesOrRecycling() {
91+
val text = SpannableString("Text")
92+
val view = TestReactTextView(RuntimeEnvironment.getApplication())
93+
val defaultTextSize = view.textSize
94+
val custom =
95+
textAttributes(
96+
"fontFamily",
97+
"serif",
98+
"fontSize",
99+
32.0,
100+
"letterSpacing",
101+
1.5,
102+
"fontVariant",
103+
JavaOnlyArray.of("small-caps"),
104+
)
105+
106+
view.setText(textUpdate(text, custom))
107+
assertThat(view.paint.typeface).isNotNull
108+
assertThat(view.textSize).isNotEqualTo(defaultTextSize)
109+
110+
view.setText(textUpdate(text, textAttributes()))
111+
assertDefaultPaint(view, defaultTextSize)
112+
113+
view.setText(textUpdate(text, custom))
114+
view.recycleView()
115+
assertDefaultPaint(view, defaultTextSize)
116+
}
117+
118+
@Test
119+
fun baseTextAttributesPreserveViewFontSizeAndLetterSpacingWhenUnset() {
120+
val view = TestReactTextView(RuntimeEnvironment.getApplication())
121+
view.setFontSize(32f)
122+
view.letterSpacing = 1.5f
123+
val expectedTextSize = view.textSize
124+
val expectedLetterSpacing = view.letterSpacing
125+
126+
view.setText(textUpdate(SpannableString("Text"), textAttributes("fontFamily", "serif")))
127+
128+
assertThat(view.paint.typeface).isNotNull
129+
assertThat(view.textSize).isEqualTo(expectedTextSize)
130+
assertThat(view.letterSpacing).isEqualTo(expectedLetterSpacing)
131+
}
132+
48133
@Test
49134
fun drawsGlyphInkOutsideLineHeightWhenOverflowIsVisible() {
50135
val bitmap = drawReactTextViewWithOverflow(null)
@@ -134,6 +219,29 @@ class ReactTextViewTest {
134219
private fun largestAbsoluteSizeSpan(text: Spanned): Int =
135220
text.getSpans(0, text.length, ReactAbsoluteSizeSpan::class.java).maxOfOrNull { it.size } ?: 0
136221

222+
private fun textAttributes(vararg keysAndValues: Any): TextAttributeProps =
223+
TextAttributeProps.fromReadableMap(ReactStylesDiffMap(JavaOnlyMap.of(*keysAndValues)))
224+
225+
private fun paintWithAttributes(attributes: TextAttributeProps): TextPaint =
226+
TextPaint(TextPaint.ANTI_ALIAS_FLAG).apply {
227+
TextLayoutManager.updateTextPaint(
228+
this,
229+
attributes,
230+
RuntimeEnvironment.getApplication().assets,
231+
0,
232+
)
233+
}
234+
235+
private fun textUpdate(text: Spanned, attributes: TextAttributeProps): ReactTextUpdate =
236+
ReactTextUpdate(text, -1, Gravity.START, 0, 0, attributes)
237+
238+
private fun assertDefaultPaint(view: ReactTextView, defaultTextSize: Float) {
239+
assertThat(view.paint.typeface).isNull()
240+
assertThat(view.textSize).isEqualTo(defaultTextSize)
241+
assertThat(view.letterSpacing).isEqualTo(0f)
242+
assertThat(view.paint.fontFeatureSettings).isNull()
243+
}
244+
137245
private fun drawReactTextViewWithOverflow(overflow: String?): Bitmap {
138246
return drawReactTextViewWithOverflow(overflow, lineHeight = 24, viewHeight = 24, gravity = null)
139247
}

0 commit comments

Comments
 (0)