LibWeb: Support strings as list-style-types

We've long claimed to support this, but then silently ignored string
values, until 4cb2063577 which would
not-so-silently crash instead. (Oops)

So, actually pass the string value along and use it in the list marker.

As part of this, rename our `list-style-type` enum to
`counter-style-name-keyword`. This is an awkward name, attempting to be
spec-based. (The spec says `<counter-style>`, which is either a
`<counter-style-name>` or a function, and the `<counter-style-name>` is
a `<custom-ident>` that also has a few predefined values. So this is the
best I could come up with.)

Unfortunately only one WPT test for this passes - the others fail
because we produce a different layout when text is in `::before` than
when it's in `::marker`, and similar issues.

Libraries/LibWeb/CSS/ComputedProperties.cpp

@@ -1008,7 +1008,9 @@ TextTransform ComputedProperties::text_transform() const
 ListStyleType ComputedProperties::list_style_type() const
 {
     auto const& value = property(PropertyID::ListStyleType);
-    return keyword_to_list_style_type(value.to_keyword()).release_value();
+    if (value.is_string())
+        return value.as_string().string_value().to_string();
+    return keyword_to_counter_style_name_keyword(value.to_keyword()).release_value();
 }
 
 ListStylePosition ComputedProperties::list_style_position() const

Libraries/LibWeb/CSS/ComputedValues.h

@@ -78,6 +78,8 @@ struct Containment {
     bool is_empty() const { return !(size_containment || inline_size_containment || layout_containment || style_containment || paint_containment); }
 };
 
+using ListStyleType = Variant<CounterStyleNameKeyword, String>;
+
 class InitialValues {
 public:
     static AspectRatio aspect_ratio() { return AspectRatio { true, {} }; }
@@ -112,7 +114,7 @@ public:
     static Vector<Gfx::Filter> backdrop_filter() { return {}; }
     static Vector<Gfx::Filter> filter() { return {}; }
     static Color background_color() { return Color::Transparent; }
-    static CSS::ListStyleType list_style_type() { return CSS::ListStyleType::Disc; }
+    static CSS::ListStyleType list_style_type() { return CSS::CounterStyleNameKeyword::Disc; }
     static CSS::ListStylePosition list_style_position() { return CSS::ListStylePosition::Outside; }
     static CSS::Visibility visibility() { return CSS::Visibility::Visible; }
     static CSS::FlexDirection flex_direction() { return CSS::FlexDirection::Row; }

Libraries/LibWeb/CSS/Enums.json

@@ -122,6 +122,22 @@
     "auto",
     "hidden"
   ],
+  "counter-style-name-keyword": [
+    "circle",
+    "decimal",
+    "decimal-leading-zero",
+    "disc",
+    "disclosure-closed",
+    "disclosure-open",
+    "lower-alpha",
+    "lower-latin",
+    "lower-roman",
+    "none",
+    "square",
+    "upper-alpha",
+    "upper-latin",
+    "upper-roman"
+  ],
   "cursor": [
     "auto",
     "default",
@@ -386,22 +402,6 @@
     "inset",
     "outset"
   ],
