LibWeb: Support reversed ordered lists

This PR adds support for the `reversed` attribute of
ordered lists.

Libraries/LibWeb/DOM/Element.cpp

@@ -49,6 +49,9 @@
 #include <LibWeb/HTML/HTMLFrameSetElement.h>
 #include <LibWeb/HTML/HTMLHtmlElement.h>
 #include <LibWeb/HTML/HTMLInputElement.h>
+#include <LibWeb/HTML/HTMLLIElement.h>
+#include <LibWeb/HTML/HTMLMenuElement.h>
+#include <LibWeb/HTML/HTMLOListElement.h>
 #include <LibWeb/HTML/HTMLOptGroupElement.h>
 #include <LibWeb/HTML/HTMLOptionElement.h>
 #include <LibWeb/HTML/HTMLSelectElement.h>
@@ -57,6 +60,7 @@
 #include <LibWeb/HTML/HTMLTableElement.h>
 #include <LibWeb/HTML/HTMLTemplateElement.h>
 #include <LibWeb/HTML/HTMLTextAreaElement.h>
+#include <LibWeb/HTML/HTMLUListElement.h>
 #include <LibWeb/HTML/Numbers.h>
 #include <LibWeb/HTML/Parser/HTMLParser.h>
 #include <LibWeb/HTML/Scripting/Agent.h>
@@ -3142,6 +3146,111 @@ bool Element::has_paint_containment() const
     return false;
 }
 
+size_t Element::number_of_owned_list_items() const
+{
+    auto number_of_owned_li_elements = 0;
+    for_each_child_of_type<DOM::Element>([&](auto& child) {
+        if (child.list_owner() == this) {
+            number_of_owned_li_elements++;
+        }
+        return IterationDecision::Continue;
+    });
+    return number_of_owned_li_elements;
+}
+
+// https://html.spec.whatwg.org/multipage/grouping-content.html#list-owner
+Element const* Element::list_owner() const
+{
+    // Any element whose computed value of 'display' is 'list-item' has a list owner, which is determined as follows:
+    if (!computed_properties()->display().is_list_item())
+        return nullptr;
+
+    // 1. If the element is not being rendered, return null; the element has no list owner.
+    if (!layout_node())
+        return nullptr;
+
+    // 2. Let ancestor be the element's parent.
+    auto const* ancestor = parent_element();
+
+    // 3. If the element has an ol, ul, or menu ancestor, set ancestor to the closest such ancestor element.
+    for_each_ancestor([&ancestor](GC::Ref<Node> node) {
+        if (is<HTML::HTMLOListElement>(*node) || is<HTML::HTMLUListElement>(*node) || is<HTML::HTMLMenuElement>(*node)) {
+            ancestor = static_cast<Element const*>(node.ptr());
+            return IterationDecision::Break;
+        }
+        return IterationDecision::Continue;
+    });
+
+    // 4. Return the closest inclusive ancestor of ancestor that produces a CSS box.
+    // Spec-Note: Such an element will always exist, as at the very least the document element will always produce a CSS box.
+    ancestor->for_each_inclusive_ancestor([&ancestor](GC::Ref<Node> node) {
+        if (is<Element>(*node) && node->paintable_box()) {
+            ancestor = static_cast<Element const*>(node.ptr());
+            return IterationDecision::Break;
+        }
+        return IterationDecision::Continue;
+    });
+    return ancestor;
+}
+
+// https://html.spec.whatwg.org/multipage/grouping-content.html#ordinal-value
+size_t Element::ordinal_value() const
+{
+    // NOTE: The spec provides an algorithm to determine the ordinal value of each element owned by a given list owner.
+    //       However, we are only interested in the ordinal value of this element.
+
+    // FIXME: 1. Let i be 1.
+
+    // 2. If owner is an ol element, let numbering be owner's starting value. Otherwise, let numbering be 1.
+    auto const* owner = list_owner();
+    if (!owner) {
+        return 1;
+    }
+
+    auto numbering = 1;
+    auto reversed = false;
+    if (is<HTML::HTMLOListElement>(owner)) {
+        auto const* ol_element = static_cast<const HTML::HTMLOListElement*>(owner);
+        numbering = ol_element->starting_value();
+        reversed = ol_element->has_attribute(HTML::AttributeNames::reversed);
+    }
+
+    // FIXME: 3. Loop : If i is greater than the number of list items that owner owns, then return; all of owner's owned list items have been assigned ordinal values.
+    // FIXME: 4. Let item be the ith of owner's owned list items, in tree order.
+
+    owner->for_each_child_of_type<DOM::Element>([&](auto& item) {
+        if (item.list_owner() == owner) {
+            // 5. If item is an li element that has a value attribute, then:
+            auto value_attribute = item.get_attribute(HTML::AttributeNames::value);
+            if (is<HTML::HTMLLIElement>(item) && value_attribute.has_value()) {
+                // 1. Let parsed be the result of parsing the value of the attribute as an integer.
+                auto parsed = HTML::parse_integer(value_attribute.value());
+
+                // 2. If parsed is not an error, then set numbering to parsed.
+                if (parsed.has_value())
+                    numbering = parsed.value();
+            }
+
+            // FIXME: 6. The ordinal value of item is numbering.
+            if (&item == this)
+                return IterationDecision::Break;
+
+            // 7. If owner is an ol element, and owner has a reversed attribute, decrement numbering by 1; otherwise, increment numbering by 1.
+            if (reversed) {
+                numbering--;
+            } else {
+                numbering++;
+            }
+
+            // FIXME: 8. Increment i by 1.
+        }
+        return IterationDecision::Continue;
+    });
+
+    // FIXME: 9. Go to the step labeled loop.
+    return numbering;
+}
+
 bool Element::id_reference_exists(String const& id_reference) const
 {
     return document().get_element_by_id(id_reference);
@@ -3687,5 +3796,4 @@ Optional<String> Element::lang() const
         return {};
     return maybe_lang.release_value();
 }
