[CSS Grid Layout] Crash at CSSParser::parseGridTemplateRowsAndAreas
authorsvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Sep 2014 09:12:18 +0000 (09:12 +0000)
committersvillar@igalia.com <svillar@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Sep 2014 09:12:18 +0000 (09:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136778

Reviewed by Darin Adler.

Source/WebCore:

An empty list of grid line names (represented by "()") does not
add anything to the list of parsed values. That's why trying to
concatenate an adjacent list of grid line names was failing,
because we were trying to concatenate a list with the last parsed
CSSValue which was not the expected grid line names list.

* css/CSSParser.cpp:
(WebCore::CSSParser::parseGridTemplateRowsAndAreas):
(WebCore::CSSParser::parseGridLineNames):
* css/CSSParser.h:

LayoutTests:

Added some new test cases to verify that we properly handle empty
lists of grid line names.

* 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@173615 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 03f92c8..bba8596 100644 (file)
@@ -1,3 +1,16 @@
+2014-09-12  Sergio Villar Senin  <svillar@igalia.com>
+
+        [CSS Grid Layout] Crash at CSSParser::parseGridTemplateRowsAndAreas
+        https://bugs.webkit.org/show_bug.cgi?id=136778
+
+        Reviewed by Darin Adler.
+
+        Added some new test cases to verify that we properly handle empty
+        lists of grid line names.
+
+        * fast/css-grid-layout/grid-template-shorthand-get-set-expected.txt:
+        * fast/css-grid-layout/grid-template-shorthand-get-set.html:
+
 2014-09-10  Jon Honeycutt  <jhoneycutt@apple.com>
 
         Re-add the request autocomplete feature
index fd03ee2..294593e 100644 (file)
@@ -40,6 +40,9 @@ PASS window.getComputedStyle(gridTemplateComplexFormWithAuto, '').getPropertyVal
 PASS window.getComputedStyle(gridTemplateComplexFormOnlyAreas, '').getPropertyValue('-webkit-grid-template-columns') is "none"
 PASS window.getComputedStyle(gridTemplateComplexFormOnlyAreas, '').getPropertyValue('-webkit-grid-template-rows') is "0px"
 PASS window.getComputedStyle(gridTemplateComplexFormOnlyAreas, '').getPropertyValue('-webkit-grid-template-areas') is "\"a\""
+PASS window.getComputedStyle(gridTemplateNoColumnsRowWithEmptyTrailingLineNames, '').getPropertyValue('-webkit-grid-template-columns') is "none"
+PASS window.getComputedStyle(gridTemplateNoColumnsRowWithEmptyTrailingLineNames, '').getPropertyValue('-webkit-grid-template-rows') is "(first) 0px"
+PASS window.getComputedStyle(gridTemplateNoColumnsRowWithEmptyTrailingLineNames, '').getPropertyValue('-webkit-grid-template-areas') is "\"a\""
 
 Test getting wrong values for grid-template shorthand through CSS (they should resolve to the default: 'none')
 PASS window.getComputedStyle(gridTemplateMultipleSlash, '').getPropertyValue('-webkit-grid-template-columns') is "none"
@@ -102,6 +105,12 @@ PASS window.getComputedStyle(gridTemplateComplexFormColumnsNotParsing2, '').getP
 PASS window.getComputedStyle(gridTemplateComplexFormWithNoneColumns, '').getPropertyValue('-webkit-grid-template-columns') is "none"
 PASS window.getComputedStyle(gridTemplateComplexFormWithNoneColumns, '').getPropertyValue('-webkit-grid-template-rows') is "none"
 PASS window.getComputedStyle(gridTemplateComplexFormWithNoneColumns, '').getPropertyValue('-webkit-grid-template-areas') is "none"
+PASS window.getComputedStyle(gridTemplateNoColumnsRowWithTwoEmptyTrailingLineNames, '').getPropertyValue('-webkit-grid-template-columns') is "none"
+PASS window.getComputedStyle(gridTemplateNoColumnsRowWithTwoEmptyTrailingLineNames, '').getPropertyValue('-webkit-grid-template-rows') is "none"
+PASS window.getComputedStyle(gridTemplateNoColumnsRowWithTwoEmptyTrailingLineNames, '').getPropertyValue('-webkit-grid-template-areas') is "none"
+PASS window.getComputedStyle(gridTemplateNoColumnsRowWithEmptyTrailingLineNamesAndNonEmptyLeadingLineNames, '').getPropertyValue('-webkit-grid-template-columns') is "none"
+PASS window.getComputedStyle(gridTemplateNoColumnsRowWithEmptyTrailingLineNamesAndNonEmptyLeadingLineNames, '').getPropertyValue('-webkit-grid-template-rows') is "none"
+PASS window.getComputedStyle(gridTemplateNoColumnsRowWithEmptyTrailingLineNamesAndNonEmptyLeadingLineNames, '').getPropertyValue('-webkit-grid-template-areas') is "none"
 
 Test the initial value
 PASS window.getComputedStyle(element, '').getPropertyValue('-webkit-grid-template-columns') is "none"
index 83aa559..1f65adb 100644 (file)
@@ -42,6 +42,9 @@
 #gridTemplateComplexFormOnlyAreas {
     -webkit-grid-template: "a";
 }
