Closed Bug 1697666 Opened 4 years ago Closed 4 years ago

non-Japanese font is used for monospace, on Japanese website

Categories

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

x86_64
macOS
defect

Tracking

()

VERIFIED FIXED
88 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox86 --- unaffected
firefox87 + verified
firefox88 + verified

People

(Reporter: arai, Assigned: jfkthame)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(6 files)

Attached file a.html

Steps to reproduce:

  1. Run Nightly 88.0a1 (2021-03-10) (64-bit), (either en-US locale or ja-JP locale), with clean Profile, on macOS 10.15.7 (19H524)
  2. save the attached page as a.html
  3. open a.html

Actual result:
text with monospace font uses "PingFang SC Regular"
(textarea in default section, and p and textarea in monospace section)

Expected result:
text with monospace font uses "Osaka-Mono"

Regression range: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=65e0a4623175a0f0966ac34c7abb93c11f3d8b9c&tochange=78dab59d989b3d5c16ca163b6a6520ca694fb729

Note:
This seems to happen intermittently on actual website
(the above STR may also be intermittent)

tested on textarea in the following pages (top 2 the search results for "textarea サンプル" on google):

tested with 2 ways:

  • paste those URL in location bar, after opening Nighly with clean profile
  • searching "textarea サンプル" from location bar, and clicking the above top 2 result

after opening the page, paste "海" in the textarea in the page.

Sometimes "Osaka-Mono" is used, and sometimes "PingFang SC Regular" is used.

Different result can be observed even with opening the same website in 2 tabs, by entering the URL.
(I see different process is used for those 2 tabs, in about:processes)

Flags: needinfo?(jfkthame)

here's screenshot of the attachment.

Also observed the problem intermittently when directly opening the attachment HTML, by opening the attachment in new tab, multiple times.
Sometimes "Osaka-Mono" is used, and sometimes "PingFang SC Regular" is used, for monospace font.

I've tried opening the attachment a bunch of times, in multiple tabs, as well as the example sites from comment 0, but I'm consistently getting Osaka-Mono as expected (using Nightly on macOS 11.1beta). Quite puzzled why you're seeing inconsistent behavior. How frequently does it happen for you -- do I need to try hundreds of times to have a chance of seeing the problem?

When the problem happens, does reloading the page (in the same tab) fix it, or does it persist in that tab until it's closed?

Flags: needinfo?(jfkthame)

tries directly opening the attachment in 20 tabs,
and 3 tabs (1st tab, 8th tab, and 16th tab) hit the issue, and I see that same single process is used for the tab, while others use multiple different processes.
(8 processes, 2-3 tabs for each)

and if the issue happens, reloading the page doesn't solve,
and also, reloading the page doesn't hit if the issue doesn't happen in the tab at first.
so, the both behavior persists until the tab is closed.

tried 5 times, with new Nightly instance for each, (quitting Nightly for each)

  • never happens for 20 tabs
  • 3 processes hit issue (first 3 tabs, and tabs that uses the same process)
  • 2 processes hit issue (first and 3rd tabs)
  • 5 processes hit issue (1-5-th tabs)
  • 5 processes hit issue (1-4, and 8)

It seems to be dependent to the time between the Nightly startup, and either child process startup or page load.

If I wait 20 seconds after starting Nightly, before opening a new tab and pasting the URL there and opening it, the issue doesn't happen.

also, I observed a bit different issue before bug 1687426:
in the same situation (the tab is opened immediately after starting Nightly),
"Hiragino Kaku Gothic ProN W3" is used for monospace font, instead of "Osaka-Mono" (expected), or "PingFang SC Regular" (actual, after bug 1687426),
and after waiting 20 seconds before opening tab, "Osaka-Mono" is used as expected.

So, looks like the issue that "wrong font is used intermittently" had been there,
and bug 1687426 changed the font for "wrong" case from "Hiragino Kaku Gothic ProN W3" to "PingFang SC Regular".