-
 }

Libraries/LibWeb/DOM/Element.h

@@ -454,6 +454,10 @@ public:
         return affected_by_sibling_combinator() || affected_by_first_or_last_child_pseudo_class() || affected_by_nth_child_pseudo_class();
     }
 
+    size_t number_of_owned_list_items() const;
+    Element const* list_owner() const;
+    size_t ordinal_value() const;
+
 protected:
     Element(Document&, DOM::QualifiedName);
     virtual void initialize(JS::Realm&) override;

Libraries/LibWeb/HTML/HTMLOListElement.cpp

@@ -7,6 +7,7 @@
 #include <LibWeb/Bindings/HTMLOListElementPrototype.h>
 #include <LibWeb/Bindings/Intrinsics.h>
 #include <LibWeb/CSS/StyleValues/CSSKeywordValue.h>
+#include <LibWeb/DOM/Document.h>
 #include <LibWeb/HTML/HTMLOListElement.h>
 #include <LibWeb/HTML/Numbers.h>
 
@@ -37,6 +38,29 @@ WebIDL::Long HTMLOListElement::start()
     return 1;
 }
 
+// https://html.spec.whatwg.org/multipage/grouping-content.html#concept-ol-start
+size_t HTMLOListElement::starting_value() const
+{
+    // 1. If the ol element has a start attribute, then:
+    auto start = get_attribute(AttributeNames::start);
+    if (start.has_value()) {
+        // 1. Let parsed be the result of parsing the value of the attribute as an integer.
+        auto parsed = parse_integer(start.value());
+
+        // 2. If parsed is not an error, then return parsed.
+        if (parsed.has_value())
+            return parsed.value();
+    }
+
+    // 2. If the ol element has a reversed attribute, then return the number of owned li elements.
+    if (has_attribute(AttributeNames::reversed)) {
+        return number_of_owned_list_items();
+    }
+
+    // 3. Return 1.
+    return 1;
+}
+
 bool HTMLOListElement::is_presentational_hint(FlyString const& name) const
 {
     if (Base::is_presentational_hint(name))

Libraries/LibWeb/HTML/HTMLOListElement.h

@@ -28,6 +28,8 @@ public:
         MUST(set_attribute(AttributeNames::start, String::number(start)));
     }
 
+    size_t starting_value() const;
+
 private:
     HTMLOListElement(DOM::Document&, DOM::QualifiedName);
 

Libraries/LibWeb/Layout/ListItemMarkerBox.cpp

