Merge pull request #494 from mnaza/fix/clonenode-structural-490

fix(js): clone nodes structurally instead of via div.innerHTML

crates/obscura-js/js/bootstrap.js

@@ -703,6 +703,33 @@ const _styleProxy = (decl) => new Proxy(decl, {
   },
 });
 
+// Clone a single node (no children), used by Node.cloneNode. Elements are built
+// with createElement/createElementNS and their content attributes copied, so no
+// HTML parsing context is involved and every attribute (including style) is
+// preserved. Text/Comment/DocumentFragment map to their factory; anything else
+// yields null.
+function _shallowCloneNode(node) {
+  const nt = node.nodeType;
+  if (nt === 3) return document.createTextNode(node.data != null ? node.data : (node.textContent || ""));
+  if (nt === 8) return document.createComment(node.data != null ? node.data : (node.nodeValue || ""));
+  if (nt === 11) return document.createDocumentFragment();
+  if (nt !== 1) return null;
+  const ns = node.namespaceURI;
+  const el = (ns && ns !== "http://www.w3.org/1999/xhtml")
+    ? document.createElementNS(ns, node.nodeName)
+    : document.createElement(node.localName || node.nodeName.toLowerCase());
+  const names = node.getAttributeNames ? node.getAttributeNames() : [];
+  for (const name of names) {
+    const v = node.getAttribute(name);
+    if (v !== null) el.setAttribute(name, v);
+  }
+  // CSS declarations currently live on the JS wrapper independently of the
+  // DOM attribute. Copy that state as well so styles assigned through
+  // `node.style` survive cloning even before attribute reflection runs.
+  if (node.style && node.style.cssText) el.style.cssText = node.style.cssText;
+  return el;
+}
+
 class Node {
   static ELEMENT_NODE = 1;
   static ATTRIBUTE_NODE = 2;
@@ -893,32 +920,38 @@ class Node {
   contains(o) { return o ? _dom("contains", this._nid, o._nid) === "true" : false; }
   hasChildNodes() { return _dom("has_child_nodes", this._nid) === "true"; }
   cloneNode(deep) {
-    const t = this.nodeType;
+    // Clone structurally via real DOM nodes rather than round-tripping through a
-    if (t === 1) {
+    // throwaway <div>.innerHTML: the fragment parser discards elements that are
-      if (deep) {
+    // not valid children of <div> (<tr>, <td>, <option>, …), so the old path
-        const wrapper = document.createElement('div');
+    // returned null for them and lost JS-set inline styles. Building each node
-        wrapper.innerHTML = _domParse("outer_html", this._nid) || "";
+    // directly with createElement(NS) + attribute copy avoids any parsing
-        const clone = wrapper.firstChild;
+    // context, and an explicit stack keeps a deep subtree from overflowing the
-        return clone;
+    // JS stack (issue #490).
-      }
+    const root = _shallowCloneNode(this);
-      const el = document.createElement(this.nodeName.toLowerCase());
+    if (!deep || !root) return root;
-      const html = _domParse("outer_html", this._nid) || "";
+    const stack = [[this, root]];
-      const attrMatch = html.match(/^<[a-zA-Z][^\s>]*([\s\S]*?)>/);
+    while (stack.length) {
-      if (attrMatch && attrMatch[1]) {
+      const [src, dst] = stack.pop();
-        const attrStr = attrMatch[1].trim();
+      // A <template>'s children hang off its content fragment, not childNodes,
-        const re = /([a-zA-Z_:][a-zA-Z0-9_.:-]*)(?:\s*=\s*(?:"([^"]*)"|'([^']*)'|(\S+)))?/g;
+      // so clone them into the clone's fragment. Gated on the tag name because
-        let m;
+      // .content means something else on other elements (e.g. <meta>).
-        while ((m = re.exec(attrStr)) !== null) {
+      if (src.localName === 'template' && dst.localName === 'template') {
-          const name = m[1];
+        const sc = src.content, dc = dst.content;
-          const val = m[2] !== undefined ? m[2] : m[3] !== undefined ? m[3] : m[4] || "";
+        if (sc && dc && sc.childNodes) {
-          if (name !== this.nodeName.toLowerCase()) el.setAttribute(name, val);
+          const tk = sc.childNodes;
+          for (let i = 0; i < tk.length; i++) {
+            const c = _shallowCloneNode(tk[i]);
+            if (c) { dc.appendChild(c); stack.push([tk[i], c]); }
           }
         }
-      return el;
       }
-    if (t === 3) return document.createTextNode(this.textContent);
+      const kids = src.childNodes;
-    if (t === 8) return document.createComment(this.nodeValue || "");
+      for (let i = 0; i < kids.length; i++) {
-    return null;
+        const c = _shallowCloneNode(kids[i]);
+        if (c) { dst.appendChild(c); stack.push([kids[i], c]); }
+      }
+    }
+    return root;
   }
   compareDocumentPosition(other) {
     if (!other) return 0;

crates/obscura-js/src/runtime.rs

@@ -1475,6 +1475,80 @@ mod tests {
         assert_eq!(color, serde_json::json!("green"));
     }
 
+    #[test]
+    fn clone_node_deep_preserves_context_sensitive_elements() {
+        // A <tr> is not a valid child of <div>, so cloning through a throwaway
+        // <div>.innerHTML dropped it and returned null. A structural clone keeps it.
+        let mut rt = setup_runtime("<html><body></body></html>");
+        let tag = rt
+            .evaluate("(document.createElement('tr').cloneNode(true) || {}).tagName || 'NULL'")
+            .unwrap();
+        assert_eq!(tag, serde_json::json!("TR"));
+        let td = rt
+            .evaluate("(document.createElement('td').cloneNode(true) || {}).tagName || 'NULL'")
+            .unwrap();
+        assert_eq!(td, serde_json::json!("TD"));
+    }
+
+    #[test]
+    fn clone_node_deep_copies_children_and_attributes() {
+        let mut rt = setup_runtime(r#"<html><body><ul id="l"><li class="a">one</li><li class="b">two</li></ul></body></html>"#);
+        let out = rt
+            .evaluate(
+                "(function(){var c=document.getElementById('l').cloneNode(true); return c.children.length + '|' + c.children[0].className + '|' + c.children[1].textContent;})()",
+            )
+            .unwrap();
+        assert_eq!(out, serde_json::json!("2|a|two"));
+    }
+
+    #[test]
+    fn clone_node_deep_preserves_table_rows() {
+        let mut rt = setup_runtime(
+            r#"<html><body><table id="t"><tbody><tr><td>1</td><td>2</td></tr></tbody></table></body></html>"#,
+        );
+        // Navigate the detached clone directly (querySelector does not traverse
+        // detached subtrees). tbody > tr > (td, td).
+        let out = rt
+            .evaluate(
+                "(function(){var tb=document.querySelector('#t tbody').cloneNode(true); var tr=tb.children[0]; return tr.tagName + '|' + tr.children.length + '|' + tr.children[1].textContent;})()",
+            )
+            .unwrap();
+        assert_eq!(out, serde_json::json!("TR|2|2"));
+    }
+
+    #[test]
+    fn clone_node_shallow_copies_attributes_without_children() {
+        let mut rt = setup_runtime(r#"<html><body><div id="d" data-x="7"><span>kid</span></div></body></html>"#);
+        let out = rt
+            .evaluate(
+                "(function(){var c=document.getElementById('d').cloneNode(false); return c.getAttribute('data-x') + '|' + c.childNodes.length;})()",
+            )
+            .unwrap();
+        assert_eq!(out, serde_json::json!("7|0"));
+    }
+
+    #[test]
+    fn clone_node_copies_js_assigned_inline_styles() {
+        let mut rt = setup_runtime("<html><body><div id='d'></div></body></html>");
+        let out = rt
+            .evaluate(
+                "(function(){var d=document.getElementById('d');d.style.color='red';d.style.fontSize='12px';var c=d.cloneNode(false);return c.style.color+'|'+c.style.fontSize+'|'+c.style.cssText;})()",
+            )
+            .unwrap();
+        assert_eq!(out, serde_json::json!("red|12px|color: red; font-size: 12px;"));
+    }
+
+    #[test]
+    fn clone_node_deep_copies_template_content() {
+        let mut rt = setup_runtime("<html><body></body></html>");
+        let out = rt
+            .evaluate(
+                "(function(){var t=document.createElement('template');t.content.appendChild(document.createElement('option')).textContent='choice';var c=t.cloneNode(true);return c.content.childNodes.length+'|'+c.content.firstChild.tagName+'|'+c.content.firstChild.textContent;})()",
+            )
+            .unwrap();
+        assert_eq!(out, serde_json::json!("1|OPTION|choice"));
+    }
+
     #[test]
     fn test_document_title() {
         let mut rt = setup_runtime("<html><head><title>Test</title></head><body></body></html>");