LibWeb/HTML: Make HTMLImageRequests currentURL a String

This is the same type as what is spec'd. We cannot use a URL record
for this member as the spec in some scenarios will set and compare
the URL string to an invalid URL value, such as the empty string.

With implicit string constructors for the URL class removed
explicitly using URL::Parser::basic_parse makes the code look
quite silly in those places.

Libraries/LibWeb/HTML/HTMLImageElement.cpp

@@ -315,10 +315,7 @@ bool HTMLImageElement::complete() const
 String HTMLImageElement::current_src() const
 {
     // The currentSrc IDL attribute must return the img element's current request's current URL.
-    auto current_url = m_current_request->current_url();
-    if (!current_url.is_valid())
-        return {};
-    return current_url.to_string();
+    return m_current_request->current_url();
 }
 
 // https://html.spec.whatwg.org/multipage/embedded-content.html#dom-img-decode
@@ -522,14 +519,15 @@ ErrorOr<void> HTMLImageElement::update_the_image_data(bool restart_animations, b
         // 1. Parse selected source, relative to the element's node document.
         //    If that is not successful, then abort this inner set of steps.
         //    Otherwise, let urlString be the resulting URL string.
-        auto url_string = document().parse_url(selected_source.value());
-        if (!url_string.has_value())
+        auto maybe_url = document().parse_url(selected_source.value());
+        if (!maybe_url.has_value())
             goto after_step_7;
+        auto url_string = maybe_url->serialize();
 
         // 2. Let key be a tuple consisting of urlString, the img element's crossorigin attribute's mode,
         //    and, if that mode is not No CORS, the node document's origin.
         ListOfAvailableImages::Key key;
-        key.url = url_string.value();
+        key.url = maybe_url.value();
         key.mode = m_cors_setting;
         key.origin = document().origin();
 
@@ -565,7 +563,7 @@ ErrorOr<void> HTMLImageElement::update_the_image_data(bool restart_animations, b
                     restart_the_animation();
 
                 // 2. Set current request's current URL to urlString.
-                m_current_request->set_current_url(realm(), *url_string);
+                m_current_request->set_current_url(realm(), url_string);
 
                 // 3. If maybe omit events is not set or previousURL is not equal to urlString, then fire an event named load at the img element.
                 if (!maybe_omit_events || previous_url != url_string)
@@ -604,7 +602,7 @@ after_step_7:
             // 2. Queue an element task on the DOM manipulation task source given the img element and the following steps:
             queue_an_element_task(HTML::Task::Source::DOMManipulation, [this, maybe_omit_events, previous_url] {
                 // 1. Change the current request's current URL to the empty string.
-                m_current_request->set_current_url(realm(), ""sv);
+                m_current_request->set_current_url(realm(), String {});
 
                 // 2. If all of the following conditions are true:
                 //    - the element has a src attribute or it uses srcset or picture; and
@@ -621,9 +619,9 @@ after_step_7:
         }
 
         // 12. Parse selected source, relative to the element's node document, and let urlString be the resulting URL string.
-        auto url_string = document().parse_url(selected_source.value().url.to_byte_string());
+        auto maybe_url = document().parse_url(selected_source.value().url.to_byte_string());
         // If that is not successful, then:
-        if (!url_string.has_value()) {
+        if (!maybe_url.has_value()) {
             // 1. Abort the image request for the current request and the pending request.
             abort_the_image_request(realm(), m_current_request);
             abort_the_image_request(realm(), m_pending_request);
@@ -647,6 +645,7 @@ after_step_7:
             // 5. Return.
             return;
         }
+        auto url_string = maybe_url->serialize();
 
         // 13. If the pending request is not null and urlString is the same as the pending request's current URL, then return.
         if (m_pending_request && url_string == m_pending_request->current_url())
@@ -674,7 +673,7 @@ after_step_7:
 
         // 16. Set image request to a new image request whose current URL is urlString.
         auto image_request = ImageRequest::create(realm(), document().page());
-        image_request->set_current_url(realm(), *url_string);
+        image_request->set_current_url(realm(), url_string);
 
         // 17. If current request's state is unavailable or broken, then set the current request to image request.
         //     Otherwise, set the pending request to image request.
@@ -691,7 +690,7 @@ after_step_7:
         if (delay_load_event)
             m_load_event_delayer.emplace(document());
 
-        add_callbacks_to_image_request(*image_request, maybe_omit_events, *url_string, previous_url);
+        add_callbacks_to_image_request(*image_request, maybe_omit_events, *maybe_url, previous_url);
 
         // AD-HOC: If the image request is already available or fetching, no need to start another fetch.
         if (image_request->is_available() || image_request->is_fetching())
@@ -699,7 +698,7 @@ after_step_7:
 
         // 18. Let request be the result of creating a potential-CORS request given urlString, "image",
         //     and the current state of the element's crossorigin content attribute.
-        auto request = create_potential_CORS_request(vm(), *url_string, Fetch::Infrastructure::Request::Destination::Image, m_cors_setting);
+        auto request = create_potential_CORS_request(vm(), *maybe_url, Fetch::Infrastructure::Request::Destination::Image, m_cors_setting);
 
         // 19. Set request's client to the element's node document's relevant settings object.
         request->set_client(&document().relevant_settings_object());
@@ -733,7 +732,7 @@ after_step_7:
     return {};
 }
 
-void HTMLImageElement::add_callbacks_to_image_request(GC::Ref<ImageRequest> image_request, bool maybe_omit_events, URL::URL const& url_string, URL::URL const& previous_url)
+void HTMLImageElement::add_callbacks_to_image_request(GC::Ref<ImageRequest> image_request, bool maybe_omit_events, URL::URL const& url_string, String const& previous_url)
 {
     image_request->add_callbacks(
         [this, image_request, maybe_omit_events, url_string, previous_url]() {
@@ -766,7 +765,7 @@ void HTMLImageElement::add_callbacks_to_image_request(GC::Ref<ImageRequest> imag
                 document().set_needs_layout();
 
                 // 4. If maybe omit events is not set or previousURL is not equal to urlString, then fire an event named load at the img element.
-                if (!maybe_omit_events || previous_url != url_string)
+                if (!maybe_omit_events || previous_url != url_string.serialize())
                     dispatch_event(DOM::Event::create(realm(), HTML::EventNames::load));
 
                 if (image_data->is_animated() && image_data->frame_count() > 1) {
@@ -795,7 +794,7 @@ void HTMLImageElement::add_callbacks_to_image_request(GC::Ref<ImageRequest> imag
             // and then, if maybe omit events is not set or previousURL is not equal to urlString,
             // queue an element task on the DOM manipulation task source given the img element
             // to fire an event named error at the img element.
-            if (!maybe_omit_events || previous_url != url_string)
+            if (!maybe_omit_events || previous_url != url_string.serialize())
                 dispatch_event(DOM::Event::create(realm(), HTML::EventNames::error));
 
             m_load_event_delayer.clear();
@@ -848,9 +847,10 @@ void HTMLImageElement::react_to_changes_in_the_environment()
 
     // 6. ⌛ Parse selected source, relative to the element's node document,
     //       and let urlString be the resulting URL string. If that is not successful, then return.
-    auto url_string = document().parse_url(selected_source.value());
-    if (!url_string.has_value())
+    auto maybe_url = document().parse_url(selected_source.value());
+    if (!maybe_url.has_value())
         return;
+    auto url_string = maybe_url->serialize();
 
     // 7. ⌛ Let corsAttributeState be the state of the element's crossorigin content attribute.
     auto cors_attribute_state = m_cors_setting;
@@ -863,14 +863,14 @@ void HTMLImageElement::react_to_changes_in_the_environment()
 
     // 10. ⌛ Let key be a tuple consisting of urlString, corsAttributeState, and, if corsAttributeState is not No CORS, origin.
     ListOfAvailableImages::Key key;
-    key.url = *url_string;
+    key.url = *maybe_url;
     key.mode = m_cors_setting;
     if (cors_attribute_state != CORSSettingAttribute::NoCORS)
         key.origin = document().origin();
 
     // 11. ⌛ Let image request be a new image request whose current URL is urlString
     auto image_request = ImageRequest::create(realm(), document().page());
-    image_request->set_current_url(realm(), *url_string);
+    image_request->set_current_url(realm(), url_string);
 
     // 12. ⌛ Let the element's pending request be image request.
     m_pending_request = image_request;
@@ -918,7 +918,7 @@ void HTMLImageElement::react_to_changes_in_the_environment()
     // Otherwise:
     else {
         // 1. Let request be the result of creating a potential-CORS request given urlString, "image", and corsAttributeState.
-        auto request = create_potential_CORS_request(vm(), *url_string, Fetch::Infrastructure::Request::Destination::Image, m_cors_setting);
+        auto request = create_potential_CORS_request(vm(), *maybe_url, Fetch::Infrastructure::Request::Destination::Image, m_cors_setting);
 
         // 2. Set request's client to client, initiator to "imageset", and set request's synchronous flag.
         request->set_client(&client);

Libraries/LibWeb/HTML/HTMLImageElement.h

@@ -134,7 +134,7 @@ private:
 
     void handle_successful_fetch(URL::URL const&, StringView mime_type, ImageRequest&, ByteBuffer, bool maybe_omit_events, URL::URL const& previous_url);
     void handle_failed_fetch();
-    void add_callbacks_to_image_request(GC::Ref<ImageRequest>, bool maybe_omit_events, URL::URL const& url_string, URL::URL const& previous_url);
+    void add_callbacks_to_image_request(GC::Ref<ImageRequest>, bool maybe_omit_events, URL::URL const& url_string, String const& previous_url);
 
     void animate();
 

Libraries/LibWeb/HTML/ImageRequest.cpp

@@ -6,6 +6,7 @@
 
 #include <AK/HashTable.h>
 #include <LibGfx/Bitmap.h>
+#include <LibURL/Parser.h>
 #include <LibWeb/Fetch/Fetching/Fetching.h>
 #include <LibWeb/Fetch/Infrastructure/FetchAlgorithms.h>
 #include <LibWeb/Fetch/Infrastructure/FetchController.h>
@@ -67,16 +68,11 @@ void ImageRequest::set_state(State state)
     m_state = state;
 }
 
-URL::URL const& ImageRequest::current_url() const
-{
-    return m_current_url;
-}
-
-void ImageRequest::set_current_url(JS::Realm& realm, URL::URL url)
+void ImageRequest::set_current_url(JS::Realm& realm, String url)
 {
     m_current_url = move(url);
-    if (m_current_url.is_valid())
-        m_shared_resource_request = SharedResourceRequest::get_or_create(realm, m_page, m_current_url);
+    if (auto url = URL::Parser::basic_parse(m_current_url); url.has_value())
+        m_shared_resource_request = SharedResourceRequest::get_or_create(realm, m_page, url.release_value());
 }
 
 // https://html.spec.whatwg.org/multipage/images.html#abort-the-image-request

Libraries/LibWeb/HTML/ImageRequest.h

@@ -39,8 +39,8 @@ public:
     State state() const;
     void set_state(State);
 
-    URL::URL const& current_url() const;
-    void set_current_url(JS::Realm&, URL::URL);
+    String const& current_url() const { return m_current_url; }
+    void set_current_url(JS::Realm&, String);
 
     [[nodiscard]] GC::Ptr<DecodedImageData> image_data() const;
     void set_image_data(GC::Ptr<DecodedImageData>);
@@ -72,7 +72,7 @@ private:
 
     // https://html.spec.whatwg.org/multipage/images.html#img-req-url
     // An image request's current URL is initially the empty string.
-    URL::URL m_current_url;
+    String m_current_url;
 
     // https://html.spec.whatwg.org/multipage/images.html#img-req-data
     GC::Ptr<DecodedImageData> m_image_data;