Closed
Conversation
Implements: * Nested style rules (https://drafts.csswg.org/css-nesting-1/#syntax) * The nesting selector (https://drafts.csswg.org/css-nesting-1/#nest-selector) TODO / decisions: * Add support for nested group rules (https://drafts.csswg.org/css-nesting-1/#conditionals) * How should the domain model be extended? * (a) Do we want to preserve the order of interspersed declarations and nested rules? * (b) Does it need to be API compatible? Or can there be minor changes? * (c) Do we want to preserve the distinction between style rule's "style" and "cssRules" attribute from the CSSOM (https://drafts.csswg.org/css-nesting-1/#nested-declarations)? * The easiest way would be to add a list of style rules, a list of media rules etc. to the "CSSStyleRule" class, which fulfills (b) but not (a) and (c). * A couple of test failures related to parsing errors. Since rules can now be nested, edge cases how invalid CSS is handled differ slightly. * Make sure nested rules appear in the output of the getCSSAsString method
…ng issues, update getAsCssString Add CSSNestedDeclarations class to represent nested declarations, like the CSSOM does. CSSStyleRule now has a list of ICSSNestedRule, which includes various rules such as CSSStyleRule or CSSMediaRule. See https://drafts.csswg.org/css-nesting-1/#conditionals Update CSSNodeToDomainObject to create CSSNestedDeclarations instances where needed. Add support for parsing nested at-rules, e.g.: .foo { @media print {} } Fix parsing issue where the parser confused an element selector with a style declaration, e.g. such as in the following 2 cases: * .foo { p { color: red } } * .foo { p: value; } Add a lookahead that checks if a style declaration follows, and only consume it in that case. Otherwise, proceed to look for rules and nested declarations. Add a new parser rule for consuming a style declaration with nested elements. Not all rules support nesting, e.g. @font-face. For these rules, we do want to keep the previous behavior where it throws on nested rules. Minor change to how whitespace is handled: getAsCssString only outputs itself as a string, it does not output any leading or trailing whitespace or newlines. Space and lines around an CSS element should be handled by the container that contains the element, as it may depend on the context of that container.
& is now a valid selector member. The test suite has changed the tests to use % instead of &, which is still invalid. See: https://github.com/web-platform-tests/wpt/blob/master/css/selectors/old-tests/css3-modsel-156.xml
support for relative selectors in nested style rules, e.g.
.foo { > .bar { ... } }
When a style rule is nested, it allows a <relativeSelector>, not just a
<selector>. But at the top-level, a <relativeSelector> is not allowed, i.e.
the following is invalid:
> .bar { ... }
To prevent the grammar from blowing up, always allow <relativeSelector> in a
style rule, even at the top-level. Check for disallowed <relativeSelector> at
the top level when mapping the parsed AST to the domain model. Issue a CSS
interpretation error if such an invalid relative selector is encountered.
The file is not actually good -- it's missing an rbrace!
…rule
With the browser compliant flag enabled, the behavior did not change (empty string).
With the browser compliant flag disabled,
.class{color:red;.class{color:green}.class{color:blue}
is now parsed as nothing. Previously, an error occurred at the second ".class", causing the
parser to skip to the next rbrace. Now, the second ".class" is the start of a nested
rule. The parser only encounters an error at the very end, when it finds a missing rbrace.
The error recovery tries looking for the next rbrace, but finds none, only an EOF, causing
it to discard the entire rule.
Use prefix "s" for string variables Use prefix "m_" for instance fields As per https://github.com/phax/meta/blob/master/CodingStyleguide.md
Support property rules such as @Property --canBeAnything { syntax: "*"; inherits: true; } * Add grammar rule * Add visitor methods * Add CSSPropertyRule model class
shagkur
reviewed
Apr 3, 2026
| ( <S> )* | ||
| <IDENT> { jjtThis.setText (token.image); } | ||
| ( <S> )* | ||
| styleDeclarationBlock() |
Contributor
There was a problem hiding this comment.
There're only 3 valid descriptors, all other are invalid and should be dropped rather than parsed and recreated (browsers behave the same)
Author
|
I just saw there's already #122 from 2 days ago which also implements So I think you should accept #122 and close this. 
|
The CSSOM exposes the syntax, initialValue, and inherits properties directly, and does not have nested child nodes.
Author
|
As mentioned in #122 (comment), adding new keywords worsens the current issue with the parser where it finds keywords when it shouldn't. So I think the approach in this PR has some merit to be discussed. Also, I changed the CSSPropertyRule model to align with the CSSOM, i.e. member fields for 
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements
@properyrule, closes #121Based on #123, i.e. includes changes from there. So that PR should be merged first (which I hope you will ;)
Support property rules such as