The default bug view has changed. See this FAQ.

stylo: Implement access to CSSFontFaceRule

NEW
Assigned to

Status

()

Core
CSS Parsing and Computation
P1
normal
14 days ago
7 hours ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

(Blocks: 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

Comment hidden (empty)
Priority: -- → P2
(Assignee)

Updated

14 days ago
Blocks: 1324701
Priority: P2 → P1
Xidorn - Jonathan is going to be looking at the @font-face stuff in bug 1281962. IIUC he'll need the CSSOM bits in this bug as well, though they're otherwise mostly-separate tasks. Does that sound right?

If so, are the CSSOM bits something you can knock out without much trouble? Assuming Jonathan needs it to test is stuff in bug 1281962, it's probably worth doing this on the sooner side.
Assignee: nobody → xidorn+moz
Flags: needinfo?(xidorn+moz)
(Assignee)

Comment 2

6 days ago
I believe you mean bug 1290237.

From my previous investigation (bug 1290237 comment 3), it seems to me our internal usage of @font-face basically depends on CSSOM interfaces of CSSFontFaceRule, so I would say bug 1290237 is going to be completely blocked by this bug, and maybe I can do all of them in this bug together, since I don't think there are much remaining work after implementing the CSSOM bits.
Flags: needinfo?(xidorn+moz)
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #2)
> I believe you mean bug 1290237.

Sorry, clipboard fail. Too many bugs flying around.
 
> From my previous investigation (bug 1290237 comment 3), it seems to me our
> internal usage of @font-face basically depends on CSSOM interfaces of
> CSSFontFaceRule, so I would say bug 1290237 is going to be completely
> blocked by this bug, and maybe I can do all of them in this bug together,
> since I don't think there are much remaining work after implementing the
> CSSOM bits.

Ok sounds good. Let's have you implement the CSSOM bits, and then if there's any non-trivial amount of followup work we can hand that off the Jonathan.
(Assignee)

Comment 4

17 hours ago
It seems to me that @font-face rules are mainly used by Gecko side, and Servo side doesn't really need that information, so I'm considering a different approach for integrating it.

I want to have this rule backed by Gecko class in Servo side. That says, I'd like to have Servo code fill the nsCSSValues of Gecko's nsCSSFontFaceRule directly, and make Servo's FontFaceRule just hold a pointer to the corresponding Gecko rule object (and own that object).

This is quite different from our previous approach that data is hold in Servo side, and Gecko class is just a thin wrapper of Servo object to C++.

Do you think there would be any problem with this approach?

The main problem to be solved is that, we need to support collecting all effective @font-face rules, and mapping them to FontFace object if existed when flushing user font set. And flushing user font set could happen quite frequently (although I suspect whether it really needs to, which might be something optimizable in Gecko side).

For supporting that, we have several choices:
(1) traverse the Gecko CSSOM tree for collecting
(2) traverse by Servo code, and we create C++ wrapper for them when collecting
(3) traverse by Servo code, and we make FontFace object able to be backed by Servo rule object
(4) hold data in C++ object, traverse by Servo code, and Servo returns C++ objects directly

Choice (3) is probably doable, but I'm a bit concerned about the performance consequence of getting descriptor value, especially font-family (which is a string) and src (which is a list of url / string). Generating string from Servo to Gecko is especially inefficient as we've seen in properties.

Choice (2) would be worse than (3) because it adds the extra cost to create wrapper object for every flushing.

Choice (1) is probably the worst that we need to duplicate the logic of stylesheet::effective_rules in C++ code, and we need to expand all group rules in CSSOM tree. Also it seems we don't have a fast way to check whether a media rule is active outside cascading at all.

So I think (4) would be the best choice, and may require the least code change in Gecko side since we can just reuse all the infrastructure based on the existing nsCSSFontFaceRule.

The only problem is that it is quite different than what we have been doing, so I want to confirm if there is any concern about this approach.
Flags: needinfo?(simon.sapin)
Flags: needinfo?(bobbyholley)
Call "parse_font_face_block() -> Result<FontFaceRule, ()>", then instead of wrapping into Arc<Locked<FontFaceRule>> convert to nsCSSFontFaceRule at parse time and keep a gecko reference-counted pointer in CssRule? Sounds good.

I suppose CSSOM implementation for CSSFontFaceRule would be almost entirely on the Gecko side. Stylo can provide function for parsing values of individual descriptors, to avoid duplicating parsing code between CSSFontFaceRule setters and stylesheet parsing.
Flags: needinfo?(simon.sapin)
Sounds fine to me!
Flags: needinfo?(bobbyholley)
You need to log in before you can comment on or make changes to this bug.