Closed Bug 1774413 Opened 2 years ago Closed 2 years ago

Bundled fonts are not picked up on macOS

Categories

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

Firefox 103
x86
macOS
defect

Tracking

()

RESOLVED FIXED
105 Branch
Tracking Status
firefox105 --- fixed

People

(Reporter: ti8bpxk2y, Assigned: pierov)

References

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

I was trying to reproduce this issue affecting the macOS version of Tor Browser, to see if it is an upstream one.

To do so I:

  1. Added --enable-bundled-fonts to my mozconfig.
  2. Built from source.
  3. Checked that the build flag showed u[ in about:buildconfig > Configure options.
  4. Added TwemojiMozilla.tff to Nightly.app/Contents/Resources/fonts.
  5. Added Krub.tcc to Nightly.app/Contents/Resources/fonts.
  6. Set gfx.bundled-fonts.activate to 1 in about:config

Actual results:

  • I used https://arkenfox.github.io/TZP/tests/fontlists.html to check my fontlist (also tweaked the code a bit locally to expect Krub) but neither one showed up.
  • I changed font.name-list.emoji to use Twemoji but I was still getting Apple emojis.
  • As you can see from the TB issue, fonts are not picked up there as well. Noto Sans Bengali is bundled but that still results in tofu.

Others:

  • I'm not sure that's very meaningful but I noticed the fonts showed up in about:preferences > Fonts, so I tried to set Krub as default font and instead it used Times as a fallback.
  • Neither font is installed system wide.

Expected results:

Bundled fonts should be fully functional.

OS: Unspecified → macOS
Hardware: Unspecified → x86
Severity: -- → S3
Component: Graphics: Text → Layout: Text and Fonts

Hello, I investigated the problem on Tor Browser.

My conclusion is that bundled fonts aren't registered to the shared font manager in content processes, but only in the main process.

Therefore, [sFontManager availableMembersOfFontFamily:family] returns nil (gfxMacFontFamily::FindStyleVariations(FontInfoData* aFontInfoData), gfx/thebes/gfxMacPlatformFontList.mm), and bundled fonts are removed from mFontFamilies in gfxPlatformFontList::CheckFamily.

I hacked a way to call ActivateBundledFonts() (gfx/thebes/gfxPlatformMac.cpp) at least once in content process, and it fixes the problem.
I'll try to prepare a proper patch.

Does Tor Browser have gfx.e10s.font-list.shared set to true or false?

TB 11.5a12 has it set to false.

(In reply to Jonathan Kew (:jfkthame) from comment #2)

Does Tor Browser have gfx.e10s.font-list.shared set to true or false?

Like Fabrizio said, it's false.

So, I have tried to set it to true, but the problem seems to persist.

On a comment on the code before Bug 1704273, I can see that registering font only on the parent process is expected to be enough on macOS Catalina (which I am using) or newer.

But in our case, loading the bundled fonts in all processes solves our issue, as long as gfx.e10s.font-list.shared remains false.
The attached patch is what I have used.
I am doing a build to ask some people to test with more recent macOS version.

But I am also going to investigate a little bit more to understand where the problem is with gfx.e10s.font-list.shared set to true.

Assignee: nobody → pierov

I think there are 3 problems with fonts on Mac, currently:

  1. bundled fonts need to be registered to the font manager also from child processes, otherwise they do not work, neither with gfx.e10s.font-list.shared set to true nor false. My proposed solution is the patch I attached yesterday;
  2. with gfx.e10s.font-list.shared set to true, ApplyWhitelist is not called in children, and mFontFamilyWhitelistActive is never set to true, if needed. From what I understood, it is equivalent to mFontFamilyWhitelistActive = !mEnabledFontsList.IsEmpty();, so I propose to do this whenever mEnabledFontsList is (re-)read from preferences, instead of setting mFontFamilyWhitelistActive in ApplyWhitelist;
  3. on Mac, the visibility for bundled fonts is User, but on the other platforms it is Base. My proposed solution is populating a set with the bundled fonts while they are being loaded, and use a function to query whether they are bundled fonts.

1 is necessary in any case.
With gfx.e10s.font-list.shared == true, also either 2 or 3 are needed, otherwise bundled fonts are discarded before FindFontForChar is even called, because IsVisibleToCSS returns false.

Jonathan: could you please review a phabricator patch, if I created one? Or could you please tell me who I could ask for?
Thanks in advance!

Flags: needinfo?(jfkthame)

Thanks for looking into this. Yes, if you post a patch to phabricator, please tag me for review and I'll take a look; it sounds like you've identified the issues that need to be fixed here.

Flags: needinfo?(jfkthame)

(In reply to Jonathan Kew (:jfkthame) from comment #7)

Thanks for looking into this. Yes, if you post a patch to phabricator, please tag me for review and I'll take a look; it sounds like you've identified the issues that need to be fixed here.

Yes, thank you!

I have already written the patches, actually, but for 91.10.
I will rebase them to a more updated version of Firefox and then then create the patch.

Bundled fonts were not picked up because also child processes need to
register them.
Also, they were assigned User visibility, instead of Base, which was
not coherent with other platforms.

@pierov, sorry for the slow response here, I'll try to review this shortly.

One thing I did wonder is about how this relates to the macOS-version-dependent behavior mentioned at https://searchfox.org/mozilla-central/rev/287583a4a605eee8cd2d41381ffaea7a93d7b987/gfx/thebes/gfxPlatformMac.cpp#166-169, which I see (in comment 5) you've noticed, but it sounds like that didn't apply in this case.

Is that comment not (no longer?) accurate, perhaps? Can you confirm whether the supplemental-language fonts are available to child processes in your testing, even though bundled fonts (activated via the same ActivateFontsFromDir function) are not?

Flags: needinfo?(pierov)

(In reply to Jonathan Kew (:jfkthame) from comment #10)

@pierov, sorry for the slow response here, I'll try to review this shortly.

No problem for the delay, and thanks for your time.

One thing I did wonder is about how this relates to the macOS-version-dependent behavior mentioned at https://searchfox.org/mozilla-central/rev/287583a4a605eee8cd2d41381ffaea7a93d7b987/gfx/thebes/gfxPlatformMac.cpp#166-169, which I see (in comment 5) you've noticed, but it sounds like that didn't apply in this case.

Is that comment not (no longer?) accurate, perhaps? Can you confirm whether the supplemental-language fonts are available to child processes in your testing, even though bundled fonts (activated via the same ActivateFontsFromDir function) are not?

My test was going to /System/Library/Fonts/Supplemental, and try to set some fonts I found there as font-family on the inspector tool: they were displayed correctly.
I have tried both with the official Firefox 102 binaries, and with Tor Browser 11.0.x (after adding them to font.system.whitelist, with the latter).
I think this is already enough, but I can try with a debugger, if needed. (Our main problem was that tofus were displayed instead of some languages, such as Bengali).

I arrived to the conclusion that bundled fonts needed to be registered in child processes because some calls to system API failed, for example:

* thread #1, name = 'MainThread', queue = 'com.apple.main-thread', stop reason = step over
    frame #0: 0x000000011b8d3ebb XUL`MacOSFontEntry::CreateFontInstance(this=0x0000000110280f10, aFontStyle=0x00000001186f6130) at gfxMacPlatformFontList.mm:278:7
   275 	  if (!unscaledFont) {
   276 	    CGFontRef baseFont = GetFontRef();
   277 	    if (!baseFont) {
-> 278 	      return nullptr;
   279 	    }
   280 	    unscaledFont = new UnscaledFontMac(baseFont, mIsDataUserFont);
   281 	    mUnscaledFont = unscaledFont;
Target 0: (plugin-container) stopped.
(lldb) bt
* thread #1, name = 'MainThread', queue = 'com.apple.main-thread', stop reason = step over
  * frame #0: 0x000000011b8d3ebb XUL`MacOSFontEntry::CreateFontInstance(this=0x0000000110280f10, aFontStyle=0x00000001186f6130) at gfxMacPlatformFontList.mm:278:7
    frame #1: 0x000000011b7bd5d1 XUL`gfxFontEntry::FindOrMakeFont(this=0x0000000110280f10, aStyle=0x00000001186f6130, aUnicodeRangeMap=0x0000000000000000) at gfxFontEntry.cpp:281:24
    frame #2: 0x000000011b7bd137 XUL`mozilla::fontlist::Family::SearchAllFontsForChar(this=0x00000001105b13f4, aList=0x00000001103bdc20, aMatchData=0x00007ffee0415470) at SharedFontList.cpp:505:38
    frame #3: 0x000000011b8629e3 XUL`gfxPlatformFontList::GlobalFontFallback(this=0x0000000117cc2000, aCh=2452, aNextCh=8204, aRunScript=BENGALI, aPresentation=Text, aMatchStyle=0x00000001186f6130, aCmapCount=0x00007ffee0415674, aMatchedFamily=0x00007ffee0415680, aFontMatchingStats=0x000000011029d940) at gfxPlatformFontList.cpp:1154:18
    frame #4: 0x000000011b8619d4 XUL`gfxPlatformFontList::SystemFindFontForChar(this=0x0000000117cc2000, aCh=2452, aNextCh=8204, aRunScript=BENGALI, aPresentation=Text, aStyle=0x00000001186f6130, aVisibility=0x00007ffee0415717, aFontMatchingStats=0x000000011029d940) at gfxPlatformFontList.cpp:982:12
    frame #5: 0x000000011b88676f XUL`gfxFontGroup::WhichSystemFontSupportsChar(this=0x00000001186f6100, aCh=2452, aNextCh=8204, aRunScript=BENGALI, aPresentation=Text) at gfxTextRun.cpp:3774:48
    frame #6: 0x000000011b882a98 XUL`gfxFontGroup::FindFontForChar(this=0x00000001186f6100, aCh=2452, aPrevCh=32, aNextCh=8204, aRunScript=BENGALI, aPrevMatchedFont=0x000000014cfd15d0, aMatchType=0x00007ffee0415a38) at gfxTextRun.cpp:3385:10

So, I thought that registering them could have solved the problem and it did.
However, I am not a macOS expert at all, so I am not sure of what is going on there.
My conjecture is that system fonts (including the supplemental ones) could be privileged. Or font APIs could have some paths where to search for fonts (ATSApplicationFontsPath could be it), and system fonts are automatically added to that path. But I have just had this idea, and I have not tested it.

Flags: needinfo?(pierov)
Assignee: nobody → pierov
See Also: → 1780233
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4979a1899d2e
Bundled fonts are not picked up on macOS r=jfkthame

@jfkthame thanks for the review and for pushing!

Would it be possible to get it uplifted to ESR 102?

Thanks again!

Flags: needinfo?(jfkthame)

I think that'd be reasonable; let's check that it lands safely in Nightly, and request uplift after a day or two if all goes well.

Flags: needinfo?(jfkthame)

Sounds great, thanks!

Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: