Closed
Bug 1341637
Opened 8 years ago
Closed 8 years ago
stylo: need to support "font-family: -moz-fixed" in Servo's CSS parser or mapping back to Gecko values
Categories
(Core :: CSS Parsing and Computation, defect, P1)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla55
People
(Reporter: bzbarsky, Assigned: canova)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
This is treated as a generic family name in Gecko's CSS parser. See AppendGeneric.
Without this, <pre> doesn't work right, for example, causing various reftest failures.
Note that 'font-family: -moz-fixed' and 'font-family: "-moz-fixed"' have different behavior, so this probably does need explicit parser support.
Flags: needinfo?(simon.sapin)
Comment 1•8 years ago
|
||
Servo/Stylo’s font-family parser already separates generic families which can only be specified as a keyword, from family names that can be space-separated keywords or quoted strings. We can easily add another generic keyword to the list here:
https://github.com/servo/servo/blob/469ed934e7/components/style/properties/longhand/font.mako.rs#L89-L93
Flags: needinfo?(simon.sapin)
Reporter | ||
Updated•8 years ago
|
Blocks: stylo-reftest
Reporter | ||
Comment 2•8 years ago
|
||
Right, the question is whether we're ok adding -moz-fixed to here, and whether we want it for servo as well as stylo....
Comment 3•8 years ago
|
||
Servo shouldn’t parse a CSS property or value if it doesn’t have corresponding layout (or in this case, font selection) support. For entire properties, there’s a `products="gecko"` mechanism. For properties For values, the corresponding lines of parsing/serialization code can be wrapped in :
% if product == "gecko":
// Rust code here, not included for Servo
% endif
(This file is a Mako template that emits Rust code, its syntax is documented at http://docs.makotemplates.org/en/latest/syntax.html , it’s mostly Python.)
Updated•8 years ago
|
Blocks: stylo-syntax
Updated•8 years ago
|
Assignee: nobody → canaltinova
Priority: -- → P1
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•8 years ago
|
||
Firefox serializes -moz-fixed as monospace(also treats as monospace in another parts of the code) so I did the same for this too.
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8846346 [details]
Bug 1341637 - Stylo: Add support for "font-family: -moz-fixed"
https://reviewboard.mozilla.org/r/119404/#review121278
Attachment #8846346 -
Flags: review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•8 years ago
|
||
Split Servo side to: https://github.com/servo/servo/pull/15916
Pushed by manishearth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8364027e1ae5
Stylo: Add support for "font-family: -moz-fixed" r=manishearth
Comment 10•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Reporter | ||
Comment 11•8 years ago
|
||
> Firefox serializes -moz-fixed as monospace(also treats as monospace in another parts of the code)
That's ... not quite true:
data:text/html,<div id="x" style="font-family: -moz-fixed"><pre><script>document.writeln(x.style.fontFamily);</script>
The specified value is "-moz-fixed". The computed value is ... it's complicated. There's the mGenericId, which I'm quite sure is kGenericFont_moz_fixed. But getComputedStyle() returns "monospace", which may just mean it's not properly capturing all of the computed style. I'm not sure.
And afaict, "monospace" and "-moz-fixed" can have different default sizes; see http://searchfox.org/mozilla-central/rev/a5c2b278897272497e14a8481513fee34bbc7e2c/layout/base/StaticPresData.cpp#220-223
What behavior was actually implemented here, and what was done to test that it actually matches Gecko's behavior?
Flags: needinfo?(canaltinova)
Reporter | ||
Comment 12•8 years ago
|
||
David, do you recall how exactly -moz-fixed is supposed to behave?
Flags: needinfo?(dbaron)
I believe -moz-fixed is mostly equivalent to monospace except that it triggers slightly magical font-size behavior in order to support the separate font-size preferences for monospace fonts. This was originally implement in bug 99010, changeset https://github.com/mozilla/gecko-dev/commit/7f1638d03979d29b54b11eb702c8e4e5cf1e1e25 . (Related: bug 380915.) Then font-size is computed according to relatively complicated rules in nsRuleNode. I believe the basic idea is that when we've inherited a font-size from the initial value of font-size, we effectively inherit a pair of values: one for when the font-family list's generic is -moz-fixed, and one for when it's not. However, the current code, rather than storing the pair, stores only a single value, but when we switch between the two categories, we do a hypothetical recomputation of font-size going back up the tree to the last matching generic. I don't particularly remember the details, but they're present in nsRuleNode::ComputeFontData, nsRuleNode::SetGenericFont, nsRuleNode::SetFont, and perhaps also nsRuleNode::SetFontSize. (Though I think the nsRuleNode part of the code is general enough that it supports separate font size preferences for every generic, which is actually rather silly...)
Flags: needinfo?(dbaron)
Updated•7 years ago
|
Assignee | ||
Comment 14•7 years ago
|
||
Sorry, it's been a while. We've talked this with Boris on IRC in that time actually and I was keeping this ni as a reminder to come back. The font size issue seems to be fixed by Manish but we still have an issue in the specified/computed style serialization parts. Currently specified value is printing as monospace and computed value is printing -moz-fixed in stylo. It should be the other way around. I can fix the specified value serialization but I still can't determine in which part gecko converts -moz-fixed to monospace :/
Flags: needinfo?(canaltinova)
You need to log in
before you can comment on or make changes to this bug.
Description
•