Attachment 369686 Details for Bug 197656 – Handle CSS generic keyword correctly

[patch] Handle CSS generic keyword correctly

bug-197656-20190513103449.patch (text/plain), 16.69 KB, created by Joonghun Park on 2019-05-12 18:35:09 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Joonghun Park

Created: 2019-05-12 18:35:09 PDT

Size: 16.69 KB

patch

obsolete

>Subversion Revision: 245215
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 880b4cabd9e6a55a771b2d66bab3633b88881644..22bcadba3813125d6ee59f7d539732c5f1a26c24 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2019-05-12  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
>+
>+        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-12  Takashi Komori  <Takashi.Komori@sony.com>
> 
>         [Curl] Suppress extra didReceiveAuthenticationChallenge call when accessing a server which checks basic auth.
>diff --git a/Source/WebCore/css/CSSProperties.json b/Source/WebCore/css/CSSProperties.json
>index d7c7656a252e02ca4124b820d15c3cf1654df223..996d0cff30caeee9a1feb4ec8d4c0b8be8832cc1 100644
>--- a/Source/WebCore/css/CSSProperties.json
>+++ b/Source/WebCore/css/CSSProperties.json
>@@ -3414,17 +3414,10 @@
>             }
>         },
>         "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",
>@@ -3432,17 +3425,10 @@
>             }
>         },
>         "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",
>@@ -3450,14 +3436,10 @@
>             }
>         },
>         "page-break-inside": {
>-            "values": [
>-                "auto",
>-                "avoid"
>-            ],
>             "codegen-properties": {
>-                "initial": "initialBreakInside",
>-                "name-for-methods": "BreakInside",
>-                "converter": "PageBreakInside"
>+                "longhands": [
>+                    "break-inside"
>+                ]
>             },
>             "specification": {
>                 "category": "css-22",
>@@ -4690,17 +4672,10 @@
>             }
>         },
>         "-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"
>@@ -4711,17 +4686,10 @@
>             }
>         },
>         "-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"
>@@ -4732,14 +4700,10 @@
>             }
>         },
>         "-webkit-column-break-inside": {
>-            "values": [
>-                "auto",
>-                "avoid"
>-            ],
>             "codegen-properties": {
>-                "initial": "initialBreakInside",
>-                "name-for-methods": "BreakInside",
>-                "converter": "ColumnBreakInside"
>+                "longhands": [
>+                    "break-inside"
>+                ]
>             },
>             "status": {
>                 "status": "experimental"
>diff --git a/Source/WebCore/css/StyleBuilderConverter.h b/Source/WebCore/css/StyleBuilderConverter.h
>index abf5caa3a8ab0215be860c52789da39302458023..6701ca571b0e4a1aed6d5f7daee4de77d0c14f04 100644
>--- a/Source/WebCore/css/StyleBuilderConverter.h
>+++ b/Source/WebCore/css/StyleBuilderConverter.h
>@@ -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)
> {
>diff --git a/Source/WebCore/css/StyleProperties.cpp b/Source/WebCore/css/StyleProperties.cpp
>index 84d512604abb4333d337c5a8ec1243fd06f391b6..5a6c18e6aa97cc20c814fa2f5d5dbbf0b5423a14 100644
>--- a/Source/WebCore/css/StyleProperties.cpp
>+++ b/Source/WebCore/css/StyleProperties.cpp
>@@ -209,6 +209,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 +719,24 @@ String StyleProperties::borderPropertyValue(const StylePropertyShorthand& width,
>     return result.toString();
> }
> 
>+String StyleProperties::pageBreakPropertyValue(const StylePropertyShorthand& shorthand) const
>+{
>+    RefPtr<CSSValue> value = getPropertyCSSValueInternal(shorthand.properties()[0]);
>+    // 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.
>+    if (value->isGlobalKeyword())
>+        return value->cssText();
>+    CSSValueID valueId = downcast<CSSPrimitiveValue>(*value).valueID();
>+    if (valueId == CSSValuePage)
>+        return "always";
>+    if (valueId == CSSValueAuto || valueId == CSSValueAvoid || valueId == CSSValueLeft
>+        || valueId == CSSValueRight)
>+        return value->cssText();
>+    return String();
>+}
>+
> RefPtr<CSSValue> StyleProperties::getPropertyCSSValue(CSSPropertyID propertyID) const
> {
>     return getPropertyCSSValueInternal(propertyID);
>diff --git a/Source/WebCore/css/StyleProperties.h b/Source/WebCore/css/StyleProperties.h
>index e8572e29751c1bbabdc298fd815d2a8886a9383b..0ca7e613468b5a78cca6b8e7692352b71e431a3f 100644
>--- a/Source/WebCore/css/StyleProperties.h
>+++ b/Source/WebCore/css/StyleProperties.h
>@@ -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;
>diff --git a/Source/WebCore/css/parser/CSSPropertyParser.cpp b/Source/WebCore/css/parser/CSSPropertyParser.cpp
>index 3eeaa080fae12947965a7ea3d3b01597486bdcf2..aa0cd67f74781827623f3c008e793ad0a890824d 100644
>--- a/Source/WebCore/css/parser/CSSPropertyParser.cpp
>+++ b/Source/WebCore/css/parser/CSSPropertyParser.cpp
>@@ -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;
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 0a05cee769733eda7e6c1e7b1324c0e23d54988d..6bb5765f64c41f47821fd95a8b3ec2c364475cf5 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-05-12  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
>+
>+        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-07  Antoine Quint  <graouts@apple.com>
> 
>         [Pointer Events] isPrimary property of pointercancel events should match previous events for that pointer
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/cssom/serialize-values-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/cssom/serialize-values-expected.txt
>index 372047759b48e9ca414ef953b23cf0eae40caa6d..64a04b94b58107bb1dc8b70ebb677156ceab090b 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/cssom/serialize-values-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/cssom/serialize-values-expected.txt
>@@ -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

Actions: View | Formatted Diff | Diff

Attachments on bug 197656: 369565 | 369574 | 369576 | 369583 | 369587 | 369588 | 369674 | 369675 | 369677 | 369684 | 369685 | 369686 | 369830 | 369831