Closed
Bug 1396731
Opened 7 years ago
Closed 7 years ago
Make the default font size for zh same as other languages
Categories
(Core :: CSS Parsing and Computation, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: kevin.hsieh, Assigned: kevin.hsieh)
References
Details
Attachments
(2 files)
The default font size for zh on macOS is 15 but it is 16 for other languages. https://searchfox.org/mozilla-central/rev/04bee69b3274bd8d5cf52d54a0a5cc14dbe8693a/modules/libpref/init/all.js#4095-4097 Per discussion in Bug 1391341, this default value does not manifest itself prominently in Gecko because of the way Gecko inherits font size. But it does in Stylo, and it's causing a permafail in Bug 1396411. We should probably change the default font size for zh so that the testcases in Bug 1391341 and Bug 1394762 render the same in Gecko and Stylo, though Gecko and Stylo will be doing different things internally.
Assignee | ||
Comment 1•7 years ago
|
||
Is there a particular reason the default font size should be set to 15, or can we safely remove that?
Flags: needinfo?(jfkthame)
Comment 2•7 years ago
|
||
It looks like this was originally done in bug 143557 as a workaround for text measurement issues we were having with old QuickDraw-based code on early OS X versions, and it has just lingered ever since (although we have long ago moved on from those ancient APIs). As such, I expect we can safely remove the custom font size setting. This may affect some Chinese sites (where the font size isn't explicitly overridden by anything else), but I doubt it will be a problem.
Flags: needinfo?(jfkthame)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → kevin.hsieh
Status: NEW → ASSIGNED
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8904634 [details] Bug 1396731 - Make the default font size for zh same as other languages. https://reviewboard.mozilla.org/r/176450/#review181400 As noted above, I think this should be fine, but let's ask Xidorn -- as someone who actually reads Chinese :) and can more meaningfully comment on how it looks -- to confirm whether it's OK.
Assignee | ||
Updated•7 years ago
|
Attachment #8904634 -
Flags: review?(jfkthame) → review?(xidorn+moz)
Comment 5•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8904634 [details] Bug 1396731 - Make the default font size for zh same as other languages. https://reviewboard.mozilla.org/r/176450/#review181400 If this is the reason for asking me to review... I think we should just proceed with this, and see if there is any incoming bug report related. I myself would set the pref on my Firefox and see whether it becomes a problem anywhere.
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8904634 [details] Bug 1396731 - Make the default font size for zh same as other languages. https://reviewboard.mozilla.org/r/176450/#review181554
Attachment #8904634 -
Flags: review?(xidorn+moz) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/ffbd69cdade3 Make the default font size for zh same as other languages. r=xidorn
Keywords: checkin-needed
Assignee | ||
Comment 8•7 years ago
|
||
It appears that the fix causes a couple unexpected passes on tests that are marked as failing in stylo (precisely because of this bug). Revising test expectations.
Attachment #8904793 -
Flags: review?(xidorn+moz)
Updated•7 years ago
|
Attachment #8904793 -
Flags: review?(xidorn+moz) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/4132428cf77c Update stylo test expectations. r=xidorn
Keywords: checkin-needed
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ffbd69cdade3 https://hg.mozilla.org/mozilla-central/rev/4132428cf77c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•