LibWeb: Support reversed ordered lists

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

Libraries/LibWeb/DOM/Element.cpp

@@ -45,6 +45,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>
@@ -53,6 +56,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>
@@ -2898,6 +2902,109 @@ bool Element::has_paint_containment() const
     return false;
 }
 
+int 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/#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 (auto const* node = ancestor; node; node = node->parent_element()) {
+        if (is<HTML::HTMLOListElement>(node) || is<HTML::HTMLUListElement>(node) || is<HTML::HTMLMenuElement>(node)) {
+            ancestor = node;
+            break;
+        }
+    }
+
+    // 4. Return the closest inclusive ancestor of ancestor that produces a CSS box.
+    for (; ancestor; ancestor = ancestor->parent_element()) {
+        if (ancestor->layout_node()) {
+            return ancestor;
+        }
+    }
+
+    // Spec-Note: Such an element will always exist, as at the very least the document element will always produce a CSS box.
+    VERIFY_NOT_REACHED();
+}
+
+// https://html.spec.whatwg.org/#ordinal-value
+int 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* 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);

Libraries/LibWeb/DOM/Element.h

@@ -414,6 +414,10 @@ class Element
     bool affected_by_has_pseudo_class_in_subject_position() const { return m_affected_by_has_pseudo_class_in_subject_position; }
     void set_affected_by_has_pseudo_class_in_subject_position(bool value) { m_affected_by_has_pseudo_class_in_subject_position = value; }
 
+    int number_of_owned_list_items() const;
+    Element const* list_owner() const;
+    int ordinal_value() const;
+
 protected:
     Element(Document&, DOM::QualifiedName);
     virtual void initialize(JS::Realm&) override;

Libraries/LibWeb/HTML/HTMLOListElement.cpp

@@ -6,6 +6,7 @@
 
 #include <LibWeb/Bindings/HTMLOListElementPrototype.h>
 #include <LibWeb/Bindings/Intrinsics.h>
+#include <LibWeb/DOM/Document.h>
 #include <LibWeb/HTML/HTMLOListElement.h>
 #include <LibWeb/HTML/Numbers.h>
 
@@ -36,4 +37,27 @@ WebIDL::Long HTMLOListElement::start()
     return 1;
 }
 
+// https://html.spec.whatwg.org/#concept-ol-start
+int 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;
+}
+
 }

Libraries/LibWeb/HTML/HTMLOListElement.h

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

Libraries/LibWeb/Layout/ListItemMarkerBox.cpp

@@ -12,53 +12,58 @@ 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)
 {
+}
+
+ListItemMarkerBox::~ListItemMarkerBox() = default;
+
+Optional<ByteString> ListItemMarkerBox::text() const
+{
+    auto index = m_list_item_element->ordinal_value();
+
     switch (m_list_style_type) {
     case CSS::ListStyleType::Square:
     case CSS::ListStyleType::Circle:
     case CSS::ListStyleType::Disc:
     case CSS::ListStyleType::DisclosureClosed:
     case CSS::ListStyleType::DisclosureOpen:
-        break;
+        return {};
     case CSS::ListStyleType::Decimal:
-        m_text = ByteString::formatted("{}.", m_index);
-        break;
+        return ByteString::formatted("{}.", index);
     case CSS::ListStyleType::DecimalLeadingZero:
         // This is weird, but in accordance to spec.
-        m_text = m_index < 10 ? ByteString::formatted("0{}.", m_index) : ByteString::formatted("{}.", m_index);
-        break;
+        return index < 10 ? ByteString::formatted("0{}.", index) : ByteString::formatted("{}.", index);
     case CSS::ListStyleType::LowerAlpha:
     case CSS::ListStyleType::LowerLatin:
-        m_text = ByteString::bijective_base_from(m_index - 1).to_lowercase();
-        break;
+        return ByteString::bijective_base_from(index - 1).to_lowercase();
     case CSS::ListStyleType::UpperAlpha:
     case CSS::ListStyleType::UpperLatin:
-        m_text = ByteString::bijective_base_from(m_index - 1);
-        break;
+        return ByteString::bijective_base_from(index - 1);
     case CSS::ListStyleType::LowerRoman:
-        m_text = ByteString::roman_number_from(m_index).to_lowercase();
-        break;
+        return ByteString::roman_number_from(index).to_lowercase();
     case CSS::ListStyleType::UpperRoman:
-        m_text = ByteString::roman_number_from(m_index);
-        break;
+        return ByteString::roman_number_from(index);
     case CSS::ListStyleType::None:
-        break;
-
+        return {};
     default:
         VERIFY_NOT_REACHED();
     }
 }
 
-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

@@ -16,25 +16,25 @@ 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<ByteString> const& text() const { return m_text; }
+    Optional<ByteString> text() const;
 
     virtual GC::Ptr<Painting::Paintable> create_paintable() const override;
 
     CSS::ListStyleType list_style_type() const { return m_list_style_type; }
     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::ListStyleType::None };
     CSS::ListStylePosition m_list_style_position { CSS::ListStylePosition::Outside };
-    size_t m_index;
-
-    Optional<ByteString> m_text {};
+    GC::Ref<DOM::Element> m_list_item_element;
 };
 
 template<>

Libraries/LibWeb/Layout/TreeBuilder.cpp

@@ -215,7 +215,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);
@@ -426,30 +426,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
@@ -629,7 +605,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);

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>