[cssom] [css-fonts] [css-page] Sort out properties and descriptors. #9686

emilio · 2023-12-08T12:32:49Z

This is intended to address #5649.

emilio · 2023-12-08T12:34:52Z

@tabatkins: Bikeshed doesn't seem to be liking the dashed attribute syntax, even though I think per the WebIDL grammar is correct. E.g., this diff on top of this PR:

diff --git a/cssom-1/Overview.bs b/cssom-1/Overview.bs
index a2ac5e91e..fdefb5521 100644
--- a/cssom-1/Overview.bs
+++ b/cssom-1/Overview.bs
@@ -2120,7 +2120,7 @@ Issue: Need to define the rules for
 [Exposed=Window]
 interface CSSPageDescriptors : CSSStyleDeclaration {
   attribute [LegacyNullToEmptyString] CSSOMString margin;
   attribute [LegacyNullToEmptyString] CSSOMString marginTop;
+  attribute [LegacyNullToEmptyString] CSSOMString margin-top;
   attribute [LegacyNullToEmptyString] CSSOMString marginRight;
   attribute [LegacyNullToEmptyString] CSSOMString marginBottom;
   attribute [LegacyNullToEmptyString] CSSOMString marginLeft;

Causes:

FATAL ERROR: IDL SYNTAX ERROR LINE: 4 - expected ";" skipped: "-top"
 ✘  Did not generate, due to errors exceeding the allowed error level.

Am I missing something?

svgeesus

Overall much cleaner than the current mess.

See comment about the new descriptors in CSS Fonts 5.

css-fonts-4/Overview.bs

svgeesus · 2023-12-08T14:19:22Z

css-fonts-4/Overview.bs

+	attribute [LegacyNullToEmptyString] CSSOMString ascentOverride;
+	attribute [LegacyNullToEmptyString] CSSOMString descentOverride;
+	attribute [LegacyNullToEmptyString] CSSOMString lineGapOverride;
+};


Should fonts 5 have the same interface too except adding the size-adjust descriptor and the subscript-* descriptors as well?

cdoublev · 2023-12-08T15:08:24Z

css-animations-1/Overview.bs

