Add tests and fix some edge cases

Namely, fixing the cursor position after insert, and adding more parsing post processing to keep the dom in an expected state with wrap_inline_nodes_into_paragraphs_if_needed and join_nodes_in_container.
Also fixing some tests.

Author: langleyd

crates/wysiwyg/src/composer_model/example_format.rs

@@ -903,7 +903,7 @@ mod test {
     #[test]
     fn selection_across_lists_roundtrips() {
         assert_that!(
-            "<ol><li>1{1</li><li>22</li></ol><ol><li>33</li><li>4}|4</li></ol>"
+            "<ol><li>1{1</li><li>22</li></ol><p>a</p><ol><li>33</li><li>4}|4</li></ol>"
         )
         .roundtrips();
     }
@@ -915,6 +915,7 @@ mod test {
                 <li>1{1</li>\
                 <li>22</li>\
             </ol>\
+            <p>a</p>\
             <ol>\
                 <li>33</li>\
                 <li>4}|4</li>\

crates/wysiwyg/src/composer_model/replace_html.rs

@@ -9,8 +9,7 @@ use regex::Regex;
 use crate::dom::html_source::HtmlSource;
 use crate::dom::nodes::ContainerNode;
 use crate::dom::parser::parse_from_source;
-
-use crate::{ComposerModel, ComposerUpdate, DomNode, Location, UnicodeString};
+use crate::{ComposerModel, ComposerUpdate, DomNode, Location, UnicodeString}; // Import the trait for to_tree
 
 impl<S> ComposerModel<S>
 where
@@ -49,16 +48,25 @@ where
 
         // We should only have 1 dom node, so add the children under a paragraph to take advantage of the exisitng
         // insert_node_at_cursor api and then delete the paragraph node promoting it's the children up a level.
-        let p = DomNode::Container(ContainerNode::new_paragraph(
-            doc_node.into_container().unwrap().take_children(),
-        ));
-        let new_cursor_index = start + p.text_len();
+        let new_children = doc_node.into_container().unwrap().take_children();
+        let child_count = new_children.len();
+        let p = DomNode::Container(ContainerNode::new_paragraph(new_children));
+
         let handle = self.state.dom.insert_node_at_cursor(&range, p);
         self.state.dom.replace_node_with_its_children(&handle);
+        self.state.dom.wrap_inline_nodes_into_paragraphs_if_needed(
+            &self.state.dom.parent(&handle).handle(),
+        );
 
-        // manually move the cursor to the end of the html
-        self.state.start = Location::from(new_cursor_index);
+        // Track the index of the last inserted node for placing the cursor
+        let last_index = handle.index_in_parent() + child_count - 1;
+        let last_handle = handle.parent_handle().child_handle(last_index);
+        let location = self.state.dom.location_for_node(&last_handle);
+
+        self.state.start =
+            Location::from(location.position + location.length - 1);
         self.state.end = self.state.start;
+        // add a trailing space in cases when we do not have a next sibling
         self.create_update_replace_all()
     }
 }
@@ -121,4 +129,68 @@ mod test {
         let html_str = html.to_string();
         assert_eq!(html_str, "<p><strong>test</strong></p>");
     }
+
+    #[test]
+    fn test_replace_html_with_existing_selection() {
+        let mut model = cm("Hello{world}|test");
+        let new_html = "<p><em>replacement</em></p>";
+
+        let _ =
+            model.replace_html(new_html.into(), HtmlSource::UnknownExternal);
+
+        let html = model.get_content_as_html();
+        let html_str = html.to_string();
+        assert_eq!(
+            html_str,
+            "<p>Hello</p><p><em>replacement</em></p><p>test</p>"
+        );
+    }
+
+    #[test]
+    fn test_replace_html_cursor_position_after_insert() {
+        let mut model = cm("Start|");
+        let new_html = "<strong>Bold text</strong>";
+        let _ = model.replace_html(new_html.into(), HtmlSource::Matrix);
+        // Cursor should be positioned after the inserted content
+        let (start, end) = model.safe_selection();
+        assert_eq!(start, end); // No selection, just cursor
+        model.bold();
+        model.enter();
+        // Insert more text to verify cursor position
+        let _ = model.replace_text("End".into());
+        let html = model.get_content_as_html();
+        let html_str = html.to_string();
+        assert_eq!(
+            html_str,
+            "<p>Start</p><p><strong>Bold text</strong></p><p>End</p>"
+        );
+    }
+
+    #[test]
+    fn test_replace_html_multiple_meta_tags() {
+        let mut model = cm("|");
+        let html_with_multiple_metas = r#"<meta charset="utf-8"><meta name="viewport" content="width=device-width"><meta http-equiv="X-UA-Compatible" content="IE=edge"><p>Content after metas</p>"#;
+
+        let _ = model.replace_html(
+            html_with_multiple_metas.into(),
+            HtmlSource::UnknownExternal,
+        );
+
+        let html = model.get_content_as_html();
+        let html_str = html.to_string();
+        assert!(!html_str.contains("<meta"));
+        assert_eq!(html_str, "<p>Content after metas</p>");
+    }
+
+    #[test]
+    fn test_replace_html_empty_content() {
+        let mut model = cm("Existing content|");
+        let empty_html = "";
+
+        let _ = model.replace_html(empty_html.into(), HtmlSource::Matrix);
+
+        let html = model.get_content_as_html();
+        let html_str = html.to_string();
+        assert_eq!(html_str, "<p>Existing content</p>");
+    }
 }

