Changeset 279358 in webkit for trunk/Source/WebCore/css/CSSCustomPropertyValue.cpp

Timestamp:

Jun 28, 2021, 6:34:20 PM (4 years ago)

Author:

Darin Adler

Message:

CSS parser "consume declaration" algorithm does not handle whitespace correctly
​https://bugs.webkit.org/show_bug.cgi?id=227368

Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

• web-platform-tests/css/css-properties-values-api/at-property-animation-expected.txt:
• web-platform-tests/css/css-properties-values-api/at-property-expected.txt:
• web-platform-tests/css/css-properties-values-api/at-property-shadow-expected.txt:
• web-platform-tests/css/css-properties-values-api/determine-registration-expected.txt:
• web-platform-tests/css/css-properties-values-api/registered-property-cssom-expected.txt:
• web-platform-tests/css/css-properties-values-api/var-reference-registered-properties-expected.txt:

Regenerated to reflect the whitespace trimming: one new pass, no new failures. Some of
these tests will also need updates to match the newer CSS specification. Not doing those
here right now.

• web-platform-tests/css/css-properties-values-api/at-property.html:

Pulled down a newer version of this test from the WPT repository with expectations in line
with newer CSS specification.

• web-platform-tests/css/css-syntax/declarations-trim-whitespace-expected.txt:
• web-platform-tests/css/css-variables/variable-cssText-expected.txt:

Expect most tests to pass instead of fail. There are still some failures. Given my reading
of the CSS specification I suspect it is the tests that are incorrect.

• web-platform-tests/css/css-variables/variable-definition-expected.txt:
• web-platform-tests/css/css-variables/variable-definition.html:

Pulled down a newer version of this test from the WPT repository with expectations in line
with newer CSS specification. Some tests are still failing because of expectations about
trailing whitespace. Given my reading of the CSS specification I suspect it is the tests
that are incorrect.

• web-platform-tests/css/css-variables/variable-reference-expected.txt:
• web-platform-tests/css/css-variables/variable-reference.html:
• web-platform-tests/css/css-variables/variable-substitution-variable-declaration-expected.txt:
• web-platform-tests/css/css-variables/variable-substitution-variable-declaration.html:

Pulled down a newer version of these tests from the WPT repository with expectations in
line with newer CSS specification.

• web-platform-tests/css/cssom/variable-names-expected.txt: Expect tests to pass, not fail.

Source/WebCore:

Test: imported/w3c/web-platform-tests/css/css-syntax/declarations-trim-whitespace.html

To avoid creating regressions in CSS variable behavior, had to make
changes there to handle whitespace correctly at the same time as the
change to the "consume declaration" algorithm.

• css/CSSComputedStyleDeclaration.cpp:

(WebCore::ComputedStyleExtractor::customPropertyValue): Restructured
to not have a case for each variant custom value type. This lets us
support the new Empty value type without extra code here.

• css/CSSCustomPropertyValue.cpp:

(WebCore::CSSCustomPropertyValue::createEmpty): Added. Used for a new
Empty value type for properties that have empty string as their value,
which is a new capability added to the CSS specification.
(WebCore::CSSCustomPropertyValue::equals const): Removed unneeded pointer
comparison optimization. Added support for Empty value.
(WebCore::CSSCustomPropertyValue::customCSSText const): Updated to use
null string for m_stringValue instead of a separate m_serialized flag.
Added support for Empty value.
(WebCore::CSSCustomPropertyValue::tokens const): Added support for
Empty value. Also call directly to customCSSText instead of calling
through cssText.

• css/CSSCustomPropertyValue.h: Updated for the above, adding Empty value.

Removed m_serialized. Greatly simplified the copy constructor since Ref
now has a copy constructor.

• css/CSSVariableReferenceValue.cpp:

(WebCore::resolveVariableFallback): Consume whitespace after the comma,
matching what is now called for in the CSS specification.

• css/calc/CSSCalcExpressionNodeParser.cpp:

(WebCore::CSSCalcExpressionNodeParser::parseCalcFunction): Use
consumeCommaIncludingWhitespace to simplify the code. No behavior change,
just refactoring.

• css/parser/CSSParserImpl.cpp:

(WebCore::CSSParserImpl::consumeDeclaration): Updated algorithm to match
the CSS specification, trimming whitespace correctly.
(WebCore::CSSParserImpl::consumeCustomPropertyValue): Added support for
a custom property value with no declaration value, as now called for in
the CSS specification, using CSSCustomPropertyValue::createEmpty.

• css/parser/CSSVariableParser.cpp:

(WebCore::isValidVariableReference): Allow empty fallback, as now called
for in the CSS specification.
(WebCore::isValidConstantReference): Ditto.

LayoutTests:

• css-custom-properties-api/inline.html: Update expectations to expect leading

whitespace to be trimmed.

• fast/css/variables/test-suite/011.html: Updated to expect a variable reference

with no fallback tokens to be valid. Change in the CSS specification since this
test was written.

• fast/css/variables/test-suite/013.html: Ditto.
• fast/css/variables/test-suite/041.html: Ditto.
• fast/css/variables/test-suite/043.html: Ditto.

• fast/css/variables/test-suite/061.html: Updated to expect a value with no tokens

to be valid. Change in the CSS specification since this test was written.

• fast/css/variables/test-suite/100.html: Updated to expect a variable reference

with no fallback tokens to be valid. Change in the CSS specification since this
test was written.

• fast/css/variables/test-suite/105.html: Ditto.
• fast/css/variables/test-suite/136.html: Ditto.

• fast/css/variables/test-suite/170.html: Updated to expect a value with no tokens

to be valid. Change in the CSS specification since this test was written.

• fast/css/variables/test-suite/171.html: Ditto.

• platform/mac/TestExpectations: Removed a line for a test that no longer exists

(not changed in this patch).

File:

• trunk/Source/WebCore/css/CSSCustomPropertyValue.cpp (modified) (4 diffs)

trunk/Source/WebCore/css/CSSCustomPropertyValue.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2016 Apple Inc.  All rights reserved.
+ * Copyright (C) 2016-2021 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -26 +26 @@
 #include "config.h"
 #include "CSSCustomPropertyValue.h"
+
 #include "CSSTokenizer.h"
 
 namespace WebCore {
 
+Ref<CSSCustomPropertyValue> CSSCustomPropertyValue::createEmpty(const AtomString& name)
+{
+    return adoptRef(*new CSSCustomPropertyValue(name, Monostate { }));
+}
+
 bool CSSCustomPropertyValue::equals(const CSSCustomPropertyValue& other) const
 {
-    if (this == &other)
-        return true;
     if (m_name != other.m_name || m_value.index() != other.m_value.index())
         return false;
-    return WTF::switchOn(m_value, [&](const Ref<CSSVariableReferenceValue>& value) {
+    return WTF::switchOn(m_value, [&](const Monostate&) {
+        return true;
+    }, [&](const Ref<CSSVariableReferenceValue>& value) {
         return value.get() == WTF::get<Ref<CSSVariableReferenceValue>>(other.m_value).get();
     }, [&](const CSSValueID& value) {
@@ -51 +57 @@
 String CSSCustomPropertyValue::customCSSText() const
 {
-    if (!m_serialized) {
-        m_serialized = true;
-
-        WTF::switchOn(m_value, [&](const Ref<CSSVariableReferenceValue>& value) {
+    if (m_stringValue.isNull()) {
+        WTF::switchOn(m_value, [&](const Monostate&) {
+            m_stringValue = emptyString();
+        }, [&](const Ref<CSSVariableReferenceValue>& value) {
             m_stringValue = value->cssText();
         }, [&](const CSSValueID& value) {
@@ -72 +78 @@
 {
     Vector<CSSParserToken> result;
-
-    WTF::switchOn(m_value, [&](const Ref<CSSVariableReferenceValue>&) {
+    WTF::switchOn(m_value, [&](const Monostate&) {
+        // Do nothing.
+    }, [&](const Ref<CSSVariableReferenceValue>&) {
         ASSERT_NOT_REACHED();
     }, [&](const CSSValueID&) {
-        // Do nothing
+        // Do nothing.
     }, [&](const Ref<CSSVariableData>& value) {
         result.appendVector(value->tokens());
-    }, [&](const Length&) {
-        CSSTokenizer tokenizer(cssText());
-
-        auto tokenizerRange = tokenizer.tokenRange();
-        while (!tokenizerRange.atEnd())
-            result.append(tokenizerRange.consume());
-    }, [&](const Ref<StyleImage>&) {
-        CSSTokenizer tokenizer(cssText());
-
+    }, [&](auto&) {
+        CSSTokenizer tokenizer(customCSSText());
         auto tokenizerRange = tokenizer.tokenRange();
         while (!tokenizerRange.atEnd())
             result.append(tokenizerRange.consume());
     });
-
     return result;
 }