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

CSS package: consider additional parsing guarantees #127

Open
tidoust opened this issue Mar 10, 2021 · 20 comments
Open

CSS package: consider additional parsing guarantees #127

tidoust opened this issue Mar 10, 2021 · 20 comments

Comments

@tidoust
Copy link
Member

tidoust commented Mar 10, 2021

See discussion in #115 (comment)

@tidoust
Copy link
Member Author

tidoust commented Jul 5, 2021

Some notes as I'm looking into adding a check for unicity for CSS property definitions.

There are a number of properties that are defined more than once in practice. In most cases, this is "by design" and needs to be accounted for in the code:

  • Many CSS 2.x properties are redefined in dedicated CSS modules. Definitions in modules take precedence.
  • Delta specs are often not pure delta specs and may redefine a property.
  • css-align > css-flexbox, see Module interactions

There remain properties defined in more than one spec, and for which I don't always know which one is the "right" one:

I suspect I should ignore problematic definitions in SVG2 But I don't know whether dropping these properties from SVG 2 is tracked somewhere. I suppose I should also prefer fill-stroke over svg-stroke.

Not sure what to do with css-logical / css-positioning duplicates.

shape-inside should rather be defined with a newValues field in css-round-display.

@tidoust
Copy link
Member Author

tidoust commented Jul 8, 2021

shape-inside should rather be defined with a newValues field in css-round-display.

Actually, the definition should rather be dropped entirely since the new display value. Filed w3c/csswg-drafts#6433

@cdoublev
Copy link

cdoublev commented Aug 20, 2021

I suppose that properties duplicated in a higher version level of the same specification, eg. background-clip in css-backgrounds and css-backgrounds-4, are filtered out from your above list, because it is obvious that specification-name-level-x > specification-name-level-y? (where x > y) will have to be checked to achieve unicity.

You write that css-position > css-logical in a comment in another issue. Can you confirm it?

I'm not sure that unicity should also be achieved for types. But I find the following duplicates with @webref/css@^2.0.7.

@tidoust
Copy link
Member Author

tidoust commented Aug 20, 2021

I suppose that properties duplicated in a higher version level of the same specification, eg. background-clip in css-backgrounds and css-backgrounds-4, are filtered out from your above list, because it is obvious that specification-name-level-x > specification-name-level-y? (where x > y) will have to be checked to achieve unicity.

Yes, the css folder in webref only contains extracts for the latest "full" level in the series (plus extracts for "delta" levels when they exist). I generated the above list from that folder.

You write that css-position > css-logical in a comment in another issue. Can you confirm it?

Yes, see w3c/csswg-drafts#6434 (comment)

I'm not sure that unicity should also be achieved for types. But I find the following duplicates with @webref/css@^2.0.7.

Thanks, I had not yet looked at valuespaces :)

Unicity may be less critical there but it seems at least valuable to have only one definition of function-like constructs not to confuse developers. For types such as <position>, it probably matters less: the name is more internal to the spec. Some of the items you report may naturally disappear from the specs once duplicate property definitions get fixed.

@tidoust
Copy link
Member Author

tidoust commented Apr 8, 2022

Looking into de-duplicating CSS properties again with the goal of dropping duplicates during the data curation phase, I think that the following logic, to be discussed, would work:

  1. Ignore property definitions that extend a base property for now (those for which newValues is set). Goal is to check them, but that's orthogonal to the problem at hand.
  2. Ignore property definitions from delta specs that re-define the same propery in the full spec. We could handle them during data curation but choosing the "right" one seems arbitrary. It depends on whether one considers that delta specs are mature enough or whether one considers that the full spec remains the authorative version. Is that a reasonable approach?
  3. Drop duplicate property definitions defined in spec A when there is a spec B that defines the same property and that is known to supersede spec A. Here are the superseding rules, expressed in JS, that seem needed to get rid of duplicates. Some of these rules need to be confirmed because it's not clear which spec should be viewed as the authoritative one.
