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

LibWeb: Treat import rules as invalid if preceded by non-import rules #3283

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tcl3
Copy link
Member

@tcl3 tcl3 commented Jan 16, 2025

If an import statement is preceded by a valid rule that isn't an import rule or a @layer statement rule, it is now treated as invalid.

Fixes: http://wpt.live/css/CSS2/css1/c11-import-000.xht

@tcl3 tcl3 requested a review from AtkinsSJ as a code owner January 16, 2025 21:59
auto is_layer_statement_rule = is<CSSLayerStatementRule>(*rule);
auto is_import_rule = is<CSSImportRule>(*rule);

// FIXME: Include @charset rules in this check when they are added.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This FIXME isn't necessary. It's confusing, but @charset is a weird beast that isn't actually an at-rule that shows up in the CSSOM. https://drafts.csswg.org/css-syntax/#charset-rule

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...there are order-requirements for @namespace though, and I think some others.

@github-actions github-actions bot added the conflicts Pull request has merge conflicts that need resolution label Feb 6, 2025
Copy link

github-actions bot commented Feb 6, 2025

Your pull request has conflicts that need to be resolved before it can be reviewed and merged. Make sure to rebase your branch on top of the latest master.

If an import statement is preceded by a valid rule that isn't an
import rule or a @layer statement rule, it is now treated as invalid.
@tcl3 tcl3 force-pushed the libweb_invalid_import_rules branch from 6796836 to 7d9305c Compare February 15, 2025 07:19
@github-actions github-actions bot removed the conflicts Pull request has merge conflicts that need resolution label Feb 15, 2025
@tcl3 tcl3 marked this pull request as draft February 15, 2025 07:19
@AtkinsSJ
Copy link
Member

Two thoughts on this:

  1. Maybe Parser::is_valid_in_the_current_context() could be modified to check the ordering, though currently it only keeps a stack of types of context, and not the actual context. That would need to change if we want to examine the siblings.
  2. We also need to enforce this when inserting/removing rules from JS.

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

Successfully merging this pull request may close these issues.

2 participants