[css-grid] Disallow repeat() in grid-template shorthand
authorrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Jul 2016 08:57:50 +0000 (08:57 +0000)
committerrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Jul 2016 08:57:50 +0000 (08:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=159200

Reviewed by Sergio Villar Senin.

Source/WebCore:

As discussed on www-style, "repeat()" notation shouldn't be allowed
in the ASCII branch of the grid-template shorthand.
https://lists.w3.org/Archives/Public/www-style/2016May/0193.html

The patch uses an enum to invalidate "repeat()" when parsing
the grid-template shorthand.

Test: fast/css-grid-layout/grid-template-shorthand-get-set.html

* css/CSSParser.cpp:
(WebCore::CSSParser::parseGridTemplateColumns): Add enum.
(WebCore::CSSParser::parseGridTemplateRowsAndAreasAndColumns): Pass "DisallowRepeat"
when calling parseGridTemplateColumns().
(WebCore::CSSParser::parseGridTrackList): Use enum to allow/disallow repeat.
* css/CSSParser.h: Define the new enum and modify method signatures to use it,
setting it to "AllowRepeat" by default.

LayoutTests:

Modified test to follow the new behavior including new cases.

* fast/css-grid-layout/grid-template-shorthand-get-set-expected.txt:
* fast/css-grid-layout/grid-template-shorthand-get-set.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@202972 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css-grid-layout/grid-template-shorthand-get-set-expected.txt
LayoutTests/fast/css-grid-layout/grid-template-shorthand-get-set.html
Source/WebCore/ChangeLog
Source/WebCore/css/CSSParser.cpp
Source/WebCore/css/CSSParser.h

index 54f55cb..ce0fd01 100644 (file)
@@ -1,3 +1,15 @@
+2016-07-08  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [css-grid] Disallow repeat() in grid-template shorthand
+        https://bugs.webkit.org/show_bug.cgi?id=159200
+
+        Reviewed by Sergio Villar Senin.
+
+        Modified test to follow the new behavior including new cases.
+
+        * fast/css-grid-layout/grid-template-shorthand-get-set-expected.txt:
+        * fast/css-grid-layout/grid-template-shorthand-get-set.html:
+
 2016-07-08  Frederic Wang  <fwang@igalia.com>
 
         Add support for movablelimits.
index 3762254..f271ec4 100644 (file)
@@ -40,10 +40,10 @@ PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRows, '
 PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRowsWithoutRowsSizes, '').getPropertyValue('grid-template-columns') is "10px"
 PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRowsWithoutRowsSizes, '').getPropertyValue('grid-template-rows') is "[head1] 0px [tail1 head2] 0px [tail2]"
 PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRowsWithoutRowsSizes, '').getPropertyValue('grid-template-areas') is "\"a\" \"b\""
-PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRowsAndColumns, '').getPropertyValue('grid-template-columns') is "[first] 10px [nav nav2] 15px [nav nav2] 15px"
+PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRowsAndColumns, '').getPropertyValue('grid-template-columns') is "[first] 10px [nav nav2] 15px [nav] 15px [last]"
 PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRowsAndColumns, '').getPropertyValue('grid-template-rows') is "100px [nav nav2] 25px [nav nav2] 25px [last]"
 PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRowsAndColumns, '').getPropertyValue('grid-template-areas') is "\"a b c\" \"d e f\" \"g h i\""
-PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRowsAndColumnsWithoutRowsSizes, '').getPropertyValue('grid-template-columns') is "[first] 10px [nav nav2] 15px [nav nav2] 15px"
+PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRowsAndColumnsWithoutRowsSizes, '').getPropertyValue('grid-template-columns') is "[first] 10px [nav nav2] 15px [nav] 15px [last]"
 PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRowsAndColumnsWithoutRowsSizes, '').getPropertyValue('grid-template-rows') is "0px [nav nav2] 0px [nav nav2] 0px [last]"
 PASS window.getComputedStyle(gridTemplateComplexFormWithLineNamesMultipleRowsAndColumnsWithoutRowsSizes, '').getPropertyValue('grid-template-areas') is "\"a b c\" \"d e f\" \"g h i\""
 PASS window.getComputedStyle(gridTemplateComplexFormWithAuto, '').getPropertyValue('grid-template-columns') is "10px"
