From f7f37eaa0f03a08da15969cb6a43689ebe264e11 Mon Sep 17 00:00:00 2001 From: Luke Wilde Date: Thu, 2 Sep 2021 19:27:42 +0100 Subject: [PATCH] LibWeb: Make Node::root return a reference The root of a node can never be null, as "the root of an object is itself, if its parent is null, or else it is the root of its parent." https://dom.spec.whatwg.org/#concept-tree-root --- .../Libraries/LibWeb/DOM/EventDispatcher.cpp | 8 +++---- Userland/Libraries/LibWeb/DOM/Node.cpp | 23 +++++++++++-------- Userland/Libraries/LibWeb/DOM/Node.h | 8 +++---- Userland/Libraries/LibWeb/DOM/ShadowRoot.cpp | 3 ++- Userland/Libraries/LibWeb/Layout/Node.cpp | 2 +- 5 files changed, 24 insertions(+), 20 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp b/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp index b133205df5..2ef8c6bd52 100644 --- a/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp +++ b/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp @@ -35,15 +35,15 @@ static EventTarget* retarget(EventTarget* left, EventTarget* right) return left; auto* left_node = verify_cast(left); - auto* left_root = left_node->root(); + auto& left_root = left_node->root(); if (!is(left_root)) return left; - if (is(right) && left_root->is_shadow_including_inclusive_ancestor_of(verify_cast(*right))) + if (is(right) && left_root.is_shadow_including_inclusive_ancestor_of(verify_cast(*right))) return left; - auto* left_shadow_root = verify_cast(left_root); - left = left_shadow_root->host(); + auto& left_shadow_root = verify_cast(left_root); + left = left_shadow_root.host(); } } diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index 1a7a05b793..cd272ef13a 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -157,25 +157,28 @@ String Node::child_text_content() const return builder.build(); } -Node* Node::root() +// https://dom.spec.whatwg.org/#concept-tree-root +Node& Node::root() { Node* root = this; while (root->parent()) root = root->parent(); - return root; + return *root; } -Node* Node::shadow_including_root() +// https://dom.spec.whatwg.org/#concept-shadow-including-root +Node& Node::shadow_including_root() { - auto node_root = root(); + auto& node_root = root(); if (is(node_root)) - return verify_cast(node_root)->host()->shadow_including_root(); + return verify_cast(node_root).host()->shadow_including_root(); return node_root; } +// https://dom.spec.whatwg.org/#connected bool Node::is_connected() const { - return shadow_including_root() && shadow_including_root()->is_document(); + return shadow_including_root().is_document(); } Element* Node::parent_element() @@ -608,7 +611,7 @@ u16 Node::compare_document_position(RefPtr other) // FIXME: Once LibWeb supports attribute nodes fix to follow the specification. VERIFY(node1->type() != NodeType::ATTRIBUTE_NODE && node2->type() != NodeType::ATTRIBUTE_NODE); - if ((node1 == nullptr || node2 == nullptr) || (node1->root() != node2->root())) + if ((node1 == nullptr || node2 == nullptr) || (&node1->root() != &node2->root())) return DOCUMENT_POSITION_DISCONNECTED | DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC | (node1 > node2 ? DOCUMENT_POSITION_PRECEDING : DOCUMENT_POSITION_FOLLOWING); if (node1->is_ancestor_of(*node2)) @@ -626,7 +629,7 @@ u16 Node::compare_document_position(RefPtr other) // 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(other.root()) && verify_cast(other.root())->host() && is_inclusive_ancestor_of(*verify_cast(other.root())->host().ptr())); + return is_inclusive_ancestor_of(other) || (is(other.root()) && verify_cast(other.root()).host() && is_inclusive_ancestor_of(*verify_cast(other.root()).host().ptr())); } // https://dom.spec.whatwg.org/#dom-node-ownerdocument @@ -691,10 +694,10 @@ bool Node::is_shadow_including_descendant_of(Node const& other) const if (!is(root())) return false; - auto shadow_root = verify_cast(root()); + auto& shadow_root = verify_cast(root()); // NOTE: While host is nullable because of inheriting from DocumentFragment, shadow roots always have a host. - return shadow_root->host()->is_shadow_including_inclusive_descendant_of(other); + return shadow_root.host()->is_shadow_including_inclusive_descendant_of(other); } // https://dom.spec.whatwg.org/#concept-shadow-including-inclusive-descendant diff --git a/Userland/Libraries/LibWeb/DOM/Node.h b/Userland/Libraries/LibWeb/DOM/Node.h index 81a389401f..d0e66209aa 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.h +++ b/Userland/Libraries/LibWeb/DOM/Node.h @@ -113,14 +113,14 @@ public: String child_text_content() const; - Node* root(); - const Node* root() const + Node& root(); + const Node& root() const { return const_cast(this)->root(); } - Node* shadow_including_root(); - const Node* shadow_including_root() const + Node& shadow_including_root(); + const Node& shadow_including_root() const { return const_cast(this)->shadow_including_root(); } diff --git a/Userland/Libraries/LibWeb/DOM/ShadowRoot.cpp b/Userland/Libraries/LibWeb/DOM/ShadowRoot.cpp index 07dc13ed38..7043ce8242 100644 --- a/Userland/Libraries/LibWeb/DOM/ShadowRoot.cpp +++ b/Userland/Libraries/LibWeb/DOM/ShadowRoot.cpp @@ -16,11 +16,12 @@ ShadowRoot::ShadowRoot(Document& document, Element& host) set_host(host); } +// https://dom.spec.whatwg.org/#ref-for-get-the-parent%E2%91%A6 EventTarget* ShadowRoot::get_parent(const Event& event) { if (!event.composed()) { auto& events_first_invocation_target = verify_cast(*event.path().first().invocation_target); - if (events_first_invocation_target.root() == this) + if (&events_first_invocation_target.root() == this) return nullptr; } diff --git a/Userland/Libraries/LibWeb/Layout/Node.cpp b/Userland/Libraries/LibWeb/Layout/Node.cpp index 8af30c93be..6d69db73e4 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.cpp +++ b/Userland/Libraries/LibWeb/Layout/Node.cpp @@ -72,7 +72,7 @@ bool Node::establishes_stacking_context() const { if (!has_style()) return false; - if (dom_node() == document().root()) + if (dom_node() == &document().root()) return true; auto position = computed_values().position(); if (position == CSS::Position::Absolute || position == CSS::Position::Relative || position == CSS::Position::Fixed || position == CSS::Position::Sticky)