1341637 - stylo: need to support "font-family: -moz-fixed" in Servo's CSS parser or mapping back to Gecko values

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

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.

Comment 1

[Simon Sapin (:SimonSapin)]

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

Comment 2

[Boris Zbarsky [:bzbarsky]]

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

[Simon Sapin (:SimonSapin)]

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.)

Comment 4

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1341637 - Stylo: Add support for "font-family: -moz-fixed"

Review commit: https://reviewboard.mozilla.org/r/119404/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/119404/

Comment 5

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

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

[Manish Goregaokar [:manishearth]]

Comment on attachment 8846346 [details]
Bug 1341637 - Stylo: Add support for "font-family: -moz-fixed"

https://reviewboard.mozilla.org/r/119404/#review121278

Comment 7

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Comment on attachment 8846346 [details]
Bug 1341637 - Stylo: Add support for "font-family: -moz-fixed"

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/119404/diff/1-2/

Comment 8

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Split Servo side to: https://github.com/servo/servo/pull/15916

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8364027e1ae5
Stylo: Add support for "font-family: -moz-fixed" r=manishearth

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8364027e1ae5

Comment 11

[Boris Zbarsky [:bzbarsky]]

> 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?

Comment 12

[Boris Zbarsky [:bzbarsky]]

David, do you recall how exactly -moz-fixed is supposed to behave?

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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...)

Comment 14

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

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 :/