Closed Bug 1593414 Opened 5 years ago Closed 5 years ago

Wrong font in Twitter on Nightly 2019-10-31

Categories

(Core :: Layout: Text and Fonts, defect, P3)

x86_64
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr68 --- unaffected
firefox70 --- unaffected
firefox71 --- unaffected
firefox72 --- fixed

People

(Reporter: saschanaz, Assigned: jfkthame)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(3 files)

  1. Access https://mobile.twitter.com/imasml_theater

Expected: The bio text "スマートフォンアプリ「アイドルマスター ミリオンライブ! シアターデイズ」の公式Twitterアカウントです!これからゲームに関する情報をお伝えしていきます♪" should use the same font as the tweets. (Meiryo if you installed Japanese language pack)

Actual: It uses something different.

This is a regression in 2019-10-31 build. mozregression says it's https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5fe1e03dbfbca52dbaec0dc096ca1884a851203d&tochange=9a2e7d38cb07538761fdef5512550618cb00b3fa, but it's a backout commit 🤔

:jfkthame, can you dupe this or get this bug into the right component?

Flags: needinfo?(jfkthame)

Pretty sure this'll be related to bug 1581822, though when I view that twitter page on my (English-locale) Mac, it still seems to be using the Japanese font. So a bit of investigation may be called for to figure out exactly which factors are involved here.

Leaving ni? flag for now to remind me to look into it...

I'll move it to the Layout: Text and Fonts component to get it out of the Firefox :: General triage queue.

Component: General → Layout: Text and Fonts
Product: Firefox → Core

Let's call this P2 as in "recent regression with known regression range that needs more investigation".

On Linux here I also see the same font used for both texts (Noto Sans CJK SC).

Priority: -- → P2
Regressed by: 1581822

Ah, I see.... inspecting the page at https://mobile.twitter.com/imasml_theater, I see the tweet content is inside a <div> tagged with lang="ja", whereas the user bio, headings, etc., just inherit lang="en" from the root <html> element.

Therefore, it's expected that they'll end up using different prefs to resolve the sans-serif generic from their font-family property. For unified Han characters, obviously, the lang="en" font prefs aren't likely to help. If the content is tagged with an appropriate language, or if the accept-languages header or the browser or system locale directs us to prefer Japanese, we'll order our CJK fallbacks accordingly. But in the absence of any such hint, in bug 1581822 we deliberately changed the order so as to put Chinese prefs ahead of Japanese, as discussed there.

So this is the expected result of bug 1581822 working as designed.

It's true that it can give a worse result for some (untagged) Japanese content, if the user doesn't have locale settings that nudge us to still use the Japanese prefs; but this is simply the trade-off for making things better in the analogous Chinese situation. (Note that users running a Japanese-localized browser and/or localized OS should not be affected, as their locale will take precedence over the last-resort fallback ordering.)

Finally, as a mitigation for affected users, in bug 1596875 it's proposed that we add a pref to allow this order to be set in about:config rather than hard-coded.

Flags: needinfo?(jfkthame)
Priority: P2 → P3

or the browser or system locale directs us to prefer Japanese

Can Windows "Preferred languages" list help here? I have "English -> Japanese -> Korean" in this list. This list matters for other parts of system where unified Han characters render based on the order: If the list prefers Japanese over Korean then a Japanese font is used, and vise versa.

Flags: needinfo?(jfkthame)