const supersededBy = {
  // All CSS modules supersede CSS 2.x
  'CSS': '*',

  // See note in https://drafts.csswg.org/css-align/#placement
  // "The property definitions here supersede those in [CSS-FLEXBOX-1]"
  'css-flexbox': 'css-align',

  // https://github.com/w3c/csswg-drafts/issues/6434#issuecomment-877447360
  'css-logical': 'css-position',

  // TODO: confirm cf https://github.com/w3c/csswg-drafts/issues/6435
  // (string-set property defined twice)
  'css-gcpm': 'css-content',

  // See https://github.com/w3c/csswg-drafts/issues/6433
  // (shape-inside should get dropped from css-round-display)
  'css-round-display': 'css-shapes',

  // See note in https://svgwg.org/specs/strokes/#sotd
  // "In the future, this specification will supersede the SVG 2 Stroke
  // definition, however at this time the SVG 2 Stroke definition must be
  // considered the normative definition."
  'svg-strokes': 'SVG',

  // TODO: confirm that CSS modules supersede SVG 2
  'SVG': [
    'css-images',   // image-rendering
    'css-logical',  // inline-size
    'css-shapes',   // shape-inside, shape-margin
    'css-ui',       // pointer-events
    'fill-stroke'   // all properties in fill-stroke
  ]
};

The superseding rules could perhaps be upstreamed to browser-specs, but I would prefer to start maintaining the list in webref and confirm that it is at the right granularity. For instance, I stuck to series shortname because that seems good enough for now but perhaps we'll end up in a situation where levels need to be taken into account. Also, in most cases, a spec does not completely supersedes another one, it just supersedes it for a couple of properties, but both specs will continue to be developed,

tidoust added a commit that referenced this issue Apr 9, 2022
This update adds a new data curation that drops duplicate CSS property
definitions from CSS extracts whenever possible, in other words when we know
which definition to keep. The code contains the list of known superseding
relationships between specs to choose the right one.

See discussion in #127 for details.

CSS properties that get re-defined in a delta spec are ignored, meaning that a
naive merge of all curated CSS extracts will still contain such duplicates.
A typical solution to end up with a consistent merge would be to exclude delta
specs.

The curated data may still contain duplicate CSS property definitions, but these
get caught by the new tests. The new tests also check that CSS extracts contain
a base CSS property definition for all CSS properties that get extended
(through `newValues`).
tidoust added a commit that referenced this issue Apr 9, 2022
This adds a new data curation step that drops duplicate CSS property definitions
from CSS extracts whenever possible, in other words when we know which
definition is the authoritative one. The code contains a list of known
superseding relationships between specs to choose the right one. See discussion
in #127 for details.

CSS properties that get re-defined in a delta spec are ignored, meaning that a
naive merge of all curated CSS extracts will still contain such duplicates. A
typical solution to end up with a consistent merge would be to exclude delta
specs from that merge. This is left to data consumers as some may choose to
rather live on the bleeding edge.

The curated data may still contain duplicate CSS property definitions, but these
will get caught by tests. The new tests also check that CSS extracts contain a
base CSS property definition for all CSS properties that get extended (through
`newValues`).
@cdoublev
Copy link

cdoublev commented Apr 9, 2022

I think I'd rather an entire delta spec be ignored than only some parts of it. I'm thinking to CSS types extracted in valuespaces that may only be used in the newValues of a property definition. This would not be a big deal but my point is that it might be a bit confusing.

Regarding these "partial" definition overrides, you may be interested by this issue, in which I ask a clarification about the way to concatenate the newValues, especially for some cases that looks ambiguous to me.

Your rules look good to me. I use the same except a few for which I'm wrong.

