The default bug view has changed. See this FAQ.

stylo: @font-face support

ASSIGNED
Assigned to

Status

()

Core
CSS Parsing and Computation
P1
normal
ASSIGNED
8 months ago
6 days ago

People

(Reporter: bholley, Assigned: jfkthame)

Tracking

(Blocks: 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

Support for downloadable fonts is another pieces of medium-large property integration work for stylo.
Chris, can you track this and bug 1290228?
Flags: needinfo?(cpeterson)
OK
Flags: needinfo?(cpeterson)
Priority: -- → P3
Whiteboard: [stylo:m3]
Blocks: 1328507
Integration of @font-face may not be very complicated. It seems to me except parsing, other part of style system in Gecko basically depends on the CSSOM interface of font-face rule. That says, as far as we add a CSSFontFaceRule impl backed by Servo and returns the requested nsCSSValue like what current Gecko impl does, then everything should just work.

A challenge is that in Rust we generally don't preserve strings, so we may need to re-serialize values of descriptors to feed Gecko. But I guess that may not be a big issue as I don't think that would usually be on a hot path.

It seems the most complicated descriptor font-variant is not supported in Gecko at the moment, fortunately :)
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #3)
> Integration of @font-face may not be very complicated. It seems to me except
> parsing, other part of style system in Gecko basically depends on the CSSOM
> interface of font-face rule. That says, as far as we add a CSSFontFaceRule
> impl backed by Servo and returns the requested nsCSSValue like what current
> Gecko impl does, then everything should just work.

That's great news!

> A challenge is that in Rust we generally don't preserve strings, so we may
> need to re-serialize values of descriptors to feed Gecko. But I guess that
> may not be a big issue as I don't think that would usually be on a hot path.

Yeah sounds good.
 
> It seems the most complicated descriptor font-variant is not supported in
> Gecko at the moment, fortunately :)

:-)

So I guess the next step here is to implement the CSSOM bits - do we have a bug on file that we can block this on?
Also, I think this bug can probably have a higher priority, because I believe quite a number of reftests depend on @font-face due to wide usage of testing fonts, especially Ahem.

Probably Simon can help implementing the parser part of @font-face in Servo?
Flags: needinfo?(simon.sapin)
Servo already supports @font-face, though currently only with the 'src' and 'font-family' descriptors. I can work on adding parsing and serialization for other descriptors. Although since there is no specified/computed value split like for properties, maybe we want to parse directly to FFI types (nsCSSValue?) rather than Rust/Servo types?
Flags: needinfo?(simon.sapin)
Note to myself: http://searchfox.org/mozilla-central/rev/225ab0637ed51b8b3f9f4ee2f9c339a37a65b626/layout/style/nsCSSParser.cpp#12188
(In reply to Simon Sapin (:SimonSapin) from comment #6)
> Servo already supports @font-face, though currently only with the 'src' and
> 'font-family' descriptors. I can work on adding parsing and serialization
> for other descriptors. Although since there is no specified/computed value
> split like for properties, maybe we want to parse directly to FFI types
> (nsCSSValue?) rather than Rust/Servo types?

If that is not too hard for Servo side, I think it would be very helpful.

It seems to me the current binding functions is not enough for that. It seems we need the additional functions to set string value, normal value, pair list value, url value, and local font value to nsCSSValue.

Ah, this makes me a bit hesitant. Probably we should make bindgen generate methods for us rather than adding so many binding functions :/ (But method generating requires build-time bindgen to be enabled first, because otherwise MSVC has different mangled name for methods.)
Blocks: 1321197
https://github.com/servo/servo/pull/15356 add support for more descriptors inside @font-face. I’ll add conversions to nsCSSValue separately since that requires new C++ functions.

Compared to Gecko’s CSSParserImpl::ParseFontDescriptorValue:

* eCSSFontDesc_FontLanguageOverride and eCSSFontDesc_Display are in Gecko but not in the spec. I don’t know what they are, but if they’re needed in stylo I can reverse-engineer Gecko to find out, and implement equivalent parsing and serialization.

* font-variant is supported inside @font-face in the spec but not in Gecko. Servo currently only supports "normal | small-caps" out of the many values specified in CSS Fonts 3. Also in level 3 the font-variant property became a shorthand, so for @font-face we’ll likely want a Rust type in font_face.rs that holds values of all of the corresponding longhands.

* The font-feature-settings property already has parsing and serialization code in Servo, but with products="none". Is it because the bindings are not there yet? Once that’s done, adding support for it in @font-face should be a matter of a couple new lines in a macro.


[1] http://searchfox.org/mozilla-central/rev/d20e4431d0cf40311afa797868bc5c58c54790a2/layout/style/nsCSSParser.cpp#12186
I'm not sure if it has made it into an official spec yet, but font-display is defined here: https://tabatkins.github.io/specs/css-font-display/#font-display-desc

font-language-override is defined here, but is marked as at risk: https://drafts.csswg.org/css-fonts-3/#propdef-font-language-override
(In reply to Cameron McCormack (:heycam) from comment #10)
> font-language-override is defined here, but is marked as at risk:
> https://drafts.csswg.org/css-fonts-3/#propdef-font-language-override

It’s a property (that applies to elements) though.

Jonathan, looks like you added support for font-language-override as an @font-face descriptor in bug 511339. How does it interact with the property? Could you write to CSSWG about specifying that?
Flags: needinfo?(jfkthame)
(Assignee)

Comment 12

2 months ago
Huh, I thought it was specified alongside font-feature-settings, but apparently it's not there.

It was always intended to parallel font-feature-settings, which exists as both a property on elements and a descriptor in @font-face rules, with the descriptor providing a default that the property can override.

I'll raise it with the WG...
Flags: needinfo?(jfkthame)
Depends on: 1340728
Shing mentioned that fonts were a major reason why a lot of sites don't work.

Simon, how much work is left on the servo side? Are we at the point where we just need to do the work in comment 3?
Flags: needinfo?(simon.sapin)
Priority: P3 → P1
No longer blocks: 1243581
Whiteboard: [stylo:m3]
As discussed over video I added parsing and serialization code for @font-face descriptors, except a couple per Comment 9. Nazım added some nsCSSValue constructors in Bug 1340728. What’s left is converting the Rust types for descriptor values to nsCSSValue (maybe implementation of the`std::ops::From` trait?), glue to call this from Gecko, and integration with Gecko’s font subsystem.
Flags: needinfo?(simon.sapin)
Assignee: nobody → simon.sapin
Status: NEW → ASSIGNED
Jonathan said he'd have a look here - thanks Jonathan!
Assignee: simon.sapin → jfkthame
You need to log in before you can comment on or make changes to this bug.