Closed Bug 1687426 Opened 4 years ago Closed 4 years ago

With CSS property "font-family: serif", non-CJK lang attributes causes fonts fallback to sans-serif fonts

Categories

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

Product:
Core
Component:
Layout: Text and Fonts
Version:
Firefox 84
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
87 Branch
Tracking Flags:
Tracking Status
firefox87 --- fixed

People

(Reporter: moposx01, Assigned: jfkthame)

References

Details

Attachments

(2 files)

The article rendered on Firefox (on the left) and Chrome (on the right).
444.87 KB, image/png
Details
Bug 1687426 - If a CSS generic font-family name is present, use this to determine which font prefs to look at during fallback. r?emk
48 bytes, text/x-phabricator-request
Details | Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:84.0) Gecko/20100101 Firefox/84.0

Steps to reproduce:

open the article at https://medium.com/@ifanr/%E5%B7%B2%E7%BB%8F%E5%89%81%E6%89%8B-apple-tv-%E4%BA%86-%E6%80%8E%E4%B9%88%E8%83%BD%E4%B8%8D%E7%9C%8B%E7%9C%8B%E8%BF%99%E4%BA%9B-apps-1af3e918d803

Actual results:

Only Latin letters are rendered in serif font, the Chinese characters are rendered in sans-serif font.

Expected results:

The texts should be rendered all in serif fonts.

If the "lang=en" is removed or changed to "lang=zh" then the issue is resolved.

I made a simple web page to demonstrate the title: https://goofy-liskov-d9fd47.netlify.app/test.html, and the source files can be accessed at https://github.com/moposx/Bug-1687426-demonstration

We can see different results of font selection in Firefox and Chrome. In FIrefox , the CJK characters will be rendered in sans-serif font when non-CJK lang attributed is specified even if we set the font to be rendered in serif fonts in CSS. In Chrome it works well.

This issue will result in that, for example, the chinese article at medium.com will have inconcsistent font families rendered, or on sites like https://lyy289065406.github.io/re0-web/gitbook/book/index.html when choosing a theme with serif font, it does not work properly.

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

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

Works for me on Ubuntu 20.04.

Reproduced on Windows 10.

If I change [Options] > [General] > [Fonts and Colors] > [Advanced] > [Proportional] to "Serif" and reopen the tab, serif font-family is selected. But, this time, serif is selected even if the CSS-specified generic font family is sans-serif. Apparently this setting has higher precedence over the CSS generic family (which shouldn't).

I think we should check the CSS-specified generic font family here:
https://searchfox.org/mozilla-central/rev/b9384b091e901b3283ce24b6610e80699d79fd06/gfx/thebes/gfxTextRun.cpp#3675

Status: UNCONFIRMED → NEW
Ever confirmed: true

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)

Works for me on Ubuntu 20.04.

Yep, I also got the same report from some Arch Linux users.

I can confirm now that the issue is reproducible on Windows 10, macOS Big Sur and Android (Fenix). On Linux things are different since there is Fontconfig I guess.

