Changeset 282397 in webkit for trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp

Timestamp:

Sep 14, 2021, 11:38:58 AM (4 years ago)

Author:

[email protected]

Message:

Implement text-decoration-skip-ink
​https://bugs.webkit.org/show_bug.cgi?id=230244

Reviewed by Antti Koivisto.

LayoutTests/imported/w3c:

• web-platform-tests/css/css-cascade/all-prop-initial-xml-expected.txt:
• web-platform-tests/css/css-pseudo/parsing/marker-supported-properties-expected.txt:
• web-platform-tests/css/css-pseudo/parsing/marker-supported-properties-in-animation-expected.txt:
• web-platform-tests/css/css-text-decor/inheritance-expected.txt:
• web-platform-tests/css/css-text-decor/parsing/text-decoration-skip-ink-computed-expected.txt:
• web-platform-tests/css/css-text-decor/parsing/text-decoration-skip-ink-valid-expected.txt:
• web-platform-tests/css/css-text-decor/text-decoration-skip-ink-expected.txt:
• web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt:
• web-platform-tests/css/cssom/getComputedStyle-detached-subtree-expected.txt:

Source/WebCore:

First, there was "text-decoration-skip: ink", and that was what we implemented in WebKit. That had a
problem, though, which was that you couldn't add a value into the set without resetting the entire
property, so the CSSWG split it up into a bunch of constituent properties:

• text-decoration-skip-self
• text-decoration-skip-box
• text-decoration-skip-inset
• text-decoration-skip-spaces
• text-decoration-skip-ink

text-decoration-skip was turned into a shorthand for all of these constituent properties.

Chrome and Firefox, however, implemented text-decoration-skip-ink rather than text-decoration-skip.
From what I understand, Firefox isn't interested in implementing the shorthand until they implement
all the longhands. Therefore, because WebKit implements only the shorthand and Firefox implements only
the longhand, authors have to write two different properties in their style for the two different
browsers. That's unfortunate, so this patch makes WebKit follow the CSSWG, and turns
text-decoration-skip into a shorthand, and adds support for text-decoration-skip-ink. Therefore,
there's no new behavior here; it's just a new way of accessing the same functionality we already have.

The spec actually removed the "ink" value from the text-decoration-skip shorthand, but we have to
maintain it because of compat. So, this patch maps "text-decoration-skip: ink" to
"text-decoration-skip-ink: auto". There's an issue in the spec about this:
​https://drafts.csswg.org/css-text-decor-4/#issue-070668ae

Back when we implemented text-decoration-skip: ink, we only implemented the "ink" value, and none of
the other values. This patch also doesn't implement the other values.

Also, previously, we used to parse "text-decoration-skip: objects" but we didn't do anything with it.
This patch removes parsing support for that, so @supports works properly.

Covered by existing tests.

• css/CSSComputedStyleDeclaration.cpp:

(WebCore::renderTextDecorationSkipToCSSValue):
(WebCore::ComputedStyleExtractor::valueForPropertyInStyle):
(WebCore::renderTextDecorationSkipFlagsToCSSValue): Deleted.

• css/CSSPrimitiveValueMappings.h:

(WebCore::CSSPrimitiveValue::CSSPrimitiveValue):
(WebCore::CSSPrimitiveValue::operator TextDecorationSkipInk const):

• css/CSSProperties.json:
• css/StyleProperties.cpp:

(WebCore::StyleProperties::getPropertyValue const):
(WebCore::StyleProperties::textDecorationSkipValue const):

• css/StyleProperties.h:
• css/html.css:

(ins, del):

• css/parser/CSSParserFastPaths.cpp:

(WebCore::CSSParserFastPaths::isValidKeywordPropertyAndValue):
(WebCore::CSSParserFastPaths::isKeywordPropertyID):

• css/parser/CSSPropertyParser.cpp:

(WebCore::CSSPropertyParser::consumeTextDecorationSkip):
(WebCore::CSSPropertyParser::parseSingleValue):
(WebCore::CSSPropertyParser::parseShorthand):
(WebCore::consumeTextDecorationSkip): Deleted.

• css/parser/CSSPropertyParser.h:
• rendering/TextDecorationPainter.cpp:

(WebCore::TextDecorationPainter::paintTextDecoration):

• rendering/style/RenderStyle.cpp:

(WebCore::RenderStyle::changeRequiresRepaintIfTextOrBorderOrOutline const):

• rendering/style/RenderStyle.h:

(WebCore::RenderStyle::textDecorationSkipInk const):
(WebCore::RenderStyle::setTextDecorationSkipInk):
(WebCore::RenderStyle::initialTextDecorationSkipInk):
(WebCore::RenderStyle::textDecorationSkip const): Deleted.
(WebCore::RenderStyle::setTextDecorationSkip): Deleted.
(WebCore::RenderStyle::initialTextDecorationSkip): Deleted.

• rendering/style/RenderStyleConstants.cpp:

(WebCore::operator<<):

• rendering/style/RenderStyleConstants.h:
• rendering/style/StyleRareInheritedData.cpp:

(WebCore::StyleRareInheritedData::StyleRareInheritedData):
(WebCore::StyleRareInheritedData::operator== const):

• rendering/style/StyleRareInheritedData.h:
• style/InlineTextBoxStyle.cpp:

(WebCore::minLogicalTopForTextDecorationLine):
(WebCore::maxLogicalBottomForTextDecorationLine):

• style/PropertyCascade.cpp:

(WebCore::Style::shouldApplyPropertyInParseOrder):

• style/StyleBuilderConverter.h:

(WebCore::Style::BuilderConverter::valueToDecorationSkip): Deleted.
(WebCore::Style::BuilderConverter::convertTextDecorationSkip): Deleted.

LayoutTests:

• fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-roundtrip-expected.txt:
• fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-roundtrip.html:
• platform/ios-wk2/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt:
• platform/ios-wk2/imported/w3c/web-platform-tests/css/cssom/getComputedStyle-detached-subtree-expected.txt:

File:

• trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp (modified) (2 diffs)

trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp

@@ -1620 +1620 @@
 }
 
-static Ref<CSSValue> renderTextDecorationSkipFlagsToCSSValue(OptionSet<TextDecorationSkip> textDecorationSkip)
-{
-    // FIXME: This should probably return a CSSValueList with the set of all TextDecorationSkips.
-    switch (static_cast<TextDecorationSkip>(textDecorationSkip.toRaw())) {
-    case TextDecorationSkip::Auto:
+static RefPtr<CSSValue> renderTextDecorationSkipToCSSValue(TextDecorationSkipInk textDecorationSkipInk)
+{
+    switch (textDecorationSkipInk) {
+    case TextDecorationSkipInk::None:
+        return CSSValuePool::singleton().createIdentifierValue(CSSValueNone);
+    case TextDecorationSkipInk::Auto:
         return CSSValuePool::singleton().createIdentifierValue(CSSValueAuto);
-    case TextDecorationSkip::None:
-        return CSSValuePool::singleton().createIdentifierValue(CSSValueNone);
-    case TextDecorationSkip::Ink:
-        return CSSValuePool::singleton().createIdentifierValue(CSSValueInk);
-    case TextDecorationSkip::Objects:
-        return CSSValuePool::singleton().createIdentifierValue(CSSValueObjects);
+    case TextDecorationSkipInk::All:
+        return nullptr;
     }
 
@@ -3234 +3231 @@
             return currentColorOrValidColor(&style, style.textDecorationColor());
         case CSSPropertyTextDecorationSkip:
-            return renderTextDecorationSkipFlagsToCSSValue(style.textDecorationSkip());
+            return renderTextDecorationSkipToCSSValue(style.textDecorationSkipInk());
+        case CSSPropertyTextDecorationSkipInk:
+            return cssValuePool.createValue(style.textDecorationSkipInk());
         case CSSPropertyTextUnderlinePosition:
             return cssValuePool.createValue(style.textUnderlinePosition());