From 193fc7ef985f4248ac65b9c053f3e073d3c23fb8 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Sun, 21 Apr 2024 18:16:27 +0200
Subject: [PATCH] LibWeb: Allow cloneNode() to clone elements with weird
 attributes

We can't rely on Element.setAttribute() in cloneNode() since that will
throw on weird attribute names. Instead, just follow the spec and copy
attributes into cloned elements verbatim.

This fixes a crash when loading the "issues" tab on GitHub repos.
They are actually sending us unintentionally broken markup, but we
should still support cloning it. :^)
---
 .../Text/expected/DOM/cloneNode-with-goofy-attribute.txt | 2 ++
 .../Text/input/DOM/cloneNode-with-goofy-attribute.html   | 9 +++++++++
 Userland/Libraries/LibWeb/DOM/Element.cpp                | 6 ++++++
 Userland/Libraries/LibWeb/DOM/Element.h                  | 1 +
 Userland/Libraries/LibWeb/DOM/Node.cpp                   | 2 +-
 5 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 Tests/LibWeb/Text/expected/DOM/cloneNode-with-goofy-attribute.txt
 create mode 100644 Tests/LibWeb/Text/input/DOM/cloneNode-with-goofy-attribute.html

diff --git a/Tests/LibWeb/Text/expected/DOM/cloneNode-with-goofy-attribute.txt b/Tests/LibWeb/Text/expected/DOM/cloneNode-with-goofy-attribute.txt
new file mode 100644
index 00000000000..ea3be288195
--- /dev/null
+++ b/Tests/LibWeb/Text/expected/DOM/cloneNode-with-goofy-attribute.txt
@@ -0,0 +1,2 @@
+  a(
+a(
diff --git a/Tests/LibWeb/Text/input/DOM/cloneNode-with-goofy-attribute.html b/Tests/LibWeb/Text/input/DOM/cloneNode-with-goofy-attribute.html
new file mode 100644
index 00000000000..d15be31cd87
--- /dev/null
+++ b/Tests/LibWeb/Text/input/DOM/cloneNode-with-goofy-attribute.html
@@ -0,0 +1,9 @@
+<script src="../include.js"></script>
+<svg><circle id=c a(></svg>
+<script>
+    test(() => {
+        let cloned = c.cloneNode()
+        println(c.attributes[1].localName)
+        println(cloned.attributes[1].localName)
+    });
+</script>
diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp
index f9b57f7782a..a43d60bcaeb 100644
--- a/Userland/Libraries/LibWeb/DOM/Element.cpp
+++ b/Userland/Libraries/LibWeb/DOM/Element.cpp
@@ -246,6 +246,12 @@ WebIDL::ExceptionOr<void> Element::set_attribute_ns(Optional<FlyString> const& n
     return {};
 }
 
+// https://dom.spec.whatwg.org/#concept-element-attributes-append
+void Element::append_attribute(FlyString const& name, String const& value)
+{
+    m_attributes->append_attribute(Attr::create(document(), name, value));
+}
+
 // https://dom.spec.whatwg.org/#concept-element-attributes-append
 void Element::append_attribute(Attr& attribute)
 {
diff --git a/Userland/Libraries/LibWeb/DOM/Element.h b/Userland/Libraries/LibWeb/DOM/Element.h
index 08dc3af09da..bf6016fc1c8 100644
--- a/Userland/Libraries/LibWeb/DOM/Element.h
+++ b/Userland/Libraries/LibWeb/DOM/Element.h
@@ -109,6 +109,7 @@ public:
     WebIDL::ExceptionOr<JS::GCPtr<Attr>> set_attribute_node(Attr&);
     WebIDL::ExceptionOr<JS::GCPtr<Attr>> set_attribute_node_ns(Attr&);
 
+    void append_attribute(FlyString const& name, String const& value);
     void append_attribute(Attr&);
     void remove_attribute(FlyString const& name);
     void remove_attribute_ns(Optional<FlyString> const& namespace_, FlyString const& name);
diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp
index 208de97763a..1667efde238 100644
--- a/Userland/Libraries/LibWeb/DOM/Node.cpp
+++ b/Userland/Libraries/LibWeb/DOM/Node.cpp
@@ -826,7 +826,7 @@ JS::NonnullGCPtr<Node> Node::clone_node(Document* document, bool clone_children)
         element.for_each_attribute([&](auto& name, auto& value) {
             // 1. Let copyAttribute be a clone of attribute.
             // 2. Append copyAttribute to copy.
-            MUST(element_copy->set_attribute(name, value));
+            element_copy->append_attribute(name, value);
         });
         copy = move(element_copy);