@@ -12,56 +12,63 @@ namespace Web::Layout {
 
 GC_DEFINE_ALLOCATOR(ListItemMarkerBox);
 
-ListItemMarkerBox::ListItemMarkerBox(DOM::Document& document, CSS::ListStyleType style_type, CSS::ListStylePosition style_position, size_t index, GC::Ref<CSS::ComputedProperties> style)
+ListItemMarkerBox::ListItemMarkerBox(DOM::Document& document, CSS::ListStyleType style_type, CSS::ListStylePosition style_position, GC::Ref<DOM::Element> list_item_element, GC::Ref<CSS::ComputedProperties> style)
     : Box(document, nullptr, move(style))
     , m_list_style_type(style_type)
     , m_list_style_position(style_position)
-    , m_index(index)
+    , m_list_item_element(list_item_element)
 {
-    m_list_style_type.visit(
-        [this](CSS::CounterStyleNameKeyword keyword) {
+}
+
+ListItemMarkerBox::~ListItemMarkerBox() = default;
+
+Optional<String> ListItemMarkerBox::text() const
+{
+    auto index = m_list_item_element->ordinal_value();
+
+    return m_list_style_type.visit(
+        [index](CSS::CounterStyleNameKeyword keyword) -> Optional<String> {
             switch (keyword) {
             case CSS::CounterStyleNameKeyword::Square:
             case CSS::CounterStyleNameKeyword::Circle:
             case CSS::CounterStyleNameKeyword::Disc:
             case CSS::CounterStyleNameKeyword::DisclosureClosed:
             case CSS::CounterStyleNameKeyword::DisclosureOpen:
-                break;
+                return {};
             case CSS::CounterStyleNameKeyword::Decimal:
-                m_text = MUST(String::formatted("{}.", m_index));
-                break;
+                return MUST(String::formatted("{}.", index));
             case CSS::CounterStyleNameKeyword::DecimalLeadingZero:
                 // This is weird, but in accordance to spec.
-                m_text = m_index < 10 ? MUST(String::formatted("0{}.", m_index)) : MUST(String::formatted("{}.", m_index));
-                break;
+                return MUST(index < 10 ? String::formatted("0{}.", index) : String::formatted("{}.", index));
             case CSS::CounterStyleNameKeyword::LowerAlpha:
             case CSS::CounterStyleNameKeyword::LowerLatin:
-                m_text = String::bijective_base_from(m_index - 1, String::Case::Lower);
-                break;
+                return String::bijective_base_from(index - 1, String::Case::Lower);
             case CSS::CounterStyleNameKeyword::UpperAlpha:
             case CSS::CounterStyleNameKeyword::UpperLatin:
-                m_text = String::bijective_base_from(m_index - 1, String::Case::Upper);
-                break;
+                return String::bijective_base_from(index - 1, String::Case::Upper);
             case CSS::CounterStyleNameKeyword::LowerRoman:
-                m_text = String::roman_number_from(m_index, String::Case::Lower);
-                break;
+                return String::roman_number_from(index, String::Case::Lower);
             case CSS::CounterStyleNameKeyword::UpperRoman:
-                m_text = String::roman_number_from(m_index, String::Case::Upper);
-                break;
+                return String::roman_number_from(index, String::Case::Upper);
             case CSS::CounterStyleNameKeyword::None:
-                break;
+                return {};
+            default:
+                VERIFY_NOT_REACHED();
             }
         },
-        [this](String const& string) {
-            m_text = string;
+        [](String const& string) -> Optional<String> {
+            return string;
         });
 }
 
-ListItemMarkerBox::~ListItemMarkerBox() = default;
-
 GC::Ptr<Painting::Paintable> ListItemMarkerBox::create_paintable() const
 {
     return Painting::MarkerPaintable::create(*this);
 }
 
+void ListItemMarkerBox::visit_edges(Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_list_item_element);
+}
 }

Libraries/LibWeb/Layout/ListItemMarkerBox.h

@@ -17,10 +17,10 @@ class ListItemMarkerBox final : public Box {
     GC_DECLARE_ALLOCATOR(ListItemMarkerBox);
 
 public:
-    explicit ListItemMarkerBox(DOM::Document&, CSS::ListStyleType, CSS::ListStylePosition, size_t index, GC::Ref<CSS::ComputedProperties>);
+    explicit ListItemMarkerBox(DOM::Document&, CSS::ListStyleType, CSS::ListStylePosition, GC::Ref<DOM::Element>, GC::Ref<CSS::ComputedProperties>);
     virtual ~ListItemMarkerBox() override;
 
-    Optional<String> const& text() const { return m_text; }
+    Optional<String> text() const;
 
     virtual GC::Ptr<Painting::Paintable> create_paintable() const override;
 
@@ -28,14 +28,14 @@ public:
     CSS::ListStylePosition list_style_position() const { return m_list_style_position; }
 
 private:
+    virtual void visit_edges(Cell::Visitor&) override;
+
     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::CounterStyleNameKeyword::None };
     CSS::ListStylePosition m_list_style_position { CSS::ListStylePosition::Outside };
-    size_t m_index;
-
-    Optional<String> m_text {};
+    GC::Ref<DOM::Element> m_list_item_element;
 };
 
 template<>

Libraries/LibWeb/Layout/TreeBuilder.cpp

@@ -214,7 +214,7 @@ void TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Se
             document,
             pseudo_element_node->computed_values().list_style_type(),
             pseudo_element_node->computed_values().list_style_position(),
-            0,
+            element,
             marker_style);
         static_cast<ListItemBox&>(*pseudo_element_node).set_marker(list_item_marker);
         element.set_pseudo_element_node({}, CSS::Selector::PseudoElement::Type::Marker, list_item_marker);
