Skip to content

Commit c93ff90

Browse files
sivefuncmeta-codesync[bot]
sivefunc
authored and
meta-codesync[bot]
committed
Fix lineHeight and letterSpacing ignoring maxFontSizeMultiplier on Android (#58982)
Summary: On Android, `maxFontSizeMultiplier` caps the font size of a `<Text>`, but not its `lineHeight` or `letterSpacing`. When the system font scale is above the cap, the glyphs stop growing while the space between lines keeps growing. For example, with `lineHeight: 20` and `maxFontSizeMultiplier={1}` at system font scale 2.0 on Android 16, each line is 34 dp tall instead of 20 dp (screenshots below). Letter spacing grows too: it is converted to pixels without the cap and then divided by the capped font size, so its em value increases. The cause is in `TextAttributeProps`, which `TextLayoutManager` uses to build the text spans: - `fontSize` goes through `toPixelFromSP(fontSize, maxFontSizeMultiplier)`. - `lineHeight` and `letterSpacing` go through `toPixelFromSP(value)`, without the cap. Everywhere else the cap already applies to all three: - `TextAttributes.kt` (used by `TextInput`) caps `fontSize`, `lineHeight` and `letterSpacing` with `effectiveMaxFontSizeMultiplier`, as introduced in #23069. - On iOS, `RCTAttributedTextUtils.mm` applies the capped multiplier to the line height as well. It looks like an oversight rather than a decision. When #47614 added `maxFontSizeMultiplier` to `TextAttributeProps`, its setter re-applies `lineHeight = lineHeightInput` so the line height follows a change of the cap, but that recomputation had no effect because the `lineHeight` setter did not pass the cap. This change passes `maxFontSizeMultiplier` in those two calls. Behavior is unchanged when there is no cap (`NaN`), when the cap is `0` (no limit), when the system font scale is below the cap, and with `allowFontScaling={false}`: `PixelUtil.toPixelFromSP` already handles all of these. Fixes #56578. #58124 looks related, but it is a different behavior and this change does not alter its numbers. There the cap never limits the line height: with Android 14's nonlinear font scaling, `lineHeight: 36` becomes 36 dp at font scale 1.5 and 43.3 dp at 2.0, both below the 54 dp cap (36 × 1.5). The difference between the two font scales comes from the nonlinear curve itself. I also added a `lineHeight` + `letterSpacing` case to the existing `maxFontSizeMultiplier` example in RNTester, on Android and iOS, so the behavior can be compared on both platforms. ## Changelog: [ANDROID] [FIXED] - Respect maxFontSizeMultiplier for lineHeight and letterSpacing in Text Pull Request resolved: #58982 Test Plan: **Unit tests.** Added seven cases to `TextAttributePropsTest`. The system font scale is simulated with `DisplayMetrics.scaledDensity`, as in `PixelUtilTest` and `TextLayoutManagerInlineViewSizeTest`: - `lineHeight` is capped, built with `fromReadableMap`. - `lineHeight` is capped, built with `fromMapBuffer`. Map buffer keys are iterated in ascending order, so `lineHeight` (11) is set before `maxFontSizeMultiplier` (29): this covers the re-application in the `maxFontSizeMultiplier` setter. - `letterSpacing` is capped. - No cap, a cap of `0`, a font scale below the cap, and `allowFontScaling={false}`: unchanged. Without the fix, the three "capped" cases fail: ``` $ ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests com.facebook.react.views.text.TextAttributePropsTest -PenableWarningsAsErrors=true TextAttributePropsTest > letterSpacingIsCappedByMaxFontSizeMultiplier FAILED (expected 0.14054054 em, actual 0.21621622) TextAttributePropsTest > lineHeightIsCappedByMaxFontSizeMultiplier FAILED (expected 52.0 px, actual 80.0) TextAttributePropsTest > lineHeightIsCappedByMaxFontSizeMultiplierFromMapBuffer FAILED (expected 52.0 px, actual 80.0) 13 tests completed, 3 failed ``` With the fix: ``` $ ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests com.facebook.react.views.text.TextAttributePropsTest -PenableWarningsAsErrors=true BUILD SUCCESSFUL (13 tests, 0 failures) $ ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest -PenableWarningsAsErrors=true BUILD SUCCESSFUL (681 tests, 0 failures) $ yarn format-check-kotlin # exit 0 $ yarn lint # exit 0 $ yarn flow-check # exit 0 ``` **RNTester.** Text > "maxFontSizeMultiplier attribute", new gray paragraph (`lineHeight: 20`, `letterSpacing: 1`, `maxFontSizeMultiplier={1}`): - Android 16 emulator (API 36, density 3.0), built with `./gradlew :packages:rn-tester:android:app:assembleDebug -PreactNativeArchitectures=arm64-v8a -Preact.internal.useHermesNightly=true`, system font scale set with `adb shell settings put system font_scale 2.0`. - Before: lines are 102 px (34 dp) apart, with wide letter spacing. - After: lines are 60 px (20 dp) apart, the same as at font scale 1.0. The other examples on the screen are unchanged. - iOS 26.5 simulator (iPhone 17), Larger Text "AX L" (`xcrun simctl ui <device> content_size accessibility-large`): lines are 20 pt apart, which is what Android shows after the fix. <table> <tr><th>Android 16, font scale 2.0: before</th><th>Android 16, font scale 2.0: after</th></tr> <tr> <td><img src="https://raw.githubusercontent.com/sivefunc/react-native/1f8fa303126b527ad01c93f9dd6067a3ffd3455e/android-before-font-scale-2.png" width="300" alt="Android, font scale 2.0, before: the gray paragraph has small text but lines 34 dp apart and wide letter spacing"></td> <td><img src="https://raw.githubusercontent.com/sivefunc/react-native/1f8fa303126b527ad01c93f9dd6067a3ffd3455e/android-after-font-scale-2.png" width="300" alt="Android, font scale 2.0, after: the gray paragraph has lines 20 dp apart and normal letter spacing"></td> </tr> <tr><th>Android 16, font scale 1.0: after</th><th>iOS 26.5, Larger Text (AX L): reference</th></tr> <tr> <td><img src="https://raw.githubusercontent.com/sivefunc/react-native/1f8fa303126b527ad01c93f9dd6067a3ffd3455e/android-after-font-scale-1.png" width="300" alt="Android, font scale 1.0, after: the gray paragraph looks the same as at font scale 2.0"></td> <td><img src="https://raw.githubusercontent.com/sivefunc/react-native/1f8fa303126b527ad01c93f9dd6067a3ffd3455e/ios-larger-text-ax-l.png" width="300" alt="iOS, Larger Text AX L: the gray paragraph has lines 20 pt apart, like Android after the fix"></td> </tr> </table> Not tested: Android 13 (API 33) or older, where font scaling is linear (the unit tests run with linear scaling), and physical devices. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: christophpurrer Differential Revision: D124966658 Pulled By: javache fbshipit-source-id: 860acd08ca69fccbc12771e34749494d68360927
1 parent 03355ef commit c93ff90

4 files changed

Lines changed: 154 additions & 2 deletions

File tree

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ public class TextAttributeProps private constructor() {
4040
if (value == ReactConstants.UNSET.toFloat()) {
4141
Float.NaN
4242
} else {
43-
if (allowFontScaling) toPixelFromSP(value) else toPixelFromDIP(value)
43+
if (allowFontScaling) toPixelFromSP(value, maxFontSizeMultiplier)
44+
else toPixelFromDIP(value)
4445
}
4546
}
4647

@@ -169,7 +170,7 @@ public class TextAttributeProps private constructor() {
169170
public var letterSpacing: Float
170171
get() {
171172
val letterSpacingPixels =
172-
if (allowFontScaling) toPixelFromSP(letterSpacingInput)
173+
if (allowFontScaling) toPixelFromSP(letterSpacingInput, maxFontSizeMultiplier)
173174
else toPixelFromDIP(letterSpacingInput)
174175

175176
require(fontSize > 0) { "FontSize should be a positive value. Current value: $fontSize" }

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

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,18 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8+
@file:Suppress("DEPRECATION") // DisplayMetrics.scaledDensity, to simulate a system font scale
9+
810
package com.facebook.react.views.text
911

12+
import android.util.DisplayMetrics
1013
import android.view.Gravity
1114
import com.facebook.react.bridge.JavaOnlyMap
15+
import com.facebook.react.common.mapbuffer.WritableMapBuffer
1216
import com.facebook.react.uimanager.DisplayMetricsHolder
1317
import com.facebook.react.uimanager.ReactStylesDiffMap
1418
import org.assertj.core.api.Assertions.assertThat
19+
import org.assertj.core.api.Assertions.within
1520
import org.junit.After
1621
import org.junit.Before
1722
import org.junit.Test
@@ -84,11 +89,133 @@ class TextAttributePropsTest {
8489
assertThat(textAlignment("end", isRTL = true)).isEqualTo(Gravity.LEFT)
8590
}
8691

92+
@Test
93+
fun lineHeightIsCappedByMaxFontSizeMultiplier() {
94+
setFontScale(2f)
95+
96+
val textAttributes =
97+
TextAttributeProps.fromReadableMap(
98+
ReactStylesDiffMap(
99+
JavaOnlyMap.of("fontSize", 14.0, "lineHeight", 20.0, "maxFontSizeMultiplier", 1.3)
100+
),
101+
)
102+
103+
assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY * 1.3f, within(0.001f))
104+
}
105+
106+
@Test
107+
fun lineHeightIsCappedByMaxFontSizeMultiplierFromMapBuffer() {
108+
setFontScale(2f)
109+
110+
// Keys are iterated in ascending order, so lineHeight is set before maxFontSizeMultiplier.
111+
val textAttributes =
112+
TextAttributeProps.fromMapBuffer(
113+
WritableMapBuffer()
114+
.put(TextAttributeProps.TA_KEY_FONT_SIZE, 14.0)
115+
.put(TextAttributeProps.TA_KEY_LINE_HEIGHT, 20.0)
116+
.put(TextAttributeProps.TA_KEY_MAX_FONT_SIZE_MULTIPLIER, 1.3),
117+
)
118+
119+
assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY * 1.3f, within(0.001f))
120+
}
121+
122+
@Test
123+
fun letterSpacingIsCappedByMaxFontSizeMultiplier() {
124+
setFontScale(2f)
125+
126+
val textAttributes =
127+
TextAttributeProps.fromReadableMap(
128+
ReactStylesDiffMap(
129+
JavaOnlyMap.of("fontSize", 14.0, "letterSpacing", 2.0, "maxFontSizeMultiplier", 1.3)
130+
),
131+
)
132+
133+
assertThat(textAttributes.fontSize).isEqualTo(37) // ceil(14 * 2 * 1.3)
134+
assertThat(textAttributes.letterSpacing)
135+
.isCloseTo(2f * DENSITY * 1.3f / textAttributes.fontSize, within(0.001f))
136+
}
137+
138+
@Test
139+
fun lineHeightScalesWithFontScaleWithoutMaxFontSizeMultiplier() {
140+
setFontScale(2f)
141+
142+
val textAttributes =
143+
TextAttributeProps.fromReadableMap(
144+
ReactStylesDiffMap(JavaOnlyMap.of("fontSize", 14.0, "lineHeight", 20.0)),
145+
)
146+
147+
assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY * 2f, within(0.001f))
148+
}
149+
150+
@Test
151+
fun lineHeightIsNotCappedWhenFontScaleIsBelowMaxFontSizeMultiplier() {
152+
setFontScale(1.2f)
153+
154+
val textAttributes =
155+
TextAttributeProps.fromReadableMap(
156+
ReactStylesDiffMap(
157+
JavaOnlyMap.of("fontSize", 14.0, "lineHeight", 20.0, "maxFontSizeMultiplier", 1.5)
158+
),
159+
)
160+
161+
assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY * 1.2f, within(0.001f))
162+
}
163+
164+
@Test
165+
fun lineHeightIsNotCappedWhenMaxFontSizeMultiplierIsZero() {
166+
setFontScale(2f)
167+
168+
val textAttributes =
169+
TextAttributeProps.fromReadableMap(
170+
ReactStylesDiffMap(
171+
JavaOnlyMap.of("fontSize", 14.0, "lineHeight", 20.0, "maxFontSizeMultiplier", 0.0)
172+
),
173+
)
174+
175+
assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY * 2f, within(0.001f))
176+
}
177+
178+
@Test
179+
fun lineHeightIgnoresFontScaleWhenFontScalingIsDisabled() {
180+
setFontScale(2f)
181+
182+
val textAttributes =
183+
TextAttributeProps.fromReadableMap(
184+
ReactStylesDiffMap(
185+
JavaOnlyMap.of(
186+
"fontSize",
187+
14.0,
188+
"lineHeight",
189+
20.0,
190+
"maxFontSizeMultiplier",
191+
1.3,
192+
"allowFontScaling",
193+
false,
194+
)
195+
),
196+
)
197+
198+
assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY, within(0.001f))
199+
}
200+
201+
private fun setFontScale(fontScale: Float) {
202+
DisplayMetricsHolder.setScreenDisplayMetrics(
203+
DisplayMetrics().apply {
204+
density = DENSITY
205+
scaledDensity = DENSITY * fontScale
206+
},
207+
)
208+
}
209+
87210
private fun textAlignment(textAlign: String, isRTL: Boolean): Int {
88211
return TextAttributeProps.getTextAlignment(
89212
ReactStylesDiffMap(JavaOnlyMap.of("textAlign", textAlign)),
90213
isRTL,
91214
Gravity.CENTER_HORIZONTAL,
92215
)
93216
}
217+
218+
private companion object {
219+
const val DENSITY = 2f
220+
}
94221
}

