Directly check values of grid-template-rows and grid-template-columns…

… when serializing grid-template with grid areas

https://bugs.webkit.org/show_bug.cgi?id=260494
rdar://114224504

Reviewed by Tim Nguyen.

The grid-template shorthand has two different options for its syntax:
- [ <'grid-template-rows'> / <'grid-template-columns'> ]
- [ <line-names>? <string> <track-size>? <line-names>? ]+ [ / <explicit-track-list> ]?

In the latter version of the syntax, ShorthandSerializer would bail out
early (and not serialize any values) if grid-template-areas (the <string>
portion of the syntax) was not set by the grid-template shorthand. We
should instead check the values of grid-template-rows and grid-template-columns
directly to see if they can be expressed inside of this shorthand.

For the rows portion we build up the shorthand iteratively, so we can
just check if the value is a valid <track-size> and return an empty
string if it is not since that would mean the value could not be
expressed in the shorthand. A value being a valid <track-size> means that
it must be one of: minmax(), fit-content(), length-percentage, flex,
min-content, max-content, or auto. If it is not one of these then the
value is not a <track-size>.

For the columns portion we have to check the entire value of grid-template-columns
beforehand since we will end up appending the whole value to the end
of the shorthand. To check to see if a value is a valid <explicit-track-list>,
we need to make sure the value contains only <line-names>s and <track-size>s
with at least one <track-size> in the value.

* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-template-shorthand-areas-valid-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-template-shorthand-valid.html:
* Source/WebCore/css/ShorthandSerializer.cpp:
(WebCore::ShorthandSerializer::commonSerializationChecks):
(WebCore::ShorthandSerializer::serializeGridTemplate const):
* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::isGridBreadthIdent):
(WebCore::CSSPropertyParserHelpers::consumeGridBreadth):
* Source/WebCore/css/parser/CSSPropertyParserHelpers.h:

Canonical link: https://commits.webkit.org/267155@main

LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-template-shorthand-areas-valid-expected.txt

@@ -1,7 +1,7 @@
 
 PASS grid-template: none / 1px and "grid-template-areas: "a";" should be valid.
 PASS grid-template: none / none and "grid-template-areas: "a";" should be valid.
-FAIL grid-template: auto / 1px and "grid-template-areas: "a a a";" should be valid. assert_equals: expected "\"a a a\" / 1px" but got ""
-FAIL grid-template: auto / auto and "grid-template-areas: "a a a";" should be valid. assert_equals: expected "\"a a a\" / auto" but got ""
-FAIL grid-template: 10px 20px 30px / 40px 50px 60px 70px and "grid-template-areas: "a . b ." "c d . e" "f g h .";" should be valid. assert_equals: expected "\"a . b .\" 10px \"c d . e\" 20px \"f g h .\" 30px / 40px 50px 60px 70px" but got ""
+PASS grid-template: auto / 1px and "grid-template-areas: "a a a";" should be valid.
+PASS grid-template: auto / auto and "grid-template-areas: "a a a";" should be valid.
+PASS grid-template: 10px 20px 30px / 40px 50px 60px 70px and "grid-template-areas: "a . b ." "c d . e" "f g h .";" should be valid.
 

LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-template-shorthand-valid-expected.txt

@@ -27,6 +27,7 @@ PASS e.style['grid-template'] = "\"a\" [a] [b] \"b\"" should set the property va
 PASS e.style['grid-template'] = "\"a\" [a] \"b\"" should set the property value
 PASS e.style['grid-template'] = "\"a\" / 0" should set the property value
 PASS e.style['grid-template'] = "\"a\" 10px / 10px" should set the property value
+PASS e.style['grid-template'] = "\"a\" calc(100% - 10px) / calc(10px)" should set the property value
 PASS e.style['grid-template'] = "\"a\" [a] \"b\" 10px" should set the property value
 PASS e.style['grid-template'] = "\"a\" [a] \"b\" 10px [a]" should set the property value
 PASS e.style['grid-template'] = "\"a\" [a] [a] \"b\" 10px" should set the property value

LayoutTests/imported/w3c/web-platform-tests/css/css-grid/parsing/grid-template-shorthand-valid.html

@@ -41,6 +41,7 @@
 test_valid_value("grid-template", '"a" [a] "b"');
 test_valid_value("grid-template", '"a" / 0', '"a" / 0px');
 test_valid_value("grid-template", '"a" 10px / 10px');
+test_valid_value("grid-template", '"a" calc(100% - 10px) / calc(10px)');
 test_valid_value("grid-template", '"a" [a] "b" 10px');
 test_valid_value("grid-template", '"a" [a] "b" 10px [a]');
 test_valid_value("grid-template", '"a" [a] [a] "b" 10px', '"a" [a a] "b" 10px');

Source/WebCore/css/ShorthandSerializer.cpp

@@ -126,7 +126,6 @@ class ShorthandSerializer {
     StylePropertyShorthand m_shorthand;
     RefPtr<CSSValue> m_longhandValues[maxShorthandLength];
     String m_result;
-    bool m_gridTemplateAreasWasSetFromShorthand { false };
     bool m_commonSerializationChecksSuppliedResult { false };
 };
 
