Browser+WebContent+WebDriver: Move Get Element Attribute to WebContent

In doing so, this also implements a FIXME to handle boolean attributes.
srikavin · Nov 10, 2022 · 1bc94e1 · 1bc94e1
1 parent 7b1f3b7
commit 1bc94e1
Show file tree

Hide file tree

Showing 16 changed files with 37 additions and 67 deletions.
diff --git a/Userland/Applications/Browser/BrowserWindow.cpp b/Userland/Applications/Browser/BrowserWindow.cpp
@@ -611,10 +611,6 @@ void BrowserWindow::create_new_tab(URL url, bool activate)
         active_tab().view().scroll_element_into_view(element_id);
     };
 
-    new_tab.webdriver_endpoints().on_get_element_attribute = [this](i32 element_id, String const& name) {
-        return active_tab().view().get_element_attribute(element_id, name);
-    };
-
     new_tab.webdriver_endpoints().on_get_element_property = [this](i32 element_id, String const& name) {
         return active_tab().view().get_element_property(element_id, name);
     };

diff --git a/Userland/Applications/Browser/WebDriverConnection.cpp b/Userland/Applications/Browser/WebDriverConnection.cpp
@@ -146,17 +146,6 @@ void WebDriverConnection::scroll_element_into_view(i32 element_id)
     }
 }
 
