From 6b580d68a38236562ae548095459b2e57134c4be Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 2 Nov 2023 14:30:00 +0100 Subject: [PATCH] LibWeb: Rename DOM::Node::id() to unique_id() The old name was pretty confusing, since it had nothing to do with the common "id" content attribute. This makes way for using id() to return the "id" attribute instead. :^) --- .../LibWeb/DOM/AccessibilityTreeNode.cpp | 2 +- Userland/Libraries/LibWeb/DOM/Element.cpp | 2 +- Userland/Libraries/LibWeb/DOM/Node.cpp | 26 +++++++++---------- Userland/Libraries/LibWeb/DOM/Node.h | 6 ++--- .../Libraries/LibWeb/Page/EventHandler.cpp | 2 +- Userland/Libraries/LibWeb/Page/Page.cpp | 2 +- .../WebContent/ConnectionFromClient.cpp | 4 +-- .../WebContent/WebDriverConnection.cpp | 10 +++---- 8 files changed, 27 insertions(+), 27 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp b/Userland/Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp index 1073496424..1cef74149b 100644 --- a/Userland/Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp +++ b/Userland/Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp @@ -41,7 +41,7 @@ void AccessibilityTreeNode::serialize_tree_as_json(JsonObjectSerializeraccessible_description(document)); MUST(object.add("description"sv, description)); - MUST(object.add("id"sv, element->id())); + MUST(object.add("id"sv, element->unique_id())); if (has_role) MUST(object.add("role"sv, ARIA::role_name(*role))); diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index 0d34c19876..410ce18413 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -995,7 +995,7 @@ void Element::serialize_pseudo_elements_as_json(JsonArraySerializer(i))))); MUST(object.add("type"sv, "pseudo-element")); - MUST(object.add("parent-id"sv, id())); + MUST(object.add("parent-id"sv, unique_id())); MUST(object.add("pseudo-element"sv, i)); MUST(object.finish()); } diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index b619e749ca..a9a20de805 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -46,33 +46,33 @@ namespace Web::DOM { -static IDAllocator s_node_id_allocator; +static IDAllocator s_unique_id_allocator; static HashMap s_node_directory; -static i32 allocate_node_id(Node* node) +static i32 allocate_unique_id(Node* node) { - i32 id = s_node_id_allocator.allocate(); + i32 id = s_unique_id_allocator.allocate(); s_node_directory.set(id, node); return id; } -static void deallocate_node_id(i32 node_id) +static void deallocate_unique_id(i32 node_id) { if (!s_node_directory.remove(node_id)) VERIFY_NOT_REACHED(); - s_node_id_allocator.deallocate(node_id); + s_unique_id_allocator.deallocate(node_id); } -Node* Node::from_id(i32 node_id) +Node* Node::from_unique_id(i32 unique_id) { - return s_node_directory.get(node_id).value_or(nullptr); + return s_node_directory.get(unique_id).value_or(nullptr); } Node::Node(JS::Realm& realm, Document& document, NodeType type) : EventTarget(realm) , m_document(&document) , m_type(type) - , m_id(allocate_node_id(this)) + , m_unique_id(allocate_unique_id(this)) { } @@ -86,7 +86,7 @@ Node::~Node() = default; void Node::finalize() { Base::finalize(); - deallocate_node_id(m_id); + deallocate_unique_id(m_unique_id); } void Node::visit_edges(Cell::Visitor& visitor) @@ -1155,7 +1155,7 @@ bool Node::is_uninteresting_whitespace_node() const void Node::serialize_tree_as_json(JsonObjectSerializer& object) const { MUST(object.add("name"sv, node_name())); - MUST(object.add("id"sv, id())); + MUST(object.add("id"sv, unique_id())); if (is_document()) { MUST(object.add("type"sv, "document")); } else if (is_element()) { @@ -1758,7 +1758,7 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con auto const* root_node = this; auto const* current_node = root_node; StringBuilder total_accumulated_text; - visited_nodes.set(id()); + visited_nodes.set(unique_id()); if (is_element()) { auto const* element = static_cast(this); @@ -1792,7 +1792,7 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con if (!node) continue; - if (visited_nodes.contains(node->id())) + if (visited_nodes.contains(node->unique_id())) continue; // a. Set the current node to the node referenced by the IDREF. current_node = node; @@ -1842,7 +1842,7 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con // iii. For each child node of the current node: element->for_each_child([&total_accumulated_text, current_node, target, &document, &visited_nodes]( DOM::Node const& child_node) mutable { - if (visited_nodes.contains(child_node.id())) + if (visited_nodes.contains(child_node.unique_id())) return; // a. Set the current node to the child node. diff --git a/Userland/Libraries/LibWeb/DOM/Node.h b/Userland/Libraries/LibWeb/DOM/Node.h index 18073eb236..698f268bd6 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.h +++ b/Userland/Libraries/LibWeb/DOM/Node.h @@ -235,8 +235,8 @@ public: bool is_shadow_including_ancestor_of(Node const&) const; bool is_shadow_including_inclusive_ancestor_of(Node const&) const; - i32 id() const { return m_id; } - static Node* from_id(i32 node_id); + i32 unique_id() const { return m_unique_id; } + static Node* from_unique_id(i32); WebIDL::ExceptionOr serialize_fragment(DOMParsing::RequireWellFormed) const; @@ -689,7 +689,7 @@ protected: bool m_needs_style_update { false }; bool m_child_needs_style_update { false }; - i32 m_id; + i32 m_unique_id {}; // https://dom.spec.whatwg.org/#registered-observer-list // "Nodes have a strong reference to registered observers in their registered observer list." https://dom.spec.whatwg.org/#garbage-collection diff --git a/Userland/Libraries/LibWeb/Page/EventHandler.cpp b/Userland/Libraries/LibWeb/Page/EventHandler.cpp index a88748b780..73e0e80022 100644 --- a/Userland/Libraries/LibWeb/Page/EventHandler.cpp +++ b/Userland/Libraries/LibWeb/Page/EventHandler.cpp @@ -312,7 +312,7 @@ bool EventHandler::handle_mouseup(CSSPixelPoint position, CSSPixelPoint screen_p }; if (auto* page = m_browsing_context->page()) - page->did_request_media_context_menu(media_element.id(), m_browsing_context->to_top_level_position(position), "", modifiers, move(menu)); + page->did_request_media_context_menu(media_element.unique_id(), m_browsing_context->to_top_level_position(position), "", modifiers, move(menu)); } else if (auto* page = m_browsing_context->page()) { page->client().page_did_request_context_menu(m_browsing_context->to_top_level_position(position)); } diff --git a/Userland/Libraries/LibWeb/Page/Page.cpp b/Userland/Libraries/LibWeb/Page/Page.cpp index 8a197ce8f9..e9a0df831b 100644 --- a/Userland/Libraries/LibWeb/Page/Page.cpp +++ b/Userland/Libraries/LibWeb/Page/Page.cpp @@ -384,7 +384,7 @@ JS::GCPtr Page::media_context_menu_element() if (!m_media_context_menu_element_id.has_value()) return nullptr; - auto* dom_node = DOM::Node::from_id(*m_media_context_menu_element_id); + auto* dom_node = DOM::Node::from_unique_id(*m_media_context_menu_element_id); if (dom_node == nullptr) return nullptr; diff --git a/Userland/Services/WebContent/ConnectionFromClient.cpp b/Userland/Services/WebContent/ConnectionFromClient.cpp index 00d7a04206..763e0b501a 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.cpp +++ b/Userland/Services/WebContent/ConnectionFromClient.cpp @@ -517,7 +517,7 @@ Messages::WebContentServer::InspectDomNodeResponse ConnectionFromClient::inspect return IterationDecision::Continue; }); - Web::DOM::Node* node = Web::DOM::Node::from_id(node_id); + Web::DOM::Node* node = Web::DOM::Node::from_unique_id(node_id); // Note: Nodes without layout (aka non-visible nodes, don't have style computed) if (!node || !node->layout_node()) { return { false, "", "", "", "", "" }; @@ -642,7 +642,7 @@ Messages::WebContentServer::GetHoveredNodeIdResponse ConnectionFromClient::get_h if (auto* document = page().top_level_browsing_context().active_document()) { auto hovered_node = document->hovered_node(); if (hovered_node) - return hovered_node->id(); + return hovered_node->unique_id(); } return (i32)0; } diff --git a/Userland/Services/WebContent/WebDriverConnection.cpp b/Userland/Services/WebContent/WebDriverConnection.cpp index 0a370466ba..57784578f5 100644 --- a/Userland/Services/WebContent/WebDriverConnection.cpp +++ b/Userland/Services/WebContent/WebDriverConnection.cpp @@ -123,7 +123,7 @@ static DeprecatedString get_or_create_a_web_element_reference(Web::DOM::Node con // FIXME: 2. Add element to the list of known elements of the current browsing context. // FIXME: 3. Return success with the element’s web element reference. - return DeprecatedString::number(element.id()); + return DeprecatedString::number(element.unique_id()); } // https://w3c.github.io/webdriver/#dfn-web-element-reference-object @@ -154,7 +154,7 @@ static ErrorOr get_known_connected_el if (!element.has_value()) return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, "Element ID is not an integer"); - auto* node = Web::DOM::Node::from_id(*element); + auto* node = Web::DOM::Node::from_unique_id(*element); if (!node || !node->is_element()) return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::NoSuchElement, DeprecatedString::formatted("Could not find element with ID: {}", element_id)); @@ -170,7 +170,7 @@ static DeprecatedString get_or_create_a_shadow_root_reference(Web::DOM::ShadowRo // FIXME: 2. Add shadow to the list of known shadow roots of the current browsing context. // FIXME: 3. Return success with the shadow’s shadow root reference. - return DeprecatedString::number(shadow_root.id()); + return DeprecatedString::number(shadow_root.unique_id()); } // https://w3c.github.io/webdriver/#dfn-shadow-root-reference-object @@ -201,7 +201,7 @@ static ErrorOr get_known_shadow_ro if (!shadow_root.has_value()) return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, "Shadow ID is not an integer"); - auto* node = Web::DOM::Node::from_id(*shadow_root); + auto* node = Web::DOM::Node::from_unique_id(*shadow_root); if (!node || !node->is_shadow_root()) return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::NoSuchElement, DeprecatedString::formatted("Could not find shadow root with ID: {}", shadow_id)); @@ -982,7 +982,7 @@ Messages::WebDriverClient::GetActiveElementResponse WebDriverConnection::get_act // 4. If active element is a non-null element, return success with data set to web element reference object for active element. // Otherwise, return error with error code no such element. if (active_element) - return DeprecatedString::number(active_element->id()); + return DeprecatedString::number(active_element->unique_id()); return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::NoSuchElement, "The current document does not have an active element"sv); }