Width measures of Text components using a custom font are wrong

charpeni commented 5 years ago

React Native version:

System:
    OS: macOS 10.14.5
    CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
    Memory: 1002.17 MB / 32.00 GB
    Shell: 5.3 - /bin/zsh
  Binaries:
    Node: 10.15.3 - ~/.nvm/versions/node/v10.15.3/bin/node
    Yarn: 1.16.0 - /usr/local/bin/yarn
    npm: 6.4.1 - ~/.nvm/versions/node/v10.15.3/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: iOS 12.2, macOS 10.14, tvOS 12.2, watchOS 5.2
    Android SDK:
      API Levels: 23, 25, 26, 27, 28
      Build Tools: 27.0.3, 28.0.2, 28.0.3, 29.0.0
      System Images: android-28 | Google APIs Intel x86 Atom, android-28 | Google Play Intel x86 Atom, android-29 | Google Play Intel x86 Atom
      Android NDK: 17.2.4988734
  IDEs:
    Android Studio: 3.4 AI-183.6156.11.34.5522156
    Xcode: 10.2.1/10E1001 - /usr/bin/xcodebuild
  npmPackages:
    react: 16.8.6 => 16.8.6
    react-native: 0.60.0 => 0.60.0

⚠️ This is not a regression from RN 0.60, can also be reproduced in RN 0.57.

On Android, some manufacturers — like Samsung, OnePlus, LG, HTC, and others — let you change the default font on your device without having a rooted device, and, of course, rooted devices can also do that.

The <Text> component provided by React Native doesn't seem to measure the layout correctly. Observed cases were when the default font is not Roboto and when a thick font weight is applied.

The result is that on these specifics devices, text components with a thicker font weight will be chopped off (seems to be wrapped in some cases?). It could be a dot missing, or even an entire word.

<Text style={{ fontWeight: 'bold' }}>
  This looks great!
</Text>

Roboto (Default font)	OnePlus Slate

Repository to reproduce this.

Workaround

To influence the layout measures of the text component, we can set a border of at least 2 on it. By doing this, the layout will be properly measured, but the text won't be vertically centered.

From there, to vertically center the text, we can subtract the double of the border we previously set as a negative marginBottom. (As seen in the screenshots above).

gnprice commented 5 years ago

Thanks @charpeni for the detailed report and debugging!

This issue has been around for a couple of years, as #15114. Some information that gradually accumulated there:

The issue goes back to RN v0.34.
It's not only users who change the default font who are affected -- on some models, the default font that comes with the device triggers this issue. This includes a number of Oppo and OnePlus models.
One workaround that seems 100% effective is to avoid setting fontWeight or using bold. This seems like a clue.
Some fascinating debugging in this comment finds that:
- the font weight (and maybe other things) isn't taken into account to determine correct size for the text box.
- In particular the box gets laid out with what looks to be exactly the right width for the non-bold, default-weight, version of the font. Then the actual text gets placed into it... and it's wider than that, so it doesn't fit.
- In particular it gets line-wrapped based on the font's actual dimensions (but inside the box sized for its non-bold dimensions), so typically the last word of the line gets moved to the next line... which if it was a single-line box, means it just doesn't appear.

To influence the layout measures of the text component, we can set a border of at least 2 on it. By doing this, the layout will be properly measured

This is an interesting workaround! It would be especially interesting to know whether this workaround keeps working even when the text is long and the border is still small.

That is, does it somehow cause the layout code to go down the right path to use the correct font metrics to make the box as wide as it needs to be? Or does it just add the border itself to the width, and then the text is able to eat into that when it turns out to be wider?

... Looking closely at your screenshots, actually, I have a new observation.

The last three green boxes -- that is, each of the variations on a text box with bold text -- have exactly the same width in the OnePlus Slate case as the Roboto case.

OTOH the first box, with unstyled text, has less width for OnePlus Slate than for Roboto. And in the OnePlus Slate case, it has less width than the (first) bold box.

So it looks as if in the bold case, the size isn't getting based on the non-bold version of the same font... instead it seems to have the right dimensions for bold Roboto.

That would be interesting to confirm or disconfirm with more examples. If true, it may be a strong clue for whoever wants to go into the code to try to track down the actual cause.

gaodeng commented 5 years ago

this is duplicate of https://github.com/facebook/react-native/issues/15114 please reopen that issue

stale[bot] commented 5 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

carloscuesta commented 5 years ago

Not fixed AFAIK

treyp commented 5 years ago

Related, but I've noticed that because the wrapping calculation is wrong on custom fonts, sometimes you will get an extra line where the text renderer thinks the text will wrap but doesn't. There is no way to get rid of it.

Example:

YesSkyscrapers commented 4 years ago

Guys, I discovered workaround. You can use <TextInput /> to render texts instead of and this problem is gone.

YesSkyscrapers commented 4 years ago

New solution https://github.com/facebook/react-native/issues/15114#issuecomment-576313266 Just change textBreakStrategy to simple fix it

