Implement page-break-* and -webkit-column-break-* as legacy-shorthands.
authorjh718.park@samsung.com <jh718.park@samsung.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 May 2019 14:02:50 +0000 (14:02 +0000)
committerjh718.park@samsung.com <jh718.park@samsung.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 May 2019 14:02:50 +0000 (14:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197656

LayoutTests/imported/w3c:

Reviewed by Darin Adler.

According to https://drafts.csswg.org/css-cascade-4/#legacy-shorthand,
implement page-break-* and -webkit-column-break-* as legacy-shorthands for break-*.

This change also serialize page-break-* properties
to CSSStyleDeclaration,
per https://drafts.csswg.org/css-break/#page-break-properties.

* web-platform-tests/css/cssom/serialize-values-expected.txt:

Source/WebCore:

Reviewed by Darin Adler.

According to https://drafts.csswg.org/css-cascade-4/#legacy-shorthand,
implement page-break-* and -webkit-column-break-* as legacy-shorthands for break-*.

This change also serialize page-break-* properties
to CSSStyleDeclaration,
per https://drafts.csswg.org/css-break/#page-break-properties.

* css/CSSProperties.json:
* css/StyleBuilderConverter.h:
(WebCore::StyleBuilderConverter::convertFontSynthesis):
(WebCore::StyleBuilderConverter::convertPageBreakBetween): Deleted.
(WebCore::StyleBuilderConverter::convertPageBreakInside): Deleted.
(WebCore::StyleBuilderConverter::convertColumnBreakBetween): Deleted.
(WebCore::StyleBuilderConverter::convertColumnBreakInside): Deleted.
* css/StyleProperties.cpp:
(WebCore::StyleProperties::getPropertyValue const):
(WebCore::StyleProperties::pageBreakPropertyValue const):
* css/StyleProperties.h:
* css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::parseValueStart):
(WebCore::mapFromPageBreakBetween):
(WebCore::CSSPropertyParser::parseShorthand):
(WebCore::isLegacyBreakProperty): Deleted.

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

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/css/cssom/serialize-values-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/css/CSSProperties.json
Source/WebCore/css/StyleBuilderConverter.h
Source/WebCore/css/StyleProperties.cpp
Source/WebCore/css/StyleProperties.h
Source/WebCore/css/parser/CSSPropertyParser.cpp

index cf295ec..e7132da 100644 (file)
@@ -1,3 +1,19 @@
+2019-05-14  Joonghun Park  <jh718.park@samsung.com>
+
+        Implement page-break-* and -webkit-column-break-* as legacy-shorthands.
+        https://bugs.webkit.org/show_bug.cgi?id=197656
+
+        Reviewed by Darin Adler.
+
+        According to https://drafts.csswg.org/css-cascade-4/#legacy-shorthand,
+        implement page-break-* and -webkit-column-break-* as legacy-shorthands for break-*.
+
+        This change also serialize page-break-* properties
+        to CSSStyleDeclaration,
+        per https://drafts.csswg.org/css-break/#page-break-properties.
+
+        * web-platform-tests/css/cssom/serialize-values-expected.txt:
+
 2019-05-14  Javier Fernandez  <jfernandez@igalia.com>
 
         Implement "line-break: anywhere"
index 3720477..64a04b9 100644 (file)
@@ -571,20 +571,20 @@ PASS padding-left: .1em
 PASS padding-left: 5% 
 PASS padding-left: .5% 
 PASS padding-left: inherit 
-FAIL page-break-after: auto assert_equals: page-break-after raw inline style declaration expected "auto" but got ""
-FAIL page-break-after: always assert_equals: page-break-after raw inline style declaration expected "always" but got ""
-FAIL page-break-after: avoid assert_equals: page-break-after raw inline style declaration expected "avoid" but got ""
-FAIL page-break-after: left assert_equals: page-break-after raw inline style declaration expected "left" but got ""
-FAIL page-break-after: right assert_equals: page-break-after raw inline style declaration expected "right" but got ""
+PASS page-break-after: auto 
+PASS page-break-after: always 
+PASS page-break-after: avoid 
+PASS page-break-after: left 
+PASS page-break-after: right 
 PASS page-break-after: inherit 
