[css-fonts] CSSFontFaceRule does not seem Web compatible

[svgeesus]

CSSFontFaceRule has a style property, introduced in DOM Level 2 Style.

DOM 2 Style says style property is of type CSSStyleDeclaration
https://www.w3.org/TR/2000/REC-DOM-Level-2-Style-20001113/css.html#CSS-CSSFontFaceRule
https://www.w3.org/TR/2000/REC-DOM-Level-2-Style-20001113/css.html#CSS-CSSStyleDeclaration

while CSS3 Fonts says style property is of type DOMString.
https://drafts.csswg.org/css-fonts/Fonts.html#om-fontface

Informal testing indicates that in browsers (tested Firefox 51, Chrome 57, Edge 14) the style property is an object, per Dom 2 Style. It isn't a DOMString and it is not clear how Web compatible it would be to change the type of the property.

Quick test http://codepen.io/svgeesus/pen/ZBZbbv

[svgeesus]

(That test has a separate issue where family == 'sample' works on Chrome and Edge, but family == '"sample"' is needed on Firefox, with double quoting).

[svgeesus]

So, is this just a copy paste error in the Fonts 3 spec? If style was CSSStyleDeclaration while family, src, etc were DOMString that would make sense to me, the old one for backwards compat and the new ones for convenience.

[tabatkins]

This was no error, it was an intentional "let's fix this better" effort by jdaggett years ago that never panned out. Old 2.1 spec gave CSSFontFaceRule a .style property, which worked identically to style rules, but jdaggett tried to fix it to just put the valid descriptors directly on the CSSFontFaceRule, like we do with other at-rules.

