LibWeb: Wait for initial navigation to complete before modifying iframe

If initial src of an iframe is "about:blank", it does synchronous navigation that is not supposed to be interleaved by other navigation or usage of Document.open(). Fixes crashing in navigation on https://twinings.co.uk/
kalenikaliaksandr · Apr 8, 2024 · a2dd749 · a2dd749
1 parent de6507e
commit a2dd749
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 0 deletions.
diff --git a/Tests/LibWeb/Text/expected/navigation/populate-iframe-using-document-write.txt b/Tests/LibWeb/Text/expected/navigation/populate-iframe-using-document-write.txt
@@ -0,0 +1 @@
+  PASS (didn't crash)
diff --git a/Tests/LibWeb/Text/input/navigation/populate-iframe-using-document-write.html b/Tests/LibWeb/Text/input/navigation/populate-iframe-using-document-write.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<script src="../include.js"></script>
+<body></body>
+<script>
+    const iframe = document.createElement("iframe");
+    document.body.appendChild(iframe);
+
+    const iframeDocument = iframe.contentDocument;
+
+    iframeDocument.open();
+    iframeDocument.write(
+        "<!DOCTYPE html><html><head><title>Iframe Content</title></head><body>"
+    );
+    iframeDocument.write("<h1>Hello</h1>");
+    iframeDocument.write("<p>from iframe</p>");
+    iframeDocument.write("</body></html>");
+    iframeDocument.close();
+
+    test(() => {
+        println("PASS (didn't crash)");
+    })
+</script>
diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp
@@ -567,6 +567,15 @@ WebIDL::ExceptionOr<void> Document::run_the_document_write_steps(StringView inpu
 // https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#dom-document-open
 WebIDL::ExceptionOr<Document*> Document::open(Optional<String> const&, Optional<String> const&)
 {
+    // If document belongs to a child navigable, we need to make sure its initial navigation is done,
+    // because subsequent steps will modify "initial about:blank" to false, which would cause
+    // initial navigation to fail in case it was "about:blank".
+    if (navigable() && navigable()->container()) {
+        HTML::main_thread_event_loop().spin_until([&] {
+            return navigable()->container()->content_navigable_initialized();
+        });
+    }
+
     // 1. If document is an XML document, then throw an "InvalidStateError" DOMException exception.
     if (m_type == Type::XML)
         return WebIDL::InvalidStateError::create(realm(), "open() called on XML document."_fly_string);

diff --git a/Userland/Libraries/LibWeb/HTML/HTMLIFrameElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLIFrameElement.cpp
@@ -76,6 +76,12 @@ void HTMLIFrameElement::process_the_iframe_attributes(bool initial_insertion)
     if (!content_navigable())
         return;
 
+    // Make sure applying of history step caused by potential sync navigation to "about:blank"
+    // is finished. Otherwise, it might interrupt navigation caused by changing src or srcdoc.
+    if (!initial_insertion) {
+        main_thread_event_loop().spin_until([this] { return content_navigable_initialized(); });
+    }
+
     // 1. If element's srcdoc attribute is specified, then:
     if (has_attribute(HTML::AttributeNames::srcdoc)) {
         // 1. Set element's current navigation was lazy loaded boolean to false.

diff --git a/Userland/Libraries/LibWeb/HTML/NavigableContainer.h b/Userland/Libraries/LibWeb/HTML/NavigableContainer.h
@@ -51,6 +51,8 @@ class NavigableContainer : public HTMLElement {
     // https://html.spec.whatwg.org/multipage/iframe-embed-object.html#potentially-delays-the-load-event
     bool currently_delays_the_load_event() const;
 
+    bool content_navigable_initialized() const { return m_content_navigable_initialized; }
+
 protected:
     NavigableContainer(DOM::Document&, DOM::QualifiedName);