[css-grid] Implement grid gutters

third_party/WebKit/Source/core/css/parser/LegacyCSSPropertyParser.cpp

 /*
  * Copyright (C) 2003 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Nicholas Shanks <webkit@nickshanks.com>
  * Copyright (C) 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2012 Adobe Systems Incorporated. All rights reserved.
  * Copyright (C) 2012 Intel Corporation. All rights reserved.
  *
@@ skipping 1154 matching lines @@
         break;
 
     case CSSPropertyGridColumnEnd:
     case CSSPropertyGridColumnStart:
     case CSSPropertyGridRowEnd:
     case CSSPropertyGridRowStart:
         ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
         parsedValue = parseGridPosition();
         break;
 
+    case CSSPropertyGridColumnGap:
+    case CSSPropertyGridRowGap:
+        ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
+        validPrimitive = validUnit(value, FLength | FNonNeg);

[rune, 2015/10/05 12:23:39]

The current spec draft doesn't say it's limited to non-negative lengths, but it
probably should, like the spec for column-gap in multicol does.

[svillar, 2015/10/05 12:36:01]

True, I think it's just an overlook, actually the Mozilla guy that is going to
implement it in Gecko sent this
https://lists.w3.org/Archives/Public/www-style/2015Oct/0028.html to the list
asking for clarifications.

I think that the assumption I made is correct in any case.

[rune, 2015/10/05 12:47:10]

Yes, I just wanted to make sure the spec issue is addressed.

+        break;
+
+    case CSSPropertyGridGap:
+        ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
+        return parseGridGapShorthand(important);
+
     case CSSPropertyGridColumn:
     case CSSPropertyGridRow:
         ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
         return parseGridItemPositionShorthand(propId, important);
 
     case CSSPropertyGridArea:
         ASSERT(RuntimeEnabledFeatures::cssGridLayoutEnabled());
         return parseGridAreaShorthand(important);
 
     case CSSPropertyGridTemplateAreas:
@@ skipping 2005 matching lines @@
             return false;
     } else {
         endValue = gridMissingGridPositionValue(startValue.get());
     }
 
     addProperty(shorthand.properties()[0], startValue, important);
     addProperty(shorthand.properties()[1], endValue, important);
     return true;
 }
 
+bool CSSPropertyParser::parseGridGapShorthand(bool important)

[fs, 2015/10/05 10:53:28]

Could you use parseShorthand instead?

[svillar, 2015/10/05 11:58:40]

I don't think so because although ideally the shorthand gets two values it might
get just one so we have to add that specific behavior. AFAIK that cannot be done
with parseShorthand...

[fs, 2015/10/05 12:01:54]

Right, I figured it might be the column-gap -> row-gap value expansion (didn't
remember if parseShorthand handled that or not.)

+{
+    ShorthandScope scope(this, CSSPropertyGridGap);
+    ASSERT(shorthandForProperty(CSSPropertyGridGap).length() == 2);
+
+    CSSParserValue* value = m_valueList->current();
+    if (!value)
+        return false;
+
+    if (value->id == CSSValueNormal) {
+        if (m_valueList->next())
+            return false;
+        addProperty(CSSPropertyGridColumnGap, cssValuePool().createImplicitInitialValue(), important);

[rune, 2015/10/05 12:23:39]

According to the spec, "normal" is not a valid value for any of the longhands.

Also, the spec doesn't say what normal mean, afaict.

[svillar, 2015/10/05 12:36:01]

That was specified in the previous draft, see
http://www.w3.org/TR/2015/WD-css-grid-1-20150806/#gutters. My first patch was
from 3 weeks ago when that was the latest version.

Anyway in the email I linked above there is another question for "normal" as
well so they'll clarify it for sure.

[rune, 2015/10/05 12:47:10]

Ah. So the property used to be shared with multicol. Now that explains the need
for 'normal' to mean something. Now that they're separate properties, and the
initial value is 0, and normal used to map to 0, I see no point in keeping
'normal'. I suggest that you just drop normal as a valid value for now and
re-introduce it if the spec for some reason re-introduces it instead of removing
it for the shorthand.

+        addProperty(CSSPropertyGridRowGap, cssValuePool().createImplicitInitialValue(), important);
+        return true;
+    }
+
+    if (!validUnit(value, FLength | FNonNeg))
+        return false;
+
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> columnGap = createPrimitiveNumericValue(value);
+
+    value = m_valueList->next();
+    if (!value) {
+        addProperty(CSSPropertyGridColumnGap, columnGap, important);
+        addProperty(CSSPropertyGridRowGap, columnGap, important);

[rune, 2015/10/05 12:23:39]

I think it this code might be easier to read if you have the addProperty calls
at the end and have two variables columnGap and rowGap which is set to the same
value if there's only one value in the declaration.

[svillar, 2015/10/05 12:36:01]

Acknowledged.

+        return true;
+    }
+
+    if (!validUnit(value, FLength | FNonNeg))
+        return false;
+
+    if (m_valueList->next())
+        return false;
+
+    RefPtrWillBeRawPtr<CSSPrimitiveValue> rowGap = createPrimitiveNumericValue(value);
+
+    addProperty(CSSPropertyGridColumnGap, columnGap, important);
+    addProperty(CSSPropertyGridRowGap, rowGap, important);
+
+    return true;
+}
+
 bool CSSPropertyParser::parseGridTemplateRowsAndAreas(PassRefPtrWillBeRawPtr<CSSValue> templateColumns, bool important)
 {
     NamedGridAreaMap gridAreaMap;
     size_t rowCount = 0;
     size_t columnCount = 0;
     bool trailingIdentWasAdded = false;
     RefPtrWillBeRawPtr<CSSValueList> templateRows = CSSValueList::createSpaceSeparated();
 
     // At least template-areas strings must be defined.
     if (!m_valueList->current())
@@ skipping 4253 matching lines @@
         }
     }
 
     if (!list->length())
         return nullptr;
 
     return list.release();
 }
 
 } // namespace blink