diff options
author | Luke <luke.wilde@live.co.uk> | 2021-04-06 19:34:49 +0100 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-04-06 21:42:00 +0200 |
commit | 5beacf08a2d578d0eb36d6320255d0f4634a1085 (patch) | |
tree | c9a269226bdd772ef6db6f012530689594dbf2e0 /Userland/Libraries/LibWeb | |
parent | 5b5d7857e3bb98ade11b06d2b392a7058b1487ec (diff) | |
download | serenity-5beacf08a2d578d0eb36d6320255d0f4634a1085.zip |
LibWeb: Make the node mutation algorithms more spec compliant
The mutation algorithms now more closely follow the spec and
fixes some assertion failures in tests such as Acid3 and Dromaeo.
The main thing that is missing right now is passing exceptions to the
bindings layer. This is because of issue #6075. I spent a while trying
to work it out and got so frustrated I just left it as a FIXME. Besides
that, the algorithms bail at the appropriate points.
This also makes the adopting steps in the document more spec compliant
as it's needed by the insertion algorithm. While I was at it, I added
the adoptNode IDL binding.
This adds a bunch of ancestor/descendant checks to TreeNode as well.
I moved the "remove_all_children" function to Node as it needs to use
the full remove algorithm instead of simply removing it from
the child list.
Diffstat (limited to 'Userland/Libraries/LibWeb')
-rw-r--r-- | Userland/Libraries/LibWeb/DOM/Document.cpp | 57 | ||||
-rw-r--r-- | Userland/Libraries/LibWeb/DOM/Document.h | 1 | ||||
-rw-r--r-- | Userland/Libraries/LibWeb/DOM/Document.idl | 2 | ||||
-rw-r--r-- | Userland/Libraries/LibWeb/DOM/Node.cpp | 226 | ||||
-rw-r--r-- | Userland/Libraries/LibWeb/DOM/Node.h | 17 | ||||
-rw-r--r-- | Userland/Libraries/LibWeb/DOM/Node.idl | 4 | ||||
-rw-r--r-- | Userland/Libraries/LibWeb/Page/EditEventHandler.cpp | 8 | ||||
-rw-r--r-- | Userland/Libraries/LibWeb/TreeNode.h | 37 |
8 files changed, 301 insertions, 51 deletions
diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 3aa3100305..ebca6e9bbd 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -43,6 +43,7 @@ #include <LibWeb/DOM/Event.h> #include <LibWeb/DOM/ExceptionOr.h> #include <LibWeb/DOM/Range.h> +#include <LibWeb/DOM/ShadowRoot.h> #include <LibWeb/DOM/Text.h> #include <LibWeb/DOM/Window.h> #include <LibWeb/Dump.h> @@ -124,7 +125,7 @@ void Document::removed_last_ref() VERIFY(&node.document() == this); VERIFY(!node.is_document()); if (node.parent()) - node.parent()->remove_child(node); + node.remove(); } } @@ -282,9 +283,7 @@ void Document::set_title(const String& title) head_element->append_child(*title_element); } - while (RefPtr<Node> child = title_element->first_child()) - title_element->remove_child(child.release_nonnull()); - + title_element->remove_all_children(true); title_element->append_child(adopt(*new Text(*this, title))); if (auto* page = this->page()) { @@ -659,12 +658,52 @@ NonnullRefPtrVector<HTML::HTMLScriptElement> Document::take_scripts_to_execute_a return move(m_scripts_to_execute_as_soon_as_possible); } -void Document::adopt_node(Node& subtree_root) +// https://dom.spec.whatwg.org/#concept-node-adopt +void Document::adopt_node(Node& node) { - subtree_root.for_each_in_inclusive_subtree([&](auto& node) { - node.set_document({}, *this); - return IterationDecision::Continue; - }); + auto& old_document = node.document(); + if (node.parent()) + node.remove(); + + if (&old_document != this) { + // FIXME: This should be shadow-including. + node.for_each_in_inclusive_subtree([&](auto& inclusive_descendant) { + inclusive_descendant.set_document({}, *this); + // FIXME: If inclusiveDescendant is an element, then set the node document of each attribute in inclusiveDescendant’s attribute list to document. + return IterationDecision::Continue; + }); + + // FIXME: For each inclusiveDescendant in node’s shadow-including inclusive descendants that is custom, + // enqueue a custom element callback reaction with inclusiveDescendant, callback name "adoptedCallback", + // and an argument list containing oldDocument and document. + + // FIXME: This should be shadow-including. + node.for_each_in_inclusive_subtree([&](auto& inclusive_descendant) { + inclusive_descendant.adopted_from(old_document); + return IterationDecision::Continue; + }); + } +} + +// https://dom.spec.whatwg.org/#dom-document-adoptnode +NonnullRefPtr<Node> Document::adopt_node_binding(NonnullRefPtr<Node> node) +{ + if (is<Document>(*node)) { + dbgln("Document::adopt_node_binding: Cannot adopt a document into a document (FIXME: throw as NotSupportedError exception, see issue #6075"); + return node; + } + + if (is<ShadowRoot>(*node)) { + dbgln("Document::adopt_node_binding: Cannot adopt a shadow root into a document (FIXME: throw as HierarchyRequestError exception, see issue #6075"); + return node; + } + + if (is<DocumentFragment>(*node) && downcast<DocumentFragment>(*node).host()) + return node; + + adopt_node(*node); + + return node; } const DocumentType* Document::doctype() const diff --git a/Userland/Libraries/LibWeb/DOM/Document.h b/Userland/Libraries/LibWeb/DOM/Document.h index c42b43f8ef..a6f87d1cbc 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.h +++ b/Userland/Libraries/LibWeb/DOM/Document.h @@ -188,6 +188,7 @@ public: void set_quirks_mode(QuirksMode mode) { m_quirks_mode = mode; } void adopt_node(Node&); + NonnullRefPtr<Node> adopt_node_binding(NonnullRefPtr<Node>); const DocumentType* doctype() const; const String& compat_mode() const; diff --git a/Userland/Libraries/LibWeb/DOM/Document.idl b/Userland/Libraries/LibWeb/DOM/Document.idl index 2886bdcd3f..aacdae09f2 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.idl +++ b/Userland/Libraries/LibWeb/DOM/Document.idl @@ -31,6 +31,8 @@ interface Document : Node { Comment createComment(DOMString data); Range createRange(); + [CEReactions, ImplementedAs=adopt_node_binding] Node adoptNode(Node node); + [ImplementedAs=style_sheets_for_bindings] readonly attribute StyleSheetList styleSheets; readonly attribute DOMString compatMode; diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index 61ebb37232..c2792df94a 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -30,11 +30,14 @@ #include <LibWeb/Bindings/EventWrapper.h> #include <LibWeb/Bindings/NodeWrapper.h> #include <LibWeb/Bindings/NodeWrapperFactory.h> +#include <LibWeb/DOM/Comment.h> +#include <LibWeb/DOM/DocumentType.h> #include <LibWeb/DOM/Element.h> #include <LibWeb/DOM/Event.h> #include <LibWeb/DOM/EventDispatcher.h> #include <LibWeb/DOM/EventListener.h> #include <LibWeb/DOM/Node.h> +#include <LibWeb/DOM/ProcessingInstruction.h> #include <LibWeb/DOM/ShadowRoot.h> #include <LibWeb/HTML/HTMLAnchorElement.h> #include <LibWeb/Layout/InitialContainingBlockBox.h> @@ -179,38 +182,191 @@ const Element* Node::parent_element() const return downcast<Element>(parent()); } -RefPtr<Node> Node::append_child(NonnullRefPtr<Node> node, bool notify) +// https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity +ExceptionOr<void> Node::ensure_pre_insertion_validity(NonnullRefPtr<Node> node, RefPtr<Node> child) const { - if (node->parent()) - node->parent()->remove_child(node); - if (&node->document() != &document()) - document().adopt_node(node); - TreeNode<Node>::append_child(node, notify); - return node; + if (!is<Document>(this) && !is<DocumentFragment>(this) && !is<Element>(this)) + return DOM::HierarchyRequestError::create("Can only insert into a document, document fragment or element"); + + if (node->is_host_including_inclusive_ancestor_of(*this)) + return DOM::HierarchyRequestError::create("New node is an ancestor of this node"); + + if (child && child->parent() != this) + return DOM::NotFoundError::create("This node is not the parent of the given child"); + + // FIXME: All the following "Invalid node type for insertion" messages could be more descriptive. + + if (!is<DocumentFragment>(*node) && !is<DocumentType>(*node) && !is<Element>(*node) && !is<Text>(*node) && !is<Comment>(*node) && !is<ProcessingInstruction>(*node)) + return DOM::HierarchyRequestError::create("Invalid node type for insertion"); + + if ((is<Text>(*node) && is<Document>(this)) || (is<DocumentType>(*node) && !is<Document>(this))) + return DOM::HierarchyRequestError::create("Invalid node type for insertion"); + + if (is<Document>(this)) { + if (is<DocumentFragment>(*node)) { + auto node_element_child_count = node->element_child_count(); + if ((node_element_child_count > 1 || node->has_child_of_type<Text>()) + || (node_element_child_count == 1 && (has_child_of_type<Element>() || is<DocumentType>(child.ptr()) /* FIXME: or child is non-null and a doctype is following child. */))) { + return DOM::HierarchyRequestError::create("Invalid node type for insertion"); + } + } else if (is<Element>(*node)) { + if (has_child_of_type<Element>() || is<DocumentType>(child.ptr()) /* FIXME: or child is non-null and a doctype is following child. */) + return DOM::HierarchyRequestError::create("Invalid node type for insertion"); + } else if (is<DocumentType>(*node)) { + if (has_child_of_type<DocumentType>() /* FIXME: or child is non-null and an element is preceding child */ || (!child && has_child_of_type<Element>())) + return DOM::HierarchyRequestError::create("Invalid node type for insertion"); + } + } + + return {}; } -RefPtr<Node> Node::remove_child(NonnullRefPtr<Node> node) +// https://dom.spec.whatwg.org/#concept-node-insert +void Node::insert_before(NonnullRefPtr<Node> node, RefPtr<Node> child, bool suppress_observers) { - TreeNode<Node>::remove_child(node); - return node; + NonnullRefPtrVector<Node> nodes; + if (is<DocumentFragment>(*node)) + nodes = downcast<DocumentFragment>(*node).child_nodes(); + else + nodes.append(node); + + auto count = nodes.size(); + if (count == 0) + return; + + if (is<DocumentFragment>(*node)) { + node->remove_all_children(true); + // FIXME: Queue a tree mutation record for node with « », nodes, null, and null. + } + + if (child) { + // FIXME: For each live range whose start node is parent and start offset is greater than child’s index, increase its start offset by count. + // FIXME: For each live range whose end node is parent and end offset is greater than child’s index, increase its end offset by count. + } + + // FIXME: Let previousSibling be child’s previous sibling or parent’s last child if child is null. (Currently unused so not included) + + for (auto& node_to_insert : nodes) { // FIXME: In tree order + document().adopt_node(node_to_insert); + + if (!child) + TreeNode<Node>::append_child(node); + else + TreeNode<Node>::insert_before(node, child); + + // FIXME: If parent is a shadow host and node is a slottable, then assign a slot for node. + // FIXME: If parent’s root is a shadow root, and parent is a slot whose assigned nodes is the empty list, then run signal a slot change for parent. + // FIXME: Run assign slottables for a tree with node’s root. + + // FIXME: This should be shadow-including. + node_to_insert.for_each_in_inclusive_subtree([&](Node& inclusive_descendant) { + inclusive_descendant.inserted(); + if (inclusive_descendant.is_connected()) { + // FIXME: If inclusiveDescendant is custom, then enqueue a custom element callback reaction with inclusiveDescendant, + // callback name "connectedCallback", and an empty argument list. + + // FIXME: Otherwise, try to upgrade inclusiveDescendant. + } + + return IterationDecision::Continue; + }); + } + + if (!suppress_observers) { + // FIXME: queue a tree mutation record for parent with nodes, « », previousSibling, and child. + } + + children_changed(); } -RefPtr<Node> Node::insert_before(NonnullRefPtr<Node> node, RefPtr<Node> child, bool notify) +// https://dom.spec.whatwg.org/#concept-node-pre-insert +NonnullRefPtr<Node> Node::pre_insert(NonnullRefPtr<Node> node, RefPtr<Node> child) { - if (!child) - return append_child(move(node), notify); - if (child->parent_node() != this) { - dbgln("FIXME: Trying to insert_before() a bogus child"); - return nullptr; + auto validity_result = ensure_pre_insertion_validity(node, child); + if (validity_result.is_exception()) { + dbgln("Node::pre_insert: ensure_pre_insertion_validity failed: {}. (FIXME: throw as exception, see issue #6075)", validity_result.exception().message()); + return node; } - if (node->parent()) - node->parent()->remove_child(node); - if (&node->document() != &document()) - document().adopt_node(node); - TreeNode<Node>::insert_before(node, child, notify); + + auto reference_child = child; + if (reference_child == node) + reference_child = node->next_sibling(); + + insert_before(node, reference_child); return node; } +// https://dom.spec.whatwg.org/#concept-node-pre-remove +NonnullRefPtr<Node> Node::pre_remove(NonnullRefPtr<Node> child) +{ + if (child->parent() != this) { + dbgln("Node::pre_remove: Child doesn't belong to this node. (FIXME: throw NotFoundError DOMException, see issue #6075)"); + return child; + } + + child->remove(); + + return child; +} + +// https://dom.spec.whatwg.org/#concept-node-append +NonnullRefPtr<Node> Node::append_child(NonnullRefPtr<Node> node) +{ + return pre_insert(node, nullptr); +} + +// https://dom.spec.whatwg.org/#concept-node-remove +void Node::remove(bool suppress_observers) +{ + auto* parent = TreeNode<Node>::parent(); + VERIFY(parent); + + // FIXME: Let index be node’s index. (Currently unused so not included) + + // FIXME: For each live range whose start node is an inclusive descendant of node, set its start to (parent, index). + // FIXME: For each live range whose end node is an inclusive descendant of node, set its end to (parent, index). + // FIXME: For each live range whose start node is parent and start offset is greater than index, decrease its start offset by 1. + // FIXME: For each live range whose end node is parent and end offset is greater than index, decrease its end offset by 1. + + // FIXME: For each NodeIterator object iterator whose root’s node document is node’s node document, run the NodeIterator pre-removing steps given node and iterator. + + // FIXME: Let oldPreviousSibling be node’s previous sibling. (Currently unused so not included) + // FIXME: Let oldNextSibling be node’s next sibling. (Currently unused so not included) + + parent->remove_child(*this); + + // FIXME: If node is assigned, then run assign slottables for node’s assigned slot. + + // FIXME: If parent’s root is a shadow root, and parent is a slot whose assigned nodes is the empty list, then run signal a slot change for parent. + + // FIXME: If node has an inclusive descendant that is a slot, then: + // Run assign slottables for a tree with parent’s root. + // Run assign slottables for a tree with node. + + removed_from(parent); + + // FIXME: Let isParentConnected be parent’s connected. (Currently unused so not included) + + // FIXME: If node is custom and isParentConnected is true, then enqueue a custom element callback reaction with node, + // callback name "disconnectedCallback", and an empty argument list. + + // FIXME: This should be shadow-including. + for_each_in_subtree([&](Node& descendant) { + descendant.removed_from(nullptr); + + // FIXME: If descendant is custom and isParentConnected is true, then enqueue a custom element callback reaction with descendant, + // callback name "disconnectedCallback", and an empty argument list. + + return IterationDecision::Continue; + }); + + if (!suppress_observers) { + // FIXME: queue a tree mutation record for parent with « », « node », oldPreviousSibling, and oldNextSibling. + } + + parent->children_changed(); +} + void Node::set_document(Badge<Document>, Document& document) { if (m_document == &document) @@ -261,13 +417,15 @@ void Node::set_needs_style_update(bool value) m_needs_style_update = value; if (m_needs_style_update) { - for (auto* ancestor = parent(); ancestor; ancestor = ancestor->parent()) + for (auto* ancestor = parent(); ancestor; ancestor = ancestor->parent()) { + //dbgln("{}", ancestor->node_name()); ancestor->m_child_needs_style_update = true; + } document().schedule_style_update(); } } -void Node::inserted_into(Node&) +void Node::inserted() { set_needs_style_update(true); } @@ -290,4 +448,26 @@ NonnullRefPtrVector<Node> Node::child_nodes() const return nodes; } +void Node::remove_all_children(bool suppress_observers) +{ + while (RefPtr<Node> child = first_child()) + child->remove(suppress_observers); +} + +// https://dom.spec.whatwg.org/#concept-tree-host-including-inclusive-ancestor +bool Node::is_host_including_inclusive_ancestor_of(const Node& other) const +{ + return is_inclusive_ancestor_of(other) || (is<DocumentFragment>(other.root()) && downcast<DocumentFragment>(other.root())->host() && is_inclusive_ancestor_of(*downcast<DocumentFragment>(other.root())->host().ptr())); +} + +size_t Node::element_child_count() const +{ + size_t count = 0; + for (auto* child = first_child(); child; child = child->next_sibling()) { + if (is<Element>(child)) + ++count; + } + return count; +} + } diff --git a/Userland/Libraries/LibWeb/DOM/Node.h b/Userland/Libraries/LibWeb/DOM/Node.h index d1a71bc768..946279ae45 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.h +++ b/Userland/Libraries/LibWeb/DOM/Node.h @@ -33,6 +33,7 @@ #include <AK/Vector.h> #include <LibWeb/Bindings/Wrappable.h> #include <LibWeb/DOM/EventTarget.h> +#include <LibWeb/DOM/ExceptionOr.h> #include <LibWeb/TreeNode.h> namespace Web::DOM { @@ -92,9 +93,13 @@ public: virtual bool is_editable() const; - RefPtr<Node> append_child(NonnullRefPtr<Node>, bool notify = true); - RefPtr<Node> insert_before(NonnullRefPtr<Node> node, RefPtr<Node> child, bool notify = true); - RefPtr<Node> remove_child(NonnullRefPtr<Node>); + NonnullRefPtr<Node> pre_insert(NonnullRefPtr<Node>, RefPtr<Node>); + NonnullRefPtr<Node> pre_remove(NonnullRefPtr<Node>); + + NonnullRefPtr<Node> append_child(NonnullRefPtr<Node>); + void insert_before(NonnullRefPtr<Node> node, RefPtr<Node> child, bool suppress_observers = false); + void remove(bool suppress_observers = false); + void remove_all_children(bool suppress_observers = false); // NOTE: This is intended for the JS bindings. bool has_child_nodes() const { return has_children(); } @@ -165,6 +170,12 @@ public: template<typename T> bool fast_is() const = delete; + ExceptionOr<void> ensure_pre_insertion_validity(NonnullRefPtr<Node> node, RefPtr<Node> child) const; + + bool is_host_including_inclusive_ancestor_of(const Node&) const; + + size_t element_child_count() const; + protected: Node(Document&, NodeType); diff --git a/Userland/Libraries/LibWeb/DOM/Node.idl b/Userland/Libraries/LibWeb/DOM/Node.idl index c79b34f2bf..5667340c8c 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.idl +++ b/Userland/Libraries/LibWeb/DOM/Node.idl @@ -15,8 +15,8 @@ interface Node : EventTarget { attribute DOMString textContent; Node appendChild(Node node); - Node insertBefore(Node node, Node? child); - Node removeChild(Node child); + [ImplementedAs=pre_insert] Node insertBefore(Node node, Node? child); + [ImplementedAs=pre_remove] Node removeChild(Node child); const unsigned short ELEMENT_NODE = 1; const unsigned short ATTRIBUTE_NODE = 2; diff --git a/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp b/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp index a91818e9b0..3e12e3dc18 100644 --- a/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp +++ b/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp @@ -62,14 +62,14 @@ void EditEventHandler::handle_delete(DOM::Range& range) for (auto* parent = end->parent(); parent; parent = parent->parent()) queued_for_deletion.remove(parent); for (auto* node : queued_for_deletion) - node->parent()->remove_child(*node); + node->remove(); // Join the parent nodes of start and end. DOM::Node *insert_after = start, *remove_from = end, *parent_of_end = end->parent(); while (remove_from) { auto* next_sibling = remove_from->next_sibling(); - remove_from->parent()->remove_child(*remove_from); + remove_from->remove(); insert_after->parent()->insert_before(*remove_from, *insert_after); insert_after = remove_from; @@ -77,7 +77,7 @@ void EditEventHandler::handle_delete(DOM::Range& range) } if (!parent_of_end->has_children()) { if (parent_of_end->parent()) - parent_of_end->parent()->remove_child(*parent_of_end); + parent_of_end->remove(); } // Join the start and end nodes. @@ -86,7 +86,7 @@ void EditEventHandler::handle_delete(DOM::Range& range) builder.append(end->data().substring_view(range.end_offset())); start->set_data(builder.to_string()); - start->parent()->remove_child(*end); + end->remove(); } // FIXME: When nodes are removed from the DOM, the associated layout nodes become stale and still diff --git a/Userland/Libraries/LibWeb/TreeNode.h b/Userland/Libraries/LibWeb/TreeNode.h index 0de95a93ce..fa4853486b 100644 --- a/Userland/Libraries/LibWeb/TreeNode.h +++ b/Userland/Libraries/LibWeb/TreeNode.h @@ -98,14 +98,15 @@ public: } bool is_ancestor_of(const TreeNode&) const; + bool is_inclusive_ancestor_of(const TreeNode&) const; + bool is_descendant_of(const TreeNode&) const; + bool is_inclusive_descendant_of(const TreeNode&) const; void append_child(NonnullRefPtr<T> node); void prepend_child(NonnullRefPtr<T> node); void insert_before(NonnullRefPtr<T> node, RefPtr<T> child); void remove_child(NonnullRefPtr<T> node); - void remove_all_children(); - bool is_child_allowed(const T&) const { return true; } T* next_in_pre_order() @@ -324,6 +325,12 @@ public: } template<typename U> + bool has_child_of_type() const + { + return first_child_of_type<U>() != nullptr; + } + + template<typename U> const U* first_ancestor_of_type() const { return const_cast<TreeNode*>(this)->template first_ancestor_of_type<U>(); @@ -366,14 +373,6 @@ private: }; template<typename T> -inline void TreeNode<T>::remove_all_children() -{ - while (RefPtr<T> child = first_child()) - remove_child(child.release_nonnull()); -} - - -template<typename T> inline void TreeNode<T>::remove_child(NonnullRefPtr<T> node) { VERIFY(node->m_parent == this); @@ -470,4 +469,22 @@ inline bool TreeNode<T>::is_ancestor_of(const TreeNode<T>& other) const return false; } +template<typename T> +inline bool TreeNode<T>::is_inclusive_ancestor_of(const TreeNode<T>& other) const +{ + return &other == this || is_ancestor_of(other); +} + +template<typename T> +inline bool TreeNode<T>::is_descendant_of(const TreeNode<T>& other) const +{ + return other.is_ancestor_of(*this); +} + +template<typename T> +inline bool TreeNode<T>::is_inclusive_descendant_of(const TreeNode<T>& other) const +{ + return other.is_inclusive_ancestor_of(*this); +} + } |