Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cssom] The way CSSStyleDeclaration exposes properties is not ideal. #5649

Open
emilio opened this issue Oct 22, 2020 · 21 comments
Open

[cssom] The way CSSStyleDeclaration exposes properties is not ideal. #5649

emilio opened this issue Oct 22, 2020 · 21 comments
Labels

Comments

@emilio
Copy link
Collaborator

emilio commented Oct 22, 2020

Right now we only have a single interface that exposes the CSS property names as attributes (CSSStyleDeclaration), but no browser does it right, and the spec behavior is also not great.

In particular, the spec behavior means that you expose stuff like fontFaceRule.style.color, which doesn't quite make sense.

WebKit and Blink (though WebKit may have changed recently) historically use named getters for this (which is also not great, see #1089).

Note also that:

  • Per recent discussion with @gsnedders WebKit trunk may have changed behavior?
  • Per spec there's no exposure of descriptors for the rules that have them (but Blink / WebKit expose them as part of the named getters).

Gecko does something which IMO is better. Gecko exposes them as attributes as the spec describes, but on an interface that derives from CSSStyleDeclaration called CSS2Properties which we use for all the "style" declarations (so, inline style declarations, CSSStyleRule.style, getComputedStyle, CSSPageRule.style, and individual keyframe declarations). (link, link). I think something like what Gecko does, but with similar interfaces for the declaration blocks that should have descriptors, like @font-face and @counter-style, would generally be much saner.

Since there's movement around here, and the interop story here is quite sad, I think there's quite a fair amount of room for improvement, and if we can align in a model that is both consistent and useful for developers it'd be great.

Does the thing I described above seem reasonable? Other ideas?

cc @gsnedders @foolip @andruud @xiaochengh @heycam

@emilio emilio added the cssom-1 label Oct 22, 2020
@gsnedders
Copy link
Contributor

Per recent discussion with @gsnedders WebKit trunk may have changed behavior?

WebKit trunk basically follows the CSSOM spec here, with each property (and descriptor, because we didn't stop exposing them) a separate IDL attribute rather than using named getters. The old named getter had some bugs that led to some "interesting" behaviour hence that change.

@emilio
Copy link
Collaborator Author

emilio commented Oct 22, 2020

I could probably live with that too, if people think it's fine to expose something like 'src' in document.body.style.

@emilio
Copy link
Collaborator Author

emilio commented Oct 22, 2020

(Though not a terrible fan of it it might be the path of least resistance...)

I assume that for stuff that doesn't apply you just always return the empty string?

@gsnedders
Copy link
Contributor

I don't think we have any opposition to changing behaviour for descriptors; it was just the simple change to move away from the named getter (and also effectively the minimal change).

@andruud
Copy link
Member

andruud commented Oct 23, 2020

@emilio Your proposal sounds good to me. Having to expose src and other descriptors on .style (etc) feels like a design failure, so it's a bit annoying. (Though maybe not a huge problem for authors in practice).

@tabatkins
Copy link
Member

Yes, I've been wanting to improve this for a long time; the current situation is ridiculous.

Having child interfaces for properties and for each at-rule that contains only the properties/descriptors relevant to the context would be great.

@LeaVerou
Copy link
Member

LeaVerou commented Nov 3, 2020

Just a note to consider introspection for whatever is decided, which is immensely useful for tooling. The situation right now is far from ideal.

@argyleink
Copy link
Contributor

issues related to this were the hardest part of getting VisBug to work across browser

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [cssom] The way CSSStyleDeclaration exposes properties is not ideal, and agreed to the following:

  • RESOLVED: Having different interfaces for declarations blocks that expose a different set of descriptors/properties
The full IRC log of that discussion <dael> Topic: [cssom] The way CSSStyleDeclaration exposes properties is not ideal
<dael> github: https://github.com//issues/5649
<dael> emilio: It's a mess everywhere. I made a proposal. Seemed to be agreement to try and go for it
<dael> emilio: Not in love with...need to decide on names. I'm fine deciding the names on GH or another call
<dael> emilio: Idea is to have different interfaces for different declarations. fontFace only has a set of descriptors. Spec is sad and doesn't match browsers. Maybe WK but it adds descriptors.
<dael> emilio: Agreement from browser engines it's a decent path forward
<dael> Rossen_: You're okay continuing discussion in GH?
<dael> emilio: It would be ideal to resolve sep interfaces for declarations that declare different things. Names tbd
<dael> TabAtkins: I would love to have this. Please have it
<dael> Rossen_: Do you have prop written up? Is it a PR or anything more?
<dael> emilio: I can summarize in IRC
<dael> fantasai: Go ahead
<dael> Rossen_: With a couple minutes remaining I doubt we'd get to anything other than this
<emilio> Having different interfaces for declarations that expose a different set of descriptors/properties
<emilio> declaration blocks* I guess, naming in the spec is wrong
<dael> fantasai: I think it's good for each to have different set. Exposing declarations that don't apply is confusing and may have future problems
<dael> Rossen_: I'm hearing support
<dael> Rossen_: We can resolve to adopt the proposal and you can come back with more concrete interfaces. Does that work?
<dael> emilio: Sounds great
<dael> Rossen_: Objections to Having different interfaces for declarations blocks that expose a different set of descriptors/properties
<dael> RESOLVED: Having different interfaces for declarations blocks that expose a different set of descriptors/properties

@emilio
Copy link
Collaborator Author

emilio commented Dec 10, 2020

So, here's a proposal. All of the interfaces below would extend CSSStyleDeclaration and thus have a getPropertyValue / setProperty / cssText / length / item / etc...:

  • CSSStyleProperties (CSS2Properties) for style declarations: inline style / getComputedStyle / CSSStyleRule.style. This is basically the current CSSStyleDeclaration. I like CSSStyleProperties a bit more, because it's more consistent, but there's precedent for CSS2Properties, and that's what Gecko calls it, so it's a bit more compatible.
  • CSSPageProperties for @page rules, with only the page descriptor.
  • CSSCounterStyleProperties for @counter-style
  • CSSFontFaceProperties for @font-face

etc. Two thoughts:

  • I'd prefer to use CSS{Style,Page,...}Declarations, but I think that might be confusing with the existing CSSStyleDeclaration.
  • An alternative that doesn't use Properties would be Block (so, CSSStyleBlock / CSSPageBlock / ...), which could be nice because it avoids using Properties for something that's "descriptors"...

But this eventually comes down to naming, so... thoughts?

@emilio emilio added the Agenda+ label Dec 10, 2020
@gsnedders
Copy link
Contributor

CSSStyleProperties (CSS2Properties) for style declarations: inline style / getComputedStyle / CSSStyleRule.style. This is basically the current CSSStyleDeclaration. I like CSSStyleProperties a bit more, because it's more consistent, but there's precedent for CSS2Properties, and that's what Gecko calls it, so it's a bit more compatible.

That precedent comes from DOM Level 2 Style. From an aesthetic point-of-view I'd like to get rid of the lie that it's only CSS2, so if you wanna try renaming it in Gecko and see if it sticks I'd quite like that?

An alternative that doesn't use Properties would be Block (so, CSSStyleBlock / CSSPageBlock / ...), which could be nice because it avoids using Properties for something that's "descriptors"...

Is there any reason why we can't go for CSSStyleProperties/CSS2Properties then CSSPageDescriptors, CSSCounterStyleDescriptors, etc?

@emilio
Copy link
Collaborator Author

emilio commented Dec 11, 2020

I guess not really, that'd be fine for me.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [cssom] The way CSSStyleDeclaration exposes properties is not ideal., and agreed to the following:

  • RESOLVED: Go with emilio naming and Sam's amendment
The full IRC log of that discussion <dael> Topic: [cssom] The way CSSStyleDeclaration exposes properties is not ideal.
<dael> github: https://github.com//issues/5649
<dael> astearns: This issue is full of Europeans. Anyone can take it?
<dael> TabAtkins: I'm happy to run it since our people are okay with it
<dael> TabAtkins: emilio points out that the way css style declarations expose properties is weird. Per current cssom definitions all prop and at rules are merged into same object and they work or don't
<dael> TabAtkins: Prop to solve is split apart interfaces for style rules and each @rule that uses .style so they are distinct name spaces
<dael> TabAtkins: sgtm. Anders says seems fine because exposing all @rule on every style rule seems weird as well
<dael> TabAtkins: Sorry, we resolved on overall proposal
<dael> astearns: emilio added specifics
<dael> TabAtkins: Specific prop is add a batch of interfaces named [reads]
<fantasai> wfm
<dael> TabAtkins: Use those as extensions of css style declarations and they have appropriate descriptor names. Looking for opinions on naming.
<dael> TabAtkins: I think names are fine
<dael> astearns: Anyone else have an opinion?
<dael> fantasai: [missed] had an opinion to have everything else named descriptors. That's the counter proposal. That also works and I think slightly better
<fantasai> s/[missed]/Sam/
<dael> astearns: Prop: Go with emilio naming and Sam's ammendment
<dael> RESOLVED: Go with emilio naming and Sam's amendment

@svgeesus
Copy link
Contributor

svgeesus commented Feb 3, 2021

Is there any reason why we can't go for CSSStyleProperties/CSS2Properties then CSSPageDescriptors, CSSCounterStyleDescriptors, etc?

I guess not really, that'd be fine for me.

I'm not following what specific change I should make to CSS Fonts 4 @font-face interface for this.

@tabatkins
Copy link
Member

You'd make a CSSFontFaceDescriptors interface, containing the @font-face descriptors, and then change the CSSFontFaceRule.style attribute to be of that type (rather than CSSStyleDeclaration).

@gsnedders
Copy link
Contributor

You'd make a CSSFontFaceDescriptors interface, containing the @font-face descriptors, and then change the CSSFontFaceRule.style attribute to be of that type (rather than CSSStyleDeclaration).

Note we probably want to make https://www.w3.org/TR/cssom/#dom-cssstyledeclaration-camel-cased-attribute, https://www.w3.org/TR/cssom/#dom-cssstyledeclaration-webkit_cased_attribute, https://www.w3.org/TR/cssom/#dom-cssstyledeclaration-dashed_attribute into their own section so each of the descriptor interfaces can reference it.

@gsnedders
Copy link
Contributor

Looking at this again, can anyone make any of the exposed attributes for the at-rules which expose a CSSStyleDeclaration do… anything? If it truly is the case that "setting" a descriptor like this does nothing in WebKit/Chromium then we should probably just drop them entirely, which means we only need CSSStyleDeclaration and CSSStyleProperties.

@cdoublev
Copy link
Collaborator

I went through issues related to this one, and read the following from a meeting transcript:

jdaggett: are people actually using this functionality, that's been there since CSS 2. Last time I looked, if you went in and changed the font family, font matching didn't respond correctly.
myles_: works in Safari.
jdaggett: not in Chrome

But I guess attributes for at-rules do nothing if you say so and if I understood you correctly.

Regarding the naming that seems to be preferred for now, is CSSStyleProperties.cssText = & { color: green } supposed to work? If so, I would say that something like CSSStyleBlock[Value|Content] makes more sense to me.

@emilio
Copy link
Collaborator Author

emilio commented Dec 8, 2023

@gsnedders Setting a descriptor "works" for some rules, like @page at least.

@svgeesus
Copy link
Contributor

svgeesus commented Dec 8, 2023

Should we change the nterfaces along the same lines for @font-palette-values ?

We currently have The CSSFontFeatureValuesRule interface and The CSSFontPaletteValuesRule interface

@emilio
Copy link
Collaborator Author

emilio commented Dec 9, 2023

That's basically https://drafts.csswg.org/css-fonts-4/#om-fontpalettevalues right?

Edit: hah, I hadn't seen the edit. I don't think exposing descriptors in a declaration block is super-desirable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

11 participants