However, no browser ever picked up the change, including Firefox, which John was nominally in charge of (for the relevant section). :/ This needs to be reverted to match reality, unless browsers actually think they can change their implementation. (I'm pretty sure Chrome's position is that we probably can't.)

[upsuper]

I still hope to push forward this change. Could we keep a style attribute inside CSSFontFaceRule which points to itself, so that we don't use CSSStyleDeclaration which includes tons of unrelated properties?

The currently state doesn't make much sense to me.

[litherum]

We definitely can't break existing code, which treats it as a CSSStyleDeclaration (using cssText and getPropertyValue()). Given this constraint, I'm not sure that we can do this.

[SimonSapin]

We probably need to keep attribute CSSStyleDeclaration style for web-compat. Given that, is it still worth adding new per-descriptor attributes? Either way the css-fonts spec needs changes.

[upsuper]

We definitely can't break existing code, which treats it as a CSSStyleDeclaration (using cssText and getPropertyValue()). Given this constraint, I'm not sure that we can do this.

Ah, cssText... probably worth adding a FontFaceDeclaration...

[SimonSapin]

@upsuper Do you mean something that has all the same methods and attributes as CSSStyleDeclaration, but isn’t CSSStyleDeclaration? Why is that preferable to CSSStyleDeclaration, which is already reused for very different things? (E.g. Element.style vs getComputedStyle())

[upsuper]

Because CSSStyleDeclaration, per the current spec, contains all other unrelated properties as attributes, which is undesirable in this case.

WebKit and Blink have already been using the new definition for CSSStyleDeclaration, but Gecko still splits CSSStyleDeclaration and CSS2Properties (the latter was the one defined in some early version of spec which includes attributes of properties and inherits from CSSStyleDeclaration).

[foolip]

I just filed https://bugs.chromium.org/p/chromium/issues/detail?id=709013 for addressing this mismatch between Blink and the spec. I mentioned in the descriptions that all engines still seem to match the old spec, and also that we have a use counter:
https://www.chromestatus.com/metrics/feature/timeline/popularity/1082

Preserving the style attribute would be the safest thing unless somebody is willing to investigate how code in the wild is impacted.

[svgeesus]

Preserving the style attribute would be the safest thing unless somebody is willing to investigate how code in the wild is impacted.

Agreed; there seems to be consensus on that point.
I'm not seeing agreement on adding new per-descriptor attributes?

If we wanted to do that, the feature should be moved to css4-fonts. If we don't, is it sufficient to drop the feature and leave it to DOM 2 Style?

[foolip]

https://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSSFontFaceRule is not maintained and I'd consider it a bug if that were the only spec defining CSSFontFaceRule.

[litherum]

If we’re keeping the .style attribute, then there's no reason to add descriptor attributes. Indeed, redundancy should be eliminated.

[svgeesus]

Why isn't this a part of CSS OM? @zcorpan
https://www.w3.org/TR/cssom-1/

Okay, https://drafts.csswg.org/cssom/ says

CSSFontFaceRule
Issue: ...

related, #728

[SimonSapin]

I think that the CSSOM spec defines interfaces for rules in CSS 2, and later specs that introduce new at-rules define their own OM. For example: https://drafts.csswg.org/css-conditional/#apis

[svgeesus]

Which is fine too, I just wanted some clarity on

1. What should be specified, and
2. Where

[SimonSapin]

I think that the conclusion from this thread is that css-fonts should specify a CSSFontFaceRule interface similar to DOM Style Level 2, with no new attribute added. (Except probably with [SameObject, PutForwards=cssText] added, like CSSStyleRule.)

[litherum]

I think we need a resolution before I can do anything here. Agenda+.

[dbaron]

So Gecko doesn't implement the merging of CSS2Properties into CSSStyleDecleration, and the declaration block for @font-face rules doesn't implement CSS2Properties. See testcase. So I think it's reasonable to think that putting all of CSS2Properties into whatever represents the declaration-block for @font-face isn't needed for Web compatibility.

[dbaron]

(And I think the issue of not wanting all the bogus properties on CSSFontFaceRule.style was one of the reasons we haven't done that merge.)

[svgeesus]

I added a few CSS Fonts 3 tests, including @font-face and @font-feature-settings to WPT. The @font-face test is just that the interface exists, the properties are not tested.
web-platform-tests/wpt#6586
@litherum waiting for your review

Happy to add further tests for CSS Fonts 4 in that area, once we agree on this issue.

[tabatkins]

@dbaron I'd be okay with a result that we unmerge the CSS2 properties from the CSSStyleDeclaration in general, and reuse that. It still means that FontFace and CSSFontFaceRule would have different shapes, but oh well, it's shipped and we're stuck with it.

I just really, really want the current "pretend all the descriptors are directly on the interface" specification to either go away or become reality.

[css-meeting-bot]

The CSS Working Group just discussed CSSFontFaceRule does not seem Web compatible.

The full IRC log of that discussion

<dael> Topic: CSSFontFaceRule does not seem Web compatible
<dael> github: https://github.com//issues/825
<dael> Rossen_: This was added by ChrisL.
<myles> https://www.w3.org/TR/2011/WD-cssom-20110712/#cssfontfacerule
<dael> myles: This is aboutwhat happens in CSSOM if you pull out a rule that represents a FontFaceRule. There's an old spec that says this only has one rule that is a style declaration.
<myles> https://drafts.csswg.org/css-fonts-4/#cssfontfacerule0
<dael> myles: This was updated to ^ which gets rid of the one style attribute and replaces with a bunch of strings.
<dael> myles: Before you would get your rule in JS and say .style.getPropertyValue and in the new rule you say .faimly to get the string.
<dael> myles: Every browser impl the old. There have been proposals of what to do. 1) revert, 2) have both 3) make a .style but have it be a new custom object with only some stuff and not all the g enerality of the style declaration.
<dael> myles: This came up b/c I started making edits and I don't know what to do.
<dael> myles: My opinion is since browsers are standardized that's what should go in spec. I imagine there are other thoughts.
<tantek> +1
<dael> Rossen_: Most people engaged in this aren't on the call today. ChrisL, dbaron, TabAtkins are all not on the call.
<dael> myles: We can push. I'll give the speech again next week.
<dael> Rossen_: I'm sympathetic with your proposed opinion to standardize on one behavior. But I'd also like to hear from others that were in this conversation.
<dael> myles: That's fine.

[css-meeting-bot]

The CSS Working Group just discussed CSSFontFaceRule isn't web-compatible, and agreed to the following resolutions:

• RESOLVED: rollback to previous state with CSSStyleDeclaration and umerged interfaces

The full IRC log of that discussion

<dbaron> Topic: CSSFontFaceRule isn't web-compatible
<myles_> https://github.com//issues/825
<astearns> github: https://github.com//issues/825
<fremy> myles_: cssom for the font-face rule
<fremy> myles_: used to be CSSStyleDeclaration with the declarations inside
<fremy> myles_: rule.style.getPropertyValue("font-family")
<fremy> myles_: the spec was then changed
<fremy> myles_: instead of having a style, you would have a bunch of strings
<fremy> myles_: e.g. rule.family
<fremy> myles_: no browser has made this change
<fremy> myles_: there is existing code that use the old way
<fremy> myles_: we don't want to break that code
<fremy> myles_: option 1: rollback to old spec text
<fremy> myles_: option 2: get browsers to support the new spec text
<fremy> myles_: option 3: get browsers to support both
<fremy> myles_: option 4: make the "style" property return new type of object that looks like CSSStyleDeclaration but is simpler
<fremy> dbaron: in the old domstyle spec, cssstyledeclaration was simple
<fremy> dbaron: the weird stuff was in css2properties
<fremy> dbaron: so you could implement both, and most things did
<fantasai> s/css2properties/css2properties, which had a property for every property in css
<fremy> dbaron: that was then changed, and the two were merged
<fremy> dbaron: so now the merge made it more difficult to implement
<fremy> dbaron: we never implemented this merge
<fremy> dbaron: (in gecko)
<fremy> dbaron: my preferred proposal would then to be unmerge things
<fantasai> dbaron: our implementation doesn't include all the other stuff in font face rules, just the 6 original get/set methods
<fantasai> (that goes up a few lines)
<fremy> dbaron: (and rollback to use cssstyledeclaration)
<fremy> myles_: we still need to explain what happens when we set properties that do not exist in @font-face but do in general style
<fremy> myles_: but I don't have a strong opinion in either ways
<fremy> myles_: but I would rather rollback the spec, and maybe we can refine after
<fremy> alan: so, instead of coming up with a perfect design, we rollback the improve iteratively
<fremy> TabAtkins: we have copied the design in font-face
<fremy> TabAtkins: and we have "style" property there
<fremy> TabAtkins: so we cannot be consistent and keep style as CSSStyleDeclaration
<fremy> myles_: life is terrible, who names these things ;)
<fremy> dbaron: if other implementations are willing to remove css2props stuff, then it leaves less questions to be answered
<fremy> myles_: i dont think we ever implemented these two interfaces
<fremy> alan: any other opinion?
<fremy> TabAtkins: i suspect if gecko is doing it, i guess we could do it
<fremy> myles_: merge allows rule.style["font-family"] and unmerge does not
<fremy> dbaron: yes
<fremy> myles_: webkit does not support this either
<dbaron> s/rule.style["font-family"]/rule.style.fontFamily/
<gsnedders> I thought both of those were supported interoperably?
<fremy> myles_: can we then try to resolve on rolling back to the unmerged version?
<fremy> no objection
<dbaron> gsnedders: rule is CSSFontFaceRule
<fremy> RESOLVED: rollback to previous state with CSSStyleDeclaration and umerged interfaces
<myles_> https://github.com//issues/1349

[astearns]

Some discussion after this noted that we'd probably need to re-introduce the style property into the fonts-3 interface.

[tabatkins]

Yeah, in more detail the edits are:

1. Reduce CSSFontFaceRule to just interface CSSFontFaceRule : CSSRule { attribute CSSStyleDeclaration style; };
2. Remove _camel_cased_attribute from CSSStyleDeclaration.
3. Create another interface with _camel_cased_attribute, somehow mix it into CSSStyleRule? The DOM2Style spec was extremely hand-wavy about this worked.

[litherum]

CSSStyleDeclaration is defined in another spec, so another issue should be raised for it. Do you know where to file it, @tabatkins?

[tabatkins]

Yeah, file it against CSSOM. The (1) in my list is still against the Fonts spec, tho, so this should probably be open.

[litherum]

15de56b completes (1), right?

[tabatkins]

Yeah.

[litherum]

#1692