@@ -1052,7 +1052,7 @@ IDL Definition</h4>
 	[Exposed=Window]
 	interface CSSKeyframeRule : CSSRule {
 	  attribute CSSOMString keyText;
-	  [SameObject, PutForwards=cssText] readonly attribute CSSStyleDeclaration style;
+	  [SameObject, PutForwards=cssText] readonly attribute CSSStyleProperties style;


Maybe the style of keyframe rules should have its own interface to prevent declaring a property defined in CSS Animations?

The <declaration-list> inside of <keyframe-block> accepts any CSS property except those defined in this specification

https://drafts.csswg.org/css-animations-1/#at-ruledef-keyframes

Possibly...

tabatkins · 2023-12-08T21:04:22Z

Bikeshed doesn't seem to be liking the dashed attribute syntax,

Huh, weird that it's valid IDL, but you're right, technically it is, per https://webidl.spec.whatwg.org/#prod-identifier

This would be a bug on plinss/widlparser, then.

This blocks w3c/csswg-drafts#9686

emilio · 2023-12-11T11:18:50Z

This would be a bug on plinss/widlparser, then.

Thanks, sent plinss/widlparser#83 for that.

* Ignore virtualenv directory. * tokenizer: Fix identifier regular expression. This blocks w3c/csswg-drafts#9686 * test: Add tests for dashed identifiers.

cdoublev · 2024-02-07T08:08:45Z

css-fonts-4/Overview.bs

+	attribute [LegacyNullToEmptyString] CSSOMString fontFamily;
+	attribute [LegacyNullToEmptyString] CSSOMString fontStyle;
+	attribute [LegacyNullToEmptyString] CSSOMString fontWeight;
+	attribute [LegacyNullToEmptyString] CSSOMString fontStretch;


font-stretch (descriptor) is now a legacy alias of font-width (b0660ee). I guess fontWidth should also be defined.

svgeesus · 2024-02-23T00:50:18Z

@emilio @tabatkins Now that plinss/widlparser#83 has been merged is this ready to go, or does it still need changes?

svgeesus

It would be easier to merge this and then iterate on any refinements

cssom-1/Overview.bs

gsnedders · 2024-02-23T13:11:36Z

cssom-1/Overview.bs

+[Exposed=Window]
+interface CSSPageDescriptors : CSSStyleDeclaration {
+  attribute [LegacyNullToEmptyString] CSSOMString margin;
+  attribute [LegacyNullToEmptyString] CSSOMString marginTop;
+  attribute [LegacyNullToEmptyString] CSSOMString marginRight;
+  attribute [LegacyNullToEmptyString] CSSOMString marginBottom;
+  attribute [LegacyNullToEmptyString] CSSOMString marginLeft;
+  attribute [LegacyNullToEmptyString] CSSOMString size;
+  attribute [LegacyNullToEmptyString] CSSOMString marks;
+  attribute [LegacyNullToEmptyString] CSSOMString bleed;
+};


There's a part of me that wonders if we actually what to define this, rather than "all descriptors supported for the @page at-rule", similar to what we do with properties? (Also CSSFontFaceDescriptors above.)

Like, if an implementation supported legacyDescriptor, do we not want it exposed on the interface? Or even a -webkit-legacy-descriptor, do we not want that exposed similarly?

This is intended to address #5649.

emilio · 2024-03-20T10:32:26Z

@tabatkins @svgeesus I updated now that the webidl parser is fixed, and it looks good from here. I should look into @gsnedders' suggestion about making descriptors more generically exposed in page and font-face descriptors, but I agree we can probably merge this and iterate on the specific text, because I don't know if legacy descriptors like -webkit-foo (is there any at all?) would be exposed using the WebKitFoo syntax or something else. Does that sound good?

emilio requested review from gsnedders, tabatkins and fantasai December 8, 2023 12:33

emilio requested a review from svgeesus December 8, 2023 12:52

svgeesus reviewed Dec 8, 2023

View reviewed changes

cdoublev reviewed Dec 8, 2023

View reviewed changes

emilio added a commit to emilio/widlparser that referenced this pull request Dec 11, 2023

tokenizer: Fix identifier regular expression.

eeadd49

This blocks w3c/csswg-drafts#9686

emilio mentioned this pull request Dec 11, 2023

tokenizer: Fix identifier regular expression. plinss/widlparser#83

Merged

cdoublev reviewed Feb 7, 2024

View reviewed changes

cdoublev mentioned this pull request Feb 8, 2024

[Meta][Editorial] Define bags of CSS properties to be referenced by allowlists #9453

Open

svgeesus approved these changes Feb 23, 2024

View reviewed changes

gsnedders reviewed Feb 23, 2024

View reviewed changes

jyasskin mentioned this pull request Mar 5, 2024

Give the CSSWG a custom triage heuristic. speced/spec-maintenance#27

Merged

[cssom] [css-fonts] [css-page] Sort out properties and descriptors.

581417f

This is intended to address #5649.

emilio force-pushed the css-style-decl-split branch from 035cd61 to 581417f Compare March 20, 2024 10:27

svgeesus merged commit 2e8c502 into main Mar 20, 2024
1 check passed

svgeesus deleted the css-style-decl-split branch March 20, 2024 20:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cssom] [css-fonts] [css-page] Sort out properties and descriptors. #9686

[cssom] [css-fonts] [css-page] Sort out properties and descriptors. #9686

emilio commented Dec 8, 2023

emilio commented Dec 8, 2023

svgeesus left a comment

svgeesus Dec 8, 2023

emilio Dec 11, 2023

cdoublev Dec 8, 2023

emilio Dec 11, 2023

tabatkins commented Dec 8, 2023

emilio commented Dec 11, 2023

cdoublev Feb 7, 2024 •

edited

svgeesus commented Feb 23, 2024

svgeesus left a comment

gsnedders Feb 23, 2024

emilio commented Mar 20, 2024

[cssom] [css-fonts] [css-page] Sort out properties and descriptors. #9686

[cssom] [css-fonts] [css-page] Sort out properties and descriptors. #9686

Conversation

emilio commented Dec 8, 2023

emilio commented Dec 8, 2023

svgeesus left a comment

Choose a reason for hiding this comment

svgeesus Dec 8, 2023

Choose a reason for hiding this comment

emilio Dec 11, 2023

Choose a reason for hiding this comment

cdoublev Dec 8, 2023

Choose a reason for hiding this comment

emilio Dec 11, 2023

Choose a reason for hiding this comment

tabatkins commented Dec 8, 2023

emilio commented Dec 11, 2023

cdoublev Feb 7, 2024 • edited

Choose a reason for hiding this comment

svgeesus commented Feb 23, 2024

svgeesus left a comment

Choose a reason for hiding this comment

gsnedders Feb 23, 2024

Choose a reason for hiding this comment

emilio commented Mar 20, 2024

cdoublev Feb 7, 2024 •

edited