(In reply to Masatoshi Kimura [:emk] from comment #4)

Reproduced on Windows 10.

If I change [Options] > [General] > [Fonts and Colors] > [Advanced] > [Proportional] to "Serif" and reopen the tab, serif font-family is selected. But, this time, serif is selected even if the CSS-specified generic font family is sans-serif. Apparently this setting has higher precedence over the CSS generic family (which shouldn't).

I think we should check the CSS-specified generic font family here:
https://searchfox.org/mozilla-central/rev/b9384b091e901b3283ce24b6610e80699d79fd06/gfx/thebes/gfxTextRun.cpp#3675

I followed this steps and got the same result. Looks like that the priority of these settings should be reconsidered during the font fallback.

Thanks for pointing that out., though I can't read those source code.

I agree this is a poor result; it has bothered me for a long time, actually. The problem arises because we only look at the CSS generics (serif, sans-serif, etc) specified on the page when initially resolving the font-family list to a list of available fonts, based on the (tagged or inferred) content language.

If we then hit font fallback -- e.g. because we find Chinese characters in an en-US page -- we just use our default generic for Chinese to look for a font via prefs, rather than respecting the CSS generic that was present on the page.

It looks like a pretty easy fix, actually. I'll push a patch to tryserver and see if tests break...

Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Pushed by jkew@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/45fb48dc681c If a CSS generic font-family name is present, use this to determine which font prefs to look at during fallback. r=emk

Backed out changeset 45fb48dc681c (bug 1687426) for arabic-final-ligature-spacing.html failures.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=HA1ltEHlRHOAbVNQ1cMO3A.0&fromchange=d5f3b4467116c4d579e8b86b815faa6436c12a18&searchStr=linux%2C18.04%2Cx64%2Cdebug%2Creftests%2Ctest-linux1804-64%2Fdebug-reftest-e10s%2Cr2&tochange=917f823a876b10e6c06d8fbb6fd43508398ce3ce

Backout link: https://hg.mozilla.org/integration/autoland/rev/917f823a876b10e6c06d8fbb6fd43508398ce3ce

Failures log: https://treeherder.mozilla.org/logviewer?job_id=328412681&repo=autoland&lineNumber=17697

[task 2021-02-01T02:43:14.418Z] 02:43:14     INFO - REFTEST TEST-START | layout/reftests/text/arabic-final-ligature-spacing.html == layout/reftests/text/arabic-final-ligature-spacing-ref.html
[task 2021-02-01T02:43:14.418Z] 02:43:14     INFO - [Child 10334, Main Thread] WARNING: Shouldn't call SchedulePaint in a detached pres context: file /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:7084
[task 2021-02-01T02:43:14.419Z] 02:43:14     INFO - [Child 10334, Main Thread] WARNING: Shouldn't call SchedulePaint in a detached pres context: file /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:7084
[task 2021-02-01T02:43:14.419Z] 02:43:14     INFO - REFTEST INFO | RESTORE PREFERENCE pref(gfx.font_rendering.fallback.async,true)
[task 2021-02-01T02:43:14.420Z] 02:43:14     INFO - REFTEST INFO | SET PREFERENCE pref(gfx.font_rendering.fallback.async,false)
[task 2021-02-01T02:43:14.420Z] 02:43:14     INFO - REFTEST TEST-LOAD | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text/arabic-final-ligature-spacing.html | 141 / 294 (47%)
[task 2021-02-01T02:43:14.437Z] 02:43:14     INFO - [Child 10334, Main Thread] WARNING: Shouldn't call SchedulePaint in a detached pres context: file /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:7084
[task 2021-02-01T02:43:14.437Z] 02:43:14     INFO - [Child 10334, Main Thread] WARNING: Shouldn't call SchedulePaint in a detached pres context: file /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:7084
[task 2021-02-01T02:43:14.437Z] 02:43:14     INFO - [Child 10334, Main Thread] WARNING: Shouldn't call SchedulePaint in a detached pres context: file /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:7084
[task 2021-02-01T02:43:14.454Z] 02:43:14     INFO - [Child 10334, Main Thread] WARNING: Shouldn't call SchedulePaint in a detached pres context: file /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:7084
[task 2021-02-01T02:43:14.454Z] 02:43:14     INFO - [Child 10334, Main Thread] WARNING: Shouldn't call SchedulePaint in a detached pres context: file /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:7084
[task 2021-02-01T02:43:14.454Z] 02:43:14     INFO - [Child 10334, Main Thread] WARNING: Shouldn't call SchedulePaint in a detached pres context: file /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:7084
[task 2021-02-01T02:43:14.538Z] 02:43:14     INFO - REFTEST INFO | RESTORE PREFERENCE pref(gfx.font_rendering.fallback.async,true)
[task 2021-02-01T02:43:14.538Z] 02:43:14     INFO - REFTEST INFO | SET PREFERENCE pref(gfx.font_rendering.fallback.async,false)
[task 2021-02-01T02:43:14.540Z] 02:43:14     INFO - REFTEST TEST-LOAD | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text/arabic-final-ligature-spacing-ref.html | 141 / 294 (47%)
[task 2021-02-01T02:43:14.563Z] 02:43:14     INFO - [Child 10334, Main Thread] WARNING: Shouldn't call SchedulePaint in a detached pres context: file /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:7084
[task 2021-02-01T02:43:14.564Z] 02:43:14     INFO - [Child 10334, Main Thread] WARNING: Shouldn't call SchedulePaint in a detached pres context: file /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:7084
[task 2021-02-01T02:43:14.565Z] 02:43:14     INFO - [Child 10334, Main Thread] WARNING: Shouldn't call SchedulePaint in a detached pres context: file /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:7084
[task 2021-02-01T02:43:14.755Z] 02:43:14     INFO - REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/text/arabic-final-ligature-spacing.html == layout/reftests/text/arabic-final-ligature-spacing-ref.html | image comparison, max difference: 255, number of differing pixels: 48
[task 2021-02-01T02:43:14.755Z] 02:43:14     INFO - REFTEST   IMAGE 1 (TEST): data:image/png;base64,iVBORw...
[task 2021-02-01T02:43:14.757Z] 02:43:14     INFO - REFTEST   IMAGE 2 (REFERENCE): data:image/png;base64,iVBORw0KGgoAAA...
[task 2021-02-01T02:43:14.759Z] 02:43:14     INFO - REFTEST INFO | Saved log: START file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text/arabic-final-ligature-spacing.html
[task 2021-02-01T02:43:14.759Z] 02:43:14     INFO - REFTEST INFO | Saved log: [CONTENT] OnDocumentLoad triggering AfterOnLoadScripts
[task 2021-02-01T02:43:14.760Z] 02:43:14     INFO - REFTEST INFO | Saved log: Initializing canvas snapshot
[task 2021-02-01T02:43:14.762Z] 02:43:14     INFO - REFTEST INFO | Saved log: DoDrawWindow 0,0,800,1000
[task 2021-02-01T02:43:14.763Z] 02:43:14     INFO - REFTEST INFO | Saved log: [CONTENT] RecordResult fired
[task 2021-02-01T02:43:14.763Z] 02:43:14     INFO - REFTEST INFO | Saved log: RecordResult fired
[task 2021-02-01T02:43:14.764Z] 02:43:14     INFO - REFTEST INFO | Saved log: START file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/text/arabic-final-ligature-spacing-ref.html
[task 2021-02-01T02:43:14.765Z] 02:43:14     INFO - REFTEST INFO | Saved log: [CONTENT] OnDocumentLoad triggering AfterOnLoadScripts
[task 2021-02-01T02:43:14.765Z] 02:43:14     INFO - REFTEST INFO | Saved log: Initializing canvas snapshot
[task 2021-02-01T02:43:14.765Z] 02:43:14     INFO - REFTEST INFO | Saved log: DoDrawWindow 0,0,800,1000
[task 2021-02-01T02:43:14.767Z] 02:43:14     INFO - REFTEST INFO | Saved log: [CONTENT] RecordResult fired
[task 2021-02-01T02:43:14.767Z] 02:43:14     INFO - REFTEST INFO | Saved log: RecordResult fired
[task 2021-02-01T02:43:14.767Z] 02:43:14     INFO - REFTEST TEST-END | layout/reftests/text/arabic-final-ligature-spacing.html == layout/reftests/text/arabic-final-ligature-spacing-ref.html

https://treeherder.mozilla.org/logviewer?job_id=328412869&repo=autoland&lineNumber=36185

[task 2021-02-01T03:02:02.634Z] 03:02:02     INFO -  REFTEST TEST-START | layout/reftests/font-matching/fallback-respects-generic-1.html == layout/reftests/font-matching/fallback-respects-generic-1-ref.html
[task 2021-02-01T03:02:02.635Z] 03:02:02     INFO -  REFTEST INFO | RESTORE PREFERENCE pref(gfx.font_rendering.fallback.async,true)
[task 2021-02-01T03:02:02.635Z] 03:02:02     INFO -  REFTEST INFO | SET PREFERENCE pref(gfx.font_rendering.fallback.async,false)
[task 2021-02-01T03:02:02.635Z] 03:02:02     INFO -  REFTEST TEST-LOAD | http://10.0.2.2:8854/tests/layout/reftests/font-matching/fallback-respects-generic-1.html | 6572 / 6606 (99%)
[task 2021-02-01T03:02:02.635Z] 03:02:02     INFO -  REFTEST INFO | RESTORE PREFERENCE pref(gfx.font_rendering.fallback.async,true)
[task 2021-02-01T03:02:02.635Z] 03:02:02     INFO -  REFTEST INFO | SET PREFERENCE pref(gfx.font_rendering.fallback.async,false)
[task 2021-02-01T03:02:02.635Z] 03:02:02     INFO -  REFTEST TEST-LOAD | http://10.0.2.2:8854/tests/layout/reftests/font-matching/fallback-respects-generic-1-ref.html | 6572 / 6606 (99%)
[task 2021-02-01T03:02:02.637Z] 03:02:02  WARNING -  REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/font-matching/fallback-respects-generic-1.html == layout/reftests/font-matching/fallback-respects-generic-1-ref.html | image comparison, max difference: 255, number of differing pixels: 595
[task 2021-02-01T03:02:02.658Z] 03:02:02     INFO -  REFTEST   IMAGE 1 (TEST): data:image/png;base64,iVBORw0...
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST   IMAGE 2 (REFERENCE): data:image/png;base64,iVBORw0KGg...
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: START http://10.0.2.2:8854/tests/layout/reftests/font-matching/fallback-respects-generic-1.html
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: [CONTENT] OnDocumentLoad triggering AfterOnLoadScripts
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: Initializing canvas snapshot
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: DoDrawWindow 0,0,800,1000
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: [CONTENT] RecordResult fired
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: RecordResult fired
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: START http://10.0.2.2:8854/tests/layout/reftests/font-matching/fallback-respects-generic-1-ref.html
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: [CONTENT] OnDocumentLoad triggering AfterOnLoadScripts
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: Initializing canvas snapshot
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: DoDrawWindow 0,0,800,1000
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: [CONTENT] RecordResult fired
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST INFO | Saved log: RecordResult fired
[task 2021-02-01T03:02:02.669Z] 03:02:02     INFO -  REFTEST TEST-END | layout/reftests/font-matching/fallback-respects-generic-1.html == layout/reftests/font-matching/fallback-respects-generic-1-ref.html
Flags: needinfo?(jfkthame)
Pushed by jkew@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/78dab59d989b If a CSS generic font-family name is present, use this to determine which font prefs to look at during fallback. r=emk

https://hg.mozilla.org/mozilla-central/rev/78dab59d989b

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox87: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch
Regressions: 1690986
Regressions: 1697666
Regressions: 1699329
Flags: needinfo?(jfkthame)
See Also: → 1748636
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: