-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix CJK fonts again and misc. font issues #14575
Conversation
Recapping the problem of
Some might find it less of a necessity to use localized fonts in the first place. Unfortunately, the current encoding of CJK characters means localized glyph forms could never be displayed by one monolithic font - doing so means at least users from one region would be unhappy with the rendering of text (or find words illegible at worst). Thus there is significant interest in maintaining correct and good rendering of localized texts on as many platforms as possible. |
FYI: https://piramisancchi.live-on.net:9175/ has my previous deployment before the conversion to CSS variables. Except for the |
Oh, I just noticed Segoe UI Symbols got added to match Bootstrap. I find that unnecessary and would now suggest removing it again, if you don't mind. |
cbdca73
to
2ca3523
Compare
Can this Windows issue be solved without JS? I'd prefer it to be CSS only and I think there must be some way to get the right fonts everywhere just be ordering alone, no? |
As stated above, the purpose of Many have thought of banning The design choice of adding that JS snippet came with several considerations in mind. First, without JS, the page would still render with To sum up, without JS, there are 3 options (all of which leave Segoe UI as first replacement after
|
Tried so hard but still cannot get the styling with variables done right. So frustrating. Ignoring the problem with I sincerely call for help with understanding how variables should be used for it to behave like before, that |
Please kindly refer to the deployed example here for further changes to the font stacks (not all are intended to be merged, of course). Changes are covered in CL-Jeremy#4 |
I've pushed some more tweaks to my repo/private PR regarding design language, specifically the use of Medium weight. As noticed already, Medium (500) is a fairly niche weight that is absent in many fonts. It's specifically wanted for body-like headings where simply using regular would be functionally inadequate. GitHub currently uses this only for highlighted buttons. I'm extending this to cover distinctions of navbar radio buttons, normal buttons and basic buttons, for selection/highlighting purposes. Previously, frame-only basic buttons would likely be thought as unclickable coming from a more traditional UI, like that of Windows XP/Aero. This would make the distinction between button status more perceptible. In browsers, weight 500 would first resolve to a thinner weight, making its usage less of an impact on systems lacking this weight, or when fallen back to such a font (note that I added some specific handling for Japanese under Mac). Everything from CL-Jeremy#4 serves as a sort of mock-ups to improve this PR based on master. I'm just working on the older branch since variables for fallback fonts don't work (and to better test it on my deployment). I tend to want to go back to Less mixins just for this part (emojis and proportional fonts, as well as monospaced fonts could remain as variables). As with |
Attempt to fix the issue with unicode-range This reverts commit 2ca3523.
Fontstack with @font-face
Codecov Report
@@ Coverage Diff @@
## master #14575 +/- ##
==========================================
- Coverage 42.21% 42.11% -0.10%
==========================================
Files 767 771 +4
Lines 81624 82079 +455
==========================================
+ Hits 34458 34571 +113
- Misses 41531 41885 +354
+ Partials 5635 5623 -12
Continue to review full report at Codecov.
|
Made my mind to pull the trigger here. Might not need to make all the Anyhow, this magically fixed my Linux Firefox issue here: #12966 (comment). It's now using |
Okay, the screenshot shown above was actually of bug with Firefox, which for some reason displays symbols from U+300x (specifically, ideographic comma and period) for newline characters using the emoji font when locale is set to zh-CN, zh-TW or ja. Fixed that and fiddled with Unicode ranges a bit more.
Edit: Found the culprit and provided a solution for the artifact. Now looking perfectly aligned everywhere. Still investigating the Firefox bug, though. Seems to affect Linux as well. |
ff48d90
to
9b10580
Compare
2ec92dd
to
d2dfb82
Compare
The last commit deals with the size difference between Safari and other platforms, as visualized in the comment section of #12623. Screenshot gallery at: https://try.gitea.io/CL-Jeremy/Test/issues/2 As always, please advise if there is a better approach than this. |
Heya @CL-Jeremy when you think this is ready it probably would be useful to put up some screenshots showing how and what it fixes. Thanks though for spending so much time on this. |
Thanks for understanding, I'm afraid this would get even larger and less reliable still. That's why I always would have liked someone to try out/review soundness of the code before reviewing for merging. Currently this code is working best with Chromium/Blink across all platforms. No alignment issues whatsoever. But targeting Chromium alone would have removed a ton of the workarounds here. Safari is working fine, or rather, macOS and iOS, being my most used platforms, have been specifically fixed with pretty high confidence (I even use 10.10), but that's also because they are least variable (shipped fonts are always preferred). Windows is fine but has some variability. It's also the reason to come up with this cumbersome To recap, the PR tries to fix a regression, aiming to display localized variants of CJK glyphs when the OS is unable to do so, a feature that worked since the introduction of local font stacks, and hopefully generalizing enough to become customizable thanks to CSS variables. The goals of this PR in logical order is:
Therefore it is highly probable that this fix will not get merged as it is right now, and remains as a reference for more elegant solutions. The best I could think of right now is still using JavaScript to eliminate As with screenshots, you are all heartily welcome to try my own deployment. I believe it's sufficient to go through existing pages without registration, and see how things differ on different platforms with different languages (NB: Windows depends on system UI language settings). |
The problem is hat most of us have no knowledge to judge what is broken and what it is you've fixed, so without labelled screenshots we're at a loss to know if there are actually even changes! |
Okay, so the current situation is: This is stock behavior with Japanese system locale. Now emulate font stack by explicitly calling for Bonus: Emulating stock with hidden font face again, note the difference in weight: Apart from this: the Hong Kong variant of Apple's stock font has been available since iOS 9/El Capitan, but not enabled until iOS 12/Mojave (cf. https://zhuanlan.zhihu.com/p/29856016). The custom font stack still caters this need, but needs the |
Regarding But also when displaying language of the same locale: All screenshots taken from unmodified, stock versions of try.gitea.io, with The issue mentioned in CL-Jeremy@7c1c2c3 is also a hardly noticeable example of this problem. The commit itself only solves the part with Western glyphs and is thus not really useful unless some approach like |
OK, so do you feel that you now have a working and decent set of results? If so, although I dislike the safari javascript hack I think the structure of your changes look OK and I would be happy to approve |
ef5fc66
to
d88b0fb
Compare
@zeripath I'm done. The last commit is due to the practice of aliasing font families with replacements in Fontconfig. This "aliasing" has nothing to do with render artifacts or so. It's just I couldn't come up with something concise and unambiguous as commit message, so I'm explaining here: some Linux distributions define fallbacks to a Western font (here: Noto Sans) much similar to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't break my test instance :)
and it looks plausible to work on Mac now ...
Could be considered WIP. Testing by fellow CJK users are highly appreciated. Summary of changes:
BlinkMacSystemFont
(Update font stack to bootstrap's latest #13834 did not address this, while the original Bootstrap commit includes this change)system-ui
for Windows (known to have caused problem as detailed in Make our font stack consistent microsoft/vscode#99428 - I didn't encounter this when working on Avoid unnecessary system-ui expansion #12522 before, as my local system wasn't set to an East Asian locale - now it's obvious to me that the'system-ui-font-family'
flag ofpostcss-preset-env
is aimed at solving this exact issue for Windows) - please do comment below if anyone finds a more elegant solution than thisLanguage
contextual variable in header template toLang
(has already been done for footer)@font-face
which should remain being of weight 700 to avoid faux-boldening in cases it does get set to bold (the real local boldface should still gets used when the whole stack is set to 500 after this change)Now the critical change:
--fonts-fallback
à la #12522/#issuecomment-681057176. However cross-browser testing has shown problems with nested partial overriding above 2 levels (that is, overriding a component variable in a higher cascade that is nested >=2 levels deeper in the lower cascade). So far only Safari 14 manages to excel in this test, showing all fonts correctly even in the language chooser popup (but may also be the browser ignoring all overrides, since macOS would natively choose the correct fonts with-apple-system
). Ignoring the fact that nested locale tag (i. e.<html lang="en"><a lang="zh-CN"></a></html>
) may become uneffective (which is puzzling, since DevTools have always shown the correct cascading consistently between browsers), this could be a great leap forward towards solving the various regressions caused by the initial refactoring into CSS variables.sans-serif
gets pushed to last place again. The problem mentioned in Css specifies emoji fonts for body text #13784 should have been taken care by the introduction ofLiberation
fonts (unless one insists on including the ancientDejaVu Sans
, which has already been proved to cause problems with emojis and more). GNU FreeSans could be added if someone finds that really necessary.