@@ -129,6 +129,15 @@ PASS window.getComputedStyle(gridTemplateNoColumnsRowWithTwoEmptyTrailingLineNam
 PASS window.getComputedStyle(gridTemplateNoColumnsRowWithEmptyTrailingLineNamesAndNonEmptyLeadingLineNames, '').getPropertyValue('grid-template-columns') is "none"
 PASS window.getComputedStyle(gridTemplateNoColumnsRowWithEmptyTrailingLineNamesAndNonEmptyLeadingLineNames, '').getPropertyValue('grid-template-rows') is "none"
 PASS window.getComputedStyle(gridTemplateNoColumnsRowWithEmptyTrailingLineNamesAndNonEmptyLeadingLineNames, '').getPropertyValue('grid-template-areas') is "none"
+PASS window.getComputedStyle(gridTemplateRepeat, '').getPropertyValue('grid-template-columns') is "none"
+PASS window.getComputedStyle(gridTemplateRepeat, '').getPropertyValue('grid-template-rows') is "none"
+PASS window.getComputedStyle(gridTemplateRepeat, '').getPropertyValue('grid-template-areas') is "none"
+PASS window.getComputedStyle(gridTemplateRepeatAutoFill, '').getPropertyValue('grid-template-columns') is "none"
+PASS window.getComputedStyle(gridTemplateRepeatAutoFill, '').getPropertyValue('grid-template-rows') is "none"
+PASS window.getComputedStyle(gridTemplateRepeatAutoFill, '').getPropertyValue('grid-template-areas') is "none"
+PASS window.getComputedStyle(gridTemplateRepeatAutoFit, '').getPropertyValue('grid-template-columns') is "none"
+PASS window.getComputedStyle(gridTemplateRepeatAutoFit, '').getPropertyValue('grid-template-rows') is "none"
+PASS window.getComputedStyle(gridTemplateRepeatAutoFit, '').getPropertyValue('grid-template-areas') is "none"
 
 Test the initial value
 PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-columns') is "none"
@@ -197,6 +206,15 @@ PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-areas'
 PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-columns') is "none"
 PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-rows') is "none"
 PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-areas') is "none"
+PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-columns') is "none"
+PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-rows') is "none"
+PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-areas') is "none"
+PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-columns') is "none"
+PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-rows') is "none"
+PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-areas') is "none"
+PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-columns') is "none"
+PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-rows') is "none"
+PASS window.getComputedStyle(element, '').getPropertyValue('grid-template-areas') is "none"
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 65e0711..e53bb21 100644 (file)
 #gridTemplateComplexFormWithLineNamesMultipleRowsAndColumns {
     grid-template:        "a b c" 100px [nav]
                            [nav2] "d e f" 25px  [nav]
-                           [nav2] "g h i" 25px  [last] / [first] 10px repeat(2, [nav nav2] 15px);
+                           [nav2] "g h i" 25px  [last] / [first] 10px [nav nav2] 15px [nav] 15px [last];
 }
 #gridTemplateComplexFormWithLineNamesMultipleRowsAndColumnsWithoutRowsSizes {
     grid-template:        "a b c" [nav]
                            [nav2] "d e f" [nav]
-                           [nav2] "g h i" [last] / [first] 10px repeat(2, [nav nav2] 15px);
+                           [nav2] "g h i" [last] / [first] 10px [nav nav2] 15px [nav] 15px [last];
 }
 #gridTemplateComplexFormWithAuto {
     grid-template: "a" / 10px;
 #gridTemplateNoColumnsRowWithEmptyTrailingLineNamesAndNonEmptyLeadingLineNames {
     grid-template: [first] "a" auto [] [tail];
 }
+#gridTemplateRepeat {
+    grid-template: "a" / repeat(1, 100px);
+}
+#gridTemplateRepeatAutoFill {
+    grid-template: "a" / repeat(auto-fill, 100px);
+}
+#gridTemplateRepeatAutoFit {
+    grid-template: "a" / repeat(auto-fit, 100px);
+}
 
 </style>
 <script src="../../resources/js-test.js"></script>
 <div class="grid" id="gridTemplateComplexFormColumnsNotParsing2"></div>
 <div class="grid" id="gridTemplateComplexFormWithNoneColumns"></div>
 <div class="grid" id="gridTemplateNoColumnsRowWithTwoEmptyTrailingLineNames"></div>
