[css-counter-styles] RenderListMarker, RenderCounter should use CSSCo…

…unterStyle::text() to handle TextDirection https://bugs.webkit.org/show_bug.cgi?id=256153 rdar://109014745 Reviewed by Matthieu Dubet and Tim Nguyen. Before we can address the bug, we need a small refactoring on how we handle disclosure-closed counter style: With this patch, we will handle disclosure-closed as one of the special complex systems, such that we can consider RTL context when calculating its text value. We fail css/css-counter-styles/counter-style-at-rule/disclosure-styles.html for 2 different reasons. Therefore, we need to fix it in 2 different points: 1. RenderListMarker: This class is used for rendering list markers for the general case. It has custom code for hadling TextDirection for counter styles due to disclosure-closed style. We can now delegate this to CSSCounterStyle::text by passing TextDirection information. The custom code currently checks if the counter-style has a "disclosure-closed" identifier but that won't work for "extends" systems. When an author extends a counter-style, it can give any identifier name to it, but it should still act as the counter-style it is extending. Since this is now handled by CSSCounterStyle itself, we no longer need the custom code. 2. RenderCounter: The class is used when rendering counters with the counter() function. These counters can also be a list-style-type that is mapped to an existent counter-style. Currently we don't handle TextDirection here, which breaks disclosure-closed style for the RTL case. Also here we can delegate TextDirection processing by passing the element's direction to CSSCounterStyle::text(). * LayoutTests/TestExpectations: * Source/WTF/wtf/unicode/CharacterNames.h: * Source/WebCore/css/CSSCounterStyle.cpp: (WebCore::CSSCounterStyle::counterForSystemDisclosureClosed): (WebCore::CSSCounterStyle::initialRepresentation const): (WebCore::CSSCounterStyle::fallbackText): (WebCore::CSSCounterStyle::text): (WebCore::CSSCounterStyle::isInRange const): * Source/WebCore/css/CSSCounterStyle.h: * Source/WebCore/css/CSSCounterStyleDescriptors.cpp: (WebCore::CSSCounterStyleDescriptors::areSymbolsValidForSystem): (WebCore::CSSCounterStyleDescriptors::systemCSSText const): * Source/WebCore/css/CSSCounterStyleDescriptors.h: * Source/WebCore/css/CSSCounterStyleRule.cpp: (WebCore::toCounterStyleSystemEnum): * Source/WebCore/css/CSSValueKeywords.in: * Source/WebCore/css/counterStyles.css: (@counter-style disclosure-closed): * Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp: (WebCore::CSSPropertyParserHelpers::consumeCounterStyleSystem): * Source/WebCore/rendering/RenderCounter.cpp: (WebCore::RenderCounter::originalText const): * Source/WebCore/rendering/RenderListMarker.cpp: (WebCore::RenderListMarker::textRun const): (WebCore::RenderListMarker::updateContent): * Source/WebCore/rendering/style/ListStyleType.cpp: (WebCore::ListStyleType::isDisclosureClosed const): Deleted. * Source/WebCore/rendering/style/ListStyleType.h: Canonical link: https://commits.webkit.org/279404@main
WebKit · May 28, 2024 · 8105a85 · 8105a85
1 parent 5dceca8
commit 8105a85
Show file tree

Hide file tree

Showing 14 changed files with 43 additions and 37 deletions.
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -6443,9 +6443,6 @@ http/wpt/loading/early-hints [ Skip ]
 # Most ports don't have a HEIC decoder.
 http/tests/misc/heic-accept-header.html [ Pass Failure ]
 
-# Needs proper bidi algorithm.
-webkit.org/b/256153 imported/w3c/web-platform-tests/css/css-counter-styles/counter-style-at-rule/disclosure-styles.html [ ImageOnlyFailure ]
-
 # Missing support to counter-style at-rule in shadow scope rdar://103873418
 imported/w3c/web-platform-tests/css/css-counter-styles/counter-style-at-rule/override-in-shadow-dom.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-counter-styles/counter-style-at-rule/fallbacks-in-shadow-dom.html [ ImageOnlyFailure ]