@@ -421,30 +421,6 @@ static bool is_ignorable_whitespace(Layout::Node const& node)
     return false;
 }
 
-i32 TreeBuilder::calculate_list_item_index(DOM::Node& dom_node)
-{
-    if (is<HTML::HTMLLIElement>(dom_node)) {
-        auto& li = static_cast<HTML::HTMLLIElement&>(dom_node);
-        if (li.value() != 0)
-            return li.value();
-    }
-
-    if (dom_node.previous_sibling() != nullptr) {
-        DOM::Node* current = dom_node.previous_sibling();
-        while (current != nullptr) {
-            if (is<HTML::HTMLLIElement>(*current))
-                return calculate_list_item_index(*current) + 1;
-            current = current->previous_sibling();
-        }
-    }
-
-    if (is<HTML::HTMLOListElement>(*dom_node.parent())) {
-        auto& ol = static_cast<HTML::HTMLOListElement&>(*dom_node.parent());
-        return ol.start();
-    }
-    return 1;
-}
-
 void TreeBuilder::update_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& context, MustCreateSubtree must_create_subtree)
 {
     bool should_create_layout_node = must_create_subtree == MustCreateSubtree::Yes
@@ -708,7 +684,7 @@ void TreeBuilder::update_layout_tree_after_children(DOM::Node& dom_node, GC::Ref
     if (is<ListItemBox>(*layout_node)) {
         auto& element = static_cast<DOM::Element&>(dom_node);
         auto marker_style = style_computer.compute_style(element, CSS::Selector::PseudoElement::Type::Marker);
-        auto list_item_marker = document.heap().allocate<ListItemMarkerBox>(document, layout_node->computed_values().list_style_type(), layout_node->computed_values().list_style_position(), calculate_list_item_index(dom_node), marker_style);
+        auto list_item_marker = document.heap().allocate<ListItemMarkerBox>(document, layout_node->computed_values().list_style_type(), layout_node->computed_values().list_style_position(), element, marker_style);
         static_cast<ListItemBox&>(*layout_node).set_marker(list_item_marker);
         element.set_pseudo_element_node({}, CSS::Selector::PseudoElement::Type::Marker, list_item_marker);
         layout_node->append_child(*list_item_marker);

Libraries/LibWeb/Painting/MarkerPaintable.cpp

@@ -66,7 +66,7 @@ void MarkerPaintable::paint(PaintContext& context, PaintPhase phase) const
 
     auto color = computed_values().color();
 
-    if (auto& text = layout_box().text(); text.has_value()) {
+    if (auto text = layout_box().text(); text.has_value()) {
         // 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);

Tests/LibWeb/Screenshot/expected/ordered-list-ref.html

@@ -0,0 +1,9 @@
+<style>
+  * {
+    margin: 0;
+  }
+  body {
+    background-color: white;
+  }
+</style>
+<img src="../images/ordered-list-ref.png">

Tests/LibWeb/Screenshot/input/ordered-list.html

@@ -0,0 +1,29 @@
+<link rel="match" href="../expected/ordered-list-ref.html" />
+<ol>
+    <li>1</li>
+    <li>2</li>
+    <li value="5">5</li>
+    <li>6</li>
+    <li>7</li>
+</ol>
+<ol start="20">
+    <li>20</li>
+    <li>21</li>
+    <li value="5">5</li>
+    <li>6</li>
+    <li>7</li>
+</ol>
+<ol reversed>
+    <li>5</li>
+    <li>4</li>
+    <li value="5">5</li>
+    <li>4</li>
+    <li>3</li>
+</ol>
+<ol reversed start="20">
+    <li>20</li>
+    <li>19</li>
+    <li value="5">5</li>
+    <li>4</li>
+    <li>3</li>
+</ol>