Diff
Modified: trunk/LayoutTests/ChangeLog (182269 => 182270)
--- trunk/LayoutTests/ChangeLog 2015-04-02 06:29:20 UTC (rev 182269)
+++ trunk/LayoutTests/ChangeLog 2015-04-02 08:51:23 UTC (rev 182270)
@@ -1,3 +1,15 @@
+2015-04-02 Joonghun Park <jh718.p...@samsung.com>
+
+ [CSS MultiColumn] Parse "columns: auto <length>" shorthand property value properly
+ https://bugs.webkit.org/show_bug.cgi?id=143248
+
+ Reviewed by Darin Adler.
+
+ * fast/css/getPropertyValue-columns-expected.txt:
+ * fast/css/getPropertyValue-columns.html:
+ * fast/multicol/columns-shorthand-parsing-2-expected.txt: Added.
+ * fast/multicol/columns-shorthand-parsing-2.html: Added.
+
2015-04-01 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r182247.
Modified: trunk/LayoutTests/fast/css/getPropertyValue-columns-expected.txt (182269 => 182270)
--- trunk/LayoutTests/fast/css/getPropertyValue-columns-expected.txt 2015-04-02 06:29:20 UTC (rev 182269)
+++ trunk/LayoutTests/fast/css/getPropertyValue-columns-expected.txt 2015-04-02 08:51:23 UTC (rev 182270)
@@ -1,19 +1,19 @@
-Bug 111011: getPropertyValue for -webkit-columns returns null, should compute the shorthand value
+Bug 111011: getPropertyValue for columns returns null, should compute the shorthand value
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-PASS webkitColumnsValue("columns1") is "10px"
-PASS webkitColumnsValue("columns2") is "10"
-PASS webkitColumnsValue("columns3") is "10px auto"
-PASS webkitColumnsValue("columns4") is "auto"
-PASS webkitColumnsValue("columns5") is "auto 2"
-PASS webkitColumnsValue("columns6") is "10px 2"
-PASS webkitColumnsValue("columns7") is "auto auto"
-NOTE: 'foo' is an illegal CSS value for '-webkit-columns'.
-PASS webkitColumnsValue("columns8") is null
+PASS columnsValue("columns1") is "10px"
+PASS columnsValue("columns2") is "10"
+PASS columnsValue("columns3") is "10px auto"
+PASS columnsValue("columns4") is "auto"
+PASS columnsValue("columns5") is "auto 2"
+PASS columnsValue("columns6") is "10px 2"
+PASS columnsValue("columns7") is "auto auto"
+NOTE: 'foo' is an illegal CSS value for 'columns'.
+PASS columnsValue("columns8") is null
NOTE: If only few longhand properties are specified, getPropertyValue for shorthand property returns null.
-PASS webkitColumnsValue("columns9") is null
+PASS columnsValue("columns9") is null
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/fast/css/getPropertyValue-columns.html (182269 => 182270)
--- trunk/LayoutTests/fast/css/getPropertyValue-columns.html 2015-04-02 06:29:20 UTC (rev 182269)
+++ trunk/LayoutTests/fast/css/getPropertyValue-columns.html 2015-04-02 08:51:23 UTC (rev 182270)
@@ -1,7 +1,7 @@
<!DOCTYPE HTML>
</html>
<head>
- <title>-webkit-columns shorthand getPropertyValue test</title>
+ <title>columns shorthand getPropertyValue test</title>
<style>
.test {
width: 100px;
@@ -15,34 +15,34 @@
</script>
</head>
<body>
- <div id="columns1" class="test" style="-webkit-columns: 10px;"></div>
- <div id="columns2" class="test" style="-webkit-columns: 10;"></div>
- <div id="columns3" class="test" style="-webkit-columns: 10px auto;"></div>
- <div id="columns4" class="test" style="-webkit-columns: auto;"></div>
- <div id="columns5" class="test" style="-webkit-columns: auto 2;"></div>
- <div id="columns6" class="test" style="-webkit-column-width: 10px; -webkit-column-count: 2;"></div>
- <div id="columns7" class="test" style="-webkit-column-width: auto; -webkit-column-count: auto;"></div>
- <div id="columns8" class="test" style="-webkit-columns: foo;"></div>
- <div id="columns9" class="test" style="-webkit-column-width: 10px;"></div>
+ <div id="columns1" class="test" style="columns: 10px;"></div>
+ <div id="columns2" class="test" style="columns: 10;"></div>
+ <div id="columns3" class="test" style="columns: 10px auto;"></div>
+ <div id="columns4" class="test" style="columns: auto;"></div>
+ <div id="columns5" class="test" style="columns: auto 2;"></div>
+ <div id="columns6" class="test" style="column-width: 10px; column-count: 2;"></div>
+ <div id="columns7" class="test" style="column-width: auto; column-count: auto;"></div>
+ <div id="columns8" class="test" style="columns: foo;"></div>
+ <div id="columns9" class="test" style="column-width: 10px;"></div>
<script>
- description("<a href="" 111011: getPropertyValue for -webkit-columns returns null, should compute the shorthand value</a>");
+ description("<a href="" 111011: getPropertyValue for columns returns null, should compute the shorthand value</a>");
- function webkitColumnsValue(id) {
+ function columnsValue(id) {
var element = document.getElementById(id);
- return element.style.getPropertyValue("-webkit-columns");
+ return element.style.getPropertyValue("columns");
}
- shouldBeEqualToString('webkitColumnsValue("columns1")', '10px');
- shouldBeEqualToString('webkitColumnsValue("columns2")', '10');
- shouldBeEqualToString('webkitColumnsValue("columns3")', '10px auto');
- shouldBeEqualToString('webkitColumnsValue("columns4")', 'auto');
- shouldBeEqualToString('webkitColumnsValue("columns5")', 'auto 2');
- shouldBeEqualToString('webkitColumnsValue("columns6")', '10px 2');
- shouldBeEqualToString('webkitColumnsValue("columns7")', 'auto auto');
- debug("NOTE: 'foo' is an illegal CSS value for '-webkit-columns'.");
- shouldBe('webkitColumnsValue("columns8")', "null");
+ shouldBeEqualToString('columnsValue("columns1")', '10px');
+ shouldBeEqualToString('columnsValue("columns2")', '10');
+ shouldBeEqualToString('columnsValue("columns3")', '10px auto');
+ shouldBeEqualToString('columnsValue("columns4")', 'auto');
+ shouldBeEqualToString('columnsValue("columns5")', 'auto 2');
+ shouldBeEqualToString('columnsValue("columns6")', '10px 2');
+ shouldBeEqualToString('columnsValue("columns7")', 'auto auto');
+ debug("NOTE: 'foo' is an illegal CSS value for 'columns'.");
+ shouldBe('columnsValue("columns8")', "null");
debug("NOTE: If only few longhand properties are specified, getPropertyValue for shorthand property returns null.")
- shouldBe('webkitColumnsValue("columns9")', "null");
+ shouldBe('columnsValue("columns9")', "null");
</script>
<script src=""
</body>
Added: trunk/LayoutTests/fast/multicol/columns-shorthand-parsing-2-expected.txt (0 => 182270)
--- trunk/LayoutTests/fast/multicol/columns-shorthand-parsing-2-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/multicol/columns-shorthand-parsing-2-expected.txt 2015-04-02 08:51:23 UTC (rev 182270)
@@ -0,0 +1,51 @@
+Test the behavior when 'auto' is part of the 'columns' property value. See http://www.w3.org/TR/css3-multicol/#columns
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS element.style.columnWidth is "auto"
+PASS element.style.columnCount is "3"
+PASS element.style.columns is "auto 3"
+PASS element.style.columnWidth is "10em"
+PASS element.style.columnCount is "auto"
+PASS element.style.columns is "10em auto"
+PASS element.style.columnWidth is "auto"
+PASS element.style.columnCount is "3"
+PASS element.style.columns is "auto 3"
+PASS element.style.columnWidth is "10em"
+PASS element.style.columnCount is "auto"
+PASS element.style.columns is "10em auto"
+PASS element.style.columnWidth is "7em"
+PASS element.style.columnCount is "7"
+PASS element.style.columns is "7em 7"
+PASS element.style.columnWidth is "7em"
+PASS element.style.columnCount is "7"
+PASS element.style.columns is "7em 7"
+PASS element.style.columnWidth is "auto"
+PASS element.style.columnCount is "auto"
+PASS element.style.columns is "auto"
+PASS element.style.columnWidth is "auto"
+PASS element.style.columnCount is "auto"
+PASS element.style.columns is "auto auto"
+PASS element.style.columnWidth is "initial"
+PASS element.style.columnCount is "initial"
+PASS element.style.columns is "initial"
+PASS element.style.columnWidth is "auto"
+PASS element.style.columnCount is "auto"
+PASS element.style.columns is "auto"
+PASS element.style.columnWidth is "inherit"
+PASS element.style.columnCount is "inherit"
+PASS element.style.columns is "inherit"
+PASS element.style.columnWidth is "auto"
+PASS element.style.columnCount is "auto"
+PASS element.style.columns is "auto"
+PASS element.style.columnWidth is "auto"
+PASS element.style.columnCount is "7"
+PASS element.style.columns is "7"
+PASS element.style.columnWidth is "7em"
+PASS element.style.columnCount is "auto"
+PASS element.style.columns is "7em"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/multicol/columns-shorthand-parsing-2.html (0 => 182270)
--- trunk/LayoutTests/fast/multicol/columns-shorthand-parsing-2.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/columns-shorthand-parsing-2.html 2015-04-02 08:51:23 UTC (rev 182270)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>'columns' property with 'auto' and something else</title>
+ <script src=""
+ <script>
+ description("Test the behavior when 'auto' is part of the 'columns' property value. See http://www.w3.org/TR/css3-multicol/#columns");
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ function runTests() {
+ var tests = [["columns:auto 3;", "auto", "3", "auto 3"],
+ ["columns:auto 10em;", "10em", "auto", "10em auto"],
+ ["columns:3 auto;", "auto", "3", "auto 3"],
+ ["columns:10em auto;", "10em", "auto", "10em auto"],
+ ["columns:7 7em; columns:auto auto auto;", "7em", "7", "7em 7"],
+ ["columns:7 7em; columns:10em auto auto;", "7em", "7", "7em 7"],
+ ["columns:7 7em; columns:auto;", "auto", "auto", "auto"],
+ ["columns:7 7em; columns:auto auto;", "auto", "auto", "auto auto"],
+ ["columns:auto; columns:initial;", "initial", "initial", "initial"],
+ ["columns:auto; columns:initial initial;", "auto", "auto", "auto"],
+ ["columns:auto; columns:inherit;", "inherit", "inherit", "inherit"],
+ ["columns:auto; columns:inherit inherit;", "auto", "auto", "auto"],
+ ["columns:7;", "auto", "7", "7"],
+ ["columns:7em;", "7em", "auto", "7em"]];
+
+ tests.forEach(function(test) {
+ element.style.cssText = test[0];
+ shouldBeEqualToString("element.style.columnWidth", test[1]);
+ shouldBeEqualToString("element.style.columnCount", test[2]);
+ shouldBeEqualToString("element.style.columns", test[3]);
+ });
+ }
+ </script>
+ </head>
+ <body>
+ <div id="element"></div>
+ <script>
+ runTests();
+ </script>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (182269 => 182270)
--- trunk/Source/WebCore/ChangeLog 2015-04-02 06:29:20 UTC (rev 182269)
+++ trunk/Source/WebCore/ChangeLog 2015-04-02 08:51:23 UTC (rev 182270)
@@ -1,3 +1,29 @@
+2015-04-02 Joonghun Park <jh718.p...@samsung.com>
+
+ [CSS MultiColumn] Parse "columns: auto <length>" shorthand property value properly
+ https://bugs.webkit.org/show_bug.cgi?id=143248
+
+ Reviewed by Darin Adler.
+
+ Test: fast/multicol/columns-shorthand-parsing-2.html
+
+ The two longhands for the 'columns' property ('column-count' and
+ 'column-width') may both take 'auto' as a value. When we encounter
+ 'auto' during parsing the value list of a declaration, we cannot just
+ make a guess at which property/properties that's meant for. Instead,
+ don't assign anything to 'auto' right away, but wait until all values
+ have been processed and at that point set the still unassigned
+ properties to 'auto'. If 'auto' isn't in the value list at all, set
+ unassigned properties to 'initial' for the 'columns' property, just
+ like we do for any other property.
+
+ * css/CSSParser.cpp:
+ (WebCore::CSSParser::parseValue):
+ (WebCore::CSSParser::parseColumnWidth):
+ (WebCore::CSSParser::parseColumnCount):
+ (WebCore::CSSParser::parseColumnsShorthand):
+ * css/CSSParser.h:
+
2015-04-01 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r182247.
Modified: trunk/Source/WebCore/css/CSSParser.cpp (182269 => 182270)
--- trunk/Source/WebCore/css/CSSParser.cpp 2015-04-02 06:29:20 UTC (rev 182269)
+++ trunk/Source/WebCore/css/CSSParser.cpp 2015-04-02 08:51:23 UTC (rev 182270)
@@ -2763,10 +2763,7 @@
validPrimitive = !id && validateUnit(valueWithCalculation, FNumber | FLength | FPercent);
break;
case CSSPropertyColumnCount:
- if (id == CSSValueAuto)
- validPrimitive = true;
- else
- validPrimitive = !id && validateUnit(valueWithCalculation, FPositiveInteger, CSSQuirksMode);
+ parsedValue = parseColumnCount();
break;
case CSSPropertyColumnGap: // normal | <length>
if (id == CSSValueNormal)
@@ -2791,12 +2788,7 @@
}
break;
case CSSPropertyColumnWidth: // auto | <length>
- if (id == CSSValueAuto)
- validPrimitive = true;
- else {
- // Always parse this property in strict mode, since it would be ambiguous otherwise when used in the 'columns' shorthand property.
- validPrimitive = validateUnit(valueWithCalculation, FLength | FNonNeg, CSSStrictMode) && parsedDouble(valueWithCalculation);
- }
+ parsedValue = parseColumnWidth();
break;
// End of CSS3 properties
@@ -2957,7 +2949,7 @@
case CSSPropertyListStyle:
return parseShorthand(propId, listStyleShorthand(), important);
case CSSPropertyColumns:
- return parseShorthand(propId, columnsShorthand(), important);
+ return parseColumnsShorthand(important);
case CSSPropertyColumnRule:
return parseShorthand(propId, columnRuleShorthand(), important);
case CSSPropertyWebkitTextStroke:
@@ -3611,6 +3603,76 @@
return true;
}
+RefPtr<CSSValue> CSSParser::parseColumnWidth()
+{
+ ValueWithCalculation valueWithCalculation(*m_valueList->current());
+ CSSValueID id = valueWithCalculation.value().id;
+ // Always parse this property in strict mode, since it would be ambiguous otherwise when used in the 'columns' shorthand property.
+ if (id != CSSValueAuto && !(validateUnit(valueWithCalculation, FLength | FNonNeg, CSSStrictMode) && parsedDouble(valueWithCalculation)))
+ return nullptr;
+
+ RefPtr<CSSValue> parsedValue = parseValidPrimitive(id, valueWithCalculation);
+ m_valueList->next();
+ return parsedValue;
+}
+
+RefPtr<CSSValue> CSSParser::parseColumnCount()
+{
+ ValueWithCalculation valueWithCalculation(*m_valueList->current());
+ CSSValueID id = valueWithCalculation.value().id;
+
+ if (id != CSSValueAuto && !validateUnit(valueWithCalculation, FPositiveInteger, CSSQuirksMode))
+ return nullptr;
+
+ RefPtr<CSSValue> parsedValue = parseValidPrimitive(id, valueWithCalculation);
+ m_valueList->next();
+ return parsedValue;
+}
+
+bool CSSParser::parseColumnsShorthand(bool important)
+{
+ RefPtr<CSSValue> columnWidth;
+ RefPtr<CSSValue> columnCount;
+ bool hasPendingExplicitAuto = false;
+
+ for (unsigned propertiesParsed = 0; CSSParserValue* value = m_valueList->current(); propertiesParsed++) {
+ if (propertiesParsed >= 2)
+ return false; // Too many values for this shorthand. Invalid declaration.
+ if (!propertiesParsed && value->id == CSSValueAuto) {
+ // 'auto' is a valid value for any of the two longhands, and at this point
+ // we don't know which one(s) it is meant for. We need to see if there are other values first.
+ m_valueList->next();
+ hasPendingExplicitAuto = true;
+ } else {
+ if (!columnWidth) {
+ if ((columnWidth = parseColumnWidth()))
+ continue;
+ }
+ if (!columnCount) {
+ if ((columnCount = parseColumnCount()))
+ continue;
+ }
+ // If we didn't find at least one match, this is an invalid shorthand and we have to ignore it.
+ return false;
+ }
+ }
+
+ // Any unassigned property at this point will become implicit 'auto'.
+ if (columnWidth)
+ addProperty(CSSPropertyColumnWidth, columnWidth, important);
+ else {
+ addProperty(CSSPropertyColumnWidth, cssValuePool().createIdentifierValue(CSSValueAuto), important, !hasPendingExplicitAuto /* implicit */);
+ hasPendingExplicitAuto = false;
+ }
+
+ if (columnCount)
+ addProperty(CSSPropertyColumnCount, columnCount, important);
+ else
+ addProperty(CSSPropertyColumnCount, cssValuePool().createIdentifierValue(CSSValueAuto), important, !hasPendingExplicitAuto /* implicit */);
+
+ return true;
+}
+
bool CSSParser::parseTransitionShorthand(CSSPropertyID propId, bool important)
{
const unsigned numProperties = 4;
Modified: trunk/Source/WebCore/css/CSSParser.h (182269 => 182270)
--- trunk/Source/WebCore/css/CSSParser.h 2015-04-02 06:29:20 UTC (rev 182269)
+++ trunk/Source/WebCore/css/CSSParser.h 2015-04-02 08:51:23 UTC (rev 182270)
@@ -192,6 +192,10 @@
bool parseTransitionShorthand(CSSPropertyID, bool important);
bool parseAnimationShorthand(CSSPropertyID, bool important);
+ RefPtr<CSSValue> parseColumnWidth();
+ RefPtr<CSSValue> parseColumnCount();
+ bool parseColumnsShorthand(bool important);
+
#if ENABLE(CSS_GRID_LAYOUT)
PassRefPtr<CSSValue> parseGridPosition();
bool parseGridItemPositionShorthand(CSSPropertyID, bool important);