diff --git a/Source/WTF/wtf/unicode/CharacterNames.h b/Source/WTF/wtf/unicode/CharacterNames.h
@@ -40,6 +40,7 @@ constexpr char32_t aegeanWordSeparatorLine = 0x10100;
 constexpr UChar apostrophe = 0x0027;
 constexpr UChar blackCircle = 0x25CF;
 constexpr UChar blackLeftPointingSmallTriangle = 0x25C2;
+constexpr UChar blackRightPointingSmallTriangle = 0x25B8;
 constexpr UChar blackSquare = 0x25A0;
 constexpr UChar blackUpPointingTriangle = 0x25B2;
 constexpr UChar bullet = 0x2022;
@@ -158,6 +159,7 @@ using WTF::Unicode::aegeanWordSeparatorDot;
 using WTF::Unicode::aegeanWordSeparatorLine;
 using WTF::Unicode::blackCircle;
 using WTF::Unicode::blackLeftPointingSmallTriangle;
+using WTF::Unicode::blackRightPointingSmallTriangle;
 using WTF::Unicode::blackSquare;
 using WTF::Unicode::blackUpPointingTriangle;
 using WTF::Unicode::bullet;

diff --git a/Source/WebCore/css/CSSCounterStyle.cpp b/Source/WebCore/css/CSSCounterStyle.cpp
@@ -31,6 +31,7 @@
 #include <cmath>
 #include <wtf/text/StringBuilder.h>
 #include <wtf/text/TextBreakIterator.h>
