diff --git a/Libraries/LibWeb/DOM/Element.cpp b/Libraries/LibWeb/DOM/Element.cpp index 38f8da5939c..2045afe47f3 100644 --- a/Libraries/LibWeb/DOM/Element.cpp +++ b/Libraries/LibWeb/DOM/Element.cpp @@ -49,6 +49,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -57,6 +60,7 @@ #include #include #include +#include #include #include #include @@ -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([&](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) { + if (is(*node) || is(*node) || is(*node)) { + ancestor = static_cast(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) { + if (is(*node) && node->paintable_box()) { + ancestor = static_cast(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(owner)) { + auto const* ol_element = static_cast(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([&](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(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 Element::lang() const return {}; return maybe_lang.release_value(); } - } diff --git a/Libraries/LibWeb/DOM/Element.h b/Libraries/LibWeb/DOM/Element.h index 0b728d24708..bc45e76fae6 100644 --- a/Libraries/LibWeb/DOM/Element.h +++ b/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; diff --git a/Libraries/LibWeb/HTML/HTMLOListElement.cpp b/Libraries/LibWeb/HTML/HTMLOListElement.cpp index 7f1b706f54f..290ab9a3185 100644 --- a/Libraries/LibWeb/HTML/HTMLOListElement.cpp +++ b/Libraries/LibWeb/HTML/HTMLOListElement.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -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)) diff --git a/Libraries/LibWeb/HTML/HTMLOListElement.h b/Libraries/LibWeb/HTML/HTMLOListElement.h index 23264cdfe42..49fc153d853 100644 --- a/Libraries/LibWeb/HTML/HTMLOListElement.h +++ b/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); diff --git a/Libraries/LibWeb/Layout/ListItemMarkerBox.cpp b/Libraries/LibWeb/Layout/ListItemMarkerBox.cpp index d87657fa34e..0f5abdc9e5c 100644 --- a/Libraries/LibWeb/Layout/ListItemMarkerBox.cpp +++ b/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 style) +ListItemMarkerBox::ListItemMarkerBox(DOM::Document& document, CSS::ListStyleType style_type, CSS::ListStylePosition style_position, GC::Ref list_item_element, GC::Ref 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 ListItemMarkerBox::text() const +{ + auto index = m_list_item_element->ordinal_value(); + + return m_list_style_type.visit( + [index](CSS::CounterStyleNameKeyword keyword) -> Optional { 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 { + return string; }); } -ListItemMarkerBox::~ListItemMarkerBox() = default; - GC::Ptr 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); +} } diff --git a/Libraries/LibWeb/Layout/ListItemMarkerBox.h b/Libraries/LibWeb/Layout/ListItemMarkerBox.h index 2ae9527c54a..dcaa29882ef 100644 --- a/Libraries/LibWeb/Layout/ListItemMarkerBox.h +++ b/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); + explicit ListItemMarkerBox(DOM::Document&, CSS::ListStyleType, CSS::ListStylePosition, GC::Ref, GC::Ref); virtual ~ListItemMarkerBox() override; - Optional const& text() const { return m_text; } + Optional text() const; virtual GC::Ptr 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 m_text {}; + GC::Ref m_list_item_element; }; template<> diff --git a/Libraries/LibWeb/Layout/TreeBuilder.cpp b/Libraries/LibWeb/Layout/TreeBuilder.cpp index 132b5bd316e..60e302bf77c 100644 --- a/Libraries/LibWeb/Layout/TreeBuilder.cpp +++ b/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(*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(dom_node)) { - auto& li = static_cast(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(*current)) - return calculate_list_item_index(*current) + 1; - current = current->previous_sibling(); - } - } - - if (is(*dom_node.parent())) { - auto& ol = static_cast(*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(*layout_node)) { auto& element = static_cast(dom_node); auto marker_style = style_computer.compute_style(element, CSS::Selector::PseudoElement::Type::Marker); - auto list_item_marker = document.heap().allocate(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(document, layout_node->computed_values().list_style_type(), layout_node->computed_values().list_style_position(), element, marker_style); static_cast(*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); diff --git a/Libraries/LibWeb/Painting/MarkerPaintable.cpp b/Libraries/LibWeb/Painting/MarkerPaintable.cpp index b84441183f4..d54e8c9c104 100644 --- a/Libraries/LibWeb/Painting/MarkerPaintable.cpp +++ b/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
  • element which looks very sad :( context.display_list_recorder().draw_text(device_enclosing.to_type(), *text, layout_box().scaled_font(context), Gfx::TextAlignment::Center, color); diff --git a/Tests/LibWeb/Screenshot/expected/ordered-list-ref.html b/Tests/LibWeb/Screenshot/expected/ordered-list-ref.html new file mode 100644 index 00000000000..7381ac59787 --- /dev/null +++ b/Tests/LibWeb/Screenshot/expected/ordered-list-ref.html @@ -0,0 +1,9 @@ + + diff --git a/Tests/LibWeb/Screenshot/images/ordered-list-ref.png b/Tests/LibWeb/Screenshot/images/ordered-list-ref.png new file mode 100644 index 00000000000..7d6b7ed183a Binary files /dev/null and b/Tests/LibWeb/Screenshot/images/ordered-list-ref.png differ diff --git a/Tests/LibWeb/Screenshot/input/ordered-list.html b/Tests/LibWeb/Screenshot/input/ordered-list.html new file mode 100644 index 00000000000..aa1ce773d6d --- /dev/null +++ b/Tests/LibWeb/Screenshot/input/ordered-list.html @@ -0,0 +1,29 @@ + +
      +
    1. 1
    2. +
    3. 2
    4. +
    5. 5
    6. +
    7. 6
    8. +
    9. 7
    10. +
    +
      +
    1. 20
    2. +
    3. 21
    4. +
    5. 5
    6. +
    7. 6
    8. +
    9. 7
    10. +
    +
      +
    1. 5
    2. +
    3. 4
    4. +
    5. 5
    6. +
    7. 4
    8. +
    9. 3
    10. +
    +
      +
    1. 20
    2. +
    3. 19
    4. +
    5. 5
    6. +
    7. 4
    8. +
    9. 3
    10. +