tidoust added a commit that referenced this issue Apr 10, 2022
…ion of @webref/css) (#555)

This adds a new data curation step that drops duplicate CSS property definitions
from CSS extracts whenever possible, in other words when we know which
definition is the authoritative one. The code contains a list of known
superseding relationships between specs to choose the right one. See discussion
in #127 for details.

CSS properties that get re-defined in a delta spec are ignored, meaning that a
naive merge of all curated CSS extracts will still contain such duplicates. A
typical solution to end up with a consistent merge would be to exclude delta
specs from that merge. This is left to data consumers as some may choose to
rather live on the bleeding edge.

The curated data may still contain duplicate CSS property definitions, but these
will get caught by tests. The new tests also check that CSS extracts contain a
base CSS property definition for all CSS properties that get extended (through
`newValues`).

READMEs updated to mention the new guarantees. Major version of @webref/css
package bumped accordingly.

Under the hood, note this update replaces the `requireFromWorkingDirectory`
utility function with a `loadJSON` function, to avoid memroy caching issues when
a module modifies the JSON data that it loads.
@tidoust
Copy link
Member Author

tidoust commented Apr 10, 2022

I think I'd rather an entire delta spec be ignored than only some parts of it. I'm thinking to CSS types extracted in valuespaces that may only be used in the newValues of a property definition. This would not be a big deal but my point is that it might be a bit confusing.

That seems sane to me but others may want to live on the bleeding edge and integrate definitions from delta specs somehow. We thought we would leave that up to the final data consumer.

If you ignore CSS extracts from delta specs, the new guarantee is that you will no longer bump into duplicate property definitions.

Note this logic does not yet deal with descriptors and value spaces.

@tidoust
Copy link
Member Author

tidoust commented Apr 11, 2022

Looking at value spaces, applying the same logic would allow us to get down to a list of 6 duplicates, which I don't think we can easily get rid of:

const duplicatedValuespaces = [
  // Defined in CSS Grid Layout Module Level 2 and CSS Box Sizing Module Level 3
  // https://drafts.csswg.org/css-grid-2/
  // https://drafts.csswg.org/css-sizing-3/
  '<fit-content()>',

  // Defined in CSS Images Module Level 3, CSS Positioned Layout Module Level 3
  // and CSS Text Module Level 3 (and CSS Values but in prose so not extracted)
  // https://drafts.csswg.org/css-images-3/
  // https://drafts.csswg.org/css-position/
  // https://drafts.csswg.org/css-text-3/
  '<length>',

  // Defined in CSS Shapes Module Level 1 and Motion Path Module Level 1
  // https://drafts.csswg.org/css-shapes/
  // https://drafts.fxtf.org/motion-1/
  '<path()>',

  // Defined in CSS Positioned Layout Module Level 3,
  // CSS Mobile Text Size Adjustment Module Level 1 and CSS Text Module Level 3
  // (and CSS Values but in prose so not extracted)
  // https://drafts.csswg.org/css-position/
  // https://drafts.csswg.org/css-size-adjust-1/
  // https://drafts.csswg.org/css-text-3/
  '<percentage>',

  // Defined in CSS Masking Module Level 1 and CSS Shapes Module Level 1
  // https://drafts.fxtf.org/css-masking-1/
  // https://drafts.csswg.org/css-shapes/
  '<rect()>',

  // Defined in CSS Masking Module Level 1, CSS Values and Units Module Level 4
  // and Filter Effects Module Level 1
  // https://drafts.fxtf.org/css-masking-1/
  // https://drafts.csswg.org/css-values-4/
  // https://drafts.fxtf.org/filter-effects-1/
  '<url>'
];

I have also started to look at the list of value spaces for which we do not have any definition (because they are defined in prose and so we fail to extract them from the specs). Not taking delta specs into account, there are ~330 value spaces referenced by properties and other value space definitions, 62 of which we do not have any definition for. See below. One way to complete the list of value space definitions would be to look at the dfns extract. For instance, we do have the definition of <string> in a dfns extract. Ideally, we'd be able to extract some prose around it.

There are also likely plenty of dangling value spaces, meaning definitions that are referenced in prose but not directly from the formal syntax of another property or value space. There again, the links exist in prose but it's going to be hard to extract them automatically.

List of the 62 missing value spaces definitions
  • <absolute-size>, referenced by font-size
  • <angle>, referenced by <hue>, font-style, image-orientation, <linear-gradient()>, <rotate()>, <skew()>, <skew()>, <skewX()>, <skewY()>, <angle-percentage>, <hue-rotate()>, offset-rotate, <ray()>
  • <any-value>, referenced by <general-enclosed>, <general-enclosed>, <pseudo-class-selector>
  • <attr()>, referenced by <::attr()>
  • <basic-shape>, referenced by clip-path, shape-outside, offset-path, shape-subtract
  • <bottom>, referenced by <rect()>
  • <counter-name>, referenced by counter-reset, counter-increment, counter-set, <reversed-counter-name>, <counter()>, <counters()>
  • <counter-style-name>, referenced by <counter-style>
  • <custom-ident>, referenced by <keyframes-name>, color-scheme, string-set, <target-counter()>, <target-counters()>, <string()>, <symbol>, <env()>, <running()>, <element()>, <nth()>, <line-names>, <grid-line>, <grid-line>, <grid-line>, page, <single-transition-property>, <animateable-feature>
  • <custom-property-name>, referenced by <var()>
  • <dashndashdigit-ident>, referenced by <an+b>
  • <decibel>, referenced by voice-volume, cue-before, cue-after
  • <declaration-list>, referenced by <keyframe-block>, <feature-value-block>
  • <declaration-value>, referenced by <env()>, <paint()>, <var()>
  • <declaration>, referenced by <import-condition>, <supports-decl>
  • <device-cmyk()>, referenced by <color>
  • <dimension-token>, referenced by <urange>, <urange>
  • <dimension>, referenced by <calc-value>, <mf-value>
  • <flex>, referenced by <track-breadth>
  • <frequency>, referenced by voice-pitch, voice-pitch, voice-range, voice-range, <frequency-percentage>
  • <function-token>, referenced by <general-enclosed>, <pseudo-class-selector>
  • <hash-token>, referenced by <id-selector>
  • <hex-color>, referenced by <absolute-color-base>
  • <hsla()>, referenced by <absolute-color-base>
  • <ident-token>, referenced by <custom-arg>, <page-selector>, <urange>, <ns-prefix>, <wq-name>, <class-selector>, <attribute-selector>, <pseudo-class-selector>
  • <ident>, referenced by <layer-name>, <layer-name>, <feature-value-name>, <namespace-prefix>, <paint()>, flow-into, flow-from, <::part()>, <media-type>, <mf-name>, <mf-value>, <na-prefix>, <na-name>
  • <left>, referenced by <rect()>
  • <media-query-list>, referenced by <import-condition>
  • <minmax()>, referenced by <track-size>, <fixed-size>, <fixed-size>
  • <mix-blend-mode>, referenced by background-blend-mode
  • <n-dimension>, referenced by <an+b>, <an+b>, <an+b>
  • <named-color>, referenced by <absolute-color-base>
  • <ndash-dimension>, referenced by <an+b>
  • <ndashdigit-dimension>, referenced by <an+b>
  • <ndashdigit-ident>, referenced by <an+b>
  • <number-percentage>, referenced by mask-border-slice, <brightness()>, <contrast()>, <grayscale()>, <invert()>, <opacity()>, <saturate()>, <sepia()>
  • <number-token>, referenced by <urange>, <urange>, <urange>, <urange>
  • <number>, referenced by <single-animation-iteration-count>, border-image-slice, border-image-width, border-image-outset, <hue>, <rgb()>, <alpha-value>, <lab()>, <lab()>, <lab()>, <lch()>, <lch()>, <oklab()>, <oklab()>, <oklab()>, <oklch()>, <oklch()>, <predefined-rgb-params>, <xyz-params>, <cubic-bezier()>, <cubic-bezier()>, <cubic-bezier()>, <cubic-bezier()>, <cubic-bezier-easing-function>, <cubic-bezier-easing-function>, <cubic-bezier-easing-function>, <cubic-bezier-easing-function>, flex-grow, flex-shrink, font-size-adjust, font-variation-settings, <font-weight-absolute>, line-height, initial-letter, initial-letter, mask-border-width, mask-border-outset, voice-balance, tab-size, <matrix()>, <scale()>, <scale()>, <scaleX()>, <scaleY()>, <ratio>, <ratio>, <calc-value>, stroke-miterlimit, <number-optional-number>, <number-optional-number>, <mf-value>, <element-offset>, <points>, <dasharray>
  • <outline-line-style>, referenced by outline-style
  • <part()>, referenced by <::part()>
  • <relative-size>, referenced by font-size
  • <repeat()>, referenced by <track-repeat>, <auto-repeat>, <fixed-repeat>, <name-repeat>
  • <repeating-linear-gradient()>, referenced by <gradient>
  • <repeating-radial-gradient()>, referenced by <gradient>
  • <reversed()>, referenced by <reversed-counter-name>
  • <rgba()>, referenced by <absolute-color-base>
  • <right>, referenced by <rect()>
  • <semitones>, referenced by voice-pitch, voice-range
  • <signed-integer>, referenced by <an+b>, <an+b>, <an+b>
  • <signless-integer>, referenced by <an+b>, <an+b>, <an+b>, <an+b>, <an+b>, <an+b>
  • <src()>, referenced by <url>
  • <string-token>, referenced by <attribute-selector>
  • <string>, referenced by <keyframes-name>, content, quotes, quotes, string-set, <leader-type>, <target-counter()>, <target-counters()>, <target-counters()>, <target-text()>, <content-list>, <symbol>, <symbols()>, font-language-override, font-variation-settings, <font-format>, <feature-tag-value>, grid-template-areas, grid-template, list-style-type, <counters()>, block-ellipsis, <path()>, text-emphasis-style, <url>, <url>, <filter()>, <path()>, d
  • <supports()>, referenced by <import-condition>
  • <system-color>, referenced by <color>
  • <time>, referenced by animation-duration, animation-delay, <single-animation>, <single-animation>, pause-before, pause-after, rest-before, rest-after, voice-duration, transition-duration, transition-delay, <single-transition>, <single-transition>, <time-percentage>
  • <top>, referenced by <rect()>, inset-block, inset-inline, inset
  • <transform-function>, referenced by <transform-list>
  • <uri>, referenced by cue-before, cue-after, shape-subtract
  • <url()>, referenced by <url>
  • <url-modifier>, referenced by <url>, <url>
  • <zero>, referenced by <rotate()>, <skew()>, <skew()>, <skewX()>, <skewY()>, <hue-rotate()>
tidoust added a commit that referenced this issue Apr 11, 2022
This applies the same logic as for CSS properties to drop duplicates from CSS
extracts. Some duplicates remain: `<fit-content()>`, `<length>`, `<path()>`,
`<percentage>`, `<rect()>` and `<url>`.

No easy way to get rid of them, as functions and terms seem to be reused on
purpose with similar though slightly different meaning each time. These
duplicates are hardcoded into tests.

No update to guarantees in README as it's not clear what interesting guarantees
we can really make for now. Value spaces remain too messy to be directly usable,
e.g. see #127 (comment)
@cdoublev
Copy link

cdoublev commented Apr 12, 2022

Regarding <rect()> (duplicate type):

  • it is defined with rect(<top>, <right>, <bottom>, <left>) (all 4 type arguments are defined in prose and missing in valuespaces) in the grammar (value definition) of the deprecated clip property, which is rect() | auto, noting that ideally it should be <rect()> | auto instead of rect() | auto otherwise it could be interpreted as a function named rect with no arguments
  • it is defined with rect([<length-percentage> | auto ]{4} [round <'border-radius'>]?) as one of the alternations resulting from the expansion of <basic-shape> (ie. the basic shapes defined in prose and missing in valuespaces) in the properties that you referenced above for this type

I do not know if this information is helpfull because I think these issues can only be fixed in the corresponding CSS specs, or if the following workaround can be achieved, but I think it is safe to ignore the <rect()> definition from CSS Masking, to override the clip property with rect(<top>, <right>, <bottom>, <left>) | auto and to define the type arguments with <length> | auto, as defined in prose. I believe it will never be updated or removed.

I just upgraded @webref/css from 3.0.8 to 3.1.0. The duplicate <rect()> was introduced in 3.0.9, and I think I will go for this solution to fix this issue, and for defining css-shapes as the authoritative spec for <rect()> (as well as for <rect()>, <shape-inside>, and <shape-margin>.

I have not carefully look at the other types because I'm not sure if that can help remove duplicates. Anyway, I can still note that <length>, <percentage> and <url> are all terminals, therefore they will always be only defined in prose. I ignore all terminal type definitions when processing @webref/css data.

I have the following list of terminals:
  • angle
  • custom-ident
  • custom-property-name
  • dashed-ident
  • dashndashdigit-ident
  • decibel
  • dimension-token
  • flex
  • frequency
  • function
  • function-token
  • hash
  • hash-token
  • hex-color
  • ident
  • ident-token
  • integer
  • keyword
  • length
  • n-dimension
  • ndash-dimension
  • ndashdigit-dimension
  • ndashdigit-ident
  • number
  • number-token
  • percentage
  • percentage-token
  • resolution
  • semitones
  • signed-integer
  • signless-integer
  • string
  • string-token
  • time
  • url-token
  • zero
tidoust added a commit that referenced this issue Apr 13, 2022
This applies the same logic as for CSS properties to drop duplicates from CSS
extracts. Some duplicates remain: `<fit-content()>`, `<length>`, `<path()>`,
`<percentage>`, `<rect()>` and `<url>`.

No easy way to get rid of them, as functions and terms seem to be reused on
purpose with similar though slightly different meaning each time. These
duplicates are hardcoded into tests.

No update to guarantees in README as it's not clear what interesting guarantees
we can really make for now. Value spaces remain too messy to be directly usable,
e.g. see #127 (comment)
@tidoust
Copy link
Member Author

tidoust commented Apr 13, 2022

I do not know if this information is helpfull because I think these issues can only be fixed in the corresponding CSS specs

One of the goals with webref is to identify problems that could be reported to underlying specs. So it's always useful to document these issues!

I think it is safe to ignore the <rect()> definition from CSS Masking, to override the clip property with rect(, , , ) | auto and to define the type arguments with | auto, as defined in prose

The duplicate definition of rect() seems to be intended, see the note in the definition of rect() in css-shapes: "This syntax is similar, but not quite identical, to the legacy rect() function used solely by the clip property."

I haven't looked into details but I don't think it is safe to override the definition of clip property with the newer syntax for rect() because the clip property is not meant to support that newer syntax.

From a CSS extract perspective, we should rather find a way to flag the rect() definition in CSS Masking, e.g. as only applying locally.

Other duplicate cases seem similar: function names are re-used, but with slightly different meaning depending on where the function is used. I don't think we can select one definition as the valid one. Both definitions are valid.

I have the following list of terminals:

Thanks for the list! That's what I'm hoping we can build automatically from prose definitions. Such definitions are flagged with a type type (see Bikeshed documentation).

We prefer not to hardcode exceptions in webref or in the crawler if we can avoid it, so as not to have to maintain these exceptions over time.

@cdoublev
Copy link

cdoublev commented Apr 14, 2022

I'm suggesting to override the definition of clip in a way that keeps the older syntax but removes its syntactically invalid (according to the CSS value definition syntax) reference to <rect()> as rect().

From:

clip = rect() | auto
<rect()> = rect( <top>, <right>, <bottom>, <left> )
<rect()> = rect( [ <length-percentage> | auto ]{4} [ round <'border-radius'> ]? )

To:

clip = rect( <top>, <right>, <bottom>, <left> ) | auto
<rect()> = rect( [ <length-percentage> | auto ]{4} [ round <'border-radius'> ]? )

But I understand that you prefer not to hardcode exceptions.

Thanks for pointing me to the Bikeshed doc. I believe that token types should be ideally splitted into a different category because all entry points of the CSS parser normalize tokens into component values (types). Therefore it will never have to match eg. <number-token>. Instead, it will have to match <number>, which is defined as <number-token>. The list of tokens is defined here.

There are some CSS values that are actually defined with a value definition that includes token types, some of which I reported as an issue, because not all terminal types are mapped to the corresponding token types. These definitions use <function-token> or tokens for blocks. These tokens are consumed into a single function/block component value, which contains other component values.

@tidoust
Copy link
Member Author

tidoust commented Apr 14, 2022

But I understand that you prefer not to hardcode exceptions.

Right but the invalid syntax should be fixed. If you can report this issue as well to the CSS WG, that would be great!

I note that the CSS WG may prefer to fix the invalid syntax by replacing clip = rect() | auto with clip = <rect()> | auto. That wouldn't solve the duplication issue, but the unicity of these tokens is apparently not a guarantee that CSS specs currently provide.

I note that a similar syntax hiccup seems to exist for fit-content(). The maximum size properties should probably rather defined as:

none | <length-percentage> | min-content | max-content | <fit-content()>`

instead of

none | <length-percentage> | min-content | max-content | fit-content(<length-percentage>)

The latter form is valid but it does not create a link to the definition of <fit-content()>.

@cdoublev
Copy link

I'm late on this, sorry. I just upgraded to v4 from v3. SVG 2 is defined as superseded by CSS UI in the PR #555 related to this issue. But some values for the pointer-events property are only defined in SVG 2 and are supported in several browsers. I will downgrade to v3 for now but I will look further in the next days.

It seems also a bit confusing to find pointer-events in SVG 2 on the repository but not in the locally installed @webref/css package. I used to look up a grammar using this repository, but I think I should use grep -r ... node_modules/@webref/css/ now.

@tidoust
Copy link
Member Author

tidoust commented May 18, 2022

But some values for the pointer-events property are only defined in SVG 2 and are supported in several browsers. I will downgrade to v3 for now but I will look further in the next days.

I think we're hitting another one of these corner cases that CSS seems to excel at.

The definition in CSS UI was indeed added to replace that in SVG 2. It contains a note that "SVG 2 § 15.6 The ‘pointer-events’ property defines a variant of this property for SVG elements, with more possible values. The effect of such values outside of SVG is currently not defined."

We don't have a way to namespace certain property values to SVG elements in webref. As long as specs have not been updated (there's an on-going plan in w3c/svgwg#744 (comment)), I would personally (and probably lazily?) be inclined to keep things as-is. The alternative would be to remove the superseding rule in webref's code for now to preserve the SVG definition and create a patch to ignore the definition of pointer-events from CSS-UI.

It seems also a bit confusing to find pointer-events in SVG 2 on the repository but not in the locally installed @webref/css package. I used to look up a grammar using this repository, but I think I should use grep -r ... node_modules/@webref/css/ now.

Sorry for the confusion. You can still look things up using the webref repository but there are two primary branches in the repository:

  • the main branch contains the raw results of crawling the specs. It contains duplicates for CSS properties, invalid IDL definitions, etc. because the specs have them.
  • the curated branch contains the curated version of the main branch. That branch gives birth to @webref/css packages. Duplicates get removed during curation. You won't find pointer-events in SVG 2 in the curated branch.

You should look things up in the curated branch (or even in the @webref/css@latest branch, which contains the snapshot of the curated branch taken when the @webref/css package was last published).

The README in the main branch contains the following note: "Important: Unless you are ready to deal with invalid content, we recommend that you rely on the content of the curated branch or NPM packages instead of on the raw content in the main branch of this repository." Suggestions to improve the description are welcome!

@cdoublev
Copy link

cdoublev commented May 18, 2022

Thanks for these informations. I find it time-consuming to search for them and it is hard to remember them.

I would personally (and probably lazily?) be inclined to keep things as-is.

Yes, I think I approve.

The README is clear. It's my fault if I was confused. I can use the main branch and keep my existing curation strategy.

I believe this is what I'm going to do, because of different problematic properties or types. For example, CSS Fill Stroke now supersedes SVG Stroke, which supersedes SVG 2. In the last two specs, fill is a longhand defined with <paint> (a color or an image, roughly). In CSS Fill Stroke it is a shorthand and its grammar is <'background'> with modifications. I can't parse this grammar as is and browsers only support the longhand.

@tidoust
Copy link
Member Author

tidoust commented May 18, 2022

I can use the main branch and keep my existing curation strategy.

We probably cannot satisfy everyone but if you need to keep on using your own curation strategy, that may be an indication that we're not doing the curation correctly. Specs are imperfect and implementations may either be in advance or lagging behind specs, so I don't know how we can publish something that can be viewed as perfect and authoritative, but the number of corner cases should remain minimal.

In other words, if you can continue to report discrepancies between your curation logic and the one we run, that's very useful for us (and thanks! :)). For pointer-events, I don't really know what definition should be seen as the authoritative one (and that is why I suggest the "let's be lazy" approach), but here and elsewhere, there may be changes that we can introduce.

I believe this is what I'm going to do, because of different problematic properties or types. For example, CSS Fill Stroke now supersedes SVG Stroke, which supersedes SVG 2. In the last two specs, fill is a longhand defined with (a color or an image, roughly). In CSS Fill Stroke it is a shorthand and its grammar is <'background'> with modifications. I can't parse this grammar as is and browsers only support the longhand.

... which seems like a spec bug to me. I filed w3c/csswg-drafts#7285 accordingly.

If browsers don't yet support the CSS Fill Stroke, that's probably a timing issue, which I confess I'm also unsure how to address during curation (having to maintain a list of property definitions that e.g. are not yet implemented cross-browsers requires more work).

@cdoublev
Copy link

cdoublev commented May 18, 2022

My requirement is to stick as closely as possible to browser behaviors and not support a future grammar if it conflicts, which theoretically should not happen due to CSS backwards compatibility policy. From this perspective, another issue related to CSS Fill Stroke, which has not been updated since 2017, is that it does not allow <number> for <stroke-dasharray>, only <length-percentage>, while both are currently allowed (on SVG elements) by browsers and SVG 2. I do not know why.

Also, note that the grammar of pointer-events in CSS UI is included in the grammar defined in SVG 2.

As noted in a previous comment, my curation strategy is very similar to the one implemented for v4. The only difference may be related to these SVG specs. I was not sure about your intent for these specs because of this note in your above comment: TODO: confirm that CSS modules supersede SVG 2.

There may be less problematic cases than I think. My script for processing @webref/css extracts allows me to override any property/type grammar anyway. I should just find enough time to properly look at it.

@tidoust
Copy link
Member Author

tidoust commented May 24, 2022

Starting with v4.2.1, the @webref/css includes fixes for the properties discussed above.

From this perspective, another issue related to CSS Fill Stroke, which has not been updated since 2017, is that it does not allow for , only , while both are currently allowed (on SVG elements) by browsers and SVG 2. I do not know why.

The CSS WG resolved to add <number> some time ago. Tracked in:
w3c/csswg-drafts#3057

We added a patch in Webref in the meantime to add <number> in the definitions of stroke-width, stroke-dasharray and stroke-dashoffset.

Also, note that the grammar of pointer-events in CSS UI is included in the grammar defined in SVG 2.

Right. The CSS WG tracks this in:
w3c/csswg-drafts#4438

Upon further thoughts, we created a patch to drop the CSS UI definition for now as long as it is not properly spec-ed. So we're back to the SVG 2 definition (and only that one) in the @webref/css package.

I was not sure about your intent for these specs because of this note in your above comment: TODO: confirm that CSS modules supersede SVG 2.

The long term story seems correct: CSS modules will supersede SVG 2 definitions, but that is not always the case at this stage, and we need to choose on a case by case basis.

We also added a patch in Webref to drop the definitions of fill and stroke from the CSS Fill and Stroke spec for the time being, and thus get back to the SVG 2 definitions as well in the @webref/css package. The SVG definitions are not perfect in that they don't reference some of the fill-xx and stoke-xx properties, but that's better than nothing.

One more difference is that definition of stroke-linejoin in CSS Fill and Stroke does not include miter-clip. I raised w3c/csswg-drafts#7295 on this but did not try to fix it in Webref: the value is not supported anywhere as far as I can tell, and has been flagged as an at-risk feature in the SVG 2 spec.

Let me know if you bump into other hiccups!

@cdoublev
Copy link

If Filter Effects and CSS Masking were linking to the <url> definition in CSS Values, would it prevent duplicate entries for this type in @webref/css? Does an open issue exists on w3c/csswg-drafts for this problem?

@tidoust
Copy link
Member Author

tidoust commented Jun 16, 2022

If Filter Effects and CSS Masking were linking to the <url> definition in CSS Values, would it prevent duplicate entries for this type in @webref/css? Does an open issue exists on w3c/csswg-drafts for this problem?

Yes, that should indeed prevent duplication in @webref/css.

The specs use a local proxy definition to describe that the URL should reference a mask element for CSS Masking or a filter element for Filter Effects. I can understand why editors may find it valuable to have <url> target the local definition instead of the one in CSS Values.

A possible fix that could perhaps work for everyone (and us in particular) is that Filter Effects could define something like <filter-value-list> = [ <filter-function> | <filter-url> ]+ and then define a <filter-url> = <url> entry (targeting CSS Values) along with the current prose.

Similarly, the text in CSS Masking could be slightly re-written to have the current prose fit as the definition of <mask-source>, so that <url> could directly target CSS Values.

I don't think there's an open issue around that in the CSS repo. A pull request would probably help to speed things up.

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