+<div class="grid" id="gridTemplateRepeat"></div>
+<div class="grid" id="gridTemplateRepeatAutoFill"></div>
+<div class="grid" id="gridTemplateRepeatAutoFit"></div>
 <script src="resources/grid-template-shorthand-parsing-utils.js"></script>
 <script>
     description("This test checks that the 'grid-template' shorthand is properly parsed and the longhand properties correctly assigned.");
     testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormWithLineNamesMultipleColumnsWithoutRowSize"), "10px 20px", "[head] 0px [tail]", '"a b"');
     testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormWithLineNamesMultipleRows"), "10px", "[head1] 15px [tail1 head2] 20px [tail2]", '"a" "b"');
     testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormWithLineNamesMultipleRowsWithoutRowsSizes"), "10px", "[head1] 0px [tail1 head2] 0px [tail2]", '"a" "b"');
-    testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormWithLineNamesMultipleRowsAndColumns"), "[first] 10px [nav nav2] 15px [nav nav2] 15px", "100px [nav nav2] 25px [nav nav2] 25px [last]", '"a b c" "d e f" "g h i"');
-    testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormWithLineNamesMultipleRowsAndColumnsWithoutRowsSizes"), "[first] 10px [nav nav2] 15px [nav nav2] 15px", "0px [nav nav2] 0px [nav nav2] 0px [last]", '"a b c" "d e f" "g h i"');
+    testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormWithLineNamesMultipleRowsAndColumns"), "[first] 10px [nav nav2] 15px [nav] 15px [last]", "100px [nav nav2] 25px [nav nav2] 25px [last]", '"a b c" "d e f" "g h i"');
+    testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormWithLineNamesMultipleRowsAndColumnsWithoutRowsSizes"), "[first] 10px [nav nav2] 15px [nav] 15px [last]", "0px [nav nav2] 0px [nav nav2] 0px [last]", '"a b c" "d e f" "g h i"');
     testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormWithAuto"), "10px", "0px", '"a"');
     testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormOnlyAreas"), "0px", "0px", '"a"');
     testGridDefinitionsValues(document.getElementById("gridTemplateNoColumnsRowWithEmptyTrailingLineNames"), "0px", "[first] 0px", '"a"');
     testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormWithNoneColumns"), "none", "none", "none");
     testGridDefinitionsValues(document.getElementById("gridTemplateNoColumnsRowWithTwoEmptyTrailingLineNames"), "none", "none", "none");
     testGridDefinitionsValues(document.getElementById("gridTemplateNoColumnsRowWithEmptyTrailingLineNamesAndNonEmptyLeadingLineNames"), "none", "none", "none");
+    testGridDefinitionsValues(document.getElementById("gridTemplateRepeat"), "none", "none", "none");
+    testGridDefinitionsValues(document.getElementById("gridTemplateRepeatAutoFill"), "none", "none", "none");
+    testGridDefinitionsValues(document.getElementById("gridTemplateRepeatAutoFit"), "none", "none", "none");
 
     debug("");
     debug("Test the initial value");
     testGridDefinitionsSetBadJSValues("15px");
     testGridDefinitionsSetBadJSValues("20px none / 15px");
     testGridDefinitionsSetBadJSValues("10px 'a' / 25px");
+    testGridDefinitionsSetBadJSValues("'a b' / 100px repeat(1, 200px)");
+    testGridDefinitionsSetBadJSValues("'a b c' / repeat(2, 200px) 100px");
+    testGridDefinitionsSetBadJSValues("'a b c d' / 100px repeat(auto-fill, 200px) 100px");
 
 </script>
 </body>
index 5a038df..4a6ea0b 100644 (file)
@@ -1,3 +1,27 @@
+2016-07-08  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [css-grid] Disallow repeat() in grid-template shorthand
+        https://bugs.webkit.org/show_bug.cgi?id=159200
+
+        Reviewed by Sergio Villar Senin.
+
+        As discussed on www-style, "repeat()" notation shouldn't be allowed
+        in the ASCII branch of the grid-template shorthand.
+        https://lists.w3.org/Archives/Public/www-style/2016May/0193.html
+
+        The patch uses an enum to invalidate "repeat()" when parsing
+        the grid-template shorthand.
+
+        Test: fast/css-grid-layout/grid-template-shorthand-get-set.html
+
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseGridTemplateColumns): Add enum.
+        (WebCore::CSSParser::parseGridTemplateRowsAndAreasAndColumns): Pass "DisallowRepeat"
+        when calling parseGridTemplateColumns().
+        (WebCore::CSSParser::parseGridTrackList): Use enum to allow/disallow repeat.
+        * css/CSSParser.h: Define the new enum and modify method signatures to use it,
+        setting it to "AllowRepeat" by default.
+
 2016-07-08  Frederic Wang  <fwang@igalia.com>
 
         Add support for movablelimits.
