Open mickdekkers opened 1 month ago
Thanks for the bug report. I'll look at this ASAP.
Thanks! In the meantime I'm using this workaround to avoid calling .pop()
on any attribute nodes:
/// Workaround for faulty attribute node .pop() method.
///
/// Takes an attribute node and returns a new parent element with all children and attributes except
/// the given attribute node. The other attributes are copied, but the children are moved.
fn attribute_pop_workaround(attr_node: RNode) -> Result<RNode, xrust::Error> {
assert_eq!(attr_node.node_type(), NodeType::Attribute);
let mut parent = attr_node.parent().unwrap();
let mut new_parent = parent.owner_document().new_element(parent.name())?;
let attrs: Vec<_> = parent
.attribute_iter()
// We want to keep all attributes except attr_node
.filter(|node| !node.is_same(&attr_node))
.map(|node| {
// .add_attribute() also calls .pop() on the attribute node, which would panic, so we
// need a copy of the attribute node that isn't attached to any parent. Using
// .deep_copy() would just clone the attribute Rc<Node>, so instead we create a new
// attribute node with the same name and value ourselves.
let new_attr = new_parent
.owner_document()
.new_attribute(node.name(), node.value())?;
Ok(new_attr)
})
.collect::<Result<_, xrust::Error>>()?;
for attr in attrs.into_iter() {
new_parent.add_attribute(attr)?;
}
let children: Vec<_> = parent.child_iter().collect();
for child in children.into_iter() {
new_parent.push(child)?;
}
// Insert the new parent before the old one
parent.insert_before(new_parent.clone())?;
// Remove the old parent
parent.pop()?;
// The new parent is now in the same position as the old one
Ok(new_parent)
}
Used like:
// [...]
let elem = root.first_child().unwrap();
let foo_attr = elem.attribute_iter().next().unwrap();
let new_elem = attribute_pop_workaround(foo_attr).unwrap();
Hey, I noticed that the
.pop()
method appears to be broken for Attribute nodes. It causes aBorrowMutError
panic when called.Example code:
Panics when run with
cargo run
:Can run with
core/debug_refcell
feature (adjust target triple to match your host):To display additional info about the panic:
The line numbers are a bit shifted for me, but it refers to these two lines:
https://github.com/ballsteve/xrust/blob/2449e4dfe85d613d8d096fed21f7c1e2f768e0f9/src/trees/smite.rs#L424 https://github.com/ballsteve/xrust/blob/2449e4dfe85d613d8d096fed21f7c1e2f768e0f9/src/trees/smite.rs#L847
Tested with latest
dev
branch commit (https://github.com/ballsteve/xrust/commit/2449e4dfe85d613d8d096fed21f7c1e2f768e0f9)