[CSS Parser] Fix region, column and page break parsing
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Oct 2016 18:35:56 +0000 (18:35 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Oct 2016 18:35:56 +0000 (18:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163743

Reviewed by Simon Fraser.

* css/parser/CSSParserFastPaths.cpp:
(WebCore::CSSParserFastPaths::isValidKeywordPropertyAndValue):
Add the missing values for break support.

* css/parser/CSSPropertyParser.cpp:
(WebCore::isLegacyBreakProperty):
(WebCore::CSSPropertyParser::parseValueStart):
Add a special case for handling legacy break properties. Blink treats them like
shorthands, but we can't do that without breaking the old parser, so for now
we add a special case.

(WebCore::mapFromPageBreakBetween):
(WebCore::mapFromColumnBreakBetween):
(WebCore::mapFromRegionBreakBetween):
Updated to have the AvoidXXX values (where XXX is Column/Page/Region).

(WebCore::CSSPropertyParser::parseShorthand):
Remove the consumeLegacyBreak from the shorthand function, since we can't treat
the legacy break properties as shorthands yet.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@207629 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/css/parser/CSSParserFastPaths.cpp
Source/WebCore/css/parser/CSSPropertyParser.cpp

index 7a30c51..befd2f8 100644 (file)
@@ -1,3 +1,30 @@
+2016-10-20  Dave Hyatt  <hyatt@apple.com>
+
+        [CSS Parser] Fix region, column and page break parsing
+        https://bugs.webkit.org/show_bug.cgi?id=163743
+
+        Reviewed by Simon Fraser.
+
+        * css/parser/CSSParserFastPaths.cpp:
+        (WebCore::CSSParserFastPaths::isValidKeywordPropertyAndValue):
+        Add the missing values for break support.
+
+        * css/parser/CSSPropertyParser.cpp:
+        (WebCore::isLegacyBreakProperty):
+        (WebCore::CSSPropertyParser::parseValueStart):
+        Add a special case for handling legacy break properties. Blink treats them like
+        shorthands, but we can't do that without breaking the old parser, so for now
+        we add a special case.
+
+        (WebCore::mapFromPageBreakBetween):
+        (WebCore::mapFromColumnBreakBetween):
+        (WebCore::mapFromRegionBreakBetween):
+        Updated to have the AvoidXXX values (where XXX is Column/Page/Region).
+
+        (WebCore::CSSPropertyParser::parseShorthand):
+        Remove the consumeLegacyBreak from the shorthand function, since we can't treat
+        the legacy break properties as shorthands yet.
+
 2016-10-20  Sam Weinig  <sam@webkit.org>
 
         Add convenience function that combines WTF::visit(...) with WTF::makeVisitor(...)
index 92c4509..1317811 100644 (file)
@@ -596,9 +596,17 @@ bool CSSParserFastPaths::isValidKeywordPropertyAndValue(CSSPropertyID propertyId
         return valueID == CSSValueVisible || valueID == CSSValueHidden || valueID == CSSValueScroll || valueID == CSSValueAuto || valueID == CSSValueOverlay || valueID == CSSValueWebkitPagedX || valueID == CSSValueWebkitPagedY;
     case CSSPropertyBreakAfter:
     case CSSPropertyBreakBefore:
-        return valueID == CSSValueAuto || valueID == CSSValueAvoid || valueID == CSSValueAvoidPage || valueID == CSSValuePage || valueID == CSSValueLeft || valueID == CSSValueRight || valueID == CSSValueRecto || valueID == CSSValueVerso || valueID == CSSValueAvoidColumn || valueID == CSSValueColumn;
+        return valueID == CSSValueAuto || valueID == CSSValueAvoid || valueID == CSSValueAvoidPage || valueID == CSSValuePage || valueID == CSSValueLeft || valueID == CSSValueRight || valueID == CSSValueRecto || valueID == CSSValueVerso || valueID == CSSValueAvoidColumn || valueID == CSSValueColumn
+#if ENABLE(CSS_REGIONS)
+            || valueID == CSSValueRegion || valueID == CSSValueAvoidRegion
+#endif
+        ;
     case CSSPropertyBreakInside:
-        return valueID == CSSValueAuto || valueID == CSSValueAvoid || valueID == CSSValueAvoidPage || valueID == CSSValueAvoidColumn;
+        return valueID == CSSValueAuto || valueID == CSSValueAvoid || valueID == CSSValueAvoidPage || valueID == CSSValueAvoidColumn
+#if ENABLE(CSS_REGIONS)
+            || valueID == CSSValueAvoidRegion
+#endif
+        ;
     case CSSPropertyPointerEvents:
         // none | visiblePainted | visibleFill | visibleStroke | visible |
         // painted | fill | stroke | auto | all | bounding-box
index 7ba6c81..88fd22e 100644 (file)
@@ -266,6 +266,27 @@ RefPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSPropertyID property, con
     return value;
 }
 
+static bool isLegacyBreakProperty(CSSPropertyID propertyID)
+{
+    switch (propertyID) {
+    case CSSPropertyPageBreakAfter:
+    case CSSPropertyPageBreakBefore:
+    case CSSPropertyPageBreakInside:
+    case CSSPropertyWebkitColumnBreakAfter:
+    case CSSPropertyWebkitColumnBreakBefore:
+    case CSSPropertyWebkitColumnBreakInside:
+#if ENABLE(CSS_REGIONS)
+    case CSSPropertyWebkitRegionBreakAfter:
+    case CSSPropertyWebkitRegionBreakBefore:
+    case CSSPropertyWebkitRegionBreakInside:
+#endif
+        return true;
+    default:
+        break;
+    }
+    return false;
+}
+
 bool CSSPropertyParser::parseValueStart(CSSPropertyID propertyID, bool important)
 {
     if (consumeCSSWideKeyword(propertyID, important))
@@ -278,6 +299,11 @@ bool CSSPropertyParser::parseValueStart(CSSPropertyID propertyID, bool important
         // Variable references will fail to parse here and will fall out to the variable ref parser below.
         if (parseShorthand(propertyID, important))
             return true;
+    } else if (isLegacyBreakProperty(propertyID)) {
+        // FIXME-NEWPARSER: Can turn this into a shorthand once old parser is gone, and then
+        // we don't need the special case.
+        if (consumeLegacyBreakProperty(propertyID, important))
+            return true;
     } else {
         RefPtr<CSSValue> parsedValue = parseSingleValue(propertyID);
         if (parsedValue && m_range.atEnd()) {
@@ -4003,8 +4029,10 @@ static inline CSSValueID mapFromPageBreakBetween(CSSValueID value)
 {
     if (value == CSSValueAlways)
         return CSSValuePage;
-    if (value == CSSValueAuto || value == CSSValueAvoid || value == CSSValueLeft || value == CSSValueRight)
+    if (value == CSSValueAuto || value == CSSValueLeft || value == CSSValueRight)
         return value;
+    if (value == CSSValueAvoid)
+        return CSSValueAvoidPage;
     return CSSValueInvalid;
 }
 
@@ -4012,8 +4040,10 @@ static inline CSSValueID mapFromColumnBreakBetween(CSSValueID value)
 {
     if (value == CSSValueAlways)
         return CSSValueColumn;
-    if (value == CSSValueAuto || value == CSSValueAvoid)
+    if (value == CSSValueAuto)
         return value;
+    if (value == CSSValueAvoid)
+        return CSSValueAvoidColumn;
     return CSSValueInvalid;
 }
 
@@ -4022,8 +4052,10 @@ static inline CSSValueID mapFromRegionBreakBetween(CSSValueID value)
 {
     if (value == CSSValueAlways)
         return CSSValueRegion;
-    if (value == CSSValueAuto || value == CSSValueAvoid)
+    if (value == CSSValueAuto)
         return value;
+    if (value == CSSValueAvoid)
+        return CSSValueAvoidRegion;
     return CSSValueInvalid;
 }
 #endif
@@ -4571,13 +4603,6 @@ bool CSSPropertyParser::parseShorthand(CSSPropertyID property, bool important)
         return consumeBorder(important);
     case CSSPropertyBorderImage:
         return consumeBorderImage(property, important);
-    case CSSPropertyPageBreakAfter:
-    case CSSPropertyPageBreakBefore:
-    case CSSPropertyPageBreakInside:
-    case CSSPropertyWebkitColumnBreakAfter:
-    case CSSPropertyWebkitColumnBreakBefore:
-    case CSSPropertyWebkitColumnBreakInside:
-        return consumeLegacyBreakProperty(property, important);
     case CSSPropertyWebkitMaskPosition:
     case CSSPropertyBackgroundPosition: {
         RefPtr<CSSValue> resultX;