From 33f04522f8742d068e310e6c02eef9f8bfe598e0 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 9 Nov 2026 07:58:49 -0800 Subject: [PATCH] JavaScript: an attribute placed among XML children goes in as text ECMA-357 sends a single attribute given to [[Insert]] through [[Replace]], which makes a text node. A list holding an attribute went into the children as the attribute itself, changed its parent and stayed on its old owner. Each attribute now goes in as a new text node with its value, and it stays on its owner. A list assigned such a value holds the new text nodes, and an attribute does not count against the depth limit. --- crates/js/src/xml.rs | 79 ++++++++++++++++++++++++++------------ crates/js/src/xml_tests.rs | 44 +++++++++++++++++++++ 2 files changed, 98 insertions(+), 25 deletions(-) diff --git a/crates/js/src/xml.rs b/crates/js/src/xml.rs index 5a02ccdc1..c84bd6a6d 100644 --- a/crates/js/src/xml.rs +++ b/crates/js/src/xml.rs @@ -907,7 +907,9 @@ impl Interp { Content::List(l) => { for at in 0..self.list_len(*l)? { let Some(item) = self.list_at(*l, at)? else { break }; - self.check_attach_node(x, item)?; + if self.xml_kind(item)? != XmlKind::Attribute { + self.check_attach_node(x, item)?; + } } Ok(()) } @@ -940,9 +942,6 @@ impl Interp { /// position in `x` to use after the removal. The caller has taken [`Self::place_fuel`]. fn detach(&mut self, x: ObjId, v: ObjId, at: usize) -> JsResult { let Some(parent) = self.xml_node(v)?.parent else { return Ok(at) }; - if self.xml_kind(v)? == XmlKind::Attribute { - return Ok(at); - } let Some(q) = self.xml_node(parent)?.children.iter().position(|&c| c == v) else { return Ok(at) }; self.remove_child(parent, q)?; Ok(if parent == x && q < at { at - 1 } else { at }) @@ -952,10 +951,7 @@ impl Interp { /// the scan and removal that take it out. fn place_fuel(&mut self, len: usize, v: ObjId) -> JsResult<()> { self.xml_text_fuel(len + 1)?; - let node = self.xml_node(v)?; - if node.kind != XmlKind::Attribute - && let Some(parent) = node.parent - { + if let Some(parent) = self.xml_node(v)?.parent { let old = self.children_len(parent)?; self.xml_text_fuel(old)?; self.xml_text_fuel(old)?; @@ -987,15 +983,30 @@ impl Interp { self.place_child(x, at, v) } - /// Takes the fuel and heap to put every item of `l` under `x` and returns the items. The tree - /// does not change here, so a limit leaves it as it was. + /// The node to place as a child for `item`: `item` itself, or a new text node holding the + /// value when it is an attribute. `roots` are held across any collection. + fn child_for_item(&mut self, item: ObjId, roots: &[Value]) -> JsResult { + if self.xml_kind(item)? != XmlKind::Attribute { + return Ok(item); + } + let value = self.xml_node(item)?.value.clone(); + let t = self.rooted(roots, |i| i.new_text_node(value, None))?; + self.xml_keep(t); + Ok(t) + } + + /// Takes the fuel and heap to put every item of `l` under `x` and returns the nodes to place, + /// from [`Self::child_for_item`]. The tree does not change here, so a limit leaves it as it + /// was. fn reserve_list_insert(&mut self, x: ObjId, l: ObjId) -> JsResult> { - let items = self.xml_list(l)?.items.clone(); + let mut items = self.xml_list(l)?.items.clone(); let bytes = XML_REF_BYTES.saturating_mul(items.len()); - self.rooted(&[Value::Object(x), Value::Object(l)], |i| i.xml_charge(bytes))?; + let roots = [Value::Object(x), Value::Object(l)]; + self.rooted(&roots, |i| i.xml_charge(bytes))?; let len = self.children_len(x)?; - for (j, &item) in items.iter().enumerate() { - self.place_fuel(len + j, item)?; + for (j, item) in items.iter_mut().enumerate() { + *item = self.child_for_item(*item, &roots)?; + self.place_fuel(len + j, *item)?; } Ok(items) } @@ -1009,6 +1020,17 @@ impl Interp { Ok(()) } + /// Puts a new text node holding `s` at position `at` of `x`. + fn insert_text(&mut self, x: ObjId, at: usize, s: &Str) -> JsResult<()> { + let t = self.new_text_node(s.clone(), Some(x))?; + self.xml_keep(t); + self.xml_charge(XML_REF_BYTES)?; + let node = self.xml_node_mut(x)?; + let at = at.min(node.children.len()); + node.children.insert(at, t); + Ok(()) + } + /// [[Insert]] (ECMA-357 section 9.1.1.11). pub(crate) fn xml_insert(&mut self, x: ObjId, at: usize, c: &Content) -> JsResult<()> { if self.xml_kind(x)? != XmlKind::Element { @@ -1016,13 +1038,10 @@ impl Interp { } self.check_attach(x, c)?; match c { - Content::Text(s) => { - let t = self.new_text_node(s.clone(), Some(x))?; - self.xml_keep(t); - self.xml_charge(XML_REF_BYTES)?; - let node = self.xml_node_mut(x)?; - let at = at.min(node.children.len()); - node.children.insert(at, t); + Content::Text(s) => self.insert_text(x, at, s)?, + Content::Node(v) if self.xml_kind(*v)? == XmlKind::Attribute => { + let value = self.xml_node(*v)?.value.clone(); + self.insert_text(x, at, &value)?; } Content::Node(v) => { self.insert_node(x, at, *v)?; @@ -1473,11 +1492,21 @@ impl Interp { Content::List(l) => { let c = self.list_shallow_copy(l)?; self.xml_keep(c); - let items = self.xml_list(c)?.items.clone(); + let place = match parent { + Some(parent) => self.index_in_parent(parent, item)?.map(|q| (parent, q)), + None => None, + }; + let mut items = self.xml_list(c)?.items.clone(); + if place.is_some() { + let roots = [Value::Object(x), Value::Object(c)]; + for item in &mut items { + self.use_fuel()?; + *item = self.child_for_item(*item, &roots)?; + } + self.xml_list_mut(c)?.items.clone_from(&items); + } self.xml_charge(XML_REF_BYTES.saturating_mul(items.len()))?; - if let Some(parent) = parent - && let Some(q) = self.index_in_parent(parent, item)? - { + if let Some((parent, q)) = place { self.xml_replace(parent, q, &Content::List(c))?; } self.xml_list_mut(x)?.items.splice(i..=i, items); diff --git a/crates/js/src/xml_tests.rs b/crates/js/src/xml_tests.rs index 58c984ad4..40d66fd64 100644 --- a/crates/js/src/xml_tests.rs +++ b/crates/js/src/xml_tests.rs @@ -911,6 +911,50 @@ fn matches_are_replaced_in_document_order() { truth("var x = ; x.b = 'v'; x.b == 'v' && x.*.length() === 2 && x.b[0].parent() === x"); } +#[test] +fn an_attribute_put_among_children_goes_in_as_text_and_stays_on_its_owner() { + let setup = "var y = ; var x = ;"; + let check = "x.b.length() === 0 && x.children().length() === 1 && x.children()[0].nodeKind() === 'text' && x.toString() === 'v' \ + && y.@p.length() === 1 && y.@p.parent() === y && y.@p[0].parent() === y"; + for edit in ["x.replace(0, y.@p)", "x.replace(0, y.@p[0])", "x.b = y.@p", "x.setChildren(y.@p)", "x.*[0] = y.@p"] { + truth(&format!("{setup} {edit}; {check}")); + } + let owner = "y.@p.length() === 1 && y.@p.parent() === y && y.@p[0].parent() === y"; + for (edit, k) in [ + ("x.prependChild(y.@p)", "0"), + ("x.prependChild(y.@p[0])", "0"), + ("x.appendChild(y.@p)", "1"), + ("x.appendChild(y.@p[0])", "1"), + ("x.insertChildAfter(x.b[0], y.@p)", "1"), + ("x.insertChildBefore(x.b[0], y.@p[0])", "0"), + ] { + truth(&format!( + "{setup} {edit}; x.children().length() === 2 && x.children()[{k}].nodeKind() === 'text' && x.children()[{k}] == 'v' && {owner}" + )); + } +} + +#[test] +fn a_list_assigned_attributes_holds_the_text_nodes_in_the_tree() { + truth( + "var y = ; var x = ; var l = x.*; l[0] = y.@*; \ + l.length() === 2 && l[0].nodeKind() === 'text' && l[0].parent() === x && l[1].nodeKind() === 'text' && l[1].parent() === x \ + && x.toString() === 'vw' && y.@*.length() === 2", + ); +} + +#[test] +fn an_attribute_list_goes_in_as_text_at_the_depth_limit() { + let open = "".repeat(7); + let close = "".repeat(7); + let src = format!( + "var x = new XML('{open}{close}'); var c = x; while (c.a.length()) c = c.a[0]; \ + var y = ; c.prependChild(y.@p); c.toString() === 'v'" + ); + let mut realm = Realm::with_options(Options { max_depth: 8, ..e4x() }, Limits::default()); + assert_eq!(realm.run(&src), Ok(JsValue::Bool(true))); +} + #[test] fn a_limit_during_normalize_does_not_repeat_text() { let setup = "var x = ; x.appendChild('p'); x.appendChild('q'); x.appendChild('r');";