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-syntax-3] "Consume a block's contents" leaves decls list with #11017

Open
AtkinsSJ opened this issue Oct 10, 2024 · 7 comments
Open

[css-syntax-3] "Consume a block's contents" leaves decls list with #11017

AtkinsSJ opened this issue Oct 10, 2024 · 7 comments

Comments

@AtkinsSJ
Copy link
Contributor

Consume a block's contents collects declarations into the decls list, but only copies that over into rules when an at-rule is encountered. If decls is not empty when <EOF-token> or <}-token> is encountered, those declarations are lost.

I think the fix is to change this:

<EOF-token>
<}-token>
Return rules.

into this:

<EOF-token>
<}-token>
If decls is not empty, append it to rules.
Return rules.

This at least worked for me.


As a side note, not sure if #8834 is asking for this particular kind of feedback, but the new algorithms have been very straightforward to implement directly, and feel like a nice improvement to how they worked previously. (Token streams are how I'd ended up implementing it before so it's nice to have that actually match the spec!) So thanks @tabatkins

@cdoublev
Copy link
Collaborator

Considering that declarations at the top of a style rule should not be wrapped in a nested declarations rule, I think this algorithm should either still return a list of declarations and rules, or CSS Nesting should specify that this top-level list should not be included in CSSStyleRule.cssRules but exposed as CSSStyleRule.style.

@cdoublev
Copy link
Collaborator

Actually, many other rules only have declarations, so it should definitely return rules and declarations, imo.

@AtkinsSJ
Copy link
Contributor Author

I can't remember if that was confusing to implement or not. I just ended up treating a list-of-declarations at the start in a different way. But being more explicit is always nice.

@cdoublev
Copy link
Collaborator

I am still unsure and considering all the options.

On one hand, a representation like Contents(Declarations) as the result from parsing against <declaration-list> seem quite odd. On the other hand, all rule block might be considered as containing rules, and lists of declarations implicitly wrapped in an invisible rule. But then it is not clear why the top-level list might not be included in the list of rules...

@AtkinsSJ
Copy link
Contributor Author

Let me know if you have anything you'd like to test and I'll give it a go. Ladybird tries to implement specs as closely as possible, so it should be a pretty good test of whether the spec algorithms work or not.

@cdoublev
Copy link
Collaborator

I am in a similar position. I was just thinking out loud about what the appropriate spec text should be. I am not in a position to decide it. 😸

@AtkinsSJ
Copy link
Contributor Author

Oh neat! I hadn't noticed sorry. 😅 I might have a look at that for some ideas.

cdoublev added a commit to cdoublev/css that referenced this issue Dec 19, 2024
Declarations are no longer hoisted or wrapped in a style rule.

Some problem remains...

1. @font-feature-values, @function, @page, can also have declarations
   interleaved with rules, but CSSNestedDeclarations is currently
   associated to style properties. (w3c/csswg-drafts#11272)

2. "}" should be ignored in a "style" attribute value (a declaration
   block) but the algorithm has been replaced with the same algorithm
   than for a block of rules and declarations. (w3c/csswg-drafts#11113)

3. The current text wants declarations interleaved by an invalid at-rule
   or an invalid (qualified) rule error to be separated, but the reason
   is not stated and no browser does this. (w3c/csswg-drafts#11271)

4. The current text clearly has some other minor typos and is not clear
   about whether rules and declaration, or only rules, should be
   returned. (w3c/csswg-drafts#11017)
cdoublev added a commit to cdoublev/css that referenced this issue Dec 19, 2024
Declarations are no longer hoisted or wrapped in a style rule.

Some problem remains...

1. @font-feature-values, @function, @page, can also have declarations
    interleaved with rules, but CSSNestedDeclarations is currently
    associated to style properties. (w3c/csswg-drafts#11272)

2. "}" should be ignored in a "style" attribute value (a declaration
    block) but the algorithm has been replaced with the same algorithm
    than for a block of rules and declarations. (w3c/csswg-drafts#11113)

3. The current text wants declarations interleaved by an invalid at-rule
    or an invalid (qualified) rule error to be separated, but the reason
    is not stated and no browser does this. (w3c/csswg-drafts#11271)

4. The current text clearly has some other minor typos and is not clear
    about whether rules and declaration, or only rules, should be
    returned. (w3c/csswg-drafts#11017)
cdoublev added a commit to cdoublev/css that referenced this issue Dec 19, 2024
Declarations are no longer hoisted or wrapped in a style rule.

Some problem remains...

1. @font-feature-values, @function, @page, can also have declarations
    interleaved with rules, but CSSNestedDeclarations is currently
    associated to style properties. (w3c/csswg-drafts#11272)

2. "}" should be ignored in a "style" attribute value (a declaration
    block) but the algorithm has been replaced with the same algorithm
    than for a block of rules and declarations. (w3c/csswg-drafts#11113)

3. The current text wants declarations interleaved by an invalid at-rule
    or an invalid (qualified) rule error to be separated, but the reason
    is not stated and no browser does this. (w3c/csswg-drafts#11271)

4. The current text clearly has some other minor typos and is not clear
    about whether rules and declaration, or only rules, should be
    returned. (w3c/csswg-drafts#11017)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants