-
Notifications
You must be signed in to change notification settings - Fork 78
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
Comments
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:
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.
|
Actually, the definition should rather be dropped entirely since the new |
I suppose that properties duplicated in a higher version level of the same specification, eg. You write that I'm not sure that unicity should also be achieved for types. But I find the following duplicates with
|
Yes, the
Yes, see w3c/csswg-drafts#6434 (comment)
Thanks, I had not yet looked at 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 |
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:
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, |
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`).
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`).
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 Regarding these "partial" definition overrides, you may be interested by this issue, in which I ask a clarification about the way to concatenate the Your rules look good to me. I use the same except a few for which I'm wrong. |
…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.
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. |
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 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
|
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)
Regarding
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 I just upgraded 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 I have the following list of terminals:
|
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)
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!
The duplicate definition of 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 From a CSS extract perspective, we should rather find a way to flag the 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.
Thanks for the list! That's what I'm hoping we can build automatically from prose definitions. Such definitions are flagged with a 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. |
I'm suggesting to override the definition of From:
To:
But I understand that you prefer not to hardcode exceptions. Thanks for pointing me to the Bikeshed doc. I believe that token 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 |
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 I note that a similar syntax hiccup seems to exist for
instead of
The latter form is valid but it does not create a link to the definition of |
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 It seems also a bit confusing to find |
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
Sorry for the confusion. You can still look things up using the webref repository but there are two primary branches in the repository:
You should look things up in the The README in the |
Thanks for these informations. I find it time-consuming to search for them and it is hard to remember them.
Yes, I think I approve. The README is clear. It's my fault if I was confused. I can use the 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, |
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
... 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). |
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 Also, note that the grammar of 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 |
Starting with v4.2.1, the
The CSS WG resolved to add We added a patch in Webref in the meantime to add
Right. The CSS WG tracks this in: 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
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 One more difference is that definition of Let me know if you bump into other hiccups! |
If Filter Effects and CSS Masking were linking to the |
Yes, that should indeed prevent duplication in 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 A possible fix that could perhaps work for everyone (and us in particular) is that Filter Effects could define something like Similarly, the text in CSS Masking could be slightly re-written to have the current prose fit as the definition of 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. |
See discussion in #115 (comment)
The text was updated successfully, but these errors were encountered: