[CSS Grid Layout] CSSParser should reject <track-list> without a <track-size>
The case of <track-list> without a <track-size> was properly handled
but that's because style resolution had to account for the parsing
being not totally right.
This refactoring will help with implementing parsing for
<repeat-function> as the code matches the specification more closely.
TEST=fast/css-grid-layout/named-grid-line-get-set.html
Review URL: https://chromiumcodereview.appspot.com/16917013
git-svn-id: svn://svn.chromium.org/blink/trunk@152914 bbb929c8-8fbe-4397-9dbb-9b2b20218538
diff --git a/LayoutTests/fast/css-grid-layout/named-grid-line-get-set-expected.txt b/LayoutTests/fast/css-grid-layout/named-grid-line-get-set-expected.txt
index a2874ff..a594811 100644
--- a/LayoutTests/fast/css-grid-layout/named-grid-line-get-set-expected.txt
+++ b/LayoutTests/fast/css-grid-layout/named-grid-line-get-set-expected.txt
@@ -46,6 +46,8 @@
Test getting and setting invalid grid-columns and grid-rows through JS
PASS getComputedStyle(gridElement, '').getPropertyValue('grid-columns') is "none"
PASS getComputedStyle(gridElement, '').getPropertyValue('grid-rows') is "none"
+PASS getComputedStyle(gridElement, '').getPropertyValue('grid-columns') is "none"
+PASS getComputedStyle(gridElement, '').getPropertyValue('grid-rows') is "none"
PASS successfullyParsed is true
TEST COMPLETE
diff --git a/LayoutTests/fast/css-grid-layout/named-grid-line-get-set.html b/LayoutTests/fast/css-grid-layout/named-grid-line-get-set.html
index f63b9f6..8cb626a 100755
--- a/LayoutTests/fast/css-grid-layout/named-grid-line-get-set.html
+++ b/LayoutTests/fast/css-grid-layout/named-grid-line-get-set.html
@@ -147,6 +147,12 @@
element.style.gridColumns = "'foo'";
element.style.gridRows = "'bar";
testValue(element, "none", "none");
+
+ element = document.createElement("div");
+ document.body.appendChild(element);
+ element.style.gridColumns = "'foo' 'bar'";
+ element.style.gridRows = "'bar' 'foo'";
+ testValue(element, "none", "none");
</script>
<script src="../js/resources/js-test-post.js"></script>
</body>
diff --git a/Source/core/css/CSSParser.cpp b/Source/core/css/CSSParser.cpp
index c8a16d8..6bf8cb9 100644
--- a/Source/core/css/CSSParser.cpp
+++ b/Source/core/css/CSSParser.cpp
@@ -4594,24 +4594,34 @@
}
RefPtr<CSSValueList> values = CSSValueList::createSpaceSeparated();
- size_t currentLineNumber = 0;
+ // Handle leading <string>*.
+ while (m_valueList->current() && m_valueList->current()->unit == CSSPrimitiveValue::CSS_STRING) {
+ RefPtr<CSSPrimitiveValue> name = createPrimitiveStringValue(m_valueList->current());
+ values->append(name);
+ m_valueList->next();
+ }
+
+ bool seenTrackSize = false;
while (m_valueList->current()) {
+ RefPtr<CSSPrimitiveValue> primitiveValue = parseGridTrackSize();
+ if (!primitiveValue)
+ return false;
+
+ seenTrackSize = true;
+ values->append(primitiveValue.release());
+
+ // This will handle the trailing <string>* in the grammar.
while (m_valueList->current() && m_valueList->current()->unit == CSSPrimitiveValue::CSS_STRING) {
RefPtr<CSSPrimitiveValue> name = createPrimitiveStringValue(m_valueList->current());
values->append(name);
m_valueList->next();
}
-
- // This allows trailing <string>* per the specification.
- if (!m_valueList->current())
- break;
-
- RefPtr<CSSPrimitiveValue> primitiveValue = parseGridTrackSize();
- if (!primitiveValue)
- return false;
-
- values->append(primitiveValue.release());
}
+
+ // We should have found a <track-size> or else it is not a valid <track-list>
+ if (!seenTrackSize)
+ return false;
+
addProperty(propId, values.release(), important);
return true;
}
diff --git a/Source/core/css/resolver/StyleResolver.cpp b/Source/core/css/resolver/StyleResolver.cpp
index 3f1e4e2..686ee33 100644
--- a/Source/core/css/resolver/StyleResolver.cpp
+++ b/Source/core/css/resolver/StyleResolver.cpp
@@ -2264,9 +2264,9 @@
trackSizes.append(trackSize);
}
- if (trackSizes.isEmpty())
- return false;
-
+ // The parser should have rejected any <track-list> without any <track-size> as
+ // this is not conformant to the syntax.
+ ASSERT(!trackSizes.isEmpty());
return true;
}