@@ -207,11 +206,6 @@ bool ShorthandSerializer::commonSerializationChecks(const StyleProperties& prope
             return true;
         importance = isImportant;
 
-        // Record one bit of data besides the property values that's needed for serializatin.
-        // FIXME: Remove this.
-        if (longhand == CSSPropertyGridTemplateAreas && property.toCSSProperty().isSetFromShorthand())
-            m_gridTemplateAreasWasSetFromShorthand = true;
-
         auto value = property.value();
 
         // Don't serialize if longhands have different CSS-wide keywords.
@@ -1084,12 +1078,37 @@ String ShorthandSerializer::serializeGridTemplate() const
         return serializeLonghands(2, " / ");
     }
 
-    // FIXME: We must remove the check below and instead check that values can be represented.
-    // We only want to try serializing the interleaved areas/templates
-    // format if it was set from this shorthand, since that automatically
-    // excludes values that can't be represented in this format (subgrid,
-    // and the repeat() function).
-    if (!m_gridTemplateAreasWasSetFromShorthand)
+    // Depending on the values of grid-template-rows and grid-template-columns, we may not
+    // be able to completely represent them in this version of the grid-template shorthand.
+    // We need to make sure that those values map to a value the syntax supports
+    auto isValidTrackSize = [&] (const CSSValue& value) {
+        auto valueID = value.valueID();
+        if (CSSPropertyParserHelpers::identMatches<CSSValueFitContent, CSSValueMinmax>(valueID) || CSSPropertyParserHelpers::isGridBreadthIdent(valueID))
+            return true;
+        if (const auto* primitiveValue = dynamicDowncast<CSSPrimitiveValue>(value))
+            return primitiveValue->isLength() || primitiveValue->isPercentage() || primitiveValue->isCalculated() || primitiveValue->isFlex();
+        return false;
+    };
+    auto isValidExplicitTrackList = [&] (const CSSValue& value) {
+        if (!value.isValueList())
+            return isValidTrackSize(value);
+
+        const auto& values = downcast<CSSValueList>(value);
+        auto hasAtLeastOneTrackSize = false;
+        for (const auto& value : values) {
+            if (isValidTrackSize(value))
+                hasAtLeastOneTrackSize = true;
+            else if (!value.isGridLineNamesValue())
+                return false;
+        }
+        return hasAtLeastOneTrackSize;
+    };
+
+    // For the rows we need to return early if the value of grid-template-rows is none because otherwise
+    // we will iteratively build up that portion of the shorthand and check to see if the value is a valid
+    // <track-size> at the appropriate position in the shorthand. For the columns we must check the entire
+    // value of grid-template-columns beforehand because we append the value as a whole to the shorthand
+    if (isLonghandValueNone(rowsIndex) || (!isLonghandValueNone(columnsIndex) && !isValidExplicitTrackList(longhandValue(columnsIndex))))
         return String();
 
     StringBuilder result;
@@ -1101,6 +1120,8 @@ String ShorthandSerializer::serializeGridTemplate() const
             result.append(lineNames->customCSSText());
         else {
             result.append('"', areasValue->stringForRow(row), '"');
+            if (!isValidTrackSize(currentValue))
+                return String();
             if (!isValueID(currentValue, CSSValueAuto))
                 result.append(' ', currentValue.cssText());
             row++;

Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp

@@ -7522,6 +7522,11 @@ bool isSelfPositionOrLeftOrRightKeyword(CSSValueID id)
     return isSelfPositionKeyword(id) || isLeftOrRightKeyword(id);
 }
 
+bool isGridBreadthIdent(CSSValueID id)
+{
+    return identMatches<CSSValueMinContent, CSSValueWebkitMinContent, CSSValueMaxContent, CSSValueWebkitMaxContent, CSSValueAuto>(id);
+}
+
 RefPtr<CSSValue> consumeSelfPositionOverflowPosition(CSSParserTokenRange& range, IsPositionKeyword isPositionKeyword)
 {
     ASSERT(isPositionKeyword);
@@ -7747,7 +7752,7 @@ bool parseGridTemplateAreasRow(StringView gridRowNames, NamedGridAreaMap& gridAr
 static RefPtr<CSSPrimitiveValue> consumeGridBreadth(CSSParserTokenRange& range, CSSParserMode mode)
 {
     const CSSParserToken& token = range.peek();
-    if (identMatches<CSSValueMinContent, CSSValueWebkitMinContent, CSSValueMaxContent, CSSValueWebkitMaxContent, CSSValueAuto>(token.id()))
+    if (isGridBreadthIdent(token.id()))
         return consumeIdent(range);
     if (token.type() == DimensionToken && token.unitType() == CSSUnitType::CSS_FR) {
         if (range.peek().numericValue() < 0)

Source/WebCore/css/parser/CSSPropertyParserHelpers.h

@@ -214,6 +214,7 @@ bool isContentPositionKeyword(CSSValueID);
 bool isContentPositionOrLeftOrRightKeyword(CSSValueID);
 bool isSelfPositionKeyword(CSSValueID);
 bool isSelfPositionOrLeftOrRightKeyword(CSSValueID);
+bool isGridBreadthIdent(CSSValueID);
 
 RefPtr<CSSValueList> consumeAlignTracks(CSSParserTokenRange&);
 RefPtr<CSSValueList> consumeJustifyTracks(CSSParserTokenRange&);