Parse "-webkit-columns: auto <length>" properly.

Source/core/css/CSSParser.cpp

Index: Source/core/css/CSSParser.cpp
diff --git a/Source/core/css/CSSParser.cpp b/Source/core/css/CSSParser.cpp
index 55ac2f7c8c20c74d24dc17b5b1c223f8a2165f09..da1af4f220e6a78ca627785cc27bad8aa2950c8c 100644
--- a/Source/core/css/CSSParser.cpp
+++ b/Source/core/css/CSSParser.cpp
@@ -3333,47 +3333,77 @@ bool CSSParser::parseTransitionShorthand(CSSPropertyID propId, bool important)
 
 bool CSSParser::parseShorthand(CSSPropertyID propId, const StylePropertyShorthand& shorthand, bool important)

[Julien - ping for review, 2013/07/23 00:56:29]

I believe that's your problem: you are using the generic parseShorthand method
and not a custom one that would keep track of what is set.

Look at this file for some properties that use ShorthandScope and implement your
own shorthand parsing.

[mstensho (USE GERRIT), 2013/07/23 14:19:07]

Done.

 {
-    // We try to match as many properties as possible
+    // We try to match as many properties as possible.
     // We set up an array of booleans to mark which property has been found,
     // and we try to search for properties until it makes no longer any sense.
     ShorthandScope scope(this, propId);
 
-    bool found = false;
     unsigned propertiesParsed = 0;
     bool propertyFound[6]= { false, false, false, false, false, false }; // 6 is enough size.
+    unsigned autoCount = 0;
 
-    while (m_valueList->current()) {
-        found = false;
-        for (unsigned propIndex = 0; !found && propIndex < shorthand.length(); ++propIndex) {
-            if (!propertyFound[propIndex] && parseValue(shorthand.properties()[propIndex], important)) {
+    while (CSSParserValue* value = m_valueList->current()) {
+        if (propertiesParsed >= shorthand.length())
+            return false; // Too many values. Invalid declaration.
+        if (shorthand.allAcceptAuto() && value->id == CSSValueAuto) {
+            // 'auto' is a valid value for all longhands, and at this point we don't know which one
+            // it is meant for. We need to look at the other values first. Just ignore 'auto' for
+            // now. After having processed the value list, set unassigned properties to 'auto'.
+            m_valueList->next();
+            propertiesParsed++;
+            autoCount++;
+        } else {
+            bool found = false;
+            for (unsigned propIndex = 0; propIndex < shorthand.length(); ++propIndex) {
+                if (!propertyFound[propIndex] && parseValue(shorthand.properties()[propIndex], important)) {
                     propertyFound[propIndex] = found = true;
                     propertiesParsed++;
+                    break;
+                }
             }
-        }
 
-        // if we didn't find at least one match, this is an
-        // invalid shorthand and we have to ignore it
-        if (!found)
-            return false;
+            // If we didn't find at least one match, this is an
+            // invalid shorthand and we have to ignore it.
+            if (!found)
+                return false;
+        }
     }
 
-    if (propertiesParsed == shorthand.length())
+    if (propertiesParsed == shorthand.length() && !autoCount)
         return true;
 
-    // Fill in any remaining properties with the initial value.
-    ImplicitScope implicitScope(this, PropertyImplicit);
+    // Fill in any remaining properties with the 'initial' or 'auto' value.
+    RefPtr<CSSValue> value;
+    if (autoCount)
+        value = cssValuePool().createIdentifierValue(CSSValueAuto);
+    else {
+        value = cssValuePool().createImplicitInitialValue();
+        m_implicitShorthand = true;
+    }
+
     const StylePropertyShorthand* const* const propertiesForInitialization = shorthand.propertiesForInitialization();
     for (unsigned i = 0; i < shorthand.length(); ++i) {
         if (propertyFound[i])
             continue;
 
         if (propertiesForInitialization) {
+            ASSERT(!autoCount); // Not properly supported.
             const StylePropertyShorthand& initProperties = *(propertiesForInitialization[i]);
             for (unsigned propIndex = 0; propIndex < initProperties.length(); ++propIndex)
-                addProperty(initProperties.properties()[propIndex], cssValuePool().createImplicitInitialValue(), important);
+                addProperty(initProperties.properties()[propIndex], value, important);
         } else
-            addProperty(shorthand.properties()[i], cssValuePool().createImplicitInitialValue(), important);
+            addProperty(shorthand.properties()[i], value, important);
+        if (autoCount) {
+            autoCount--;
+            if (!autoCount) {
+                // We have assigned as many 'auto' values as were specified. Now make the remaining
+                // ones implicit, so that we don't get too many of them if we later decide to
+                // reconstruct a shorthand based on longhands.
+                m_implicitShorthand = true;
+            }
+        }
     }
+    m_implicitShorthand = false;
 
     return true;
 }