-  "list-style-type": [
-    "circle",
-    "decimal",
-    "decimal-leading-zero",
-    "disc",
-    "disclosure-closed",
-    "disclosure-open",
-    "lower-alpha",
-    "lower-latin",
-    "lower-roman",
-    "none",
-    "square",
-    "upper-alpha",
-    "upper-latin",
-    "upper-roman"
-  ],
   "list-style-position": [
     "inside",
     "outside"
@@ -419,10 +419,10 @@
     "compact"
   ],
   "mix-blend-mode": [
-    "normal", 
+    "normal",
-    "multiply", 
+    "multiply",
-    "screen", 
+    "screen",
-    "overlay", 
+    "overlay",
     "darken",
     "lighten",
     "color-dodge",
@@ -434,8 +434,8 @@
     "hue",
     "saturation",
     "color",
-    "luminosity", 
+    "luminosity",
-    "plus-darker", 
+    "plus-darker",
     "plus-lighter"
   ],
   "object-fit": [

Libraries/LibWeb/CSS/Properties.json

@@ -1815,8 +1815,8 @@
     "inherited": true,
     "initial": "disc",
     "valid-types": [
-      "string",
+      "counter-style-name-keyword",
-      "list-style-type"
+      "string"
     ]
   },
   "margin": {

Libraries/LibWeb/CSS/StyleValues/CounterStyleValue.cpp

@@ -53,35 +53,34 @@ static String generate_a_counter_representation(CSSStyleValue const& counter_sty
         auto counter_style_name = counter_style.as_custom_ident().custom_ident();
         auto keyword = keyword_from_string(counter_style_name);
         if (keyword.has_value()) {
-            auto list_style_type = keyword_to_list_style_type(*keyword);
+            if (auto list_style_type = keyword_to_counter_style_name_keyword(*keyword); list_style_type.has_value()) {
-            if (list_style_type.has_value()) {
                 switch (*list_style_type) {
-                case ListStyleType::Square:
+                case CounterStyleNameKeyword::Square:
                     return "▪"_string;
-                case ListStyleType::Circle:
+                case CounterStyleNameKeyword::Circle:
                     return "◦"_string;
-                case ListStyleType::Disc:
+                case CounterStyleNameKeyword::Disc:
                     return "•"_string;
-                case ListStyleType::DisclosureClosed:
+                case CounterStyleNameKeyword::DisclosureClosed:
                     return "▸"_string;
-                case ListStyleType::DisclosureOpen:
+                case CounterStyleNameKeyword::DisclosureOpen:
                     return "▾"_string;
-                case ListStyleType::Decimal:
+                case CounterStyleNameKeyword::Decimal:
                     return MUST(String::formatted("{}", value));
-                case ListStyleType::DecimalLeadingZero:
+                case CounterStyleNameKeyword::DecimalLeadingZero:
                     // This is weird, but in accordance to spec.
                     if (value < 10)
                         return MUST(String::formatted("0{}", value));
                     return MUST(String::formatted("{}", value));
-                case ListStyleType::LowerAlpha:
+                case CounterStyleNameKeyword::LowerAlpha:
-                case ListStyleType::LowerLatin:
+                case CounterStyleNameKeyword::LowerLatin:
                     return String::bijective_base_from(value - 1, String::Case::Lower);
-                case ListStyleType::UpperAlpha:
+                case CounterStyleNameKeyword::UpperAlpha:
-                case ListStyleType::UpperLatin:
+                case CounterStyleNameKeyword::UpperLatin:
                     return String::bijective_base_from(value - 1, String::Case::Upper);
-                case ListStyleType::LowerRoman:
+                case CounterStyleNameKeyword::LowerRoman:
                     return String::roman_number_from(value, String::Case::Lower);
-                case ListStyleType::UpperRoman:
+                case CounterStyleNameKeyword::UpperRoman:
                     return String::roman_number_from(value, String::Case::Upper);
                 default:
                     break;

Libraries/LibWeb/Layout/ListItemMarkerBox.cpp

@@ -18,40 +18,43 @@ ListItemMarkerBox::ListItemMarkerBox(DOM::Document& document, CSS::ListStyleType
     , m_list_style_position(style_position)
     , m_index(index)
 {
-    switch (m_list_style_type) {
+    m_list_style_type.visit(
-    case CSS::ListStyleType::Square:
+        [this](CSS::CounterStyleNameKeyword keyword) {
-    case CSS::ListStyleType::Circle:
+            switch (keyword) {
-    case CSS::ListStyleType::Disc:
+            case CSS::CounterStyleNameKeyword::Square:
-    case CSS::ListStyleType::DisclosureClosed:
+            case CSS::CounterStyleNameKeyword::Circle:
-    case CSS::ListStyleType::DisclosureOpen:
+            case CSS::CounterStyleNameKeyword::Disc:
-        break;
+            case CSS::CounterStyleNameKeyword::DisclosureClosed:
-    case CSS::ListStyleType::Decimal:
+            case CSS::CounterStyleNameKeyword::DisclosureOpen:
-        m_text = MUST(String::formatted("{}.", m_index));
+                break;
-        break;
+            case CSS::CounterStyleNameKeyword::Decimal:
-    case CSS::ListStyleType::DecimalLeadingZero:
+                m_text = MUST(String::formatted("{}.", m_index));
-        // This is weird, but in accordance to spec.
+                break;
-        m_text = m_index < 10 ? MUST(String::formatted("0{}.", m_index)) : MUST(String::formatted("{}.", m_index));
+            case CSS::CounterStyleNameKeyword::DecimalLeadingZero:
-        break;
+                // This is weird, but in accordance to spec.
-    case CSS::ListStyleType::LowerAlpha:
+                m_text = m_index < 10 ? MUST(String::formatted("0{}.", m_index)) : MUST(String::formatted("{}.", m_index));
-    case CSS::ListStyleType::LowerLatin:
+                break;
-        m_text = String::bijective_base_from(m_index - 1, String::Case::Lower);
+            case CSS::CounterStyleNameKeyword::LowerAlpha:
-        break;
+            case CSS::CounterStyleNameKeyword::LowerLatin:
-    case CSS::ListStyleType::UpperAlpha:
+                m_text = String::bijective_base_from(m_index - 1, String::Case::Lower);
-    case CSS::ListStyleType::UpperLatin:
+                break;
-        m_text = String::bijective_base_from(m_index - 1, String::Case::Upper);
+            case CSS::CounterStyleNameKeyword::UpperAlpha:
-        break;
+            case CSS::CounterStyleNameKeyword::UpperLatin:
-    case CSS::ListStyleType::LowerRoman:
+                m_text = String::bijective_base_from(m_index - 1, String::Case::Upper);
-        m_text = String::roman_number_from(m_index, String::Case::Lower);
+                break;
-        break;
+            case CSS::CounterStyleNameKeyword::LowerRoman:
-    case CSS::ListStyleType::UpperRoman:
+                m_text = String::roman_number_from(m_index, String::Case::Lower);
-        m_text = String::roman_number_from(m_index, String::Case::Upper);
+                break;
-        break;
+            case CSS::CounterStyleNameKeyword::UpperRoman:
-    case CSS::ListStyleType::None:
+                m_text = String::roman_number_from(m_index, String::Case::Upper);
-        break;
+                break;
-
+            case CSS::CounterStyleNameKeyword::None:
-    default:
+                break;
-        VERIFY_NOT_REACHED();
+            }
-    }
+        },
+        [this](String const& string) {
+            m_text = string;
+        });
 }
 
 ListItemMarkerBox::~ListItemMarkerBox() = default;

Libraries/LibWeb/Layout/ListItemMarkerBox.h

@@ -24,14 +24,14 @@ public:
 
     virtual GC::Ptr<Painting::Paintable> create_paintable() const override;
 
-    CSS::ListStyleType list_style_type() const { return m_list_style_type; }
+    CSS::ListStyleType const& list_style_type() const { return m_list_style_type; }
     CSS::ListStylePosition list_style_position() const { return m_list_style_position; }
 
 private:
     virtual bool is_list_item_marker_box() const final { return true; }
     virtual bool can_have_children() const override { return false; }
 
-    CSS::ListStyleType m_list_style_type { CSS::ListStyleType::None };
+    CSS::ListStyleType m_list_style_type { CSS::CounterStyleNameKeyword::None };
     CSS::ListStylePosition m_list_style_position { CSS::ListStylePosition::Outside };
     size_t m_index;
 

Libraries/LibWeb/Painting/MarkerPaintable.cpp

@@ -66,65 +66,63 @@ void MarkerPaintable::paint(PaintContext& context, PaintPhase phase) const
 
     auto color = computed_values().color();
 
-    switch (layout_box().list_style_type()) {
+    if (auto& text = layout_box().text(); text.has_value()) {
-    case CSS::ListStyleType::Square:
-        context.display_list_recorder().fill_rect(device_marker_rect.to_type<int>(), color);
-        break;
-    case CSS::ListStyleType::Circle:
-        context.display_list_recorder().draw_ellipse(device_marker_rect.to_type<int>(), color, 1);
-        break;
-    case CSS::ListStyleType::Disc:
-        context.display_list_recorder().fill_ellipse(device_marker_rect.to_type<int>(), color);
-        break;
-    case CSS::ListStyleType::DisclosureClosed: {
-        // https://drafts.csswg.org/css-counter-styles-3/#disclosure-closed
-        // For the disclosure-open and disclosure-closed counter styles, the marker must be an image or character suitable for indicating the open and closed states of a disclosure widget, such as HTML’s details element.
-        // FIXME: If the image is directional, it must respond to the writing mode of the element, similar to the bidi-sensitive images feature of the Images 4 module.
-
-        // Draw an equilateral triangle pointing right.
-        auto path = Gfx::Path();
-        path.move_to({ left, top });
-        path.line_to({ left + sin_60_deg * (right - left), (top + bottom) / 2 });
-        path.line_to({ left, bottom });
-        path.close();
-        context.display_list_recorder().fill_path({ .path = path, .color = color, .winding_rule = Gfx::WindingRule::EvenOdd });
-        break;
-    }
-    case CSS::ListStyleType::DisclosureOpen: {
-        // https://drafts.csswg.org/css-counter-styles-3/#disclosure-open
-        // For the disclosure-open and disclosure-closed counter styles, the marker must be an image or character suitable for indicating the open and closed states of a disclosure widget, such as HTML’s details element.
-        // FIXME: If the image is directional, it must respond to the writing mode of the element, similar to the bidi-sensitive images feature of the Images 4 module.
-
-        // Draw an equilateral triangle pointing down.
-        auto path = Gfx::Path();
-        path.move_to({ left, top });
-        path.line_to({ right, top });
-        path.line_to({ (left + right) / 2, top + sin_60_deg * (bottom - top) });
-        path.close();
-        context.display_list_recorder().fill_path({ .path = path, .color = color, .winding_rule = Gfx::WindingRule::EvenOdd });
-        break;
-    }
-    case CSS::ListStyleType::Decimal:
-    case CSS::ListStyleType::DecimalLeadingZero:
-    case CSS::ListStyleType::LowerAlpha:
-    case CSS::ListStyleType::LowerLatin:
-    case CSS::ListStyleType::LowerRoman:
-    case CSS::ListStyleType::UpperAlpha:
-    case CSS::ListStyleType::UpperLatin:
-    case CSS::ListStyleType::UpperRoman: {
-        auto text = layout_box().text();
-        if (!text.has_value())
-            break;
         // FIXME: This should use proper text layout logic!
         // This does not line up with the text in the <li> element which looks very sad :(
         context.display_list_recorder().draw_text(device_enclosing.to_type<int>(), *text, layout_box().scaled_font(context), Gfx::TextAlignment::Center, color);
-        break;
+    } else if (auto const* counter_style = layout_box().list_style_type().get_pointer<CSS::CounterStyleNameKeyword>()) {
-    }
+        switch (*counter_style) {
-    case CSS::ListStyleType::None:
+        case CSS::CounterStyleNameKeyword::Square:
-        return;
+            context.display_list_recorder().fill_rect(device_marker_rect.to_type<int>(), color);
+            break;
+        case CSS::CounterStyleNameKeyword::Circle:
+            context.display_list_recorder().draw_ellipse(device_marker_rect.to_type<int>(), color, 1);
+            break;
+        case CSS::CounterStyleNameKeyword::Disc:
+            context.display_list_recorder().fill_ellipse(device_marker_rect.to_type<int>(), color);
+            break;
+        case CSS::CounterStyleNameKeyword::DisclosureClosed: {
+            // https://drafts.csswg.org/css-counter-styles-3/#disclosure-closed
+            // For the disclosure-open and disclosure-closed counter styles, the marker must be an image or character suitable for indicating the open and closed states of a disclosure widget, such as HTML’s details element.
+            // FIXME: If the image is directional, it must respond to the writing mode of the element, similar to the bidi-sensitive images feature of the Images 4 module.
 
-    default:
+            // Draw an equilateral triangle pointing right.
-        VERIFY_NOT_REACHED();
+            auto path = Gfx::Path();
+            path.move_to({ left, top });
+            path.line_to({ left + sin_60_deg * (right - left), (top + bottom) / 2 });
+            path.line_to({ left, bottom });
+            path.close();
+            context.display_list_recorder().fill_path({ .path = path, .color = color, .winding_rule = Gfx::WindingRule::EvenOdd });
+            break;
+        }
+        case CSS::CounterStyleNameKeyword::DisclosureOpen: {
+            // https://drafts.csswg.org/css-counter-styles-3/#disclosure-open
+            // For the disclosure-open and disclosure-closed counter styles, the marker must be an image or character suitable for indicating the open and closed states of a disclosure widget, such as HTML’s details element.
+            // FIXME: If the image is directional, it must respond to the writing mode of the element, similar to the bidi-sensitive images feature of the Images 4 module.
+
+            // Draw an equilateral triangle pointing down.
+            auto path = Gfx::Path();
+            path.move_to({ left, top });
+            path.line_to({ right, top });
+            path.line_to({ (left + right) / 2, top + sin_60_deg * (bottom - top) });
+            path.close();
+            context.display_list_recorder().fill_path({ .path = path, .color = color, .winding_rule = Gfx::WindingRule::EvenOdd });
+            break;
+        }
+        case CSS::CounterStyleNameKeyword::None:
+            return;
+        case CSS::CounterStyleNameKeyword::Decimal:
+        case CSS::CounterStyleNameKeyword::DecimalLeadingZero:
+        case CSS::CounterStyleNameKeyword::LowerAlpha:
+        case CSS::CounterStyleNameKeyword::LowerLatin:
+        case CSS::CounterStyleNameKeyword::LowerRoman:
+        case CSS::CounterStyleNameKeyword::UpperAlpha:
+        case CSS::CounterStyleNameKeyword::UpperLatin:
+        case CSS::CounterStyleNameKeyword::UpperRoman:
+            // These are handled by text() already.
+        default:
+            VERIFY_NOT_REACHED();
+        }
     }
 }
 

Tests/LibWeb/Ref/expected/wpt-import/css/css-lists/list-style-type-string-004-ref.html

@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <title>CSS Test: String value of list-style-type changing dynamically</title>
+  <link rel="author" title="Oriol Brufau" href="mailto:obrufau@gmail.com">
+  <style>
+    .list { list-style-type: "bar" }
+  </style>
+</head>
+<body>
+  <ol class="list">
+    <li>item 1</li>
+    <li>item 2</li>
+    <li>item 3</li>
+    <li>item 4</li>
+  </ol>
+  <ul class="list">
+    <li>item 1</li>
+    <li>item 2</li>
+    <li>item 3</li>
+    <li>item 4</li>
+  </ul>
+</body>
+</html>

Tests/LibWeb/Ref/input/wpt-import/css/css-lists/list-style-type-string-004.html

@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html lang="en" class="reftest-wait">
+<head>
+  <meta charset="UTF-8">
+  <title>CSS Test: String value of list-style-type changing dynamically</title>
+  <link rel="author" title="Oriol Brufau" href="mailto:obrufau@gmail.com">
+  <link rel="help" href="https://drafts.csswg.org/css-lists-3/#valdef-list-style-type-string">
+  <link rel="match" href="../../../../expected/wpt-import/css/css-lists/list-style-type-string-004-ref.html">
+  <meta name="assert" content="This test checks that the marker text is updated when list-style-type changes.">
+  <style>
+    .list { list-style-type: "foo" }
+  </style>
+</head>
+<body>
+  <ol class="list">
+    <li>item 1</li>
+    <li>item 2</li>
+    <li>item 3</li>
+    <li>item 4</li>
+  </ol>
+  <ul class="list">
+    <li>item 1</li>
+    <li>item 2</li>
+    <li>item 3</li>
+    <li>item 4</li>
+  </ul>
+  <script src="../../common/reftest-wait.js"></script>
+  <script>
+    "use strict";
+    addEventListener("load", function() {
+      requestAnimationFrame(() => {
+        for (const list of document.querySelectorAll(".list")) {
+          list.style.listStyleType = '"bar"';
+        }
+        takeScreenshot();
+      });
+    }, {once: true});
+  </script>
+</body>
+</html>