Closed
Bug 1361550
Opened 8 years ago
Closed 8 years ago
Make font-size: larger/smaller a simple ratio
Categories
(Core :: CSS Parsing and Computation, enhancement, P2)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: manishearth, Assigned: manishearth)
References
Details
Attachments
(5 files)
We currently implement font-size: larger and font-size: smaller by basically mapping back to the keyword sizes and bumping the keyword up or down by 1.
This means, for example, that font-size:larger when the parent is 16px on a serif font will map to font-size: large (18px). Something like 17px will take its position between font-size:medium and font-size:large (50%) and map it to the same position between font-size:large and font-size:x-large.
This is what the spec says too: https://drafts.csswg.org/css-fonts-3/#relative-size-value
However, Chrome/Safari just multiply by 1.2 or 0.85, as does Servo. IE seems to also multiply, but by different (close) ratios. It doesn't seem to handle the generic font change behavior.
Gecko's behavior in itself is not hard to implement in stylo, however it becomes nontrivial when we need to deal with generic font family changes, because we need to persist the keyword it mapped to. This makes it trickier to implement in Servo's model, which does not walk up the rule tree and instead keeps track of keyword sizes and ratios (this is something I want to eventually do in Gecko too). It's possible to implement, but after discussing in #layout the consensus seemed to be that we should just remove this behavior and use simple ratios like other browsers.
Assignee | ||
Comment 1•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7529a19c7881a899639a65758fc5b85ebf9cc40 shows that Gecko tests don't fail due to this change. One stylo test is an unexpected pass. There are multiple other tests affected by this, but they also fail due to scrollbars and other things so they won't show up.
Comment hidden (mozreview-request) |
Comment 3•8 years ago
|
||
So looking at this a little more -- I find it a little disturbing that the ratios aren't the inverse of each other. I'd have hoped that larger and then smaller (or vice-versa) would get you back to where you started.
Do you happen to know if the IE ratios produce that result? Maybe it would be better to use them...
Flags: needinfo?(manishearth)
Comment 4•8 years ago
|
||
Chromium seems to use multiplication and division by 1.2, actually:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/fonts/FontDescription.cpp?rcl=29b751a9ca7189238f4ad3703fd9db726698af91&l=154
Comment 5•8 years ago
|
||
... and that appears to have been true both before and after https://chromium.googlesource.com/chromium/src/+/17b5d686f5066fd209815697ca8c9e1419018133
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8863934 [details]
Bug 1361550 - Make font-size: larger/smaller a simple ratio; add stylo support;
https://reviewboard.mozilla.org/r/135674/#review139382
r=dbaron if you change both occurrences of 0.85 to 0.833333 (which seems equivalent to what Chromium does)... or convince me why 0.85 is better.
Attachment #8863934 -
Flags: review?(dbaron) → review+
Updated•8 years ago
|
Flags: needinfo?(manishearth)
Assignee | ||
Comment 7•8 years ago
|
||
Assignee | ||
Comment 8•8 years ago
|
||
Assignee | ||
Comment 9•8 years ago
|
||
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•8 years ago
|
||
yeah, I guess I got confused between the numbers there. I put 1 / 1.2 in both places (clearer that way)
I looked at IE and I have no idea what it's doing here. Firstly, font sizes aren't affected by the generic. Ok. That saves them a bunch of trouble.
The font sizes themselves are arranged differently than the 9/10/13/16/18/24/32 that Firefox and Chrome do. Ok. I expected that. Interestingly they're not whole numbers, but whatever.
font-size: larger and font-size: smaller do take you to the next keyword size. In most cases. Except when apply smaller on x-small, in which case it becomes even smaller than xx-small. Wat. This seems to be the worst of both worlds? Perhaps they have some weird scaling function here.
Anyway, changed to 1.2^-1, so we can land this I guess.
Comment 12•8 years ago
|
||
mozreview-review |
Comment on attachment 8863934 [details]
Bug 1361550 - Make font-size: larger/smaller a simple ratio; add stylo support;
https://reviewboard.mozilla.org/r/135674/#review139384
::: servo/components/style/properties/longhand/font.mako.rs:774
(Diff revision 2)
> }
> }
> + } else if let SpecifiedValue::Larger = *self {
> + return Some(1.2)
> + } else if let SpecifiedValue::Smaller = *self {
> + return Some(1./1.2)
(Just two drive-by comments, feel free to not address them if you don't feel the same way)
First, this could use a `match` expression, which could perhaps be a bit more idiomatic.
Second, it'd be nice if the `1.2` would be moved to a constant with a descriptive name, and perhaps some rationale of why it was chosen.
Comment 13•8 years ago
|
||
Pushed by manishearth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/60c719808cf4
Make font-size: larger/smaller a simple ratio; add stylo support; r=dbaron
Assignee | ||
Comment 14•8 years ago
|
||
Addressed in the servo side, at https://github.com/servo/servo/pull/16730
(Landing separately since they're pretty independent)
Comment 15•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee | ||
Updated•7 years ago
|
Attachment #8863933 -
Attachment mime type: text/plain → text/html
You need to log in
before you can comment on or make changes to this bug.
Description
•