+#include <wtf/unicode/CharacterNames.h>
 
 namespace WebCore {
 
@@ -236,6 +237,11 @@ static String counterForSystemCJK(int number, const std::array<UChar, 17>& table
     return std::span<const UChar> { characters, length };
 }
 
+String CSSCounterStyle::counterForSystemDisclosureClosed(TextDirection textDirection)
+{
+    return textDirection == TextDirection::LTR ? span(blackRightPointingSmallTriangle) : span(blackLeftPointingSmallTriangle);
+}
+
 String CSSCounterStyle::counterForSystemSimplifiedChineseInformal(int value)
 {
     static constexpr std::array<UChar, 17> simplifiedChineseInformalTable {
@@ -326,7 +332,7 @@ String CSSCounterStyle::counterForSystemEthiopicNumeric(unsigned value)
     return std::span<const UChar> { buffer, length };
 }
 
-String CSSCounterStyle::initialRepresentation(int value) const
+String CSSCounterStyle::initialRepresentation(int value, TextDirection textDirection) const
 {
     unsigned absoluteValue = std::abs(value);
     switch (system()) {
@@ -342,6 +348,8 @@ String CSSCounterStyle::initialRepresentation(int value) const
         return counterForSystemAdditive(absoluteValue);
     case CSSCounterStyleDescriptors::System::Fixed:
         return counterForSystemFixed(value);
+    case CSSCounterStyleDescriptors::System::DisclosureClosed:
+        return counterForSystemDisclosureClosed(textDirection);
     case CSSCounterStyleDescriptors::System::SimplifiedChineseInformal:
         return CSSCounterStyle::counterForSystemSimplifiedChineseInformal(value);
     case CSSCounterStyleDescriptors::System::SimplifiedChineseFormal:
@@ -360,26 +368,26 @@ String CSSCounterStyle::initialRepresentation(int value) const
     return { };
 }
 
-String CSSCounterStyle::fallbackText(int value)
+String CSSCounterStyle::fallbackText(int value, TextDirection textDirection)
 {
     if (m_isFallingBack || !fallback().get()) {
         m_isFallingBack = false;
-        return CSSCounterStyleRegistry::decimalCounter()->text(value);
+        return CSSCounterStyleRegistry::decimalCounter()->text(value, textDirection);
     }
     m_isFallingBack = true;
-    auto fallbackText = fallback()->text(value);
+    auto fallbackText = fallback()->text(value, textDirection);
     m_isFallingBack = false;
     return fallbackText;
 }
 
-String CSSCounterStyle::text(int value)
+String CSSCounterStyle::text(int value, TextDirection textDirection)
 {
     if (!isInRange(value))
-        return fallbackText(value);
+        return fallbackText(value, textDirection);
 
-    auto result = initialRepresentation(value);
+    auto result = initialRepresentation(value, textDirection);
     if (result.isNull())
-        return fallbackText(value);
+        return fallbackText(value, textDirection);
     applyPadSymbols(result, value);
     if (shouldApplyNegativeSymbols(value))
         applyNegativeSymbols(result);
@@ -421,6 +429,7 @@ bool CSSCounterStyle::isInRange(int value) const
         case CSSCounterStyleDescriptors::System::Cyclic:
         case CSSCounterStyleDescriptors::System::Numeric:
         case CSSCounterStyleDescriptors::System::Fixed:
+        case CSSCounterStyleDescriptors::System::DisclosureClosed:
             return true;
         case CSSCounterStyleDescriptors::System::Alphabetic:
         case CSSCounterStyleDescriptors::System::Symbolic:

diff --git a/Source/WebCore/css/CSSCounterStyle.h b/Source/WebCore/css/CSSCounterStyle.h
@@ -26,6 +26,7 @@
 #pragma once
 
 #include "CSSCounterStyleDescriptors.h"
+#include "TextDirection.h"
 #include <wtf/Forward.h>
 #include <wtf/WeakPtr.h>
 #include <wtf/text/AtomString.h>
@@ -44,7 +45,7 @@ class CSSCounterStyle : public RefCounted<CSSCounterStyle>, public CanMakeWeakPt
             && m_predefinedCounterStyle == other.m_predefinedCounterStyle;
     }
 
-    String text(int);
+    String text(int, TextDirection = TextDirection::LTR);
     const CSSCounterStyleDescriptors::Name& name() const { return m_descriptors.m_name; }
     CSSCounterStyleDescriptors::System system() const { return m_descriptors.m_system; }
     const CSSCounterStyleDescriptors::NegativeSymbols& negative() const { return m_descriptors.m_negativeSymbols; }
@@ -83,6 +84,7 @@ class CSSCounterStyle : public RefCounted<CSSCounterStyle>, public CanMakeWeakPt
     static String counterForSystemTraditionalChineseInformal(int);
     static String counterForSystemTraditionalChineseFormal(int);
     static String counterForSystemEthiopicNumeric(unsigned);
+    static String counterForSystemDisclosureClosed(TextDirection);
 
 private:
     CSSCounterStyle(const CSSCounterStyleDescriptors&, bool isPredefinedCounterStyle);
@@ -94,12 +96,12 @@ class CSSCounterStyle : public RefCounted<CSSCounterStyle>, public CanMakeWeakPt
     bool shouldApplyNegativeSymbols(int) const;
     // https://www.w3.org/TR/css-counter-styles-3/#counter-style-fallback
     WeakPtr<CSSCounterStyle> fallback() const { return m_fallbackReference; };
-    String fallbackText(int);
+    String fallbackText(int, TextDirection);
     // Generates a CSSCounterStyle object as it was defined by a 'decimal' descriptor. It is used as a last-resource in case we can't resolve fallback references.
     void applyPadSymbols(String&, int) const;
     void applyNegativeSymbols(String&) const;
     // Initial text representation for the counter, before applying pad and/or negative symbols. Suffix and Prefix are also not considered as described by https://www.w3.org/TR/css-counter-styles-3/#counter-styles.
-    String initialRepresentation(int) const;
+    String initialRepresentation(int, TextDirection) const;
 
     String counterForSystemCyclic(int) const;
     String counterForSystemFixed(int) const;

diff --git a/Source/WebCore/css/CSSCounterStyleDescriptors.cpp b/Source/WebCore/css/CSSCounterStyleDescriptors.cpp
@@ -300,6 +300,8 @@ bool CSSCounterStyleDescriptors::areSymbolsValidForSystem(CSSCounterStyleDescrip
     case System::EthiopicNumeric:
     case System::Extends:
         return !symbols.size() && !additiveSymbols.size();
+    case System::DisclosureClosed:
+        return true;
     default:
         ASSERT_NOT_REACHED();
         return false;
@@ -433,6 +435,7 @@ String CSSCounterStyleDescriptors::systemCSSText() const
     case System::TraditionalChineseInformal:
     case System::TraditionalChineseFormal:
     case System::EthiopicNumeric:
+    case System::DisclosureClosed:
         return emptyString();
     }
     return emptyString();

diff --git a/Source/WebCore/css/CSSCounterStyleDescriptors.h b/Source/WebCore/css/CSSCounterStyleDescriptors.h
@@ -54,6 +54,7 @@ struct CSSCounterStyleDescriptors {
         Symbolic,
         Additive,
         Fixed,
+        DisclosureClosed,
         SimplifiedChineseInformal,
         SimplifiedChineseFormal,
         TraditionalChineseInformal,

diff --git a/Source/WebCore/css/CSSCounterStyleRule.cpp b/Source/WebCore/css/CSSCounterStyleRule.cpp
@@ -76,6 +76,8 @@ CSSCounterStyleDescriptors::System toCounterStyleSystemEnum(const CSSValue* syst
         return CSSCounterStyleDescriptors::System::Numeric;
     case CSSValueAdditive:
         return CSSCounterStyleDescriptors::System::Additive;
+    case CSSValueInternalDisclosureClosed:
+        return CSSCounterStyleDescriptors::System::DisclosureClosed;
     case CSSValueInternalSimplifiedChineseInformal:
         return CSSCounterStyleDescriptors::System::SimplifiedChineseInformal;
     case CSSValueInternalSimplifiedChineseFormal:

diff --git a/Source/WebCore/css/CSSValueKeywords.in b/Source/WebCore/css/CSSValueKeywords.in
@@ -1664,6 +1664,7 @@ numeric
 symbolic
 additive
 // fixed
+-internal-disclosure-closed
 -internal-simplified-chinese-informal
 -internal-simplified-chinese-formal
 -internal-traditional-chinese-informal

diff --git a/Source/WebCore/css/counterStyles.css b/Source/WebCore/css/counterStyles.css
@@ -279,7 +279,7 @@
 }
 
 @counter-style disclosure-closed {
-    system: cyclic;
+    system: -internal-disclosure-closed;
     suffix: " ";
     /* for symbols, see normative text */
     symbols: "\25B8";

diff --git a/Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp b/Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp
@@ -5496,6 +5496,7 @@ RefPtr<CSSValue> consumeCounterStyleSystem(CSSParserTokenRange& range, const CSS
 
     if (isUASheetBehavior(context.mode)) {
         auto internalKeyword = consumeIdent<
+            CSSValueInternalDisclosureClosed,
             CSSValueInternalSimplifiedChineseInformal,
             CSSValueInternalSimplifiedChineseFormal,
             CSSValueInternalTraditionalChineseInformal,

diff --git a/Source/WebCore/rendering/RenderCounter.cpp b/Source/WebCore/rendering/RenderCounter.cpp
@@ -466,26 +466,24 @@ String RenderCounter::originalText() const
     RefPtr child = m_counterNode.get();
     int value = child->actsAsReset() ? child->value() : child->countInParent();
 
-    auto counterText = [](const ListStyleType& styleType, int value, CSSCounterStyle* counterStyle) {
-        if (styleType.type == ListStyleType::Type::None)
+    auto counterText = [this](int value) {
+        if (this->m_counter.listStyleType().type == ListStyleType::Type::None)
             return emptyString();
 
-        if (styleType.type == ListStyleType::Type::CounterStyle) {
-            ASSERT(counterStyle);
-            return counterStyle->text(value);
+        if (this->m_counter.listStyleType().type == ListStyleType::Type::CounterStyle) {
+            ASSERT(this->counterStyle());
+            return this->counterStyle()->text(value, this->style().direction());
         }
 
         ASSERT_NOT_REACHED();
         return emptyString();
     };
-    auto counterStyle = this->counterStyle();
-    String text = counterText(m_counter.listStyleType(), value, counterStyle.get());
-
+    auto text = counterText(value);
     if (!m_counter.separator().isNull()) {
         if (!child->actsAsReset())
             child = child->parent();
         while (CounterNode* parent = child->parent()) {
-            text = counterText(m_counter.listStyleType(), child->countInParent(), counterStyle.get())
+            text = counterText(child->countInParent())
                 + m_counter.separator() + text;
             child = parent;
         }

diff --git a/Source/WebCore/rendering/RenderListMarker.cpp b/Source/WebCore/rendering/RenderListMarker.cpp
@@ -126,12 +126,8 @@ auto RenderListMarker::textRun() const -> TextRunWithUnderlyingString
     if (m_textIsLeftToRightDirection) {
         if (style().isLeftToRightDirection())
             textForRun = m_textWithSuffix;
-        else {
-            if (style().listStyleType().isDisclosureClosed())
-                textForRun = span(blackLeftPointingSmallTriangle);
-            else
-                textForRun = makeString(reversed(StringView(m_textWithSuffix).substring(m_textWithoutSuffixLength)), m_textWithSuffix.left(m_textWithoutSuffixLength));
-        }
+        else
+            textForRun = makeString(reversed(StringView(m_textWithSuffix).substring(m_textWithoutSuffixLength)), m_textWithSuffix.left(m_textWithoutSuffixLength));
     } else {
         if (!style().isLeftToRightDirection())
             textForRun = reversed(m_textWithSuffix);
@@ -317,7 +313,7 @@ void RenderListMarker::updateContent()
     case ListStyleType::Type::CounterStyle: {
         auto counter = counterStyle();
         ASSERT(counter);
-        auto text = makeString(counter->prefix().text, counter->text(m_listItem->value()));
+        auto text = makeString(counter->prefix().text, counter->text(m_listItem->value(), style().direction()));
         m_textWithSuffix = makeString(text, counter->suffix().text);
         m_textWithoutSuffixLength = text.length();
         m_textIsLeftToRightDirection = u_charDirection(text[0]) != U_RIGHT_TO_LEFT;

diff --git a/Source/WebCore/rendering/style/ListStyleType.cpp b/Source/WebCore/rendering/style/ListStyleType.cpp
@@ -46,11 +46,6 @@ bool ListStyleType::isDisc() const
     return type == Type::CounterStyle && identifier == nameLiteral(CSSValueDisc);
 }
 
-bool ListStyleType::isDisclosureClosed() const
-{
-    return type == Type::CounterStyle && identifier == nameLiteral(CSSValueDisclosureClosed);
-}
-
 TextStream& operator<<(TextStream& ts, ListStyleType::Type styleType)
 {
     return ts << nameLiteral(toCSSValueID(styleType)).characters();

diff --git a/Source/WebCore/rendering/style/ListStyleType.h b/Source/WebCore/rendering/style/ListStyleType.h
@@ -42,7 +42,6 @@ struct ListStyleType {
     bool isCircle() const;
     bool isSquare() const;
     bool isDisc() const;
-    bool isDisclosureClosed() const;
 
     friend bool operator==(const ListStyleType&, const ListStyleType&) = default;