1882613 - pdf font renders badly

Status: RESOLVED FIXED

(GeckoView :: PDF Viewer, defect, P1)

Description

[mozbugz]

Attachment: Screen shot of Fenix

Steps to reproduce:

I opened the pdf at: https://www.whitehouse.gov/wp-content/uploads/2024/02/Final-ONCD-Technical-Report.pdf
on Firefox Nightly: 125.0a1 build # 2016006119 12b61f9318+

Actual results:

It used an incorrect font which made (at least parts of) the document very difficult to read.

Expected results:

It should have used a better font so that the document is readable.

Comment 1

[mozbugz]

Attachment: Google drive screen cap

Attached is from google drive on the save device, and what I expect it to look like (and it looks like this on a computer as well).

Comment 2

[Calixte Denizet (:calixte)]

The font used to render the text is not embedded in the pdf so the viewer fallbacks on the default serif font from the system (very likely it's Noto Serif) which hasn't the same metrics as Times New Roman.
We're already embedding Liberation Sans to be used as a substitution for Helvetica, maybe we should embed Liberation too.

Comment 3

[Vasilica Tamas]

I was able to reproduce this on Firefox Nightly 125 (2024-03-04), Firefox Beta 124.0b6 and Firefox 123 using a Samsung A32 with Android 13. Considering also the suggestion from Comment 2, I confirm this as an Enhancement.

Comment 4

[Cathy Lu [:calu]]

Hey, is this mobile specific code? Where are the default fonts set?

Comment 5

[Calixte Denizet (:calixte)]

No it isn't mobile specific.
When the font isn't embedded we try to fallback on a font in the system:
https://github.com/mozilla/pdf.js/blob/master/src/core/font_substitutions.js
but if nothing is found we just fallback on serif or sans serif.
On mobile, it's very likely we won't be able to find a "good" substitution so very likely we'll fallback on serif/sans serif.
In the pdf case the missing fonts are TimesNewRomanPSMT and TimesNewRomanPS-BoldMT.
Some fonts are supposed to be available in every pdf viewer.
We already embed some fonts:
https://github.com/mozilla/pdf.js/tree/master/external/standard_fonts
At least for Fenix we shouldn't use the system fonts but the ones we already embed.

Comment 6

[Cathy Lu [:calu]]

Gotcha, do you need me to file a separate bug in pdf.js to add Liberation as a substitute for Times New Roman fonts, or leave this bug open for this work?

Comment 7

[Calixte Denizet (:calixte)]

There's no need to file a new bug: this one is enough, thank you.

Comment 8

[BMO Github Automation]

Attachment: [mozilla/pdf.js] [GeckoView] Don't use the system fonts (bug 1882613) (#18156)

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:calixte, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox127 to wontfix.

For more information, please visit BugBot documentation.

Comment 10

[Calixte Denizet (:calixte)]

The text rendering isn't perfect but at least it's readable so it can wait for the next cycle I guess to be sure that it won't induce any regressions (even if it's a one-liner).