From 8162dc5ee6c5e7cf78786aa485c1ad967a26d717 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Wed, 6 Dec 2023 09:34:53 -0500 Subject: [PATCH] LibWeb+LibWebView+WebContent: Separate tag/attribute in Inspector menu It was a bit short-sighted to combine the tag and attribute names into one string when the Inspector requests a context menu. We will want both values for some context menu actions. Send both names, as well as the attribute value, when requesting the context menu. --- Base/res/ladybird/inspector.js | 23 ++++++-- Ladybird/AppKit/UI/Inspector.mm | 7 +-- Ladybird/Qt/InspectorWidget.cpp | 7 +-- .../Applications/Browser/InspectorWidget.cpp | 7 +-- .../Libraries/LibWeb/Internals/Inspector.cpp | 4 +- .../Libraries/LibWeb/Internals/Inspector.h | 2 +- .../Libraries/LibWeb/Internals/Inspector.idl | 2 +- Userland/Libraries/LibWeb/Page/Page.h | 2 +- .../Libraries/LibWebView/InspectorClient.cpp | 54 +++++++++---------- .../Libraries/LibWebView/InspectorClient.h | 10 ++-- .../Libraries/LibWebView/ViewImplementation.h | 2 +- .../Libraries/LibWebView/WebContentClient.cpp | 4 +- .../Libraries/LibWebView/WebContentClient.h | 2 +- Userland/Services/WebContent/PageClient.cpp | 8 ++- Userland/Services/WebContent/PageClient.h | 2 +- .../Services/WebContent/WebContentClient.ipc | 2 +- 16 files changed, 80 insertions(+), 58 deletions(-) diff --git a/Base/res/ladybird/inspector.js b/Base/res/ladybird/inspector.js index c7a2a58b43..9124056002 100644 --- a/Base/res/ladybird/inspector.js +++ b/Base/res/ladybird/inspector.js @@ -332,16 +332,29 @@ const requestContextMenu = (clientX, clientY, domNode) => { return; } - let tagOrAttributeName = null; - pendingEditDOMNode = domNode; + let tag = null; + let attributeName = null; + let attributeValue = null; if (type === "tag") { - tagOrAttributeName = domNode.innerText; + tag = domNode.dataset.tag; } else if (type === "attribute") { - tagOrAttributeName = domNode.dataset.attributeName; + tag = domNode.dataset.tag; + attributeName = domNode.dataset.attributeName; + attributeValue = domNode.dataset.attributeValue; } - inspector.requestDOMTreeContextMenu(domNodeID, clientX, clientY, type, tagOrAttributeName); + pendingEditDOMNode = domNode; + + inspector.requestDOMTreeContextMenu( + domNodeID, + clientX, + clientY, + type, + tag, + attributeName, + attributeValue + ); }; const executeConsoleScript = consoleInput => { diff --git a/Ladybird/AppKit/UI/Inspector.mm b/Ladybird/AppKit/UI/Inspector.mm index 3e633c2c13..cabaad4dd7 100644 --- a/Ladybird/AppKit/UI/Inspector.mm +++ b/Ladybird/AppKit/UI/Inspector.mm @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include @@ -91,14 +92,14 @@ static constexpr NSInteger CONTEXT_MENU_REMOVE_ATTRIBUTE_TAG = 2; [NSMenu popUpContextMenu:strong_self.dom_node_tag_context_menu withEvent:event forView:strong_self.web_view]; }; - m_inspector_client->on_requested_dom_node_attribute_context_menu = [weak_self](auto position, auto const& attribute) { + m_inspector_client->on_requested_dom_node_attribute_context_menu = [weak_self](auto position, auto const&, auto const& attribute) { Inspector* strong_self = weak_self; if (strong_self == nil) { return; } - auto edit_attribute_text = MUST(String::formatted("Edit attribute \"{}\"", attribute)); - auto remove_attribute_text = MUST(String::formatted("Remove attribute \"{}\"", attribute)); + auto edit_attribute_text = MUST(String::formatted("Edit attribute \"{}\"", attribute.name)); + auto remove_attribute_text = MUST(String::formatted("Remove attribute \"{}\"", attribute.name)); auto* edit_node_menu_item = [strong_self.dom_node_attribute_context_menu itemWithTag:CONTEXT_MENU_EDIT_NODE_TAG]; [edit_node_menu_item setTitle:Ladybird::string_to_ns_string(edit_attribute_text)]; diff --git a/Ladybird/Qt/InspectorWidget.cpp b/Ladybird/Qt/InspectorWidget.cpp index 0735b90c07..aae1cf2336 100644 --- a/Ladybird/Qt/InspectorWidget.cpp +++ b/Ladybird/Qt/InspectorWidget.cpp @@ -6,6 +6,7 @@ #include "InspectorWidget.h" #include +#include #include #include #include @@ -68,9 +69,9 @@ InspectorWidget::InspectorWidget(QWidget* tab, WebContentView& content_view) m_dom_node_tag_context_menu->exec(to_widget_position(position)); }; - m_inspector_client->on_requested_dom_node_attribute_context_menu = [this](auto position, auto const& attribute) { - m_edit_node_action->setText(qstring_from_ak_string(MUST(String::formatted("&Edit attribute \"{}\"", attribute)))); - m_remove_attribute_action->setText(qstring_from_ak_string(MUST(String::formatted("&Remove attribute \"{}\"", attribute)))); + m_inspector_client->on_requested_dom_node_attribute_context_menu = [this](auto position, auto const&, auto const& attribute) { + m_edit_node_action->setText(qstring_from_ak_string(MUST(String::formatted("&Edit attribute \"{}\"", attribute.name)))); + m_remove_attribute_action->setText(qstring_from_ak_string(MUST(String::formatted("&Remove attribute \"{}\"", attribute.name)))); m_dom_node_attribute_context_menu->exec(to_widget_position(position)); }; diff --git a/Userland/Applications/Browser/InspectorWidget.cpp b/Userland/Applications/Browser/InspectorWidget.cpp index 0ef08ac618..122db4b1e1 100644 --- a/Userland/Applications/Browser/InspectorWidget.cpp +++ b/Userland/Applications/Browser/InspectorWidget.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -63,9 +64,9 @@ InspectorWidget::InspectorWidget(WebView::OutOfProcessWebView& content_view) m_dom_node_tag_context_menu->popup(to_widget_position(position)); }; - m_inspector_client->on_requested_dom_node_attribute_context_menu = [this](auto position, auto const& attribute) { - m_edit_node_action->set_text(DeprecatedString::formatted("&Edit attribute \"{}\"", attribute)); - m_remove_attribute_action->set_text(DeprecatedString::formatted("&Remove attribute \"{}\"", attribute)); + m_inspector_client->on_requested_dom_node_attribute_context_menu = [this](auto position, auto const&, auto const& attribute) { + m_edit_node_action->set_text(DeprecatedString::formatted("&Edit attribute \"{}\"", attribute.name)); + m_remove_attribute_action->set_text(DeprecatedString::formatted("&Remove attribute \"{}\"", attribute.name)); m_dom_node_attribute_context_menu->popup(to_widget_position(position)); }; diff --git a/Userland/Libraries/LibWeb/Internals/Inspector.cpp b/Userland/Libraries/LibWeb/Internals/Inspector.cpp index c21898df3f..e62c3d277f 100644 --- a/Userland/Libraries/LibWeb/Internals/Inspector.cpp +++ b/Userland/Libraries/LibWeb/Internals/Inspector.cpp @@ -66,9 +66,9 @@ void Inspector::replace_dom_node_attribute(i32 node_id, String const& name, JS:: global_object().browsing_context()->page().client().inspector_did_replace_dom_node_attribute(node_id, name, replacement_attributes); } -void Inspector::request_dom_tree_context_menu(i32 node_id, i32 client_x, i32 client_y, String const& type, Optional const& tag_or_attribute_name) +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) { - global_object().browsing_context()->page().client().inspector_did_request_dom_tree_context_menu(node_id, { client_x, client_y }, type, tag_or_attribute_name); + 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); } 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 a7e7ab42fd..6f6e967685 100644 --- a/Userland/Libraries/LibWeb/Internals/Inspector.h +++ b/Userland/Libraries/LibWeb/Internals/Inspector.h @@ -26,7 +26,7 @@ public: 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 request_dom_tree_context_menu(i32 node_id, i32 client_x, i32 client_y, String const& type, Optional const& tag_or_attribute_name); + 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 execute_console_script(String const& script); diff --git a/Userland/Libraries/LibWeb/Internals/Inspector.idl b/Userland/Libraries/LibWeb/Internals/Inspector.idl index 37e12273da..a72e8a18cd 100644 --- a/Userland/Libraries/LibWeb/Internals/Inspector.idl +++ b/Userland/Libraries/LibWeb/Internals/Inspector.idl @@ -10,7 +10,7 @@ undefined addDOMNodeAttributes(long nodeID, NamedNodeMap attributes); undefined replaceDOMNodeAttribute(long nodeID, DOMString name, NamedNodeMap replacementAttributes); - undefined requestDOMTreeContextMenu(long nodeID, long clientX, long clientY, DOMString type, DOMString? tagOrAttributeName); + undefined requestDOMTreeContextMenu(long nodeID, long clientX, long clientY, DOMString type, DOMString? tag, DOMString? attributeName, DOMString? attributeValue); undefined executeConsoleScript(DOMString script); diff --git a/Userland/Libraries/LibWeb/Page/Page.h b/Userland/Libraries/LibWeb/Page/Page.h index 84216739db..45ca4932f5 100644 --- a/Userland/Libraries/LibWeb/Page/Page.h +++ b/Userland/Libraries/LibWeb/Page/Page.h @@ -276,7 +276,7 @@ public: 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_or_attribute_name) { } + 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_execute_console_script([[maybe_unused]] String const& script) { } protected: diff --git a/Userland/Libraries/LibWebView/InspectorClient.cpp b/Userland/Libraries/LibWebView/InspectorClient.cpp index 8a221adf5d..fee639a97c 100644 --- a/Userland/Libraries/LibWebView/InspectorClient.cpp +++ b/Userland/Libraries/LibWebView/InspectorClient.cpp @@ -81,23 +81,23 @@ 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_or_attribute_name) { - m_context_menu_dom_node_id = node_id; - m_context_menu_tag_or_attribute_name = tag_or_attribute_name; + 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_context_menu_data = ContextMenuData { node_id, tag, attribute }; if (type.is_one_of("text"sv, "comment"sv)) { if (on_requested_dom_node_text_context_menu) on_requested_dom_node_text_context_menu(position); } else if (type == "tag"sv) { - VERIFY(m_context_menu_tag_or_attribute_name.has_value()); + VERIFY(tag.has_value()); if (on_requested_dom_node_tag_context_menu) - on_requested_dom_node_tag_context_menu(position, *m_context_menu_tag_or_attribute_name); + on_requested_dom_node_tag_context_menu(position, *tag); } else if (type == "attribute"sv) { - VERIFY(m_context_menu_tag_or_attribute_name.has_value()); + VERIFY(tag.has_value()); + VERIFY(attribute.has_value()); if (on_requested_dom_node_attribute_context_menu) - on_requested_dom_node_attribute_context_menu(position, *m_context_menu_tag_or_attribute_name); + on_requested_dom_node_attribute_context_menu(position, *tag, *attribute); } }; @@ -225,51 +225,47 @@ void InspectorClient::select_node(i32 node_id) void InspectorClient::context_menu_edit_dom_node() { - VERIFY(m_context_menu_dom_node_id.has_value()); + VERIFY(m_context_menu_data.has_value()); - auto script = MUST(String::formatted("inspector.editDOMNodeID({});", *m_context_menu_dom_node_id)); + auto script = MUST(String::formatted("inspector.editDOMNodeID({});", m_context_menu_data->dom_node_id)); m_inspector_web_view.run_javascript(script); - m_context_menu_dom_node_id.clear(); - m_context_menu_tag_or_attribute_name.clear(); + m_context_menu_data.clear(); } void InspectorClient::context_menu_remove_dom_node() { - VERIFY(m_context_menu_dom_node_id.has_value()); + VERIFY(m_context_menu_data.has_value()); - m_content_web_view.remove_dom_node(*m_context_menu_dom_node_id); + m_content_web_view.remove_dom_node(m_context_menu_data->dom_node_id); m_pending_selection = m_body_node_id; inspect(); - m_context_menu_dom_node_id.clear(); - m_context_menu_tag_or_attribute_name.clear(); + m_context_menu_data.clear(); } void InspectorClient::context_menu_add_dom_node_attribute() { - VERIFY(m_context_menu_dom_node_id.has_value()); + VERIFY(m_context_menu_data.has_value()); - auto script = MUST(String::formatted("inspector.addAttributeToDOMNodeID({});", *m_context_menu_dom_node_id)); + auto script = MUST(String::formatted("inspector.addAttributeToDOMNodeID({});", m_context_menu_data->dom_node_id)); m_inspector_web_view.run_javascript(script); - m_context_menu_dom_node_id.clear(); - m_context_menu_tag_or_attribute_name.clear(); + m_context_menu_data.clear(); } void InspectorClient::context_menu_remove_dom_node_attribute() { - VERIFY(m_context_menu_dom_node_id.has_value()); - VERIFY(m_context_menu_tag_or_attribute_name.has_value()); + VERIFY(m_context_menu_data.has_value()); + VERIFY(m_context_menu_data->attribute.has_value()); - m_content_web_view.replace_dom_node_attribute(*m_context_menu_dom_node_id, *m_context_menu_tag_or_attribute_name, {}); + m_content_web_view.replace_dom_node_attribute(m_context_menu_data->dom_node_id, m_context_menu_data->attribute->name, {}); - m_pending_selection = m_context_menu_dom_node_id; + m_pending_selection = m_context_menu_data->dom_node_id; inspect(); - m_context_menu_dom_node_id.clear(); - m_context_menu_tag_or_attribute_name.clear(); + m_context_menu_data.clear(); } void InspectorClient::load_inspector() @@ -465,14 +461,16 @@ 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(); + auto tag = name.to_lowercase(); + builder.appendff("", data_attributes.string_view()); builder.append("<"sv); - builder.appendff("{}", name.to_lowercase()); + builder.appendff("{0}", tag); if (auto attributes = node.get_object("attributes"sv); attributes.has_value()) { - attributes->for_each_member([&builder](auto const& name, auto const& value) { + attributes->for_each_member([&](auto const& name, auto const& value) { builder.append(" "sv); - builder.appendff("", name); + builder.appendff("", tag, name, value); builder.appendff("{}", name); builder.append('='); builder.appendff("\"{}\"", value); diff --git a/Userland/Libraries/LibWebView/InspectorClient.h b/Userland/Libraries/LibWebView/InspectorClient.h index 16fbb6ab27..5e1cce5cf1 100644 --- a/Userland/Libraries/LibWebView/InspectorClient.h +++ b/Userland/Libraries/LibWebView/InspectorClient.h @@ -33,7 +33,7 @@ public: Function on_requested_dom_node_text_context_menu; Function on_requested_dom_node_tag_context_menu; - Function on_requested_dom_node_attribute_context_menu; + Function on_requested_dom_node_attribute_context_menu; private: void load_inspector(); @@ -61,8 +61,12 @@ private: bool m_dom_tree_loaded { false }; - Optional m_context_menu_dom_node_id; - Optional m_context_menu_tag_or_attribute_name; + struct ContextMenuData { + i32 dom_node_id { 0 }; + Optional tag; + Optional attribute; + }; + Optional m_context_menu_data; i32 m_highest_notified_message_index { -1 }; i32 m_highest_received_message_index { -1 }; diff --git a/Userland/Libraries/LibWebView/ViewImplementation.h b/Userland/Libraries/LibWebView/ViewImplementation.h index aa3cd19082..487305506a 100644 --- a/Userland/Libraries/LibWebView/ViewImplementation.h +++ b/Userland/Libraries/LibWebView/ViewImplementation.h @@ -159,7 +159,7 @@ public: 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&)> on_inspector_requested_dom_tree_context_menu; + Function const&, Optional const&)> on_inspector_requested_dom_tree_context_menu; Function on_inspector_executed_console_script; virtual Gfx::IntRect viewport_rect() const = 0; diff --git a/Userland/Libraries/LibWebView/WebContentClient.cpp b/Userland/Libraries/LibWebView/WebContentClient.cpp index c1ada606d5..8cee533bec 100644 --- a/Userland/Libraries/LibWebView/WebContentClient.cpp +++ b/Userland/Libraries/LibWebView/WebContentClient.cpp @@ -438,10 +438,10 @@ void WebContentClient::inspector_did_replace_dom_node_attribute(i32 node_id, Str m_view.on_inspector_replaced_dom_node_attribute(node_id, name, replacement_attributes); } -void WebContentClient::inspector_did_request_dom_tree_context_menu(i32 node_id, Gfx::IntPoint position, String const& type, Optional const& tag_or_attribute_name) +void WebContentClient::inspector_did_request_dom_tree_context_menu(i32 node_id, Gfx::IntPoint position, String const& type, Optional const& tag, Optional const& attribute) { if (m_view.on_inspector_requested_dom_tree_context_menu) - m_view.on_inspector_requested_dom_tree_context_menu(node_id, m_view.to_widget_position(position), type, tag_or_attribute_name); + m_view.on_inspector_requested_dom_tree_context_menu(node_id, m_view.to_widget_position(position), type, tag, attribute); } void WebContentClient::inspector_did_execute_console_script(String const& script) diff --git a/Userland/Libraries/LibWebView/WebContentClient.h b/Userland/Libraries/LibWebView/WebContentClient.h index 5471db0566..ecfb9e503b 100644 --- a/Userland/Libraries/LibWebView/WebContentClient.h +++ b/Userland/Libraries/LibWebView/WebContentClient.h @@ -92,7 +92,7 @@ private: 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, Vector const& attributes) override; virtual void inspector_did_replace_dom_node_attribute(i32 node_id, String const& name, Vector const& replacement_attributes) override; - virtual void inspector_did_request_dom_tree_context_menu(i32 node_id, Gfx::IntPoint position, String const& type, Optional const& tag_or_attribute_name) override; + virtual void inspector_did_request_dom_tree_context_menu(i32 node_id, Gfx::IntPoint position, String const& type, Optional const& tag, Optional const& attribute) override; virtual void inspector_did_execute_console_script(String const& script) override; ViewImplementation& m_view; diff --git a/Userland/Services/WebContent/PageClient.cpp b/Userland/Services/WebContent/PageClient.cpp index 6cd2a1a908..4c53f457fa 100644 --- a/Userland/Services/WebContent/PageClient.cpp +++ b/Userland/Services/WebContent/PageClient.cpp @@ -558,9 +558,13 @@ void PageClient::inspector_did_replace_dom_node_attribute(i32 node_id, String co client().async_inspector_did_replace_dom_node_attribute(node_id, name, 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_or_attribute_name) +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) { - client().async_inspector_did_request_dom_tree_context_menu(node_id, page().css_to_device_point(position).to_type(), type, tag_or_attribute_name); + 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(node_id, page().css_to_device_point(position).to_type(), type, tag, move(attribute)); } 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 3c54f73a2a..88c4489afd 100644 --- a/Userland/Services/WebContent/PageClient.h +++ b/Userland/Services/WebContent/PageClient.h @@ -127,7 +127,7 @@ private: 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_or_attribute_name) 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_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 1c8b1d474b..b4fd9a79e7 100644 --- a/Userland/Services/WebContent/WebContentClient.ipc +++ b/Userland/Services/WebContent/WebContentClient.ipc @@ -77,7 +77,7 @@ endpoint WebContentClient inspector_did_set_dom_node_tag(i32 node_id, String tag) =| inspector_did_add_dom_node_attributes(i32 node_id, Vector attributes) =| inspector_did_replace_dom_node_attribute(i32 node_id, String name, Vector replacement_attributes) =| - inspector_did_request_dom_tree_context_menu(i32 node_id, Gfx::IntPoint position, String type, Optional tag_or_attribute_name) =| + inspector_did_request_dom_tree_context_menu(i32 node_id, Gfx::IntPoint position, String type, Optional tag, Optional attribute) =| inspector_did_execute_console_script(String script) =| }