‎packages/rn-tester/js/examples/Text/TextExample.android.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,18 @@ function MaxFontSizeMultiplierExample(props: {}): React.Node {
531531
<Text allowFontScaling={true} maxFontSizeMultiplier={1}>
532532
<Text maxFontSizeMultiplier={0}>Ignore inherited max (no max)</Text>
533533
</Text>
534+
<Text
535+
allowFontScaling={true}
536+
maxFontSizeMultiplier={1}
537+
style={{
538+
marginTop: 10,
539+
lineHeight: 20,
540+
letterSpacing: 1,
541+
backgroundColor: 'lightgray',
542+
}}>
543+
lineHeight and letterSpacing are capped too (max 1x): these lines keep
544+
the same spacing at any system font size
545+
</Text>
534546
</View>
535547
);
536548
}

‎packages/rn-tester/js/examples/Text/TextExample.ios.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,6 +1269,18 @@ const examples = [
12691269
<Text allowFontScaling={true} maxFontSizeMultiplier={1}>
12701270
<Text maxFontSizeMultiplier={0}>Ignore inherited max (no max)</Text>
12711271
</Text>
1272+
<Text
1273+
allowFontScaling={true}
1274+
maxFontSizeMultiplier={1}
1275+
style={{
1276+
marginTop: 10,
1277+
lineHeight: 20,
1278+
letterSpacing: 1,
1279+
backgroundColor: 'lightgray',
1280+
}}>
1281+
lineHeight and letterSpacing are capped too (max 1x): these lines
1282+
keep the same spacing at any system font size
1283+
</Text>
12721284
<Text
12731285
allowFontScaling={true}
12741286
style={{fontSize: 22}}

0 commit comments

Comments
 (0)