From f07f5a26229580835595a805c4160f31170c3a7c Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Sun, 18 Feb 2024 13:59:28 -0500 Subject: [PATCH] LibWeb+WebContent: Do not embed attributes as data in the Inspector HTML Attribute values may contain HTML, and may contain invalid HTML at that. If the latter occurs, let's not generate invalid Inspector HTML when we embed the attribute values as data attributes. Instead, cache the values in the InspectorClient, and embed just a lookup index into the HTML. This also nicely reduces the size of the generated HTML. The Inspector on https://github.com/SerenityOS/serenity reduces from 2.3MB to 1.9MB (about 318KB, or 13.8%). --- Base/res/ladybird/inspector.js | 21 ++++---------- .../Libraries/LibWeb/Internals/Inspector.cpp | 8 ++--- .../Libraries/LibWeb/Internals/Inspector.h | 5 ++-- .../Libraries/LibWeb/Internals/Inspector.idl | 4 +-- Userland/Libraries/LibWeb/Page/Page.h | 4 +-- .../Libraries/LibWebView/InspectorClient.cpp | 29 ++++++++++++++----- .../Libraries/LibWebView/InspectorClient.h | 5 ++++ .../Libraries/LibWebView/ViewImplementation.h | 4 +-- .../Libraries/LibWebView/WebContentClient.cpp | 8 ++--- .../Libraries/LibWebView/WebContentClient.h | 4 +-- Userland/Services/WebContent/PageClient.cpp | 12 +++----- Userland/Services/WebContent/PageClient.h | 4 +-- .../Services/WebContent/WebContentClient.ipc | 4 +-- 13 files changed, 60 insertions(+), 52 deletions(-) diff --git a/Base/res/ladybird/inspector.js b/Base/res/ladybird/inspector.js index e0cc78685c..47df682d1a 100644 --- a/Base/res/ladybird/inspector.js +++ b/Base/res/ladybird/inspector.js @@ -288,8 +288,10 @@ const editDOMNode = domNode => { const element = document.createElement(value); inspector.setDOMNodeTag(domNodeID, value); } else if (type === "attribute") { + const attributeIndex = domNode.dataset.attributeIndex; const attributes = parseDOMAttributes(value); - inspector.replaceDOMNodeAttribute(domNodeID, domNode.dataset.attributeName, attributes); + + inspector.replaceDOMNodeAttribute(domNodeID, attributeIndex, attributes); } }; @@ -354,28 +356,17 @@ const requestContextMenu = (clientX, clientY, domNode) => { } let tag = null; - let attributeName = null; - let attributeValue = null; + let attributeIndex = null; if (type === "tag") { tag = domNode.dataset.tag; } else if (type === "attribute") { tag = domNode.dataset.tag; - attributeName = domNode.dataset.attributeName; - attributeValue = domNode.dataset.attributeValue; + attributeIndex = domNode.dataset.attributeIndex; } pendingEditDOMNode = domNode; - - inspector.requestDOMTreeContextMenu( - domNodeID, - clientX, - clientY, - type, - tag, - attributeName, - attributeValue - ); + inspector.requestDOMTreeContextMenu(domNodeID, clientX, clientY, type, tag, attributeIndex); }; const executeConsoleScript = consoleInput => { diff --git a/Userland/Libraries/LibWeb/Internals/Inspector.cpp b/Userland/Libraries/LibWeb/Internals/Inspector.cpp index 0b579f1e25..2334bd4520 100644 --- a/Userland/Libraries/LibWeb/Internals/Inspector.cpp +++ b/Userland/Libraries/LibWeb/Internals/Inspector.cpp @@ -61,14 +61,14 @@ void Inspector::add_dom_node_attributes(i32 node_id, JS::NonnullGCPtrpage().client().inspector_did_add_dom_node_attributes(node_id, attributes); } -void Inspector::replace_dom_node_attribute(i32 node_id, String const& name, JS::NonnullGCPtr replacement_attributes) +void Inspector::replace_dom_node_attribute(i32 node_id, WebIDL::UnsignedLongLong attribute_index, JS::NonnullGCPtr replacement_attributes) { - global_object().browsing_context()->page().client().inspector_did_replace_dom_node_attribute(node_id, name, replacement_attributes); + global_object().browsing_context()->page().client().inspector_did_replace_dom_node_attribute(node_id, static_cast(attribute_index), replacement_attributes); } -void Inspector::request_dom_tree_context_menu(i32 node_id, i32 client_x, i32 client_y, String const& type, Optional const& tag, Optional const& attribute_name, Optional const& attribute_value) +void Inspector::request_dom_tree_context_menu(i32 node_id, i32 client_x, i32 client_y, String const& type, Optional const& tag, Optional const& attribute_index) { - global_object().browsing_context()->page().client().inspector_did_request_dom_tree_context_menu(node_id, { client_x, client_y }, type, tag, attribute_name, attribute_value); + global_object().browsing_context()->page().client().inspector_did_request_dom_tree_context_menu(node_id, { client_x, client_y }, type, tag, attribute_index.map([](auto index) { return static_cast(index); })); } void Inspector::execute_console_script(String const& script) diff --git a/Userland/Libraries/LibWeb/Internals/Inspector.h b/Userland/Libraries/LibWeb/Internals/Inspector.h index 6f6e967685..0058984d72 100644 --- a/Userland/Libraries/LibWeb/Internals/Inspector.h +++ b/Userland/Libraries/LibWeb/Internals/Inspector.h @@ -8,6 +8,7 @@ #include #include +#include namespace Web::Internals { @@ -24,9 +25,9 @@ public: void set_dom_node_text(i32 node_id, String const& text); void set_dom_node_tag(i32 node_id, String const& tag); void add_dom_node_attributes(i32 node_id, JS::NonnullGCPtr attributes); - void replace_dom_node_attribute(i32 node_id, String const& name, JS::NonnullGCPtr replacement_attributes); + void replace_dom_node_attribute(i32 node_id, WebIDL::UnsignedLongLong attribute_index, JS::NonnullGCPtr replacement_attributes); - void request_dom_tree_context_menu(i32 node_id, i32 client_x, i32 client_y, String const& type, Optional const& tag, Optional const& attribute_name, Optional const& attribute_value); + void request_dom_tree_context_menu(i32 node_id, i32 client_x, i32 client_y, String const& type, Optional const& tag, Optional const& attribute_index); void execute_console_script(String const& script); diff --git a/Userland/Libraries/LibWeb/Internals/Inspector.idl b/Userland/Libraries/LibWeb/Internals/Inspector.idl index a72e8a18cd..a87479bb35 100644 --- a/Userland/Libraries/LibWeb/Internals/Inspector.idl +++ b/Userland/Libraries/LibWeb/Internals/Inspector.idl @@ -8,9 +8,9 @@ undefined setDOMNodeText(long nodeID, DOMString text); undefined setDOMNodeTag(long nodeID, DOMString tag); undefined addDOMNodeAttributes(long nodeID, NamedNodeMap attributes); - undefined replaceDOMNodeAttribute(long nodeID, DOMString name, NamedNodeMap replacementAttributes); + undefined replaceDOMNodeAttribute(long nodeID, unsigned long long attributeIndex, NamedNodeMap replacementAttributes); - undefined requestDOMTreeContextMenu(long nodeID, long clientX, long clientY, DOMString type, DOMString? tag, DOMString? attributeName, DOMString? attributeValue); + undefined requestDOMTreeContextMenu(long nodeID, long clientX, long clientY, DOMString type, DOMString? tag, unsigned long long? attributeIndex); undefined executeConsoleScript(DOMString script); diff --git a/Userland/Libraries/LibWeb/Page/Page.h b/Userland/Libraries/LibWeb/Page/Page.h index 9e05a7d7ef..9d3c9a98ca 100644 --- a/Userland/Libraries/LibWeb/Page/Page.h +++ b/Userland/Libraries/LibWeb/Page/Page.h @@ -298,8 +298,8 @@ public: virtual void inspector_did_set_dom_node_text([[maybe_unused]] i32 node_id, [[maybe_unused]] String const& text) { } virtual void inspector_did_set_dom_node_tag([[maybe_unused]] i32 node_id, [[maybe_unused]] String const& tag) { } virtual void inspector_did_add_dom_node_attributes([[maybe_unused]] i32 node_id, [[maybe_unused]] JS::NonnullGCPtr attributes) { } - virtual void inspector_did_replace_dom_node_attribute([[maybe_unused]] i32 node_id, [[maybe_unused]] String const& name, [[maybe_unused]] JS::NonnullGCPtr replacement_attributes) { } - virtual void inspector_did_request_dom_tree_context_menu([[maybe_unused]] i32 node_id, [[maybe_unused]] CSSPixelPoint position, [[maybe_unused]] String const& type, [[maybe_unused]] Optional const& tag, [[maybe_unused]] Optional const& attribute_name, [[maybe_unused]] Optional const& attribute_value) { } + virtual void inspector_did_replace_dom_node_attribute([[maybe_unused]] i32 node_id, [[maybe_unused]] size_t attribute_index, [[maybe_unused]] JS::NonnullGCPtr replacement_attributes) { } + virtual void inspector_did_request_dom_tree_context_menu([[maybe_unused]] i32 node_id, [[maybe_unused]] CSSPixelPoint position, [[maybe_unused]] String const& type, [[maybe_unused]] Optional const& tag, [[maybe_unused]] Optional const& attribute_index) { } virtual void inspector_did_execute_console_script([[maybe_unused]] String const& script) { } virtual void schedule_repaint() = 0; diff --git a/Userland/Libraries/LibWebView/InspectorClient.cpp b/Userland/Libraries/LibWebView/InspectorClient.cpp index 28ef546355..7c3e7274e3 100644 --- a/Userland/Libraries/LibWebView/InspectorClient.cpp +++ b/Userland/Libraries/LibWebView/InspectorClient.cpp @@ -96,6 +96,7 @@ InspectorClient::InspectorClient(ViewImplementation& content_web_view, ViewImple m_content_web_view.on_finshed_editing_dom_node = [this](auto const& node_id) { m_pending_selection = node_id; m_dom_tree_loaded = false; + m_dom_node_attributes.clear(); inspect(); }; @@ -122,7 +123,11 @@ InspectorClient::InspectorClient(ViewImplementation& content_web_view, ViewImple m_content_web_view.js_console_request_messages(0); }; - m_inspector_web_view.on_inspector_requested_dom_tree_context_menu = [this](auto node_id, auto position, auto const& type, auto const& tag, auto const& attribute) { + m_inspector_web_view.on_inspector_requested_dom_tree_context_menu = [this](auto node_id, auto position, auto const& type, auto const& tag, auto const& attribute_index) { + Optional attribute; + if (attribute_index.has_value()) + attribute = m_dom_node_attributes.get(node_id)->at(*attribute_index); + m_context_menu_data = ContextMenuData { node_id, tag, attribute }; if (type.is_one_of("text"sv, "comment"sv)) { @@ -158,8 +163,9 @@ InspectorClient::InspectorClient(ViewImplementation& content_web_view, ViewImple m_content_web_view.add_dom_node_attributes(node_id, attributes); }; - m_inspector_web_view.on_inspector_replaced_dom_node_attribute = [this](auto node_id, auto const& name, auto const& replacement_attributes) { - m_content_web_view.replace_dom_node_attribute(node_id, name, replacement_attributes); + m_inspector_web_view.on_inspector_replaced_dom_node_attribute = [this](auto node_id, u32 attribute_index, auto const& replacement_attributes) { + auto const& attribute = m_dom_node_attributes.get(node_id)->at(attribute_index); + m_content_web_view.replace_dom_node_attribute(node_id, attribute.name, replacement_attributes); }; m_inspector_web_view.on_inspector_executed_console_script = [this](auto const& script) { @@ -201,6 +207,8 @@ void InspectorClient::reset() m_pending_selection.clear(); m_dom_tree_loaded = false; + m_dom_node_attributes.clear(); + m_highest_notified_message_index = -1; m_highest_received_message_index = -1; m_waiting_for_messages = false; @@ -461,13 +469,17 @@ String InspectorClient::generate_dom_tree(JsonObject const& dom_tree) data_attributes.appendff("data-{}=\"{}\"", name, value); }; + i32 node_id = 0; + if (auto pseudo_element = node.get_integer("pseudo-element"sv); pseudo_element.has_value()) { - append_data_attribute("id"sv, node.get_integer("parent-id"sv).value()); + node_id = node.get_integer("parent-id"sv).value(); append_data_attribute("pseudo-element"sv, *pseudo_element); } else { - append_data_attribute("id"sv, node.get_integer("id"sv).value()); + node_id = node.get_integer("id"sv).value(); } + append_data_attribute("id"sv, node_id); + if (type == "text"sv) { auto deprecated_text = node.get_byte_string("text"sv).release_value(); deprecated_text = escape_html_entities(deprecated_text); @@ -513,7 +525,7 @@ String InspectorClient::generate_dom_tree(JsonObject const& dom_tree) } if (name.equals_ignoring_ascii_case("BODY"sv)) - m_body_node_id = node.get_integer("id"sv).value(); + m_body_node_id = node_id; auto tag = name.to_lowercase(); @@ -523,14 +535,17 @@ String InspectorClient::generate_dom_tree(JsonObject const& dom_tree) if (auto attributes = node.get_object("attributes"sv); attributes.has_value()) { attributes->for_each_member([&](auto const& name, auto const& value) { + auto& dom_node_attributes = m_dom_node_attributes.ensure(node_id); auto value_string = value.as_string(); builder.append(" "sv); - builder.appendff("", tag, name, value_string); + builder.appendff("", tag, dom_node_attributes.size()); builder.appendff("{}", name); builder.append('='); builder.appendff("\"{}\"", value_string); builder.append(""sv); + + dom_node_attributes.empend(MUST(String::from_byte_string(name)), MUST(String::from_byte_string(value_string))); }); } diff --git a/Userland/Libraries/LibWebView/InspectorClient.h b/Userland/Libraries/LibWebView/InspectorClient.h index 90a0f97320..15289f62e3 100644 --- a/Userland/Libraries/LibWebView/InspectorClient.h +++ b/Userland/Libraries/LibWebView/InspectorClient.h @@ -5,9 +5,12 @@ */ #include +#include #include #include +#include #include +#include #include #pragma once @@ -76,6 +79,8 @@ private: }; Optional m_context_menu_data; + HashMap> m_dom_node_attributes; + i32 m_highest_notified_message_index { -1 }; i32 m_highest_received_message_index { -1 }; bool m_waiting_for_messages { false }; diff --git a/Userland/Libraries/LibWebView/ViewImplementation.h b/Userland/Libraries/LibWebView/ViewImplementation.h index dbb4a9f661..e8203ad49f 100644 --- a/Userland/Libraries/LibWebView/ViewImplementation.h +++ b/Userland/Libraries/LibWebView/ViewImplementation.h @@ -174,8 +174,8 @@ public: Function on_inspector_set_dom_node_text; Function on_inspector_set_dom_node_tag; Function const&)> on_inspector_added_dom_node_attributes; - Function const&)> on_inspector_replaced_dom_node_attribute; - Function const&, Optional const&)> on_inspector_requested_dom_tree_context_menu; + Function const&)> on_inspector_replaced_dom_node_attribute; + Function const&, Optional const&)> on_inspector_requested_dom_tree_context_menu; Function on_inspector_executed_console_script; Function on_request_worker_agent; diff --git a/Userland/Libraries/LibWebView/WebContentClient.cpp b/Userland/Libraries/LibWebView/WebContentClient.cpp index 0d042ab548..fbe18b9107 100644 --- a/Userland/Libraries/LibWebView/WebContentClient.cpp +++ b/Userland/Libraries/LibWebView/WebContentClient.cpp @@ -923,7 +923,7 @@ void WebContentClient::inspector_did_add_dom_node_attributes(u64 page_id, i32 no view.on_inspector_added_dom_node_attributes(node_id, attributes); } -void WebContentClient::inspector_did_replace_dom_node_attribute(u64 page_id, i32 node_id, String const& name, Vector const& replacement_attributes) +void WebContentClient::inspector_did_replace_dom_node_attribute(u64 page_id, i32 node_id, size_t attribute_index, Vector const& replacement_attributes) { auto maybe_view = m_views.get(page_id); if (!maybe_view.has_value()) { @@ -933,10 +933,10 @@ void WebContentClient::inspector_did_replace_dom_node_attribute(u64 page_id, i32 auto& view = *maybe_view.value(); if (view.on_inspector_replaced_dom_node_attribute) - view.on_inspector_replaced_dom_node_attribute(node_id, name, replacement_attributes); + view.on_inspector_replaced_dom_node_attribute(node_id, attribute_index, replacement_attributes); } -void WebContentClient::inspector_did_request_dom_tree_context_menu(u64 page_id, i32 node_id, Gfx::IntPoint position, String const& type, Optional const& tag, Optional const& attribute) +void WebContentClient::inspector_did_request_dom_tree_context_menu(u64 page_id, i32 node_id, Gfx::IntPoint position, String const& type, Optional const& tag, Optional const& attribute_index) { auto maybe_view = m_views.get(page_id); if (!maybe_view.has_value()) { @@ -946,7 +946,7 @@ void WebContentClient::inspector_did_request_dom_tree_context_menu(u64 page_id, auto& view = *maybe_view.value(); if (view.on_inspector_requested_dom_tree_context_menu) - view.on_inspector_requested_dom_tree_context_menu(node_id, view.to_widget_position(position), type, tag, attribute); + view.on_inspector_requested_dom_tree_context_menu(node_id, view.to_widget_position(position), type, tag, attribute_index); } void WebContentClient::inspector_did_execute_console_script(u64 page_id, String const& script) diff --git a/Userland/Libraries/LibWebView/WebContentClient.h b/Userland/Libraries/LibWebView/WebContentClient.h index 03d654adc8..6c1a29d9ec 100644 --- a/Userland/Libraries/LibWebView/WebContentClient.h +++ b/Userland/Libraries/LibWebView/WebContentClient.h @@ -99,8 +99,8 @@ private: virtual void inspector_did_set_dom_node_text(u64 page_id, i32 node_id, String const& text) override; virtual void inspector_did_set_dom_node_tag(u64 page_id, i32 node_id, String const& tag) override; virtual void inspector_did_add_dom_node_attributes(u64 page_id, i32 node_id, Vector const& attributes) override; - virtual void inspector_did_replace_dom_node_attribute(u64 page_id, i32 node_id, String const& name, Vector const& replacement_attributes) override; - virtual void inspector_did_request_dom_tree_context_menu(u64 page_id, i32 node_id, Gfx::IntPoint position, String const& type, Optional const& tag, Optional const& attribute) override; + virtual void inspector_did_replace_dom_node_attribute(u64 page_id, i32 node_id, size_t attribute_index, Vector const& replacement_attributes) override; + virtual void inspector_did_request_dom_tree_context_menu(u64 page_id, i32 node_id, Gfx::IntPoint position, String const& type, Optional const& tag, Optional const& attribute_index) override; virtual void inspector_did_execute_console_script(u64 page_id, String const& script) override; virtual Messages::WebContentClient::RequestWorkerAgentResponse request_worker_agent(u64 page_id) override; diff --git a/Userland/Services/WebContent/PageClient.cpp b/Userland/Services/WebContent/PageClient.cpp index 52f0f882b6..faf1f24b1c 100644 --- a/Userland/Services/WebContent/PageClient.cpp +++ b/Userland/Services/WebContent/PageClient.cpp @@ -621,18 +621,14 @@ void PageClient::inspector_did_add_dom_node_attributes(i32 node_id, JS::NonnullG client().async_inspector_did_add_dom_node_attributes(m_id, node_id, named_node_map_to_vector(attributes)); } -void PageClient::inspector_did_replace_dom_node_attribute(i32 node_id, String const& name, JS::NonnullGCPtr replacement_attributes) +void PageClient::inspector_did_replace_dom_node_attribute(i32 node_id, size_t attribute_index, JS::NonnullGCPtr replacement_attributes) { - client().async_inspector_did_replace_dom_node_attribute(m_id, node_id, name, named_node_map_to_vector(replacement_attributes)); + client().async_inspector_did_replace_dom_node_attribute(m_id, node_id, attribute_index, named_node_map_to_vector(replacement_attributes)); } -void PageClient::inspector_did_request_dom_tree_context_menu(i32 node_id, Web::CSSPixelPoint position, String const& type, Optional const& tag, Optional const& attribute_name, Optional const& attribute_value) +void PageClient::inspector_did_request_dom_tree_context_menu(i32 node_id, Web::CSSPixelPoint position, String const& type, Optional const& tag, Optional const& attribute_index) { - Optional attribute; - if (attribute_name.has_value() && attribute_value.has_value()) - attribute = WebView::Attribute { *attribute_name, *attribute_value }; - - client().async_inspector_did_request_dom_tree_context_menu(m_id, node_id, page().css_to_device_point(position).to_type(), type, tag, move(attribute)); + client().async_inspector_did_request_dom_tree_context_menu(m_id, node_id, page().css_to_device_point(position).to_type(), type, tag, attribute_index); } void PageClient::inspector_did_execute_console_script(String const& script) diff --git a/Userland/Services/WebContent/PageClient.h b/Userland/Services/WebContent/PageClient.h index 11af94568f..a627faefac 100644 --- a/Userland/Services/WebContent/PageClient.h +++ b/Userland/Services/WebContent/PageClient.h @@ -144,8 +144,8 @@ private: virtual void inspector_did_set_dom_node_text(i32 node_id, String const& text) override; virtual void inspector_did_set_dom_node_tag(i32 node_id, String const& tag) override; virtual void inspector_did_add_dom_node_attributes(i32 node_id, JS::NonnullGCPtr attributes) override; - virtual void inspector_did_replace_dom_node_attribute(i32 node_id, String const& name, JS::NonnullGCPtr replacement_attributes) override; - virtual void inspector_did_request_dom_tree_context_menu(i32 node_id, Web::CSSPixelPoint position, String const& type, Optional const& tag, Optional const& attribute_name, Optional const& attribute_value) override; + virtual void inspector_did_replace_dom_node_attribute(i32 node_id, size_t attribute_index, JS::NonnullGCPtr replacement_attributes) override; + virtual void inspector_did_request_dom_tree_context_menu(i32 node_id, Web::CSSPixelPoint position, String const& type, Optional const& tag, Optional const& attribute_index) override; virtual void inspector_did_execute_console_script(String const& script) override; Web::Layout::Viewport* layout_root(); diff --git a/Userland/Services/WebContent/WebContentClient.ipc b/Userland/Services/WebContent/WebContentClient.ipc index 99cedcee79..57b8729a56 100644 --- a/Userland/Services/WebContent/WebContentClient.ipc +++ b/Userland/Services/WebContent/WebContentClient.ipc @@ -87,8 +87,8 @@ endpoint WebContentClient inspector_did_set_dom_node_text(u64 page_id, i32 node_id, String text) =| inspector_did_set_dom_node_tag(u64 page_id, i32 node_id, String tag) =| inspector_did_add_dom_node_attributes(u64 page_id, i32 node_id, Vector attributes) =| - inspector_did_replace_dom_node_attribute(u64 page_id, i32 node_id, String name, Vector replacement_attributes) =| - inspector_did_request_dom_tree_context_menu(u64 page_id, i32 node_id, Gfx::IntPoint position, String type, Optional tag, Optional attribute) =| + inspector_did_replace_dom_node_attribute(u64 page_id, i32 node_id, size_t attribute_index, Vector replacement_attributes) =| + inspector_did_request_dom_tree_context_menu(u64 page_id, i32 node_id, Gfx::IntPoint position, String type, Optional tag, Optional attribute_index) =| inspector_did_execute_console_script(u64 page_id, String script) =| }