From 6314ecbf88f5c4342bc70104222ac022fa0d5b81 Mon Sep 17 00:00:00 2001 From: Andrey Mnatsakanov Date: Sun, 26 Jul 2026 12:52:19 +0200 Subject: [PATCH] fix(js): make setAttributeNS/getAttributeNS/removeAttributeNS namespace-aware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three NS methods ignored the namespace: setAttributeNS stored the qualified name against the non-namespaced store and getAttributeNS/removeAttributeNS looked up by local name only, so a namespaced attribute was not addressable by (namespace, localName) — getAttributeNS(xlink, 'href') returned null. Record the namespace on the attribute and add namespace-aware DOM ops that match by (namespace, localName), where the local name is the attribute name after any prefix (so both setAttributeNS-created 'xlink:href' and parser-created 'href' resolve). The non-namespaced getAttribute/serialization paths are untouched, and getAttributeNS(null, name) still reads plain attributes. Fixes #503 --- crates/obscura-dom/src/tree.rs | 53 +++++++++++++++++++++++++++++++ crates/obscura-js/js/bootstrap.js | 6 ++-- crates/obscura-js/src/ops.rs | 30 +++++++++++++++++ crates/obscura-js/src/runtime.rs | 28 ++++++++++++++++ 4 files changed, 114 insertions(+), 3 deletions(-) diff --git a/crates/obscura-dom/src/tree.rs b/crates/obscura-dom/src/tree.rs index 01db8af..afcd545 100644 --- a/crates/obscura-dom/src/tree.rs +++ b/crates/obscura-dom/src/tree.rs @@ -32,6 +32,16 @@ pub struct Attribute { pub value: String, } +/// The local-name portion of an attribute name, i.e. the part after any prefix. +/// `"xlink:href"` -> `"href"`, `"href"` -> `"href"`. Used for namespace-aware +/// attribute lookup, which keys on `(namespace, localName)`. +fn attr_local_suffix(name: &str) -> &str { + match name.rfind(':') { + Some(i) => &name[i + 1..], + None => name, + } +} + #[derive(Clone, Debug)] pub enum NodeData { Document, @@ -126,6 +136,49 @@ impl Node { } } + // Read a namespaced attribute by (namespace, localName). The stored local + // name may be qualified (e.g. "xlink:href" for setAttributeNS-created attrs) + // or a bare local (e.g. "href" for parser-created ones), so compare against + // the part after any prefix. + pub fn get_attribute_ns(&self, ns: &str, local: &str) -> Option<&str> { + self.attrs()?.iter().find_map(|a| { + if a.name.ns.as_ref() == ns && attr_local_suffix(a.name.local.as_ref()) == local { + Some(a.value.as_str()) + } else { + None + } + }) + } + + // Set a namespaced attribute. `qualified` is the qualified name + // (e.g. "xlink:href"); it is stored as the local name with the namespace + // recorded, matching how the non-namespaced path stores names, so + // getAttribute(qualified) and serialization keep working unchanged. + pub fn set_attribute_ns(&mut self, ns: &str, qualified: &str, value: String) { + if let NodeData::Element { attrs, .. } = &mut self.data { + let local = attr_local_suffix(qualified); + if let Some(attr) = attrs + .iter_mut() + .find(|a| a.name.ns.as_ref() == ns && attr_local_suffix(a.name.local.as_ref()) == local) + { + attr.value = value; + } else { + attrs.push(Attribute { + name: QualName::new(None, Namespace::from(ns), LocalName::from(qualified)), + value, + }); + } + } + } + + pub fn remove_attribute_ns(&mut self, ns: &str, local: &str) { + if let NodeData::Element { attrs, .. } = &mut self.data { + attrs.retain(|a| { + !(a.name.ns.as_ref() == ns && attr_local_suffix(a.name.local.as_ref()) == local) + }); + } + } + pub fn text_content_of_text_node(&self) -> Option<&str> { match &self.data { NodeData::Text { contents } => Some(contents), diff --git a/crates/obscura-js/js/bootstrap.js b/crates/obscura-js/js/bootstrap.js index 3ef371a..67b25b6 100644 --- a/crates/obscura-js/js/bootstrap.js +++ b/crates/obscura-js/js/bootstrap.js @@ -1352,9 +1352,9 @@ class Element extends Node { if (popoverPrev !== undefined) this._popoverTypeMaybeChanged(popoverPrev); if (globalThis.__mutationObservers?.length) globalThis.__notifyMutation('attributes', this._nid, [], [], n); } - setAttributeNS(ns, n, v) { _dom("set_attribute", this._nid, String(n) + "\0" + String(v)); } // exact name, no HTML folding + setAttributeNS(ns, n, v) { _dom("set_attribute_ns", this._nid, String(ns == null ? "" : ns) + "\0" + String(n) + "\0" + String(v)); } // n is the qualified name; stored with its namespace removeAttribute(n) { n = _htmlAttrName(this, n); const popoverPrev = (n === "popover") ? this.popover : undefined; _dom("remove_attribute", this._nid, n); if (popoverPrev !== undefined) this._popoverTypeMaybeChanged(popoverPrev); } - removeAttributeNS(ns, n) { _dom("remove_attribute", this._nid, String(n)); } + removeAttributeNS(ns, n) { _dom("remove_attribute_ns", this._nid, String(ns == null ? "" : ns) + "\0" + String(n)); } hasAttribute(n) { return this.getAttribute(n) !== null; } hasAttributes() { return true; } // Simplified getAttributeNames() { return _domParse("attribute_names", this._nid) || []; } @@ -1386,7 +1386,7 @@ class Element extends Node { } return list; } - getAttributeNS(ns, n) { return _domParse("get_attribute", this._nid, String(n)); } + getAttributeNS(ns, n) { return _domParse("get_attribute_ns", this._nid, String(ns == null ? "" : ns) + "\0" + String(n)); } querySelector(s) { return _wrapEl(+_dom("query_selector_scoped", this._nid, s)); } querySelectorAll(s) { const ids = _domParse("query_selector_all_scoped", this._nid, s) || []; diff --git a/crates/obscura-js/src/ops.rs b/crates/obscura-js/src/ops.rs index 64aad05..68cba17 100644 --- a/crates/obscura-js/src/ops.rs +++ b/crates/obscura-js/src/ops.rs @@ -378,6 +378,36 @@ fn op_dom_inner(state: &OpState, cmd: String, arg1: String, arg2: String) -> Str }); "true".into() } + // Namespace-aware attribute ops. arg2 packs the pieces with a NUL: + // get/remove: "\0" + // set: "\0\0" + "get_attribute_ns" => { + let nid = arg1.parse::().unwrap_or(0); + let (ns, local) = arg2.split_once('\0').unwrap_or(("", arg2.as_str())); + let val = dom + .with_node(NodeId::new(nid), |n| n.get_attribute_ns(ns, local).map(|s| s.to_string())) + .flatten(); + serde_json::to_string(&val).unwrap_or("null".into()) + } + "set_attribute_ns" => { + let nid = arg1.parse::().unwrap_or(0); + let mut parts = arg2.splitn(3, '\0'); + let ns = parts.next().unwrap_or(""); + let qualified = parts.next().unwrap_or(""); + let value = parts.next().unwrap_or(""); + if !qualified.is_empty() { + dom.with_node_mut(NodeId::new(nid), |n| { + n.set_attribute_ns(ns, qualified, value.to_string()) + }); + } + "true".into() + } + "remove_attribute_ns" => { + let nid = arg1.parse::().unwrap_or(0); + let (ns, local) = arg2.split_once('\0').unwrap_or(("", arg2.as_str())); + dom.with_node_mut(NodeId::new(nid), |n| n.remove_attribute_ns(ns, local)); + "true".into() + } "set_inner_html" => { let nid = match arg1.parse::() { Ok(n) if n > 0 => n, diff --git a/crates/obscura-js/src/runtime.rs b/crates/obscura-js/src/runtime.rs index fdfeb4d..281550f 100644 --- a/crates/obscura-js/src/runtime.rs +++ b/crates/obscura-js/src/runtime.rs @@ -1317,6 +1317,34 @@ mod tests { rt } + #[test] + fn set_attribute_ns_is_retrievable_by_namespace_and_local_name() { + let mut rt = setup_runtime(""); + let v = rt + .evaluate("(function(){var s=document.createElementNS('http://www.w3.org/2000/svg','svg'); s.setAttributeNS('http://www.w3.org/1999/xlink','xlink:href','#g'); return s.getAttributeNS('http://www.w3.org/1999/xlink','href');})()") + .unwrap(); + assert_eq!(v, serde_json::json!("#g")); + } + + #[test] + fn remove_attribute_ns_removes_by_namespace() { + let mut rt = setup_runtime(""); + let v = rt + .evaluate("(function(){var s=document.createElementNS('http://www.w3.org/2000/svg','svg'); s.setAttributeNS('http://www.w3.org/1999/xlink','xlink:href','#g'); s.removeAttributeNS('http://www.w3.org/1999/xlink','href'); return s.getAttributeNS('http://www.w3.org/1999/xlink','href');})()") + .unwrap(); + assert_eq!(v, serde_json::json!(null)); + } + + #[test] + fn get_attribute_ns_reads_plain_attributes_with_null_namespace() { + // Backward-compat: getAttributeNS(null, name) still reads a plain attr. + let mut rt = setup_runtime(r#"
"#); + let v = rt + .evaluate("document.getElementById('d').getAttributeNS(null,'title')") + .unwrap(); + assert_eq!(v, serde_json::json!("hi")); + } + #[test] fn test_document_title() { let mut rt = setup_runtime("Test");