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
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions Libraries/LibWeb/CSS/Parser/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
*/

#include <AK/Debug.h>
#include <AK/GenericLexer.h>
#include <AK/SourceLocation.h>
#include <AK/TemporaryChange.h>
#include <LibWeb/CSS/CSSImportRule.h>
#include <LibWeb/CSS/CSSLayerStatementRule.h>
#include <LibWeb/CSS/CSSStyleDeclaration.h>
#include <LibWeb/CSS/CSSStyleSheet.h>
#include <LibWeb/CSS/MediaList.h>
Expand Down Expand Up @@ -123,6 +128,7 @@ CSSStyleSheet* Parser::parse_as_css_stylesheet(Optional<URL::URL> location)

// Interpret all of the resulting top-level qualified rules as style rules, defined below.
GC::RootVector<CSSRule*> rules(realm().heap());
auto more_import_rules_allowed = true;
for (auto const& raw_rule : style_sheet.rules) {
auto rule = convert_to_rule(raw_rule, Nested::No);
// If any style rule is invalid, or any at-rule is not recognized or is invalid according to its grammar or context, it’s a parse error.
Expand All @@ -131,6 +137,19 @@ CSSStyleSheet* Parser::parse_as_css_stylesheet(Optional<URL::URL> location)
log_parse_error();
continue;
}
// https://drafts.csswg.org/css-cascade-5/#at-import
// Any @import rules must precede all other valid at-rules and style rules in a style sheet (ignoring @charset and @layer statement rules)
// and must not have any other valid at-rules or style rules between it and previous @import rules, or else the @import rule is invalid.
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.

if (!(is_layer_statement_rule || is_import_rule)) {
more_import_rules_allowed = false;
} else if (is_import_rule && !more_import_rules_allowed) {
log_parse_error();
continue;
}
rules.append(rule);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">

<html xmlns="http://www.w3.org/1999/xhtml">

<head>

<title>CSS Reftest Reference</title>
<link rel="author" title="Gérard Talbot" href="http://www.gtalbot.org/BrowserBugsSection/css21testsuite/" />

<style type="text/css"><![CDATA[
p {color: green;}
]]></style>

</head>

<body>

<p>This text should be green.</p>

<p>This text should be green.</p>

<p>This text should be green.</p>

<p>This text should be green.</p>

</body>
</html>
29 changes: 29 additions & 0 deletions Tests/LibWeb/Ref/input/wpt-import/css/CSS2/css1/c11-import-000.xht
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>CSS Test: Basic Containment</title>
<link rel="help" href="http://www.w3.org/TR/REC-CSS1#containment-in-html"/>
<link rel="author" title="CSS1 Test Suite Contributors" href="http://www.w3.org/Style/CSS/Test/CSS1/current/tsack.html"/>
<link rel="author" title="Ian Hickson" href="mailto:[email protected]"/>
<style type="text/css"><![CDATA[
@import url(support/a-green.css);
@import "support/b-green.css";
.c { color: green; }
@import url(support/c-red.css);
<!-- .d { color: green; } -->
]]></style>
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html#at-import" title="6.3 The @import rule"/>
<link rel="help" href="http://www.w3.org/TR/css-cascade-3/#at-import"/>
<link rel="help" href="http://www.w3.org/TR/css-cascade-4/#at-import"/>
<link rel="help" href="http://www.w3.org/TR/CSS21/syndata.html#comments" title="4.1.9 Comments"/>
<link rel="help" href="http://www.w3.org/TR/CSS21/syndata.html#uri" title="4.3.4 URLs and URIs"/>
<link rel="match" href="../../../../../expected/wpt-import/css/CSS2/css1/c11-import-000-ref.xht" />

</head>
<body>
<p class="a">This text should be green.</p>
<p class="b">This text should be green.</p>
<p class="c">This text should be green.</p>
<p class="d">This text should be green.</p>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.a { color: green; }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.b { color: green; }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.c { color: red; }
Loading