index 1eac879..64d4c3d 100644 (file)
@@ -5631,13 +5631,13 @@ bool CSSParser::parseGridGapShorthand(bool important)
     return true;
 }
 
-RefPtr<CSSValue> CSSParser::parseGridTemplateColumns()
+RefPtr<CSSValue> CSSParser::parseGridTemplateColumns(TrackListType trackListType)
 {
     ASSERT(isCSSGridLayoutEnabled());
 
     if (!(m_valueList->current() && isForwardSlashOperator(*m_valueList->current()) && m_valueList->next()))
         return nullptr;
-    if (auto columnsValue = parseGridTrackList()) {
+    if (auto columnsValue = parseGridTrackList(trackListType)) {
         if (m_valueList->current())
             return nullptr;
         return columnsValue;
@@ -5690,11 +5690,11 @@ bool CSSParser::parseGridTemplateRowsAndAreasAndColumns(bool important)
             trailingIdentWasAdded = parseGridLineNames(*m_valueList, templateRows);
     }
 
-    // [<track-list> /]?
+    // [/ <explicit-track-list> ]?
     RefPtr<CSSValue> templateColumns;
     if (m_valueList->current()) {
         ASSERT(isForwardSlashOperator(*m_valueList->current()));
-        templateColumns = parseGridTemplateColumns();
+        templateColumns = parseGridTemplateColumns(DisallowRepeat);
         if (!templateColumns)
             return false;
         // The template-columns <track-list> can't be 'none'.
@@ -5930,7 +5930,7 @@ static bool isGridTrackFixedSized(const CSSValue& value)
     return isGridTrackFixedSized(min) || isGridTrackFixedSized(max);
 }
 
-RefPtr<CSSValue> CSSParser::parseGridTrackList()
+RefPtr<CSSValue> CSSParser::parseGridTrackList(TrackListType trackListType)
 {
     ASSERT(isCSSGridLayoutEnabled());
 
@@ -5953,6 +5953,8 @@ RefPtr<CSSValue> CSSParser::parseGridTrackList()
         if (isForwardSlashOperator(*currentValue))
             break;
         if (currentValue->unit == CSSParserValue::Function && equalLettersIgnoringASCIICase(currentValue->function->name, "repeat(")) {
+            if (trackListType == DisallowRepeat)
+                return nullptr;
             bool isAutoRepeat;
             if (!parseGridTrackRepeatFunction(values, isAutoRepeat, allTracksAreFixedSized))
                 return nullptr;
index 26837f5..235d1e1 100644 (file)
@@ -235,14 +235,15 @@ public:
     bool isCSSGridLayoutEnabled() const;
     RefPtr<CSSValue> parseGridPosition();
     bool parseGridItemPositionShorthand(CSSPropertyID, bool important);
-    RefPtr<CSSValue> parseGridTemplateColumns();
+    enum TrackListType { AllowRepeat, DisallowRepeat };
+    RefPtr<CSSValue> parseGridTemplateColumns(TrackListType = AllowRepeat);
     bool parseGridTemplateRowsAndAreasAndColumns(bool important);
     bool parseGridTemplateShorthand(bool important);
     bool parseGridShorthand(bool important);
     bool parseGridAreaShorthand(bool important);
     bool parseGridGapShorthand(bool important);
     bool parseSingleGridAreaLonghand(RefPtr<CSSValue>&);
-    RefPtr<CSSValue> parseGridTrackList();
+    RefPtr<CSSValue> parseGridTrackList(TrackListType = AllowRepeat);
     bool parseGridTrackRepeatFunction(CSSValueList&, bool& isAutoRepeat, bool& allTracksAreFixedSized);
     RefPtr<CSSValue> parseGridTrackSize(CSSParserValueList& inputList);
     RefPtr<CSSPrimitiveValue> parseGridBreadth(CSSParserValue&);