(In reply to saschanaz from comment #8)

or the browser or system locale directs us to prefer Japanese

Can Windows "Preferred languages" list help here? I have "English -> Japanese -> Korean" in this list. This list matters for other parts of system where unified Han characters render based on the order: If the list prefers Japanese over Korean then a Japanese font is used, and vise versa.

In principle, this should help.... but unfortunately in https://bugzilla.mozilla.org/show_bug.cgi?id=1581822#c2, :m_kato says that we currently use only the first locale from the system on Windows.

From the stackoverflow post referenced there, it looks as though we might be able to get at least one more fallback language, which would improve the behavior in a case like yours. This sounds worth investigating further, at least.

Flags: needinfo?(jfkthame)

After experimenting a bit with GetUserPreferredUILanguages, I can confirm that (as :m_kato said in bug 1581822), it does not seem to be capable of correctly returning a multi-language list, even though I can configure multiple "preferred languages" in the Win10 settings app.

This issue is described at https://stackoverflow.com/questions/52849233/getuserpreferreduilanguages-never-returns-more-than-two-languages, and also mentioned at https://www.autohotkey.com/boards/viewtopic.php?p=73863&sid=163a23d1f0a9f26e17483bcb5c75337d#p73863. Until this API is fixed (or some alternative provided), there doesn't seem to be a supported way for us to get the full list from Windows. :(

For now, the pref in bug 1596875 may be the best we can do. It's unfortunate that we can't just use the Windows system setting for this.

I could get the correct preferred language list using GlobalizationPreferences.Languages WinRT API. This API is usable from Win32 desktop apps. I added an answer to the stackoverflow question.

Ah, that sounds promising!

It looks like using it in Firefox might take a bit of work, though.... I tried to #include the necessary winrt headers in OSPreferences_win.cpp, but that results in an error saying that they require C++17 features (and AFAIK we're currently compiling in C++14 mode). So I think some work on the build configuration will be needed if we're going to try adopting this.

I see we have a bug in progress about moving to C++17 as default, but don't know how close that is to being completed. Would you have time to look into this further, and see if we can hook it up in OSPreferences?

C++/WinRT is just a WinRT API wrapper. If we can't use C++/WinRT (yet), we can use WRL instead. WRL is already used in our tree. See WindowsUIUtils and ToastNotificationHandler under widget/windows for examples.

I added a WRL example to my SO answer.

Thanks, that's awesome. I tried incorporating this into OSPreferences::ReadSystemLocales on Windows, and this correctly lets us use the full list of Preferred Languages from the Windows settings to determine the ordering of the CJK font prefs (for content where we didn't have an explicit lang tag, etc). So with the reporter's example, with a Preferred Languages list of "English -> Japanese -> Korean", we'd expect to see the Japanese fonts used.

The pref that was added in bug 1596875 would then only be relevant if the user's Preferred Languages list doesn't provide an order for CJK.

BTW, is it also expected that my example also uses different font for non-Unified-Han characters? (Katakana characters specifically)

Flags: needinfo?(jfkthame)

Yes, because for the purpose of choosing which font prefs to use, Firefox groups the Kana characters together with unified-Han in a comprehensive "CJK set of character ranges", and then applies the CJK font prefs ordered according to the various possible language hints from the environment, as discussed here.

I suppose we could argue that for the Kana ranges, it'd make sense to separate them from the more general "CJK set" and always prefer the Japanese font settings. But that would increase the chance of getting an ugly mixture of fonts, with kana coming from a Japanese font but nearby kanji coming from a Chinese one.

So yes, that's expected given how the font selection code is currently organized.

Flags: needinfo?(jfkthame)
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/af71a7b69a5e
Try to use the full list of user-preferred languages on Windows to guide CJK font-preference priority. r=emk
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Assignee: nobody → jfkthame
Regressions: 1598628
Blocks: 1603030
Flags: qe-verify+
Regressions: 1606744
Has Regression Range: --- → yes

I reproduced this on the affected Nightly build(2019-10-31) on a VM running Win10 64 bits. Checked on latest Nightly and Beta and it seems that the font is still broken.
I left below 2 screenshots regarding the issue.

Nightly
Beta

The point in comment #0 was "should use the same font as the tweets". Both screenshots in comment #22 uses the same fonts consistently throughout the page. Mozregression says it's fixed in 2019-11-22 build, which includes https://hg.mozilla.org/mozilla-central/rev/af71a7b69a5e947be09494bbdb33cdc15c4ef343 (exactly the patch attached here).

You should install Japanese language pack (especially the font pack) for Windows to repro this, btw.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: