Changeset 286668 in webkit

Timestamp:

Dec 8, 2021 11:16:05 AM (7 months ago)

Author:

Antti Koivisto

Message:

[CSS Cascade Layers] CSSImportRule.cssText doesn't include layer parameter
​https://bugs.webkit.org/show_bug.cgi?id=234010

Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

• web-platform-tests/css/css-cascade/parsing/layer-import-parsing-expected.txt:

Source/WebCore:

Fix serialization of import rules than include layer, like

@import url(foo.css) layer(A);

Also make the parsing of the layer() argument stricter.

• css/CSSImportRule.cpp:

(WebCore::CSSImportRule::cssText const):

Serialize the layer.

• css/CSSLayerBlockRule.cpp:

(WebCore::stringFromCascadeLayerName):

Serialize layer name segments as identifiers.

• css/parser/CSSParserImpl.cpp:

(WebCore::CSSParserImpl::consumeImportRule):

Fail parsing if there is anything left in the function argument after consuming the layer name.
If the parsing fails, revert and try the full string as media query.

Location:

trunk

Files:

• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/parsing/layer-import-parsing-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSImportRule+Layer.idl (modified) (1 diff)
• Source/WebCore/css/CSSImportRule.cpp (modified) (1 diff)
• Source/WebCore/css/CSSLayerBlockRule.cpp (modified) (2 diffs)
• Source/WebCore/css/parser/CSSParserImpl.cpp (modified) (1 diff)

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2021-12-08  Antti Koivisto  <antti@apple.com>
+
+        [CSS Cascade Layers] CSSImportRule.cssText doesn't include layer parameter
+        https://bugs.webkit.org/show_bug.cgi?id=234010
+
+        Reviewed by Simon Fraser.
+
+        * web-platform-tests/css/css-cascade/parsing/layer-import-parsing-expected.txt:
+
 2021-12-08  Antti Koivisto  <antti@apple.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-cascade/parsing/layer-import-parsing-expected.txt

@@ -1 +1 @@
 
-FAIL @import url("nonexist.css") layer; should be a valid layered import rule assert_equals: serialization should be canonical expected "@import url(\"nonexist.css\") layer;" but got "@import url(\"nonexist.css\");"
-FAIL @import url("nonexist.css") layer(A); should be a valid layered import rule assert_equals: serialization should be canonical expected "@import url(\"nonexist.css\") layer(A);" but got "@import url(\"nonexist.css\");"
-FAIL @import url("nonexist.css") layer(A.B); should be a valid layered import rule assert_equals: serialization should be canonical expected "@import url(\"nonexist.css\") layer(A.B);" but got "@import url(\"nonexist.css\");"
-FAIL @import url(nonexist.css) layer; should be a valid layered import rule assert_equals: serialization should be canonical expected "@import url(\"nonexist.css\") layer;" but got "@import url(\"nonexist.css\");"
-FAIL @import url(nonexist.css) layer(A); should be a valid layered import rule assert_equals: serialization should be canonical expected "@import url(\"nonexist.css\") layer(A);" but got "@import url(\"nonexist.css\");"
-FAIL @import url(nonexist.css) layer(A.B); should be a valid layered import rule assert_equals: serialization should be canonical expected "@import url(\"nonexist.css\") layer(A.B);" but got "@import url(\"nonexist.css\");"
-FAIL @import "nonexist.css" layer; should be a valid layered import rule assert_equals: serialization should be canonical expected "@import url(\"nonexist.css\") layer;" but got "@import url(\"nonexist.css\");"
-FAIL @import "nonexist.css" layer(A); should be a valid layered import rule assert_equals: serialization should be canonical expected "@import url(\"nonexist.css\") layer(A);" but got "@import url(\"nonexist.css\");"
-FAIL @import "nonexist.css" layer(A.B); should be a valid layered import rule assert_equals: serialization should be canonical expected "@import url(\"nonexist.css\") layer(A.B);" but got "@import url(\"nonexist.css\");"
-FAIL @import url("nonexist.css") layer(); should still be a valid import rule with an invalid layer declaration assert_not_equals: invalid layer declaration should be parsed as <general-enclosed> media query got disallowed value 0
-FAIL @import url("nonexist.css") layer(A B); should still be a valid import rule with an invalid layer declaration assert_not_equals: invalid layer declaration should be parsed as <general-enclosed> media query got disallowed value 0
-FAIL @import url("nonexist.css") layer(A . B); should still be a valid import rule with an invalid layer declaration assert_not_equals: invalid layer declaration should be parsed as <general-enclosed> media query got disallowed value 0
-FAIL @import url("nonexist.css") layer(A, B, C); should still be a valid import rule with an invalid layer declaration assert_not_equals: invalid layer declaration should be parsed as <general-enclosed> media query got disallowed value 0
+PASS @import url("nonexist.css") layer; should be a valid layered import rule
+PASS @import url("nonexist.css") layer(A); should be a valid layered import rule
+PASS @import url("nonexist.css") layer(A.B); should be a valid layered import rule
+PASS @import url(nonexist.css) layer; should be a valid layered import rule
+PASS @import url(nonexist.css) layer(A); should be a valid layered import rule
+PASS @import url(nonexist.css) layer(A.B); should be a valid layered import rule
+PASS @import "nonexist.css" layer; should be a valid layered import rule
+PASS @import "nonexist.css" layer(A); should be a valid layered import rule
+PASS @import "nonexist.css" layer(A.B); should be a valid layered import rule
+PASS @import url("nonexist.css") layer(); should still be a valid import rule with an invalid layer declaration
+PASS @import url("nonexist.css") layer(A B); should still be a valid import rule with an invalid layer declaration
+PASS @import url("nonexist.css") layer(A . B); should still be a valid import rule with an invalid layer declaration
+PASS @import url("nonexist.css") layer(A, B, C); should still be a valid import rule with an invalid layer declaration
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-12-08  Antti Koivisto  <antti@apple.com>
+
+        [CSS Cascade Layers] CSSImportRule.cssText doesn't include layer parameter
+        https://bugs.webkit.org/show_bug.cgi?id=234010
+
+        Reviewed by Simon Fraser.
+
+        Fix serialization of import rules than include layer, like
+
+        @import url(foo.css) layer(A);
+
+        Also make the parsing of the layer() argument stricter.
+
+        * css/CSSImportRule.cpp:
+        (WebCore::CSSImportRule::cssText const):
+
+        Serialize the layer.
+
+        * css/CSSLayerBlockRule.cpp:
+        (WebCore::stringFromCascadeLayerName):
+
+        Serialize layer name segments as identifiers.
+
+        * css/parser/CSSParserImpl.cpp:
+        (WebCore::CSSParserImpl::consumeImportRule):
+
+        Fail parsing if there is anything left in the function argument after consuming the layer name.
+        If the parsing fails, revert and try the full string as media query.
+
 2021-12-08  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebCore/css/CSSImportRule+Layer.idl

@@ -33 +33 @@
     Exposed=Window
 ] partial interface CSSImportRule {
-  readonly attribute CSSOMString? layerName;
+    readonly attribute CSSOMString? layerName;
 };

trunk/Source/WebCore/css/CSSImportRule.cpp

@@ -69 +69 @@
 String CSSImportRule::cssText() const
 {
+    StringBuilder builder;
+
+    builder.append("@import ", serializeURL(m_importRule.get().href()));
+
+    if (auto layerName = this->layerName(); !layerName.isNull()) {
+        if (layerName.isEmpty())
+            builder.append(" layer");
+        else
+            builder.append(" layer(", layerName, ')');
+    }
+
     if (auto queries = m_importRule.get().mediaQueries()) {
         if (auto mediaText = queries->mediaText(); !mediaText.isEmpty())
-            return makeString("@import url(\"", m_importRule.get().href(), "\") ", mediaText, ';');
+            builder.append(' ', mediaText);
     }
-    return makeString("@import url(\"", m_importRule.get().href(), "\");");
+
+    builder.append(';');
+
+    return builder.toString();
 }
 

trunk/Source/WebCore/css/CSSLayerBlockRule.cpp

@@ -31 +31 @@
 #include "CSSLayerBlockRule.h"
 
+#include "CSSMarkup.h"
 #include "CSSStyleSheet.h"
 #include "StyleRule.h"
@@ -76 +77 @@
     StringBuilder result;
     for (auto& segment : name) {
-        result.append(segment);
+        serializeIdentifier(segment, result);
         if (&segment != &name.last())
             result.append('.');

trunk/Source/WebCore/css/parser/CSSParserImpl.cpp

@@ -566 +566 @@
         auto& token = prelude.peek();
         if (token.type() == FunctionToken && equalIgnoringASCIICase(token.value(), "layer")) {
+            auto savedPreludeForFailure = prelude;
             auto contents = CSSPropertyParserHelpers::consumeFunction(prelude);
-            return consumeCascadeLayerName(contents, AllowAnonymous::No);
+            auto layerName = consumeCascadeLayerName(contents, AllowAnonymous::No);
+            if (!layerName || !contents.atEnd()) {
+                prelude = savedPreludeForFailure;
+                return { };
+            }
+            return layerName;
         }
         if (token.type() == IdentToken && equalIgnoringASCIICase(token.value(), "layer")) {