diff --git a/CHANGELOG.md b/CHANGELOG.md index c757dbf..e922d34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ All notable changes to the `dom_query` crate will be documented in this file. ### Fixed - Fixed `Selection::append_selection` to work with selections with multiple nodes and selections from another tree. - Fixed `Selection::replace_with_selection` to work with selections with multiple nodes and selections from another tree. - +- Fixed `Node::append_child`, `Node::append_children`, `Node::prepend_child`, and `Node::prepend_children`: these methods now internally remove the child/children from their previous parent node before attachment. ## [0.8.0] - 2024-11-03 diff --git a/src/dom_tree.rs b/src/dom_tree.rs index 8280875..d426c88 100644 --- a/src/dom_tree.rs +++ b/src/dom_tree.rs @@ -312,7 +312,19 @@ impl Tree { /// Appends a child node by `new_child_id` to a node by `id`. `new_child_id` must exist in the tree. pub fn append_child_of(&self, id: &NodeId, new_child_id: &NodeId) { let mut nodes = self.nodes.borrow_mut(); - let last_child_id = nodes.get_mut(id.value).and_then(|node| node.last_child); + + let Some(parent) = nodes.get_mut(id.value) else { + // TODO: panic or not? + return; + }; + + let last_child_id = parent.last_child; + + if last_child_id.is_none() { + parent.first_child = Some(*new_child_id); + } + + parent.last_child = Some(*new_child_id); if let Some(id) = last_child_id { if let Some(last_child) = nodes.get_mut(id.value) { @@ -320,13 +332,7 @@ impl Tree { } } - if let Some(parent) = nodes.get_mut(id.value) { - if last_child_id.is_none() { - parent.first_child = Some(*new_child_id); - } - - parent.last_child = Some(*new_child_id); - + { if let Some(child) = nodes.get_mut(new_child_id.value) { child.prev_sibling = last_child_id; child.parent = Some(*id); @@ -337,7 +343,18 @@ impl Tree { /// Prepend a child node by `new_child_id` to a node by `id`. `new_child_id` must exist in the tree. pub fn prepend_child_of(&self, id: &NodeId, new_child_id: &NodeId) { let mut nodes = self.nodes.borrow_mut(); - let first_child_id = nodes.get_mut(id.value).and_then(|node| node.first_child); + + let Some(parent) = nodes.get_mut(id.value) else { + // TODO: panic or not? + return; + }; + let first_child_id = parent.first_child; + + if first_child_id.is_none() { + parent.last_child = Some(*new_child_id); + } + + parent.first_child = Some(*new_child_id); if let Some(id) = first_child_id { if let Some(first_child) = nodes.get_mut(id.value) { @@ -345,13 +362,7 @@ impl Tree { } } - if let Some(parent) = nodes.get_mut(id.value) { - if first_child_id.is_none() { - parent.last_child = Some(*new_child_id); - } - - parent.first_child = Some(*new_child_id); - + { if let Some(child) = nodes.get_mut(new_child_id.value) { child.next_sibling = first_child_id; child.parent = Some(*id); @@ -372,32 +383,30 @@ impl Tree { let prev_sibling_id = node.prev_sibling; let next_sibling_id = node.next_sibling; + if parent_id.is_none() && prev_sibling_id.is_none() && next_sibling_id.is_none() { + return; + } + node.parent = None; node.next_sibling = None; node.prev_sibling = None; - if let Some(parent_id) = parent_id { - if let Some(parent) = nodes.get_mut(parent_id.value) { - if parent.first_child == Some(*id) { - parent.first_child = next_sibling_id; - } + if let Some(parent) = parent_id.and_then(|id| nodes.get_mut(id.value)) { + if parent.first_child == Some(*id) { + parent.first_child = next_sibling_id; + } - if parent.last_child == Some(*id) { - parent.last_child = prev_sibling_id; - } + if parent.last_child == Some(*id) { + parent.last_child = prev_sibling_id; } } - if let Some(prev_sibling_id) = prev_sibling_id { - if let Some(prev_sibling) = nodes.get_mut(prev_sibling_id.value) { - prev_sibling.next_sibling = next_sibling_id; - } + if let Some(prev_sibling) = prev_sibling_id.and_then(|id| nodes.get_mut(id.value)) { + prev_sibling.next_sibling = next_sibling_id; } - if let Some(next_sibling_id) = next_sibling_id { - if let Some(next_sibling) = nodes.get_mut(next_sibling_id.value) { - next_sibling.prev_sibling = prev_sibling_id; - }; + if let Some(next_sibling) = next_sibling_id.and_then(|id| nodes.get_mut(id.value)) { + next_sibling.prev_sibling = prev_sibling_id; } } @@ -421,18 +430,14 @@ impl Tree { new_sibling.next_sibling = Some(*id); }; - if let Some(parent_id) = parent_id { - if let Some(parent) = nodes.get_mut(parent_id.value) { - if parent.first_child == Some(*id) { - parent.first_child = Some(*new_sibling_id); - } - }; + if let Some(parent) = parent_id.and_then(|id| nodes.get_mut(id.value)) { + if parent.first_child == Some(*id) { + parent.first_child = Some(*new_sibling_id); + } } - if let Some(prev_sibling_id) = prev_sibling_id { - if let Some(prev_sibling) = nodes.get_mut(prev_sibling_id.value) { - prev_sibling.next_sibling = Some(*new_sibling_id); - }; + if let Some(prev_sibling) = prev_sibling_id.and_then(|id| nodes.get_mut(id.value)) { + prev_sibling.next_sibling = Some(*new_sibling_id); } } diff --git a/src/node/node_ref.rs b/src/node/node_ref.rs index 45a4120..81b95a0 100644 --- a/src/node/node_ref.rs +++ b/src/node/node_ref.rs @@ -164,7 +164,9 @@ impl<'a> NodeRef<'a> { /// Appends another node by id to the selected node. #[inline] pub fn append_child(&self, id_provider: P) { - self.tree.append_child_of(&self.id, id_provider.node_id()) + let new_child_id = id_provider.node_id(); + self.tree.remove_from_parent(new_child_id); + self.tree.append_child_of(&self.id, new_child_id) } /// Appends another node and it's siblings to the selected node. @@ -173,15 +175,19 @@ impl<'a> NodeRef<'a> { let mut next_node = self.tree.get(id_provider.node_id()); while let Some(ref node) = next_node { - self.tree.append_child_of(&self.id, &node.id); + let node_id = node.id; next_node = node.next_sibling(); + self.tree.remove_from_parent(&node_id); + self.tree.append_child_of(&self.id, &node_id); } } /// Prepend another node by id to the selected node. #[inline] pub fn prepend_child(&self, id_provider: P) { - self.tree.prepend_child_of(&self.id, id_provider.node_id()) + let new_child_id = id_provider.node_id(); + self.tree.remove_from_parent(new_child_id); + self.tree.prepend_child_of(&self.id, new_child_id) } /// Prepend another node and it's siblings to the selected node. @@ -196,6 +202,7 @@ impl<'a> NodeRef<'a> { while let Some(ref node) = next_node { let node_id = node.id; next_node = node.prev_sibling(); + self.tree.remove_from_parent(&node_id); self.tree.prepend_child_of(&self.id, &node_id); } } diff --git a/tests/data.rs b/tests/data.rs index 78c9434..c04017a 100644 --- a/tests/data.rs +++ b/tests/data.rs @@ -61,7 +61,8 @@ pub static REPLACEMENT_CONTENTS: &str = r#"

-

Something

+

Something

+

AboutMe

"#; diff --git a/tests/node-manipulation.rs b/tests/node-manipulation.rs index 6b9120e..95cf518 100644 --- a/tests/node-manipulation.rs +++ b/tests/node-manipulation.rs @@ -31,6 +31,77 @@ fn test_create_element() { assert!(doc.select("#main #inline").exists()); } +#[cfg_attr(not(target_arch = "wasm32"), test)] +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] +fn test_append_existing_element() { + let doc = Document::from(REPLACEMENT_CONTENTS); + let origin_sel = doc.select_single("p#origin"); + let origin_node = origin_sel.nodes().first().unwrap(); + + assert_eq!(doc.select_single("#origin").text(), "Something".into()); + + let span_sel = doc.select_single(" #after-origin span"); + let span_node = span_sel.nodes().first().unwrap(); + + origin_node.append_child(span_node); + + assert_eq!(doc.select_single("#origin").text(), "SomethingAbout".into()); +} + +#[cfg_attr(not(target_arch = "wasm32"), test)] +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] +fn test_append_existing_children() { + let doc = Document::from(REPLACEMENT_CONTENTS); + let origin_sel = doc.select_single("p#origin"); + let origin_node = origin_sel.nodes().first().unwrap(); + + assert_eq!(doc.select_single("#origin").text(), "Something".into()); + + let span_sel = doc.select_single(" #after-origin span"); + let span_node = span_sel.nodes().first().unwrap(); + + // this thing adds a child element and its sibling after existing child nodes. + origin_node.append_children(span_node); + + assert_eq!(doc.select_single("#origin").text(), "SomethingAboutMe".into()); +} + +#[cfg_attr(not(target_arch = "wasm32"), test)] +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] +fn test_prepend_existing_element() { + let doc = Document::from(REPLACEMENT_CONTENTS); + let origin_sel = doc.select_single("p#origin"); + let origin_node = origin_sel.nodes().first().unwrap(); + + assert_eq!(doc.select_single("#origin").text(), "Something".into()); + + let span_sel = doc.select_single(" #after-origin span"); + let span_node = span_sel.nodes().first().unwrap(); + + origin_node.prepend_child(span_node); + + assert_eq!(doc.select_single("#origin").text(), "AboutSomething".into()); +} + + +#[cfg_attr(not(target_arch = "wasm32"), test)] +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] +fn test_prepend_existing_children() { + let doc = Document::from(REPLACEMENT_CONTENTS); + let origin_sel = doc.select_single("p#origin"); + let origin_node = origin_sel.nodes().first().unwrap(); + + assert_eq!(doc.select_single("#origin").text(), "Something".into()); + + let span_sel = doc.select_single(" #after-origin span"); + let span_node = span_sel.nodes().first().unwrap(); + + // this thing adds a child element and its sibling before existing child nodes. + origin_node.prepend_children(span_node); + + assert_eq!(doc.select_single("#origin").text(), "AboutMeSomething".into()); +} + #[cfg_attr(not(target_arch = "wasm32"), test)] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_append_element_html() { diff --git a/tests/selection-manipulation.rs b/tests/selection-manipulation.rs index 29ddbd6..991954d 100644 --- a/tests/selection-manipulation.rs +++ b/tests/selection-manipulation.rs @@ -185,7 +185,6 @@ fn test_replace_with_another_tree_selection() { assert_eq!(doc_dst.select(".ad-content .source").length(), 4) } - #[cfg_attr(not(target_arch = "wasm32"), test)] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_append_tree_selection() {