-FAIL page-break-before: auto assert_equals: page-break-before raw inline style declaration expected "auto" but got ""
-FAIL page-break-before: always assert_equals: page-break-before raw inline style declaration expected "always" but got ""
-FAIL page-break-before: avoid assert_equals: page-break-before raw inline style declaration expected "avoid" but got ""
-FAIL page-break-before: left assert_equals: page-break-before raw inline style declaration expected "left" but got ""
-FAIL page-break-before: right assert_equals: page-break-before raw inline style declaration expected "right" but got ""
+PASS page-break-before: auto 
+PASS page-break-before: always 
+PASS page-break-before: avoid 
+PASS page-break-before: left 
+PASS page-break-before: right 
 PASS page-break-before: inherit 
-FAIL page-break-inside: avoid assert_equals: page-break-inside raw inline style declaration expected "avoid" but got ""
-FAIL page-break-inside: auto assert_equals: page-break-inside raw inline style declaration expected "auto" but got ""
+PASS page-break-inside: avoid 
+PASS page-break-inside: auto 
 PASS page-break-inside: inherit 
 PASS position: static 
 PASS position: relative 
index 4abfecf..4f30011 100644 (file)
@@ -1,3 +1,34 @@
+2019-05-14  Joonghun Park  <pjh0718@gmail.com>
+
+        Implement page-break-* and -webkit-column-break-* as legacy-shorthands.
+        https://bugs.webkit.org/show_bug.cgi?id=197656
+
+        Reviewed by Darin Adler.
+
+        According to https://drafts.csswg.org/css-cascade-4/#legacy-shorthand,
+        implement page-break-* and -webkit-column-break-* as legacy-shorthands for break-*.
+
+        This change also serialize page-break-* properties
+        to CSSStyleDeclaration,
+        per https://drafts.csswg.org/css-break/#page-break-properties.
+
+        * css/CSSProperties.json:
+        * css/StyleBuilderConverter.h:
+        (WebCore::StyleBuilderConverter::convertFontSynthesis):
+        (WebCore::StyleBuilderConverter::convertPageBreakBetween): Deleted.
+        (WebCore::StyleBuilderConverter::convertPageBreakInside): Deleted.
+        (WebCore::StyleBuilderConverter::convertColumnBreakBetween): Deleted.
+        (WebCore::StyleBuilderConverter::convertColumnBreakInside): Deleted.
+        * css/StyleProperties.cpp:
+        (WebCore::StyleProperties::getPropertyValue const):
+        (WebCore::StyleProperties::pageBreakPropertyValue const):
+        * css/StyleProperties.h:
+        * css/parser/CSSPropertyParser.cpp:
+        (WebCore::CSSPropertyParser::parseValueStart):
+        (WebCore::mapFromPageBreakBetween):
+        (WebCore::CSSPropertyParser::parseShorthand):
+        (WebCore::isLegacyBreakProperty): Deleted.
+
 2019-05-14  Javier Fernandez  <jfernandez@igalia.com>
 
         Implement "line-break: anywhere"
