[CSS Grid Layout] grid-template-areas should accept none value (issue 1117123005 by rego@igalia.com)

[re...@igalia.com]

Reviewers: dsinclair, leviw, Julien - OOO 18 Apr - 14 May,

Description:
[CSS Grid Layout] grid-template-areas should accept none value

Default value for grid-template-areas property is "none":
http://dev.w3.org/csswg/css-grid/#propdef-grid-template-areas

Currently if you set the property to "none" from JavaScript, the value
doesn't get reseted and it keeps the old value.

This patch adds support to parse it successfully and updates the test
adding the new case.

TEST=fast/css-grid-layout/grid-template-areas-get-set.html

Please review this at https://codereview.chromium.org/1117123005/

Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Affected files (+13, -0 lines):
M LayoutTests/fast/css-grid-layout/grid-template-areas-get-set.html
M
LayoutTests/fast/css-grid-layout/grid-template-areas-get-set-expected.txt
M Source/core/css/parser/CSSPropertyParser.cpp


Index:
LayoutTests/fast/css-grid-layout/grid-template-areas-get-set-expected.txt
diff --git
a/LayoutTests/fast/css-grid-layout/grid-template-areas-get-set-expected.txt
b/LayoutTests/fast/css-grid-layout/grid-template-areas-get-set-expected.txt
index
cc3fd4fcfd52215f968cf94d1d3c63f56c72160b..fbf07102e178378963aed31e2695e813c42d1c22
100644
---
a/LayoutTests/fast/css-grid-layout/grid-template-areas-get-set-expected.txt
+++
b/LayoutTests/fast/css-grid-layout/grid-template-areas-get-set-expected.txt
@@ -15,6 +15,9 @@ PASS
window.getComputedStyle(gridWithHorizontalRectangle).getPropertyValue('grid
PASS
window.getComputedStyle(gridWithVerticalRectangle).getPropertyValue('grid-template-areas')
is "\"a a\" \"a a\" \"a a\""
Test grid-template-areas: initial
PASS
window.getComputedStyle(element).getPropertyValue('grid-template-areas')
is "\"foobar\""
+PASS
window.getComputedStyle(element).getPropertyValue('grid-template-areas')
is "none"
+PASS
window.getComputedStyle(element).getPropertyValue('grid-template-areas')
is "\"foobar\""
+PASS
window.getComputedStyle(element).getPropertyValue('grid-template-areas')
is "none"
Test grid-template-areas: inherit
PASS
window.getComputedStyle(parentElement).getPropertyValue('grid-template-areas')
is "\"foo bar\""
PASS
window.getComputedStyle(element).getPropertyValue('grid-template-areas')
is "\"foo bar\""
Index: LayoutTests/fast/css-grid-layout/grid-template-areas-get-set.html
diff --git
a/LayoutTests/fast/css-grid-layout/grid-template-areas-get-set.html
b/LayoutTests/fast/css-grid-layout/grid-template-areas-get-set.html
index
64c702e7f14c44766a5bbbcff33391a43a9c1c3f..ab89756add5118fb62e4ad91ce0600d49d0ee4e1
100644
--- a/LayoutTests/fast/css-grid-layout/grid-template-areas-get-set.html
+++ b/LayoutTests/fast/css-grid-layout/grid-template-areas-get-set.html
@@ -91,6 +91,11 @@
element.style.gridTemplateAreas = "'foobar'";

shouldBeEqualToString("window.getComputedStyle(element).getPropertyValue('grid-template-areas')", '"foobar"')
element.style.gridTemplateAreas = "initial";
+
shouldBeEqualToString("window.getComputedStyle(element).getPropertyValue('grid-template-areas')", 'none');
+ element.style.gridTemplateAreas = "'foobar'";
+
shouldBeEqualToString("window.getComputedStyle(element).getPropertyValue('grid-template-areas')", '"foobar"')
+ element.style.gridTemplateAreas = "none";
+
shouldBeEqualToString("window.getComputedStyle(element).getPropertyValue('grid-template-areas')", 'none');
document.body.removeChild(element);

debug("Test grid-template-areas: inherit");
Index: Source/core/css/parser/CSSPropertyParser.cpp
diff --git a/Source/core/css/parser/CSSPropertyParser.cpp
b/Source/core/css/parser/CSSPropertyParser.cpp
index
17a2b6113f22a83f9cdab3c68c21d7250b6f3ed8..b09d42d932671ea83c2b3d8e3715f6bb1d1e58fa
100644
--- a/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/Source/core/css/parser/CSSPropertyParser.cpp
@@ -3715,6 +3715,11 @@ bool
CSSPropertyParser::parseGridTemplateAreasRow(NamedGridAreaMap& gridAreaMap,

PassRefPtrWillBeRawPtr<CSSValue>
CSSPropertyParser::parseGridTemplateAreas()
{
+ if (m_valueList->current() && m_valueList->current()->id ==
CSSValueNone) {
+ m_valueList->next();
+ return cssValuePool().createIdentifierValue(CSSValueNone);
+ }
+
NamedGridAreaMap gridAreaMap;
size_t rowCount = 0;
size_t columnCount = 0;

[svi...@igalia.com]

Informal lgtm

https://codereview.chromium.org/1117123005/

[dsin...@chromium.org]

lgtm



https://codereview.chromium.org/1117123005/

[commi...@chromium.org]

CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1117123005/1


https://codereview.chromium.org/1117123005/

[commi...@chromium.org]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194926

https://codereview.chromium.org/1117123005/