-Messages::WebDriverSessionClient::GetElementAttributeResponse WebDriverConnection::get_element_attribute(i32 element_id, String const& name)
-{
-    dbgln_if(WEBDRIVER_DEBUG, "WebDriverConnection: get_element_attribute");
-    if (auto browser_window = m_browser_window.strong_ref()) {
-        auto& tab = browser_window->active_tab();
-        if (tab.webdriver_endpoints().on_get_element_attribute)
-            return { tab.webdriver_endpoints().on_get_element_attribute(element_id, name) };
-    }
-    return { {} };
-}
-
 Messages::WebDriverSessionClient::GetElementPropertyResponse WebDriverConnection::get_element_property(i32 element_id, String const& name)
 {
     dbgln_if(WEBDRIVER_DEBUG, "WebDriverConnection: get_element_property");

diff --git a/Userland/Applications/Browser/WebDriverConnection.h b/Userland/Applications/Browser/WebDriverConnection.h
@@ -49,7 +49,6 @@ class WebDriverConnection final
     virtual void add_cookie(Web::Cookie::ParsedCookie const&) override;
     virtual void update_cookie(Web::Cookie::Cookie const&) override;
     virtual void scroll_element_into_view(i32 element_id) override;
-    virtual Messages::WebDriverSessionClient::GetElementAttributeResponse get_element_attribute(i32 element_id, String const& name) override;
     virtual Messages::WebDriverSessionClient::GetElementPropertyResponse get_element_property(i32 element_id, String const& name) override;
     virtual Messages::WebDriverSessionClient::GetActiveDocumentsTypeResponse get_active_documents_type() override;
     virtual Messages::WebDriverSessionClient::GetComputedValueForElementResponse get_computed_value_for_element(i32 element_id, String const& property_name) override;

diff --git a/Userland/Applications/Browser/WebDriverEndpoints.h b/Userland/Applications/Browser/WebDriverEndpoints.h
@@ -24,7 +24,6 @@ class WebDriverEndpoints {
     ~WebDriverEndpoints() = default;
 
     Function<void(i32 element_id)> on_scroll_element_into_view;
-    Function<Optional<String>(i32 element_id, String const&)> on_get_element_attribute;
     Function<Optional<String>(i32 element_id, String const&)> on_get_element_property;
     Function<String()> on_get_active_documents_type;
     Function<String(i32 element_id, String const&)> on_get_computed_value_for_element;

diff --git a/Userland/Applications/Browser/WebDriverSessionClient.ipc b/Userland/Applications/Browser/WebDriverSessionClient.ipc
@@ -25,7 +25,6 @@ endpoint WebDriverSessionClient {
     add_cookie(Web::Cookie::ParsedCookie cookie) =|
     update_cookie(Web::Cookie::Cookie cookie) =|
     scroll_element_into_view(i32 element_id) => ()
-    get_element_attribute(i32 element_id, String name) => (Optional<String> atttibute)
     get_element_property(i32 element_id, String name) => (Optional<String> property)
     get_active_documents_type() => (String type)
     get_computed_value_for_element(i32 element_id, String property_name) => (String computed_value)

diff --git a/Userland/Libraries/LibWebView/OutOfProcessWebView.cpp b/Userland/Libraries/LibWebView/OutOfProcessWebView.cpp
@@ -559,11 +559,6 @@ void OutOfProcessWebView::scroll_element_into_view(i32 element_id)
     return client().scroll_element_into_view(element_id);
 }
 
-Optional<String> OutOfProcessWebView::get_element_attribute(i32 element_id, String const& name)
-{
-    return client().get_element_attribute(element_id, name);
-}
-
 Optional<String> OutOfProcessWebView::get_element_property(i32 element_id, String const& name)
 {
     return client().get_element_property(element_id, name);

diff --git a/Userland/Libraries/LibWebView/OutOfProcessWebView.h b/Userland/Libraries/LibWebView/OutOfProcessWebView.h
@@ -64,7 +64,6 @@ class OutOfProcessWebView final
     OrderedHashMap<String, String> get_session_storage_entries();
 
     void scroll_element_into_view(i32 element_id);
-    Optional<String> get_element_attribute(i32 element_id, String const& name);
     Optional<String> get_element_property(i32 element_id, String const& name);
     String get_active_documents_type();
     String get_computed_value_for_element(i32 element_id, String const& property_name);

diff --git a/Userland/Services/WebContent/ConnectionFromClient.cpp b/Userland/Services/WebContent/ConnectionFromClient.cpp
@@ -495,15 +495,6 @@ void ConnectionFromClient::scroll_element_into_view(i32 element_id)
     element->scroll_into_view(options);
 }
 
-Messages::WebContentServer::GetElementAttributeResponse ConnectionFromClient::get_element_attribute(i32 element_id, String const& name)
-{
-    auto element = find_element_by_id(element_id);
-    if (!element.has_value())
-        return Optional<String> {};
-
-    return { element->get_attribute(name) };
-}
-
 Messages::WebContentServer::GetElementPropertyResponse ConnectionFromClient::get_element_property(i32 element_id, String const& name)
 {
     auto element = find_element_by_id(element_id);

diff --git a/Userland/Services/WebContent/ConnectionFromClient.h b/Userland/Services/WebContent/ConnectionFromClient.h
@@ -84,7 +84,6 @@ class ConnectionFromClient final
     virtual void js_console_request_messages(i32) override;
 
     virtual void scroll_element_into_view(i32 element_id) override;
-    virtual Messages::WebContentServer::GetElementAttributeResponse get_element_attribute(i32 element_id, String const& name) override;
     virtual Messages::WebContentServer::GetElementPropertyResponse get_element_property(i32 element_id, String const& name) override;
     virtual Messages::WebContentServer::GetActiveDocumentsTypeResponse get_active_documents_type() override;
     virtual Messages::WebContentServer::GetComputedValueForElementResponse get_computed_value_for_element(i32 element_id, String const& property_name) override;

diff --git a/Userland/Services/WebContent/WebContentServer.ipc b/Userland/Services/WebContent/WebContentServer.ipc
@@ -43,7 +43,6 @@ endpoint WebContentServer
     js_console_request_messages(i32 start_index) =|
 
     scroll_element_into_view(i32 element_id) => ()
-    get_element_attribute(i32 element_id, String name) => (Optional<String> attribute)
     get_element_property(i32 element_id, String name) => (Optional<String> property)
     get_active_documents_type() => (String type)
     get_computed_value_for_element(i32 element_id, String property_name) => (String computed_value)

diff --git a/Userland/Services/WebContent/WebDriverClient.ipc b/Userland/Services/WebContent/WebDriverClient.ipc
@@ -14,4 +14,5 @@ endpoint WebDriverClient {
     find_element_from_element(JsonValue payload, String element_id) => (Web::WebDriver::Response response)
     find_elements_from_element(JsonValue payload, String element_id) => (Web::WebDriver::Response response)
     is_element_selected(String element_id) => (Web::WebDriver::Response response)
+    get_element_attribute(String element_id, String name) => (Web::WebDriver::Response response)
 }
diff --git a/Userland/Services/WebContent/WebDriverConnection.cpp b/Userland/Services/WebContent/WebDriverConnection.cpp
@@ -13,6 +13,7 @@
 #include <AK/Vector.h>
 #include <LibWeb/DOM/Document.h>
 #include <LibWeb/DOM/Element.h>
+#include <LibWeb/HTML/AttributeNames.h>
 #include <LibWeb/HTML/BrowsingContext.h>
 #include <LibWeb/HTML/HTMLInputElement.h>
 #include <LibWeb/HTML/HTMLOptionElement.h>
@@ -491,6 +492,38 @@ Messages::WebDriverClient::IsElementSelectedResponse WebDriverConnection::is_ele
     return make_success_response(selected);
 }
 
+// 12.4.2 Get Element Attribute, https://w3c.github.io/webdriver/#dfn-get-element-attribute
+Messages::WebDriverClient::GetElementAttributeResponse WebDriverConnection::get_element_attribute(String const& element_id, String const& name)
+{
+    // 1. If the current browsing context is no longer open, return error with error code no such window.
+    TRY(ensure_open_top_level_browsing_context());
+
+    // FIXME: 2. Handle any user prompts and return its value if it is an error.
+
+    // 3. Let element be the result of trying to get a known connected element with url variable element id.
+    auto* element = TRY(get_known_connected_element(element_id));
+
+    // 4. Let result be the result of the first matching condition:
+    Optional<String> result;
+
+    // -> If name is a boolean attribute
+    if (Web::HTML::is_boolean_attribute(name)) {
+        // "true" (string) if the element has the attribute, otherwise null.
+        if (element->has_attribute(name))
+            result = "true"sv;
+    }
+    // -> Otherwise
+    else {
+        // The result of getting an attribute by name name.
+        result = element->get_attribute(name);
+    }
+
+    // 5. Return success with data result.
+    if (result.has_value())
+        return make_success_response(result.release_value());
+    return make_success_response({});
+}
+
 // https://w3c.github.io/webdriver/#dfn-no-longer-open
 ErrorOr<void, Web::WebDriver::Error> WebDriverConnection::ensure_open_top_level_browsing_context()
 {

diff --git a/Userland/Services/WebContent/WebDriverConnection.h b/Userland/Services/WebContent/WebDriverConnection.h
@@ -44,6 +44,7 @@ class WebDriverConnection final
     virtual Messages::WebDriverClient::FindElementFromElementResponse find_element_from_element(JsonValue const& payload, String const& element_id) override;
     virtual Messages::WebDriverClient::FindElementsFromElementResponse find_elements_from_element(JsonValue const& payload, String const& element_id) override;
     virtual Messages::WebDriverClient::IsElementSelectedResponse is_element_selected(String const& element_id) override;
+    virtual Messages::WebDriverClient::GetElementAttributeResponse get_element_attribute(String const& element_id, String const& name) override;
 
     ErrorOr<void, Web::WebDriver::Error> ensure_open_top_level_browsing_context();
     void restore_the_window();

diff --git a/Userland/Services/WebDriver/Client.cpp b/Userland/Services/WebDriver/Client.cpp
@@ -659,12 +659,11 @@ Web::WebDriver::Response Client::handle_is_element_selected(Vector<StringView> c
 
 // 12.4.2 Get Element Attribute, https://w3c.github.io/webdriver/#dfn-get-element-attribute
 // GET /session/{session id}/element/{element id}/attribute/{name}
-Web::WebDriver::Response Client::handle_get_element_attribute(Vector<StringView> const& parameters, JsonValue const& payload)
+Web::WebDriver::Response Client::handle_get_element_attribute(Vector<StringView> const& parameters, JsonValue const&)
 {
     dbgln_if(WEBDRIVER_DEBUG, "Handling GET /session/<session_id>/element/<element_id>/attribute/<name>");
     auto* session = TRY(find_session_with_id(parameters[0]));
-    auto result = TRY(session->get_element_attribute(payload, parameters[1], parameters[2]));
-    return make_json_value(result);
+    return session->web_content_connection().get_element_attribute(parameters[1], parameters[2]);
 }
 
 // 12.4.3 Get Element Property, https://w3c.github.io/webdriver/#dfn-get-element-property

diff --git a/Userland/Services/WebDriver/Session.cpp b/Userland/Services/WebDriver/Session.cpp
@@ -323,34 +323,6 @@ static ErrorOr<i32, Web::WebDriver::Error> get_known_connected_element(StringVie
     return maybe_element_id.release_value();
 }
 
-// 12.4.2 Get Element Attribute, https://w3c.github.io/webdriver/#dfn-get-element-attribute
-Web::WebDriver::Response Session::get_element_attribute(JsonValue const&, StringView parameter_element_id, StringView name)
-{
-    // 1. If the current browsing context is no longer open, return error with error code no such window.
-    TRY(check_for_open_top_level_browsing_context_or_return_error());
-
-    // FIXME: 2. Handle any user prompts and return its value if it is an error.
-
-    // 3. Let element be the result of trying to get a known connected element with url variable element id.
-    auto element_id = TRY(get_known_connected_element(parameter_element_id));
-
-    // FIXME: The case that the element does not exist is not handled at all and null is returned in that case.
-
-    // 4. Let result be the result of the first matching condition:
-    // -> FIXME: If name is a boolean attribute
-    //    NOTE: LibWeb doesn't know about boolean attributes directly
-    //    "true" (string) if the element has the attribute, otherwise null.
-    // -> Otherwise
-    //    The result of getting an attribute by name name.
-    auto result = m_browser_connection->get_element_attribute(element_id, name);
-
-    if (!result.has_value())
-        return JsonValue(AK::JsonValue::Type::Null);
-
-    // 5. Return success with data result.
-    return JsonValue(result.release_value());
-}
-
 // 12.4.3 Get Element Property, https://w3c.github.io/webdriver/#dfn-get-element-property
 Web::WebDriver::Response Session::get_element_property(JsonValue const&, StringView parameter_element_id, StringView name)
 {

diff --git a/Userland/Services/WebDriver/Session.h b/Userland/Services/WebDriver/Session.h
@@ -58,7 +58,6 @@ class Session {
     Web::WebDriver::Response get_window_handle();
     ErrorOr<void, Variant<Web::WebDriver::Error, Error>> close_window();
     Web::WebDriver::Response get_window_handles() const;
-    Web::WebDriver::Response get_element_attribute(JsonValue const& payload, StringView element_id, StringView name);
     Web::WebDriver::Response get_element_property(JsonValue const& payload, StringView element_id, StringView name);
     Web::WebDriver::Response get_element_css_value(JsonValue const& payload, StringView element_id, StringView property_name);
     Web::WebDriver::Response get_element_text(JsonValue const& payload, StringView element_id);