stale[bot] commented 4 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

saadibrahim commented 4 years ago

Still an issue.

stale[bot] commented 4 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

gnprice commented 4 years ago

No reason to think this isn't still an issue.

If someone can label this as "Backlog", that seems like it'd be appropriate.

MoOx commented 4 years ago

Is it correct to assume that the bug is located in TextLayoutManager.java inside measureText function ? https://github.com/facebook/react-native/blob/a650696f0d144ab705a617bc48925da8805dc58b/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java#L342-L499

MoOx commented 3 years ago

I discovered a "trick" to avoid this kind of issue: don't rely on fontWeight on Android alone! Instead use fontFamily. Android Roboto have only 6 variants, same for OnePlus Slate. I have spend a few hours trying to find an acceptable combo on stock Android (using a Pixel 3), ColorsOS (Oppo) and OxygenOS (OnePlus)

You can safely (on Android) use the following name instead of weight

sans-serif-thin (= 100/200 depending on android custom font)
sans-serif-light (=300)
sans-serif / sans-serif-regular (400) (note that OnePlus Slate doesn't handle sans-serif like sans-serif-regular, when used with a weight)
sans-serif-medium (500)
sans-serif-bold (600/700)
sans-serif-black (800/900)

Note that fontWeight coupled with a fontFamily give better result on Android, if you want to rely on OS font.

Here is a stylesheet you can use safely to think about "weight" and forget about the trick

  // iOS: no comment, weight are perfect
  // Android: only has 100/300/400/500/700/900
  // it's a total mess depanding on brands...
  // more details here https://github.com/MoOx/LifeTime/blob/5ce54e13096642da02e46bf54b48548f7b5b1890/src/components/shareable/Theme.res#L120-L168
  // ios | st. and. | oppo | 1+
  // 100 | 100 | 100 | 200
  // 200 | 300 | 300 | 300
  // 300 | 300 | 300 | 300
  // 400 | 400 | 400 | 400
  // 500 | 500 | 500 | 500
  // 600 | 500 | 500 | 500
  // 700 | 700 | 700 | 700
  // 800 | 700 | 700 | 900
  // 900 | 800 | 800 | 900
const weightStyles = StyleSheet.create({
  "weight100": Platform.OS === "android"
    ? {fontFamily: "sans-serif-thin", fontWeight: "100"}
    : {fontWeight: "100"},
  "weight200": Platform.OS === "android"
    ? {fontFamily: "sans-serif-light", fontWeight: "200"}
    : {fontWeight: "200"},
  "weight300": Platform.OS === "android"
    ? {fontFamily: "sans-serif-light", fontWeight: "300"}
    : {fontWeight: "300"},
  "weight400": Platform.OS === "android"
    ? {fontFamily: "sans-serif", fontWeight: "400"}
    : {fontWeight: "400"},
  "weight500": Platform.OS === "android"
    ? {fontFamily: "sans-serif-medium", fontWeight: "500"}
    : {fontWeight: "500"},
  "weight600": Platform.OS === "android"
    ? {fontFamily: "sans-serif-medium", fontWeight: "600"}
    : {fontWeight: "600"},
  // reminder: oneplus handle sans-serif-regular / 700 != sans-serif / 700 !!
  "weight700": Platform.OS === "android"
    ? {fontFamily: "sans-serif-regular", fontWeight: "700"}
    : {fontWeight: "700"},
  "weight800": Platform.OS === "android"
    ? {fontFamily: "sans-serif-bold", fontWeight: "700"}
    : {fontWeight: "800"},
  "weight900": Platform.OS === "android"
    ? {fontFamily: "sans-serif-black", fontWeight: "900"}
    : {fontWeight: "900"},
})

Just to tag issues so people would find the workaround: #15114 #25258 #29259 #18258 #17629 #21729

hussainahmad commented 3 years ago

@MoOx Not working for #29259

lexicalninja commented 3 years ago

Experiencing this issue on Samsung phones. The app I am responsible for uses a globally assigned custom font (SourceSansPro-Regular). We are seeing this issue where the font weight is not bolded. If I change the global font to 'System' the ellipses show up correctly. When using the custom font the ellipses are cut off at varying points -- sometimes half the third dot is missing, sometimes the last two dots are missing, etc.

I assume that this has something to do with the render function, but am looking for suggestions on how to fix this that includes pointing me at something in the base lib that might be the cause.

This is definitely a large issue considering we see it on one of the largest manufacturers of Android phones.

Thanks for any suggestions

MoOx commented 3 years ago

You cannot expect a regular font "SourceSansPro-Regular" to work correctly as "bold". Best thing you can do: don't use fontWeight without explicit fontFamily that you are sure cover the font. Do yourself a favour an do a map like I mentioned above, but with your own font families that cover all weight.

lexicalninja commented 3 years ago

That's great, but as I said we are seeing the issue when the font weight is NOT bold, so your font weight map would not help in this situation. We also have brand fonts, so switching fonts because the text rendering function isn't working properly isn't an option