crates/wysiwyg/src/dom/dom_struct.rs

@@ -11,6 +11,7 @@ use crate::dom::nodes::{ContainerNode, DomNode};
 use crate::dom::to_html::ToHtmlState;
 use crate::dom::to_markdown::{MarkdownError, MarkdownOptions, ToMarkdown};
 use crate::dom::unicode_string::UnicodeStrExt;
+use crate::dom::DomLocation;
 use crate::dom::{
     find_range, to_raw_text::ToRawText, DomHandle, Range, ToTree, UnicodeString,
 };
@@ -295,14 +296,24 @@ where
         find_range::find_range(self, start, end)
     }
 
-    pub fn find_range_by_node(&self, node_handle: &DomHandle) -> Range {
-        let result = find_range::find_pos(self, node_handle, 0, usize::MAX);
+    pub fn location_for_node(&self, node_handle: &DomHandle) -> DomLocation {
+        let locations = find_range::find_range(self, 0, usize::MAX);
+        return locations.find_location(node_handle).unwrap().clone();
+    }
 
-        let locations = match result {
+    pub fn locations_for_node(
+        &self,
+        node_handle: &DomHandle,
+    ) -> Vec<DomLocation> {
+        let result = find_range::find_pos(self, &node_handle, 0, usize::MAX);
+        match result {
             FindResult::Found(locations) => locations,
             _ => panic!("Node does not exist"),
-        };
+        }
+    }
 
+    pub fn find_range_by_node(&self, node_handle: &DomHandle) -> Range {
+        let locations = self.locations_for_node(node_handle);
         let leaves = locations.iter().filter(|l| l.is_leaf());
 
         let s = leaves.clone().map(|l| l.position).min().unwrap();

crates/wysiwyg/src/dom/insert_node_at_cursor.rs

@@ -67,6 +67,10 @@ where
             };
         }
 
+        self.wrap_inline_nodes_into_paragraphs_if_needed(
+            &self.parent(&inserted_handle).handle(),
+        );
+
         #[cfg(any(test, feature = "assert-invariants"))]
         self.assert_invariants();
 

crates/wysiwyg/src/dom/parser/parse.rs

@@ -9,7 +9,7 @@ use regex::Regex;
 use crate::dom::dom_creation_error::HtmlParseError;
 use crate::dom::html_source::HtmlSource;
 use crate::dom::nodes::dom_node::DomNodeKind::{self};
-use crate::dom::nodes::{container_node, ContainerNode, ContainerNodeKind};
+use crate::dom::nodes::{ContainerNode, ContainerNodeKind};
 use crate::dom::Dom;
 use crate::{DomHandle, DomNode, UnicodeString};
 
@@ -1190,30 +1190,25 @@ mod sys {
                 r#"
 
                 ├>ol
-                │ └>li
-                │   └>p
-                │     └>i
-                │       └>"Italic"
-                ├>ol
-                │ └>li
-                │   └>p
-                │     └>b
-                │       └>"Bold"
-                ├>ol
-                │ └>li
-                │   └>p
-                │     └>"Unformatted"
-                ├>ol
-                │ └>li
-                │   └>p
-                │     └>del
-                │       └>"Strikethrough"
-                ├>ol
-                │ └>li
-                │   └>p
-                │     └>u
-                │       └>"Underlined"
-                ├>ol
+                │ ├>li
+                │ │ └>p
+                │ │   └>i
+                │ │     └>"Italic"
+                │ ├>li
+                │ │ └>p
+                │ │   └>b
+                │ │     └>"Bold"
+                │ ├>li
+                │ │ └>p
+                │ │   └>"Unformatted"
+                │ ├>li
+                │ │ └>p
+                │ │   └>del
+                │ │     └>"Strikethrough"
+                │ ├>li
+                │ │ └>p
+                │ │   └>u
+                │ │     └>"Underlined"
                 │ └>li
                 │   └>p
                 │     └>a "https://matrix.org/"
@@ -1253,49 +1248,15 @@ fn post_process_adjacent_text<S: UnicodeString>(
 fn post_process_for_block_and_inline_siblings<S: UnicodeString>(
     mut dom: Dom<S>,
 ) -> Dom<S> {
-    let continer_handles = find_containers_with_inline_and_block_children(&dom);
-    for handle in continer_handles.iter().rev() {
-        dom = post_process_container_for_block_and_inline_siblings(dom, handle);
-    }
-    dom
-}
-
-fn find_containers_with_inline_and_block_children<S: UnicodeString>(
-    dom: &Dom<S>,
-) -> Vec<DomHandle> {
-    dom.iter_containers()
-        .filter(|n| {
-            if n.children().is_empty() {
-                return false; // Skip empty containers
-            }
-            let all_nodes_are_inline =
-                n.children().iter().all(|n| !n.is_block_node());
-            let all_nodes_are_block =
-                n.children().iter().all(|n| n.is_block_node());
-            !all_nodes_are_inline && !all_nodes_are_block
-        })
-        .map(|n| n.handle())
-        .collect::<Vec<_>>()
-}
-
-fn post_process_container_for_block_and_inline_siblings<S: UnicodeString>(
-    mut dom: Dom<S>,
-    handle: &DomHandle,
-) -> Dom<S> {
-    // upate the container node by grouping inline nodes, to avoid
-    // having inline nodes as siblings of block nodes.
-    let container_node =
-        dom.lookup_node_mut(handle).as_container_mut().unwrap();
-    let new_children =
-        group_inline_nodes(container_node.remove_children().to_vec());
-    container_node.insert_children(0, new_children.clone());
+    dom.wrap_inline_nodes_into_paragraphs_if_needed(&DomHandle::root());
     dom
 }
 
 fn post_process_blocks<S: UnicodeString>(mut dom: Dom<S>) -> Dom<S> {
     let block_handles = find_blocks(&dom);
     for handle in block_handles.iter().rev() {
         dom = post_process_block_lines(dom, handle);
+        dom.join_nodes_in_container(&handle);
     }
     dom
 }
@@ -1575,6 +1536,7 @@ mod js {
             self.webdom_to_dom(document, html_source)
                 .map_err(to_dom_creation_error)
                 .map(post_process_blocks)
+                .map(post_process_for_block_and_inline_siblings)
                 .map(post_process_for_adjacent_text)
         }
 

crates/wysiwyg/src/dom/range.rs

@@ -7,6 +7,7 @@
 use crate::dom::dom_handle::DomHandle;
 use crate::dom::nodes::dom_node::DomNodeKind;
 use std::cmp::{min, Ordering};
+use std::fmt;
 
 /// Represents the relative position of a DomLocation towards
 /// the range start and end.
@@ -202,6 +203,21 @@ impl DomLocation {
     }
 }
 
+impl fmt::Display for DomLocation {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "DomLocation[node_handle: {:?}, position: {}, start_offset: {}, end_offset: {}, length: {}, kind: {:?}]",
+            self.node_handle.raw(),
+            self.position,
+            self.start_offset,
+            self.end_offset,
+            self.length,
+            self.kind
+        )
+    }
+}
+
 impl PartialOrd<Self> for DomLocation {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         Some(self.cmp(other))

crates/wysiwyg/src/tests/test_deleting.rs

@@ -878,7 +878,7 @@ fn backspace_immutable_link_from_inside_link() {
 #[test]
 fn backspace_immutable_link_multiple() {
     let mut model = cm(
-        "<a contenteditable=\"false\" href=\"https://matrix.org\">first</a><a contenteditable=\"false\" href=\"https://matrix.org\">second|</a>",
+        "<a contenteditable=\"false\" href=\"https://matrix.org\">first</a><a contenteditable=\"false\" href=\"https://element.io.org\">second|</a>",
     );
     model.backspace();
     assert_eq!(
@@ -956,12 +956,12 @@ fn delete_mention_from_start() {
 #[test]
 fn delete_first_immutable_link_of_multiple() {
     let mut model = cm(
-        "<a contenteditable=\"false\" href=\"https://matrix.org\">|first</a><a contenteditable=\"false\" href=\"https://matrix.org\">second</a>",
+        "<a contenteditable=\"false\" href=\"https://matrix.org\">|first</a><a contenteditable=\"false\" href=\"https://element.io\">second</a>",
     );
     model.delete();
     assert_eq!(
         restore_whitespace(&tx(&model)),
-        "<a contenteditable=\"false\" href=\"https://matrix.org\">|second</a>"
+        "<a contenteditable=\"false\" href=\"https://element.io\">|second</a>"
     );
     model.delete();
     assert_eq!(restore_whitespace(&tx(&model)), "|");
@@ -984,7 +984,7 @@ fn delete_first_mention_of_multiple() {
 #[test]
 fn delete_second_immutable_link_of_multiple() {
     let mut model = cm(
-        "<a contenteditable=\"false\" href=\"https://matrix.org\">first</a><a contenteditable=\"false\" href=\"https://matrix.org\">second|</a>",
+        "<a contenteditable=\"false\" href=\"https://matrix.org\">first</a><a contenteditable=\"false\" href=\"https://element.io\">second|</a>",
     );
     model.backspace();
     assert_eq!(