+#gridTemplateNoColumnsRowWithEmptyTrailingLineNames {
+    -webkit-grid-template: (first) "a" auto ();
+}
 
 /* Bad values. */
 
 #gridTemplateComplexFormWithNoneColumns {
     -webkit-grid-template: none / "a" (name) 10px;
 }
+#gridTemplateNoColumnsRowWithTwoEmptyTrailingLineNames {
+    -webkit-grid-template: (first) "a" auto () ();
+}
+#gridTemplateNoColumnsRowWithEmptyTrailingLineNamesAndNonEmptyLeadingLineNames {
+    -webkit-grid-template: (first) "a" auto () (tail);
+}
 
 </style>
 <script src="../../resources/js-test.js"></script>
 <div class="grid" id="gridTemplateComplexFormWithLineNamesMultipleRowsAndColumns"></div>
 <div class="grid" id="gridTemplateComplexFormWithAuto"></div>
 <div class="grid" id="gridTemplateComplexFormOnlyAreas"></div>
+<div class="grid" id="gridTemplateNoColumnsRowWithEmptyTrailingLineNames"></div>
+<div class="grid" id="gridTemplateNoColumnsRowWithEmptyTrailingLineNamesAndNonEmptyLeadingLineNames"></div>
+<div class="grid" id="gridTemplateNoColumnsRowWithNonEmptyLeadingLineNamesAndEmptyTrailingLineNames"></div>
 <div class="grid" id="gridTemplateMultipleSlash"></div>
 <div class="grid" id="gridTemplateSimpleFormJustColumns"></div>
 <div class="grid" id="gridTemplateSimpleFormNoRows"></div>
 <div class="grid" id="gridTemplateComplexFormColumnsNotParsing1"></div>
 <div class="grid" id="gridTemplateComplexFormColumnsNotParsing2"></div>
 <div class="grid" id="gridTemplateComplexFormWithNoneColumns"></div>
