diff --git a/Libraries/LibWeb/DOM/Element.cpp b/Libraries/LibWeb/DOM/Element.cpp index 65496f8d492..b659e227c36 100644 --- a/Libraries/LibWeb/DOM/Element.cpp +++ b/Libraries/LibWeb/DOM/Element.cpp @@ -1,12 +1,16 @@ /* * Copyright (c) 2018-2024, Andreas Kling - * Copyright (c) 2022-2023, San Atkins + * Copyright (c) 2022-2023, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ #include +#include +#include #include +#include +#include #include #include #include @@ -3184,23 +3188,22 @@ bool Element::skips_its_contents() return false; } -size_t Element::number_of_owned_list_items() const +i32 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++; - } + AK::Checked number_of_owned_li_elements = 0; + for_each_numbered_item_owned_by_list_owner([&number_of_owned_li_elements]([[maybe_unused]] Element* item) { + number_of_owned_li_elements++; return IterationDecision::Continue; }); - return number_of_owned_li_elements; + + return number_of_owned_li_elements.value(); } // https://html.spec.whatwg.org/multipage/grouping-content.html#list-owner Element* 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() || !computed_properties()->display().is_list_item()) + if (!m_is_contained_in_list_subtree && (!computed_properties() || !computed_properties()->display().is_list_item())) return nullptr; // 1. If the element is not being rendered, return null; the element has no list owner. @@ -3216,8 +3219,8 @@ Element* Element::list_owner() const // 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()); + if (node->is_html_ol_ul_menu_element()) { + ancestor = static_cast(node.ptr()); return IterationDecision::Break; } return IterationDecision::Continue; @@ -3226,7 +3229,7 @@ Element* Element::list_owner() const // 4. Return the closest inclusive ancestor of ancestor that produces a CSS box. ancestor->for_each_inclusive_ancestor([&ancestor](GC::Ref node) { if (is(*node) && node->paintable_box()) { - ancestor = static_cast(node.ptr()); + ancestor = static_cast(node.ptr()); return IterationDecision::Break; } return IterationDecision::Continue; @@ -3234,62 +3237,75 @@ Element* Element::list_owner() const return const_cast(ancestor.ptr()); } -// https://html.spec.whatwg.org/multipage/grouping-content.html#ordinal-value -size_t Element::ordinal_value() const +void Element::maybe_invalidate_ordinals_for_list_owner(Optional skip_node) { - // 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. + if (Element* owner = list_owner()) + owner->for_each_numbered_item_owned_by_list_owner([&](Element* item) { + if (skip_node.has_value() && item == skip_node.value()) + return IterationDecision::Continue; - // FIXME: 1. Let i be 1. + item->m_ordinal_value = {}; - // 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) { + // Invalidate just the first ordinal in the list of numbered items. + // NOTE: This works since this item is the first accessed (preorder) when rendering the list. + // It will trigger a recalculation of all ordinals on the [first] call to ordinal_value(). + return IterationDecision::Break; + }); +} + +// https://html.spec.whatwg.org/multipage/grouping-content.html#ordinal-value +i32 Element::ordinal_value() +{ + if (m_ordinal_value.has_value()) + return m_ordinal_value.value(); + + auto* owner = list_owner(); + if (!owner) return 1; - } - auto numbering = 1; + // 1. Let i be 1. [Not necessary] + // 2. If owner is an ol element, let numbering be owner's starting value. Otherwise, let numbering be 1. + AK::Checked numbering = 1; auto reversed = false; - if (is(owner)) { + + if (owner->is_html_olist_element()) { auto const* ol_element = static_cast(owner); numbering = ol_element->starting_value().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. + // 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. + // NOTE: We use `owner->for_each_numbered_item_in_list` to iterate through the owner's list of owned elements. + // As a result, we don't need `i` as counter (spec) in the list of children, with no material consequences. + owner->for_each_numbered_item_owned_by_list_owner([&](Element* item) { + // 4. Let item be the ith of owner's owned list items, in tree order. [Not necessary] + // 5. If item is an li element that has a value attribute, then: + auto value_attribute = item->get_attribute(HTML::AttributeNames::value); + if (item->is_html_li_element() && 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()); - 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. + // 2. If parsed is not an error, then set numbering to parsed. + if (parsed.has_value()) + numbering = parsed.value(); } + + // 6. The ordinal value of item is numbering. + item->m_ordinal_value = numbering.value(); + + // 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++; + } + + // 8. Increment i by 1. [Not necessary] + // 9. Go to the step labeled loop. return IterationDecision::Continue; }); - // FIXME: 9. Go to the step labeled loop. - return numbering; + return m_ordinal_value.value_or(1); } bool Element::id_reference_exists(String const& id_reference) const diff --git a/Libraries/LibWeb/DOM/Element.h b/Libraries/LibWeb/DOM/Element.h index 5410d169fe6..44cc6efc0d7 100644 --- a/Libraries/LibWeb/DOM/Element.h +++ b/Libraries/LibWeb/DOM/Element.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include #include @@ -14,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -465,9 +467,46 @@ public: return affected_by_direct_sibling_combinator() || affected_by_indirect_sibling_combinator() || affected_by_sibling_position_or_count_pseudo_class() || affected_by_nth_child_pseudo_class(); } - size_t number_of_owned_list_items() const; + i32 number_of_owned_list_items() const; Element* list_owner() const; - size_t ordinal_value() const; + void maybe_invalidate_ordinals_for_list_owner(Optional skip_node = {}); + i32 ordinal_value(); + + template + void for_each_numbered_item_owned_by_list_owner(Callback callback) const + { + const_cast(this)->for_each_numbered_item_owned_by_list_owner(move(callback)); + } + + template + void for_each_numbered_item_owned_by_list_owner(Callback callback) + { + for (auto* node = this->first_child(); node != nullptr; node = node->next_in_pre_order(this)) { + if (!is(*node)) + continue; + + static_cast(node)->m_is_contained_in_list_subtree = true; + + if (node->is_html_ol_ul_menu_element()) { + // Skip list nodes and their descendents. They have their own, unrelated ordinals. + while (node->last_child() != nullptr) // Find the last node (preorder) in the subtree headed by node. O(1). + node = node->last_child(); + + continue; + } + + if (!node->layout_node()) + continue; // Skip nodes that do not participate in the layout. + + auto* element = static_cast(node); + + if (!element->computed_properties()->display().is_list_item()) + continue; // Skip nodes that are not list items. + + if (callback(element) == IterationDecision::Break) + return; + } + } void set_pointer_capture(WebIDL::Long pointer_id); void release_pointer_capture(WebIDL::Long pointer_id); @@ -594,6 +633,10 @@ private: // https://drafts.csswg.org/css-contain/#proximity-to-the-viewport ProximityToTheViewport m_proximity_to_the_viewport { ProximityToTheViewport::NotDetermined }; + + // https://html.spec.whatwg.org/multipage/grouping-content.html#ordinal-value + Optional m_ordinal_value; + bool m_is_contained_in_list_subtree { false }; }; template<> diff --git a/Libraries/LibWeb/DOM/Node.cpp b/Libraries/LibWeb/DOM/Node.cpp index 58e3bbbccca..0d787832b8b 100644 --- a/Libraries/LibWeb/DOM/Node.cpp +++ b/Libraries/LibWeb/DOM/Node.cpp @@ -805,6 +805,16 @@ void Node::insert_before(GC::Ref node, GC::Ptr child, bool suppress_ set_needs_layout_tree_update(true, SetNeedsLayoutTreeUpdateReason::NodeInsertBefore); } + // AD-HOC: invalidate the ordinal of the first list_item of the list_owner of the child node, if any. + if (child && child->is_element()) + static_cast(child.ptr())->maybe_invalidate_ordinals_for_list_owner(); + else if (this->is_element() && !this->is_html_ol_ul_menu_element()) + static_cast(this)->maybe_invalidate_ordinals_for_list_owner(); + // NOTE: If the child node is null and the parent node is an ol, ul or menu element then: + // the new node will be the first in the list of a potential list owner and it will not have + // an ordinal value (default from constructor). + // FIXME: This will not work if the child or the parent is not an element. Is insert_before even possible in this situation? + document().bump_dom_tree_version(); } @@ -854,6 +864,11 @@ WebIDL::ExceptionOr> Node::append_child(GC::Ref node) { // To append a node to a parent, pre-insert node into parent before null. return pre_insert(node, nullptr); + + // AD-HOC: invalidate the ordinal of the first list_item of the first child sibling of the appended node, if any. + // NOTE: This works since ordinal values are accessed (for layout and paint) in the preorder of list_item nodes !! + if (auto* first_child_element = this->first_child_of_type()) + first_child_element->maybe_invalidate_ordinals_for_list_owner(); } // https://dom.spec.whatwg.org/#live-range-pre-remove-steps @@ -917,6 +932,12 @@ void Node::remove(bool suppress_observers) // 6. Let oldNextSibling be node’s next sibling. GC::Ptr old_next_sibling = next_sibling(); + // AD-HOC: invalidate the ordinal of the first list_item of the list_owner of the removed node, if any. + if (is_element()) { + auto* this_element = static_cast(this); + this_element->maybe_invalidate_ordinals_for_list_owner(this_element); + } + if (is_connected()) { // Since the tree structure is about to change, we need to invalidate both style and layout. // In the future, we should find a way to only invalidate the parts that actually need it. diff --git a/Libraries/LibWeb/HTML/HTMLLIElement.cpp b/Libraries/LibWeb/HTML/HTMLLIElement.cpp index 2cd315e0c55..266e4794633 100644 --- a/Libraries/LibWeb/HTML/HTMLLIElement.cpp +++ b/Libraries/LibWeb/HTML/HTMLLIElement.cpp @@ -34,6 +34,7 @@ void HTMLLIElement::attribute_changed(FlyString const& local_name, Optionalset_needs_layout_tree_update(true, DOM::SetNeedsLayoutTreeUpdateReason::HTMLOListElementOrdinalValues); + maybe_invalidate_ordinals_for_list_owner(); } } } diff --git a/Libraries/LibWeb/HTML/HTMLOListElement.cpp b/Libraries/LibWeb/HTML/HTMLOListElement.cpp index 97adbffbfab..d447c4a1908 100644 --- a/Libraries/LibWeb/HTML/HTMLOListElement.cpp +++ b/Libraries/LibWeb/HTML/HTMLOListElement.cpp @@ -36,6 +36,8 @@ void HTMLOListElement::attribute_changed(FlyString const& local_name, Optional()->maybe_invalidate_ordinals_for_list_owner(); } }