SVG: Promote d to a property

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/css/parser/CSSPropertyParser.h"
 
 #include "core/StylePropertyShorthand.h"
 #include "core/css/CSSCalculationValue.h"
 #include "core/css/CSSCustomIdentValue.h"
@@ skipping 1929 matching lines @@
     RefPtrWillBeRawPtr<CSSValueList> dashes = CSSValueList::createCommaSeparated();
     do {
         RefPtrWillBeRawPtr<CSSPrimitiveValue> dash = consumeLengthOrPercent(range, SVGAttributeMode, ValueRangeNonNegative, UnitlessQuirk::Allow);
         if (!dash || (consumeCommaIncludingWhitespace(range) && range.atEnd()))
             return nullptr;
         dashes->append(dash.release());
     } while (!range.atEnd());
     return dashes.release();
 }
 
+static PassRefPtrWillBeRawPtr<CSSValue> consumeSVGDPath(CSSParserTokenRange& range)

[fs, 2015/11/26 12:51:39]

Shouldn't the syntax for this be the same as for motion-path - i.e
path(<string>) ?

[Eric Willigers, 2015/12/10 23:52:52]

Done. Should we have None or path('') if the element has no path attribute?

[fs, 2015/12/11 13:01:58]

The way you've done it in the latest PS seems ok to me. As I mentioned earlier -
they are semantically equivalent. I'd think that "path('')" should not turn into
"none" (and probably vice versa) if they both existed. No realy opinion if any
form is better though...

+{
+    if (range.peek().type() != StringToken)
+        return nullptr;
+
+    String pathString = range.consumeIncludingWhitespace().value();
+    Path path;
+    if (!buildPathFromString(pathString, path) || !range.atEnd())

[fs, 2015/11/26 12:51:39]

Here we build a Path (not entirely free) just to throw it away. If we just want
to check the syntax of the path it should be possible to add a new
SVGPathUtilities function for that. Let me know and I can add one (I see this is
used for motion-path too.)

This does mean that the semantics for 'd' changes though:

<path d="M100,100h100v100h-100z ballongdansen"/>

should produce a rectangle@(100,100) - it currently does w/ this patch because
we re-serialize to string after parsing the attribute - it'd be good to not rely
on an intermediate step "cleaning up" the path data though. I guess whether
parsing as an presentation attribute and via a stylesheet/inline style should be
treated differently is a different question though.

[fs, 2015/12/11 13:01:58]

Just noting: We should probably make sure this is addressed (spec-wise) before
we ship this.

[Eric Willigers, 2015/12/14 05:36:46]

The implemented approach is conservative, and consistent with other CSS
properties.

+        return nullptr;
+    return CSSStringValue::create(pathString);
+}
+
 PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSPropertyID unresolvedProperty)
 {
     CSSPropertyID property = resolveCSSPropertyID(unresolvedProperty);
     switch (property) {
     case CSSPropertyWillChange:
         return consumeWillChange(m_range);
     case CSSPropertyPage:
         return consumePage(m_range);
     case CSSPropertyQuotes:
         return consumeQuotes(m_range);
@@ skipping 163 matching lines @@
     case CSSPropertyMarkerMid:
     case CSSPropertyMarkerEnd:
         return consumeNoneOrURI(m_range);
     case CSSPropertyFlexBasis:
         return consumeFlexBasis(m_range, m_context.mode());
     case CSSPropertyFlexGrow:
     case CSSPropertyFlexShrink:
         return consumeNumber(m_range, ValueRangeNonNegative);
     case CSSPropertyStrokeDasharray:
         return consumeStrokeDasharray(m_range);
+    case CSSPropertyD:
+        return consumeSVGDPath(m_range);
     default:
         return nullptr;
     }
 }
 
 static PassRefPtrWillBeRawPtr<CSSValueList> consumeFontFaceUnicodeRange(CSSParserTokenRange& range)
 {
     RefPtrWillBeRawPtr<CSSValueList> values = CSSValueList::createCommaSeparated();
 
     do {
@@ skipping 575 matching lines @@
         return consumeFlex(important);
     case CSSPropertyFlexFlow:
         return consumeShorthandGreedily(flexFlowShorthand(), important);
     default:
         m_currentShorthand = oldShorthand;
         return false;
     }
 }
 
 } // namespace blink