+<div class="grid" id="gridTemplateNoColumnsRowWithTwoEmptyTrailingLineNames"></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("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("gridTemplateComplexFormWithAuto"), "10px", "0px", '"a"');
     testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormOnlyAreas"), "none", "0px", '"a"');
+    testGridDefinitionsValues(document.getElementById("gridTemplateNoColumnsRowWithEmptyTrailingLineNames"), "none", "(first) 0px", '"a"');
 
     debug("");
     debug("Test getting wrong values for grid-template shorthand through CSS (they should resolve to the default: 'none')");
     testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormColumnsNotParsing1"), "none", "none", "none");
     testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormColumnsNotParsing2"), "none", "none", "none");
     testGridDefinitionsValues(document.getElementById("gridTemplateComplexFormWithNoneColumns"), "none", "none", "none");
+    testGridDefinitionsValues(document.getElementById("gridTemplateNoColumnsRowWithTwoEmptyTrailingLineNames"), "none", "none", "none");
+    testGridDefinitionsValues(document.getElementById("gridTemplateNoColumnsRowWithEmptyTrailingLineNamesAndNonEmptyLeadingLineNames"), "none", "none", "none");
 
     debug("");
     debug("Test the initial value");
index eedfcbb..30888f1 100644 (file)
@@ -1,3 +1,21 @@
+2014-09-12  Sergio Villar Senin  <svillar@igalia.com>
+
+        [CSS Grid Layout] Crash at CSSParser::parseGridTemplateRowsAndAreas
+        https://bugs.webkit.org/show_bug.cgi?id=136778
+
+        Reviewed by Darin Adler.
+
+        An empty list of grid line names (represented by "()") does not
+        add anything to the list of parsed values. That's why trying to
+        concatenate an adjacent list of grid line names was failing,
+        because we were trying to concatenate a list with the last parsed
+        CSSValue which was not the expected grid line names list.
+
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseGridTemplateRowsAndAreas):
+        (WebCore::CSSParser::parseGridLineNames):
+        * css/CSSParser.h:
+
 2014-09-15  Andres Gomez  <agomez@igalia.com>
 
         [GStreamer] don't send transferMode HTTP header
index da7d4ba..c643dd9 100644 (file)
@@ -5015,10 +5015,8 @@ bool CSSParser::parseGridTemplateRowsAndAreas(PassRefPtr<CSSValue> templateColum
 
         // This will handle the trailing/leading <custom-ident>* in the grammar.
         trailingIdentWasAdded = false;
-        if (m_valueList->current() && m_valueList->current()->unit == CSSParserValue::ValueList) {
-            parseGridLineNames(*m_valueList, *templateRows);
-            trailingIdentWasAdded = true;
-        }
+        if (m_valueList->current() && m_valueList->current()->unit == CSSParserValue::ValueList)
+            trailingIdentWasAdded = parseGridLineNames(*m_valueList, *templateRows);
     } while (m_valueList->current());
 
     // [<track-list> /]?
@@ -5194,14 +5192,14 @@ bool CSSParser::parseSingleGridAreaLonghand(RefPtr<CSSValue>& property)
     return true;
 }
 
-void CSSParser::parseGridLineNames(CSSParserValueList& inputList, CSSValueList& valueList, CSSGridLineNamesValue* previousNamedAreaTrailingLineNames)
+bool CSSParser::parseGridLineNames(CSSParserValueList& inputList, CSSValueList& valueList, CSSGridLineNamesValue* previousNamedAreaTrailingLineNames)
 {
     ASSERT(inputList.current() && inputList.current()->unit == CSSParserValue::ValueList);
 
     CSSParserValueList* identList = inputList.current()->valueList;
     if (!identList->size()) {
         inputList.next();
-        return;
+        return false;
     }
 
     // Need to ensure the identList is at the heading index, since the parserList might have been rewound.
@@ -5217,6 +5215,7 @@ void CSSParser::parseGridLineNames(CSSParserValueList& inputList, CSSValueList&
         valueList.append(lineNames.releaseNonNull());
 
     inputList.next();
+    return true;
 }
 
 PassRefPtr<CSSValue> CSSParser::parseGridTrackList()
index 62c6d3a..59e1f6e 100644 (file)
@@ -171,7 +171,7 @@ public:
     PassRefPtr<CSSPrimitiveValue> parseGridBreadth(CSSParserValue*);
     bool parseGridTemplateAreasRow(NamedGridAreaMap&, const unsigned, unsigned&);
     PassRefPtr<CSSValue> parseGridTemplateAreas();
-    void parseGridLineNames(CSSParserValueList&, CSSValueList&, CSSGridLineNamesValue* = nullptr);
+    bool parseGridLineNames(CSSParserValueList&, CSSValueList&, CSSGridLineNamesValue* = nullptr);
     PassRefPtr<CSSValue> parseGridAutoFlow(CSSParserValueList&);
 #endif