index 1d9b6b8..8a4ef59 100644 (file)
             }
         },
         "page-break-after": {
-            "values": [
-                "auto",
-                "always",
-                "avoid",
-                "left",
-                "right"
-            ],
             "codegen-properties": {
-                "initial": "initialBreakBetween",
-                "name-for-methods": "BreakAfter",
-                "converter": "PageBreakBetween"
+                "longhands": [
+                    "break-after"
+                ]
             },
             "specification": {
                 "category": "css-22",
             }
         },
         "page-break-before": {
-            "values": [
-                "auto",
-                "always",
-                "avoid",
-                "left",
-                "right"
-            ],
             "codegen-properties": {
-                "initial": "initialBreakBetween",
-                "name-for-methods": "BreakBefore",
-                "converter": "PageBreakBetween"
+                "longhands": [
+                    "break-before"
+                ]
             },
             "specification": {
                 "category": "css-22",
             }
         },
         "page-break-inside": {
-            "values": [
-                "auto",
-                "avoid"
-            ],
             "codegen-properties": {
-                "initial": "initialBreakInside",
-                "name-for-methods": "BreakInside",
-                "converter": "PageBreakInside"
+                "longhands": [
+                    "break-inside"
+                ]
             },
             "specification": {
                 "category": "css-22",
             }
         },
         "-webkit-column-break-after": {
-            "values": [
-                "auto",
-                "always",
-                "avoid",
-                "left",
-                "right"
-            ],
             "codegen-properties": {
-                "initial": "initialBreakBetween",
-                "name-for-methods": "BreakAfter",
-                "converter": "ColumnBreakBetween"
+                "longhands": [
+                    "break-after"
+                ]
             },
             "status": {
                 "status": "experimental"
             }
         },
         "-webkit-column-break-before": {
-            "values": [
-                "auto",
-                "always",
-                "avoid",
-                "left",
-                "right"
-            ],
             "codegen-properties": {
-                "initial": "initialBreakBetween",
-                "name-for-methods": "BreakBefore",
-                "converter": "ColumnBreakBetween"
+                "longhands": [
+                    "break-before"
+                ]
             },
             "status": {
                 "status": "experimental"
             }
         },
         "-webkit-column-break-inside": {
-            "values": [
-                "auto",
-                "avoid"
-            ],
             "codegen-properties": {
-                "initial": "initialBreakInside",
-                "name-for-methods": "BreakInside",
-                "converter": "ColumnBreakInside"
+                "longhands": [
+                    "break-inside"
+                ]
             },
             "status": {
                 "status": "experimental"
index abf5caa..6701ca5 100644 (file)
@@ -1566,42 +1566,6 @@ inline FontSynthesis StyleBuilderConverter::convertFontSynthesis(StyleResolver&,
 
     return result;
 }
-
-inline BreakBetween StyleBuilderConverter::convertPageBreakBetween(StyleResolver&, const CSSValue& value)
-{
-    auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
-    if (primitiveValue.valueID() == CSSValueAlways)
-        return BreakBetween::Page;
-    if (primitiveValue.valueID() == CSSValueAvoid)
-        return BreakBetween::AvoidPage;
-    return primitiveValue;
-}
-
-inline BreakInside StyleBuilderConverter::convertPageBreakInside(StyleResolver&, const CSSValue& value)
-{
-    auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
-    if (primitiveValue.valueID() == CSSValueAvoid)
-        return BreakInside::AvoidPage;
-    return primitiveValue;
-}
-
-inline BreakBetween StyleBuilderConverter::convertColumnBreakBetween(StyleResolver&, const CSSValue& value)
-{
-    auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
-    if (primitiveValue.valueID() == CSSValueAlways)
-        return BreakBetween::Column;
-    if (primitiveValue.valueID() == CSSValueAvoid)
-        return BreakBetween::AvoidColumn;
-    return primitiveValue;
-}
-
-inline BreakInside StyleBuilderConverter::convertColumnBreakInside(StyleResolver&, const CSSValue& value)
-{
-    auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
-    if (primitiveValue.valueID() == CSSValueAvoid)
-        return BreakInside::AvoidColumn;
-    return primitiveValue;
-}
     
 inline OptionSet<SpeakAs> StyleBuilderConverter::convertSpeakAs(StyleResolver&, const CSSValue& value)
 {
index 84d5126..4cf6c6f 100644 (file)
@@ -133,6 +133,11 @@ String StyleProperties::getPropertyValue(CSSPropertyID propertyID) const
         RefPtr<CSSValue> value = getPropertyCSSValueInternal(shorthand.properties()[0]);
         if (!value || value->isPendingSubstitutionValue())
             return String();
+    // FIXME: If all longhands are the same css-generic keyword(e.g. initial or inherit),
+    // then the shorthand should be serialized to that keyword.
+    // It seems to be needed to handle this in a single function commonly for all the shorthands,
+    // not in each of the shorthand serialization function.
+    // We could call that function here.
     }
 
     // Shorthand and 4-values properties
@@ -209,6 +214,12 @@ String StyleProperties::getPropertyValue(CSSPropertyID propertyID) const
         return getShorthandValue(gridColumnShorthand());
     case CSSPropertyGridRow:
         return getShorthandValue(gridRowShorthand());
+    case CSSPropertyPageBreakAfter:
+        return pageBreakPropertyValue(pageBreakAfterShorthand());
+    case CSSPropertyPageBreakBefore:
+        return pageBreakPropertyValue(pageBreakBeforeShorthand());
+    case CSSPropertyPageBreakInside:
+        return pageBreakPropertyValue(pageBreakInsideShorthand());
     case CSSPropertyPlaceContent:
         return getAlignmentShorthandValue(placeContentShorthand());
     case CSSPropertyPlaceItems:
@@ -713,6 +724,26 @@ String StyleProperties::borderPropertyValue(const StylePropertyShorthand& width,
     return result.toString();
 }
 
+String StyleProperties::pageBreakPropertyValue(const StylePropertyShorthand& shorthand) const
+{
+    RefPtr<CSSValue> value = getPropertyCSSValueInternal(shorthand.properties()[0]);
+    // FIXME: Remove this isGlobalKeyword check after we do this consistently for all shorthands in getPropertyValue.
+    if (value->isGlobalKeyword())
+        return value->cssText();
+    CSSValueID valueId = downcast<CSSPrimitiveValue>(*value).valueID();
+    switch (valueId) {
+    case CSSValuePage:
+        return "always"_s;
+    case CSSValueAuto:
+    case CSSValueAvoid:
+    case CSSValueLeft:
+    case CSSValueRight:
+        return value->cssText();
+    default:
+        return String();
+    }
+}
+
 RefPtr<CSSValue> StyleProperties::getPropertyCSSValue(CSSPropertyID propertyID) const
 {
     return getPropertyCSSValueInternal(propertyID);
index e8572e2..0ca7e61 100644 (file)
@@ -163,6 +163,7 @@ private:
     String getCommonValue(const StylePropertyShorthand&) const;
     String getAlignmentShorthandValue(const StylePropertyShorthand&) const;
     String borderPropertyValue(const StylePropertyShorthand&, const StylePropertyShorthand&, const StylePropertyShorthand&) const;
+    String pageBreakPropertyValue(const StylePropertyShorthand&) const;
     String getLayeredShorthandValue(const StylePropertyShorthand&) const;
     String get2Values(const StylePropertyShorthand&) const;
     String get4Values(const StylePropertyShorthand&) const;
index 3eeaa08..aa0cd67 100644 (file)
@@ -314,22 +314,6 @@ void CSSPropertyParser::collectParsedCustomPropertyValueDependencies(const Strin
     parser.collectParsedCustomPropertyValueDependencies(syntax, isRoot, dependencies);
 }
 
-static bool isLegacyBreakProperty(CSSPropertyID propertyID)
-{
-    switch (propertyID) {
-    case CSSPropertyPageBreakAfter:
-    case CSSPropertyPageBreakBefore:
-    case CSSPropertyPageBreakInside:
-    case CSSPropertyWebkitColumnBreakAfter:
-    case CSSPropertyWebkitColumnBreakBefore:
-    case CSSPropertyWebkitColumnBreakInside:
-        return true;
-    default:
-        break;
-    }
-    return false;
-}
-
 bool CSSPropertyParser::parseValueStart(CSSPropertyID propertyID, bool important)
 {
     if (consumeCSSWideKeyword(propertyID, important))
@@ -342,11 +326,6 @@ 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()) {
@@ -5139,10 +5118,8 @@ static inline CSSValueID mapFromPageBreakBetween(CSSValueID value)
 {
     if (value == CSSValueAlways)
         return CSSValuePage;
-    if (value == CSSValueAuto || value == CSSValueLeft || value == CSSValueRight)
+    if (value == CSSValueAuto || value == CSSValueAvoid || value == CSSValueLeft || value == CSSValueRight)
         return value;
-    if (value == CSSValueAvoid)
-        return CSSValueAvoidPage;
     return CSSValueInvalid;
 }
 
@@ -5887,6 +5864,13 @@ bool CSSPropertyParser::parseShorthand(CSSPropertyID property, bool 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;