will check regression range for "wrong font" bug.

there are 2 changes before that:

before the following regression range, monospace font was always "Osaka-Mono" (that's expected behavior),
and after the following regression range, monospace font was always "Hiragino Kaku Gothic ProN W3" (so, always wrong),
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=268543e53e1b11ce0e468d985ea3777563e7b8a8&tochange=a7a6063bd5a4b49911b5a956c252a16961aec5a0

and after bug 1533462 (following fix range), the issue becomes intermittent (happens when the tab is opened immediately after starting Nightly, and doesn't happen if wait for a while before opening the tab), instead of always:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f6329f10fe5ccc4ca03304d6c5b52a9e116bf668&tochange=17b0768200fb16f96e26400b11c9ee0de53d8828

and then, bug 1687426 changed the "wrong" case's font.

See Also: → 1533462

(In reply to Tooru Fujisawa [:arai] from comment #8)

there are 2 changes before that:

before the following regression range, monospace font was always "Osaka-Mono" (that's expected behavior),
and after the following regression range, monospace font was always "Hiragino Kaku Gothic ProN W3" (so, always wrong),
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=268543e53e1b11ce0e468d985ea3777563e7b8a8&tochange=a7a6063bd5a4b49911b5a956c252a16961aec5a0

I don't currently understand why it regressed there (but was able to confirm this locally -- it's a pity the it is too old for mozregression to give us a more precise range). There are a couple of patches in the range that touched font-related code but I can't see how they could have affected this behavior.

and after bug 1533462 (following fix range), the issue becomes intermittent (happens when the tab is opened immediately after starting Nightly, and doesn't happen if wait for a while before opening the tab), instead of always:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f6329f10fe5ccc4ca03304d6c5b52a9e116bf668&tochange=17b0768200fb16f96e26400b11c9ee0de53d8828

This depends on whether the font-list initialization that runs shortly after startup (the "delayed font-info loader") has completed before the content process first tries to look up the Osaka-Mono font. Osaka-Mono is "special" because it's not really a font family in its own right, it is a face within the Osaka family; but it is also exposed via a special hack as a separate family name (see the "single face family" code in gfxMacPlatformFontList). This works via the shared font-list's "alias" table, but that table is populated when the loader completes.

If the content process has attempted to resolve the monospace-Japanese font pref before that happens, it will fail to find Osaka-Mono, and that result will be recorded in the font-prefs cache, so that content process will permanently fail to get the right monospace font for locale=ja.

To fix this, we need to ensure the alias table is populated with the Osaka-Mono entry at initialization time, even though the remaining alias entries aren't yet known; we'll then update it when the loader completes.

Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Blocks: 1694174
Severity: -- → S3

Is this nightly only or does this affect beta/release?

Flags: needinfo?(arai.unmht)

This affects beta 87.0 (64-bit) and dev edition 87.0b9 (64-bit) as well.
and the issue is a bit different than nightly there (it's worse), that the "PingFang SC Regular" (wrong font) is used regardless of how long I wait before opening the page.

Looks like it's because bug 1533462 change isn't enabled for beta/dev edition.
I see that gfx.e10s.font-list.shared pref is false there, and flipping it to true changes the behavior that the wrong font is used only when I open the page immediately after starting Firefox (so, same as nightly)

Then, the issue about "wrong font is used for monospace" affects release 86.0.1 (64-bit).
as described in comment #8 above, it's always "Hiragino Kaku Gothic ProN W3" there.
(so I guess the issue has been there from 76)

:jfkthame, does the patch also fix gfx.e10s.font-list.shared == false case?
(I'll test locally, but want to make sure)

Flags: needinfo?(arai.unmht) → needinfo?(jfkthame)

No, the patch here is only for the shared=true case (which is what we want to be shipping as the default soon).

I don't currently understand why this broke in the shared=false case (the legacy code), but it looks like it's been that way for about 10 releases so it's not a recent regression.

Flags: needinfo?(jfkthame)

Thanks.

Tested the behavior with gfx.e10s.font-list.shared == false.

For bug 1687426 regression ("PingFang SC Regular" is used for monospace), the issue seems to be the following:

In the following code, mFirstGeneric is set to StyleGenericFontFamily::Monospace, and pfl->GetDefaultGeneric(currentLang) is StyleGenericFontFamily::SansSerif for ja lang.

https://searchfox.org/mozilla-central/rev/1a47a74bd5ba89f2474aa27c40bd478e853f3276/gfx/thebes/gfxTextRun.cpp#3687-3690

  for (i = 0; i < numLangs; i++) {
    eFontPrefLang currentLang = prefLangs[i];
    StyleGenericFontFamily generic =
        mFirstGeneric != StyleGenericFontFamily::None
            ? mFirstGeneric
            : pfl->GetDefaultGeneric(currentLang);

Before the patch, the code was the following:

  for (i = 0; i < numLangs; i++) {
    eFontPrefLang currentLang = prefLangs[i];
    StyleGenericFontFamily defaultGeneric = pfl->GetDefaultGeneric(currentLang);

So, before bug 1687426 patch, it was using sans-serif font,
and after bug 1687426 patch, it's using monospace,
and that is the reason why the different font is used between them.

I'm not sure why choosing StyleGenericFontFamily::Monospace results in "PingFang SC Regular" tho... (it's not monospace font I guess?)

Then, I'll investigate the first regression about "wrong font is used for monospace".
(if it's just because sans-serif is used for monospace, there might be yet another regression about monospace font lookup, in between first and second regression)

So actually bug 1687426 "fixes" a bug where a monospace font was not selected correctly. The problem is that the monospace generic family resolves to "PingFang SC Regular" somehow instead of "Osaka-mono".

the first regression that "wrong font is used for monospace" is from bug 1621248.
https://hg.mozilla.org/mozilla-central/rev/4a936097f68049ccca78c7d9e6efcfc53b23fce8

4a936097f68049ccca78c7d9e6efcfc53b23fce8 itself doesn't build because of earlier bustage, so I've tested by rebasing it onto 6eab5a5a91c13ef5c91fccfe354fc49130810df3.

without 4a936097f68049ccca78c7d9e6efcfc53b23fce8, "Osaka-Mono" is used for monospace, and with 4a936097f68049ccca78c7d9e6efcfc53b23fce8, "Hiragino Kaku Gothic ProN W3" is used for monospace.

Regressed by: 1621248
Has Regression Range: --- → yes

Thanks for working to pin down the regression. That patch was not supposed to change behavior at all, but obviously it's broken in some way.

Of the files touched in that changeset, only gfxMacPlatformFontList.h and gfxMacPlatformFontList.mm are even used in the Mac build; the others are all for non-Mac platforms. The changes all look OK to me, but maybe the compiler is doing something I don't understand. If you're willing to spend some more time, maybe try reverting each individual change (addition of final) in those Mac files, and pin down exactly which one is causing the problem. (My guess: maybe it's the declaration of gfxSingleFaceMacFontFamily, because that's used for Osaka-mono. But I don't see what's wrong with it.)

it's gfxMacFontFamily.
If I remove final from it, on m-c tip, gfx.e10s.font-list.shared == false case also uses Osaka-Mono correctly.

would it be reasonable to backout https://hg.mozilla.org/mozilla-central/rev/4a936097f68049ccca78c7d9e6efcfc53b23fce8 on central and beta?
so that the breakage doesn't hit release channel.

Flags: needinfo?(jfkthame)

meanwhile, I'll investigate how it breaks.

I think this cast breaks things:
https://searchfox.org/mozilla-central/source/gfx/thebes/gfxMacPlatformFontList.mm#936-937
Adding final to gfxMacFontFamily will de-virtualize methods, so macFamily->IsSingleFaceFamily() will always return false.

Why gfxSingleFaceMacFontFamily derives from gfxFontFamily instead of gfxMacFontFamily? In this case, it is an undefined behavior to cast gfxSingleFaceMacFontFamily* to gfxMacFontFamily*. gfxSingleFaceMacFontFamily should have derived from gfxMacFontFamily. The compiler would have caught the bug (gfxMacFontFamily can not be final) if it did.

Or gfxSingleFaceMacFontFamily and gfxMacFontFamily should have derived from a common base class that declares IsSingleFaceFamily.

[Tracking Requested - why for this release]:
on macOS, TEXTAREA and other "monospace" text uses non-Japanese font "PingFang SC Regular" on Japanese website, while it should be "Osaka-Mono".

The similar issue had been there from Firefox 76, but there it was using Japanese font "Hiragino Kaku Gothic ProN W3".

There are several differences in the glyph between "PingFang SC Regular" and "Hiragino Kaku Gothic ProN W3" (or "Osaka-Mono"),
because of the difference between Japanese Kanji and Chinese Kanji, and some of them look completely different character.

Using "Hiragino Kaku Gothic ProN W3" instead of "Osaka-Mono" wouldn't be much noticeable (given there wasn't bug report for 10 cycles),
but using "PingFang SC Regular" is really noticeable and it looks like wrong character is shown, or inserted to text area.

We've already built the first release candidate for 87. We'll probably have a second one, but would need a patch very soon to make it in time.

Pushed by arai_a@mac.com: https://hg.mozilla.org/integration/autoland/rev/0b056792a101 Move IsSingleFaceFamily into gfxFontFamily. r=jfkthame

Comment on attachment 9209799 [details]
Bug 1697666 - Move IsSingleFaceFamily into gfxFontFamily. r?jfkthame!

Beta/Release Uplift Approval Request

  • User impact if declined: on macOS, TEXTAREA and other "monospace" text uses non-Japanese font "PingFang SC Regular" on Japanese website, while it should be "Osaka-Mono".
    comment #24 has more context.

The fix isn't covered by automated tests, because Osaka-Mono isn't available on automation.
Testcase itself is included in the patch, with disabled, and I've confirmed the fix locally with them.

  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Run Nightly, open https://bug1697666.bmoattachments.org/attachment.cgi?id=9208267 , make sure Osaka-Mono is used for textarea in default section, and text/textarea in monospace section (make sure Osaka-Mono font is enabled in Font Book.app)
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Essentially, this reverts bug 1621248 change, with proper fix, so the behavior is reverted to before the patch, that had been there for long time.
  • String changes made/needed: None
Attachment #9209799 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Marking as leave-open for https://phabricator.services.mozilla.com/D108186, which has not yet landed. (But is not critical to uplift, as it relates to the shared font-list configuration that is not enabled by default on release.)

Flags: needinfo?(jfkthame)
Keywords: leave-open
Blocks: 1699246
Attachment #9209799 - Flags: approval-mozilla-beta? → approval-mozilla-release?

Comment on attachment 9209799 [details]
Bug 1697666 - Move IsSingleFaceFamily into gfxFontFamily. r?jfkthame!

fix a regression in mac font selection, approved for 87 rc2.

Attachment #9209799 - Flags: approval-mozilla-release? → approval-mozilla-release+
Pushed by jkew@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0438c8585f5f Ensure the Osaka-Mono font family alias is available immediately to satisfy font-prefs lookups. r=jwatt
Keywords: leave-open
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch

Hello Tooru, I am trying to Verify the fix for this issue but I can’t seem to reproduce it, I’m trying to reproduce it on a Mac 10.15.5 using ja-JP nightly builds that were a few days/weeks older than todays build. At first I noticed a slight difference in the character from the first “default” section and its text area from your attached test case (“a.html”), there were a few extra lines inside the character (attachment - Comment 1)but that issue also occurs on the latest build.

After opening Font Book.app I noticed that I had to download the Osaka family font (regular, regular mono) manually and then I retested using the steps from Comment 28 only now everything is displayed correctly (no differences between the text area and the example above it).
I also noticed that in about:preferences the Default font is Hiragino while in Advanced font settings the Monospace font = Osaka - Mono.

I also tried your second set of steps by reaching and pasting the character in the text areas from :
https://www.scollabo.com/banban/ref/sample/sample_17.html
https://codeforfun.jp/reference-html-tag-textarea/

But It would be displayed correctly.

The attached screenshot from Comment 1 happens only if I remove the Fonts from the Font Book app, and that Also occurs in our latest Nightly build 88.0a1 (2021-03-18) .
I also tested this issue comparing Release 86 with our latest 87 RC build 3 and the only noticeable difference is that the character in RC 87 is displayed in Bold. Pleasee see the screenshot.

Can you please explain which is the correct behavior and which one is not ? And maybe a few screenshots with the wrong Font displayed ?
Can you please check our latest Nightly build and see if the issue still occurs on your end ?

Thank you

Flags: needinfo?(arai.unmht)
Attached image difference.png

Thank you for checking!

If "Osaka-Mono" is installed, the expected behavior is that,
"Osaka-Mono" is used for default textarea, and monospace section,
so, screenshot of RC 87 above is expected.
Osaka-Mono is slightly bolder than Hiragino, so, in the expected case, textarea in default section looks bolder than plain text above the textarea,
and what you've observed looks expected.

If "Osaka-Mono" isn't installed, the case is still buggy. bug 1699329 patch is going to fix it.
On current Nightly, the expected behavior (still wrong, but expected at this point) is that,
"PingFang SC Regular" is used for default textarea, and monospace section.
so, comment #1 screenshot is expected, and also what you've observed looks expected.

Once bug 1699329 patch gets landed, the expected behavior for the case Osaka-Mono isn't installed becomes that,
"Hiragino Kaku Gothic ProN W3" is used for default textarea, and monospace section.
so, screenshot of Release 86.0 above becomes expected.

Flags: needinfo?(arai.unmht)

I've checked RC build1 and RC build3, and confirmed the fix, when Osaka-Mono is installed.
build1 uses "PingFang SC Regular" for monospace font, and build3 uses "Osaka-Mono" for monospace font.

Also checked Nightly 88.0a1 (2021-03-19) (64-bit), and also confirmed the both fix, when Osaka-Mono is installed.
Osaka-Mono is always used for monospace font regardless of the time between startup and page load (https://hg.mozilla.org/mozilla-central/rev/0438c8585f5f fixed), instead of "PingFang SC Regular"
and also Osaka-Mono is used for monospace font even if I flipped gfx.e10s.font-list.shared pref to false (https://hg.mozilla.org/mozilla-central/rev/0b056792a101 fixed), instead of "PingFang SC Regular"

Based on Comments 37 and 38 I'm inclined to update the flags for this issue to Verified but should we wait until Bug 1699329 is landed ? can we update the flags for this issue ?
Thank you for all the info, do you think we can update the flags for this issue ?

Flags: needinfo?(arai.unmht)

IMO we don't have to wait until bug 1699329 for this issue.

Flags: needinfo?(arai.unmht)

Great, Thanks again for everything. Updating the flags for this issue.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

This version of the test passes for me locally with Osaka/Osaka-Mono installed, and also passes
(because both test and reference fall back to serif) if they're absent. It should fail only if
the Osaka family is present, but the Osaka-Mono family name (which is the special-case entry
that was broken) is not recognized.

Pushed by jkew@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/14feeaed5794 followup - Make the osaka-mono-exists test confirm that if Osaka is present, then Osaka-Mono is also available. r=arai
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: