diff --git a/docx_editor/comments.py b/docx_editor/comments.py index 181bb68..6d40b11 100644 --- a/docx_editor/comments.py +++ b/docx_editor/comments.py @@ -10,7 +10,7 @@ from pathlib import Path from .exceptions import CommentError, TextNotFoundError -from .xml_editor import DocxXMLEditor, _generate_hex_id +from .xml_editor import DocxXMLEditor, _generate_hex_id, get_text_node_data # Path to template files TEMPLATE_DIR = Path(__file__).parent / "ooxml" / "templates" @@ -412,11 +412,7 @@ def _parse_comment(self, comment_elem) -> Comment | None: except ValueError: date = None - # Extract text content from w:t elements - text_parts = [] - for t_elem in comment_elem.getElementsByTagName("w:t"): - if t_elem.firstChild: - text_parts.append(t_elem.firstChild.data) + text_parts = [get_text_node_data(t_elem) for t_elem in comment_elem.getElementsByTagName("w:t")] return Comment( id=int(comment_id), diff --git a/docx_editor/ooxml/pack.py b/docx_editor/ooxml/pack.py index 8256333..ddc34b0 100644 --- a/docx_editor/ooxml/pack.py +++ b/docx_editor/ooxml/pack.py @@ -120,8 +120,10 @@ def condense_xml(xml_file: Path) -> None: # Process each element to remove whitespace and comments for element in dom.getElementsByTagName("*"): - # Skip w:t elements and their processing (preserve text content) - if element.tagName.endswith(":t"): + # Skip text-bearing OOXML elements: w:t, w:delText, w:instrText (and their + # namespace variants). Their content is significant — including whitespace + # fragments that minidom may split off as their own TEXT_NODE (issue #9). + if element.tagName.endswith((":t", ":delText", ":instrText")): continue # Remove whitespace-only text nodes and comment nodes diff --git a/docx_editor/track_changes.py b/docx_editor/track_changes.py index 38cc3d1..5d9262d 100644 --- a/docx_editor/track_changes.py +++ b/docx_editor/track_changes.py @@ -27,6 +27,7 @@ build_text_map, compute_paragraph_hash, find_in_text_map, + get_text_node_data, ) @@ -372,7 +373,7 @@ def _rewrite_insert_at(self, paragraph, text_map: TextMap, char_pos: int, text: ins_ancestor = self._find_ancestor(run, "w:ins") if ins_ancestor: node_text = self._get_node_text(last_pos.node) - last_pos.node.firstChild.data = node_text + text + self._set_node_text(last_pos.node, node_text + text) _set_xml_space_preserve(last_pos.node) return @@ -391,7 +392,7 @@ def _rewrite_insert_at(self, paragraph, text_map: TextMap, char_pos: int, text: if ins_ancestor: node_text = self._get_node_text(pos.node) offset = pos.offset_in_node - pos.node.firstChild.data = node_text[:offset] + text + node_text[offset:] + self._set_node_text(pos.node, node_text[:offset] + text + node_text[offset:]) _set_xml_space_preserve(pos.node) return @@ -544,7 +545,7 @@ def replace_text(self, find: str, replace_with: str, occurrence: int = 0, paragr raise RevisionError("Could not find parent w:r element") # Get the full text content - full_text = elem.firstChild.data if elem.firstChild else "" + full_text = self._get_node_text(elem) start_idx = full_text.find(find) if start_idx == -1: @@ -563,7 +564,7 @@ def replace_text(self, find: str, replace_with: str, occurrence: int = 0, paragr # Site A: If inside , edit text in-place (no del/ins wrappers) if self._find_ancestor(run, "w:ins"): - elem.firstChild.data = before_text + replace_with + after_text + self._set_node_text(elem, before_text + replace_with + after_text) _set_xml_space_preserve(elem) return -1 @@ -647,7 +648,7 @@ def suggest_deletion(self, text: str, occurrence: int = 0, paragraph: str | None raise RevisionError("Could not find parent w:r element") # Get the full text content - full_text = elem.firstChild.data if elem.firstChild else "" + full_text = self._get_node_text(elem) start_idx = full_text.find(text) if start_idx == -1: @@ -673,7 +674,7 @@ def suggest_deletion(self, text: str, occurrence: int = 0, paragraph: str | None if ins_ancestor: remaining = before_text + after_text if remaining: - elem.firstChild.data = remaining + self._set_node_text(elem, remaining) _set_xml_space_preserve(elem) else: # Entire w:t text removed — check if other w:t nodes exist @@ -735,12 +736,25 @@ def _get_non_text_children_xml(self, run) -> str: return "".join(parts) def _get_node_text(self, node) -> str: - """Get text content of a w:t node.""" - text = "" - for child in node.childNodes: + """Get text content of a w:t node by concatenating ALL child text nodes. + + Thin wrapper around :func:`get_text_node_data` — kept as a method so + existing call sites (and external subclasses) don't have to change. + """ + return get_text_node_data(node) + + def _set_node_text(self, node, text: str) -> None: + """Replace all text content of a w:t/w:delText element with ``text``. + + Removes every existing TEXT_NODE child and appends a single new one + carrying the full content. Necessary because assigning to + ``firstChild.data`` would leave any sibling text nodes behind, + corrupting the document when the element holds split text (issue #9). + """ + for child in list(node.childNodes): if child.nodeType == child.TEXT_NODE: - text += child.data - return text + node.removeChild(child) + node.appendChild(node.ownerDocument.createTextNode(text)) def _build_cross_boundary_parts(self, match: TextMapMatch) -> list[tuple[Element, str, str, str, str, int]]: """Build per-node data for a cross-boundary match. @@ -1049,12 +1063,15 @@ def _remove_from_insertion(self, positions: list) -> None: # Other w:t nodes exist — remove just this w:t (and run if empty) self._remove_wt_and_maybe_run(first_node) elif not before_text: - first_node.firstChild.data = after_text + self._set_node_text(first_node, after_text) + _set_xml_space_preserve(first_node) elif not after_text: - first_node.firstChild.data = before_text + self._set_node_text(first_node, before_text) + _set_xml_space_preserve(first_node) else: # Middle split - first_node.firstChild.data = before_text + self._set_node_text(first_node, before_text) + _set_xml_space_preserve(first_node) run = self._find_ancestor(first_node, "w:r") if ins_elem and run: rPr_xml = "" @@ -1068,13 +1085,13 @@ def _remove_from_insertion(self, positions: list) -> None: # remove intermediate nodes entirely. # Only remove the w:t node; remove the run only if no w:t children remain. if before: - first_node.firstChild.data = before + self._set_node_text(first_node, before) _set_xml_space_preserve(first_node) else: self._remove_wt_and_maybe_run(first_node) if after: - last_node.firstChild.data = after + self._set_node_text(last_node, after) _set_xml_space_preserve(last_node) else: self._remove_wt_and_maybe_run(last_node) @@ -1386,7 +1403,7 @@ def _insert_text( rPr_xml = rPr_elems[0].toxml() # Site C: If inside , edit text in-place (safe for multi-w:t) - full_text = elem.firstChild.data if elem.firstChild else "" + full_text = self._get_node_text(elem) anchor_idx = full_text.find(anchor) if anchor_idx == -1: @@ -1397,9 +1414,9 @@ def _insert_text( if self._find_ancestor(run, "w:ins"): if position == "before": - elem.firstChild.data = before_text + text + anchor + after_text + self._set_node_text(elem, before_text + text + anchor + after_text) else: - elem.firstChild.data = before_text + anchor + text + after_text + self._set_node_text(elem, before_text + anchor + text + after_text) _set_xml_space_preserve(elem) return -1 @@ -1528,10 +1545,7 @@ def _parse_revision(self, elem, rev_type: Literal["insertion", "deletion"]) -> R else: text_elems = elem.getElementsByTagName("w:delText") - text_parts = [] - for t_elem in text_elems: - if t_elem.firstChild: - text_parts.append(t_elem.firstChild.data) + text_parts = [self._get_node_text(t_elem) for t_elem in text_elems] return Revision( id=int(rev_id), diff --git a/docx_editor/xml_editor.py b/docx_editor/xml_editor.py index a67e2c3..75aa383 100644 --- a/docx_editor/xml_editor.py +++ b/docx_editor/xml_editor.py @@ -131,6 +131,20 @@ def _is_inside_element(node, tag_name: str) -> bool: return False +def get_text_node_data(elem) -> str: + """Concatenate all direct TEXT_NODE children of ``elem`` into one string. + + minidom can split a single ````'s text across multiple TEXT_NODE + children — reproducibly when smart quotes (U+2018/U+2019) are present + (issue #9). Anywhere code naively read ``elem.firstChild.data`` would + only see the first fragment. This helper returns the complete string. + + Does NOT recurse into element children. For recursive extraction across + a subtree see ``XMLEditor._get_element_text``. + """ + return "".join(c.data for c in elem.childNodes if c.nodeType == c.TEXT_NODE) + + def build_text_map(paragraph) -> TextMap: """Build a text map for a paragraph element. @@ -150,11 +164,7 @@ def build_text_map(paragraph) -> TextMap: continue inside_ins = _is_inside_element(node, "w:ins") - # Get text content from the text node - node_text = "" - for child in node.childNodes: - if child.nodeType == child.TEXT_NODE: - node_text += child.data + node_text = get_text_node_data(node) for i, char in enumerate(node_text): text_chars.append(char) @@ -335,20 +345,30 @@ def find_all_nodes( def _get_element_text(self, elem) -> str: """Recursively extract all text content from an element. - Skips text nodes that contain only whitespace (spaces, tabs, newlines). + A whitespace-only TEXT_NODE child is treated as content (rather than + pretty-print indentation) when any of: + * ``elem`` carries ``xml:space="preserve"`` — the document explicitly + marks its whitespace significant. + * Another TEXT_NODE sibling has non-whitespace content — the lone + whitespace is a fragment of a split text node (issue #9). + Otherwise the whitespace TEXT_NODE is discarded as inter-element + formatting. Args: elem: DOM element to extract text from Returns: - Concatenated text from all non-whitespace text nodes + Concatenated text content """ + preserve_whitespace = elem.getAttribute("xml:space") == "preserve" or any( + n.nodeType == n.TEXT_NODE and n.data.strip() for n in elem.childNodes + ) text_parts = [] for node in elem.childNodes: if node.nodeType == node.TEXT_NODE: - # Skip whitespace-only text nodes (XML formatting) - if node.data.strip(): - text_parts.append(node.data) + if not node.data.strip() and not preserve_whitespace: + continue + text_parts.append(node.data) elif node.nodeType == node.ELEMENT_NODE: text_parts.append(self._get_element_text(node)) return "".join(text_parts) @@ -609,12 +629,11 @@ def add_comment_extensible_date(elem) -> None: elem.setAttribute("w16cex:dateUtc", timestamp) def add_xml_space_to_t(elem) -> None: - # Add xml:space="preserve" to w:t if text has leading/trailing whitespace - if elem.firstChild and elem.firstChild.nodeType == elem.firstChild.TEXT_NODE: - text = elem.firstChild.data - if text and (text[0].isspace() or text[-1].isspace()): - if not elem.hasAttribute("xml:space"): - elem.setAttribute("xml:space", "preserve") + # Add xml:space="preserve" to w:t if text has leading/trailing whitespace. + text = get_text_node_data(elem) + if text and (text[0].isspace() or text[-1].isspace()): + if not elem.hasAttribute("xml:space"): + elem.setAttribute("xml:space", "preserve") for node in nodes: if node.nodeType != node.ELEMENT_NODE: diff --git a/tests/test_comments.py b/tests/test_comments.py index 005ca9b..0be8d11 100644 --- a/tests/test_comments.py +++ b/tests/test_comments.py @@ -998,3 +998,35 @@ def test_add_comment_intermediate_parent_traversal(self, clean_workspace): assert isinstance(comment_id, int) doc.close() + + +class TestSplitTextNodeComment: + """Issue #9: comments with multi-TEXT_NODE w:t must round-trip full text.""" + + def test_list_comments_full_text_with_split_w_t(self, clean_workspace): + doc = Document.open(clean_workspace) + try: + doc.add_comment("fox", "Hello World") + except TextNotFoundError: + pytest.skip("Anchor text not found in document") + + editor = doc._comment_manager._get_editor(doc._comment_manager.comments_path) + wts = editor.dom.getElementsByTagName("w:t") + target = None + for wt in wts: + full = "".join(c.data for c in wt.childNodes if c.nodeType == c.TEXT_NODE) + if "Hello World" in full: + target = wt + break + assert target is not None, "comment w:t not found" + + while target.firstChild: + target.removeChild(target.firstChild) + owner = target.ownerDocument + target.appendChild(owner.createTextNode("Hello ")) + target.appendChild(owner.createTextNode("World")) + + comments = doc.list_comments() + texts = [c.text for c in comments] + assert "Hello World" in texts + doc.close() diff --git a/tests/test_ooxml.py b/tests/test_ooxml.py index 2e317c6..60071b3 100644 --- a/tests/test_ooxml.py +++ b/tests/test_ooxml.py @@ -90,3 +90,65 @@ def test_pack_excludes_root_meta_json(self, simple_docx, temp_dir): names = zf.namelist() assert "meta.json" not in names assert "word/meta.json" in names + + +def _read_doc_xml(docx_path): + """Open a packed .docx and return its word/document.xml content as text.""" + import zipfile + + with zipfile.ZipFile(docx_path) as zf: + return zf.read("word/document.xml").decode("utf-8") + + +class TestCondenseXmlPreservesTextContent: + """Issue #9: condense_xml must not strip whitespace fragments from text-bearing + OOXML elements (w:t, w:delText, w:instrText). Pre-fix only :t was protected.""" + + def _write_xml(self, base_dir, body_xml): + """Replace word/document.xml's body with body_xml, returning the dir.""" + doc_xml = base_dir / "word" / "document.xml" + original = doc_xml.read_text(encoding="utf-8") + head, _, tail = original.partition("") + _, _, end = tail.partition("") + new = f"{head}{body_xml}{end}" + doc_xml.write_text(new, encoding="utf-8") + return base_dir + + def test_condense_preserves_whitespace_in_delText(self, simple_docx, temp_dir): + unpack_document(simple_docx, temp_dir / "unpacked") + body = ( + '' + 'foo bar' + "" + ) + self._write_xml(temp_dir / "unpacked", body) + + # Inject a multi-TEXT_NODE state into the w:delText, including a lone space. + import defusedxml.minidom + + doc_xml = temp_dir / "unpacked" / "word" / "document.xml" + dom = defusedxml.minidom.parseString(doc_xml.read_text(encoding="utf-8")) + delText = dom.getElementsByTagName("w:delText")[0] + while delText.firstChild: + delText.removeChild(delText.firstChild) + owner = delText.ownerDocument + delText.appendChild(owner.createTextNode("foo")) + delText.appendChild(owner.createTextNode(" ")) + delText.appendChild(owner.createTextNode("bar")) + doc_xml.write_bytes(dom.toxml(encoding="UTF-8")) + + out = temp_dir / "out.docx" + pack_document(temp_dir / "unpacked", out) + repacked = _read_doc_xml(out) + # The space must survive condensation + assert "foo bar" in repacked + + def test_condense_preserves_whitespace_in_instrText(self, simple_docx, temp_dir): + unpack_document(simple_docx, temp_dir / "unpacked") + body = ' HYPERLINK ' + self._write_xml(temp_dir / "unpacked", body) + + out = temp_dir / "out.docx" + pack_document(temp_dir / "unpacked", out) + repacked = _read_doc_xml(out) + assert " HYPERLINK " in repacked diff --git a/tests/test_track_changes.py b/tests/test_track_changes.py index b49649a..bff0fa0 100644 --- a/tests/test_track_changes.py +++ b/tests/test_track_changes.py @@ -11,6 +11,19 @@ from docx_editor.track_changes import Revision, RevisionManager, _escape_xml +def _attach_text_node(mock_elem, text): + """Configure a MagicMock to look like a w:t element with TEXT_NODE child. + + Mirrors the real minidom structure so methods iterating ``childNodes`` + and filtering by ``TEXT_NODE`` (e.g. ``_get_node_text``) work. + """ + mock_elem.firstChild = MagicMock() + mock_elem.firstChild.data = text + # Make nodeType == TEXT_NODE evaluate True without depending on actual ints. + mock_elem.firstChild.nodeType = mock_elem.firstChild.TEXT_NODE + mock_elem.childNodes = [mock_elem.firstChild] + + class TestTrackedReplace: """Tests for tracked text replacement.""" @@ -799,8 +812,7 @@ def get_attr(name): # Mock text element with content mock_text_elem = MagicMock() - mock_text_elem.firstChild = MagicMock() - mock_text_elem.firstChild.data = "test content" + _attach_text_node(mock_text_elem, "test content") mock_elem.getElementsByTagName.return_value = [mock_text_elem] @@ -958,8 +970,7 @@ def test_replace_text_parent_traversal_loop(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_intermediate - mock_elem.firstChild = MagicMock() - mock_elem.firstChild.data = "hello world" + _attach_text_node(mock_elem, "hello world") mock_editor.find_all_nodes.return_value = [mock_elem] @@ -994,8 +1005,7 @@ def test_suggest_deletion_parent_traversal_loop(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_intermediate - mock_elem.firstChild = MagicMock() - mock_elem.firstChild.data = "hello world" + _attach_text_node(mock_elem, "hello world") mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1030,7 +1040,7 @@ def test_insert_text_parent_traversal_loop(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_intermediate - mock_elem.firstChild.data = "anchor" + _attach_text_node(mock_elem, "anchor") mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1064,8 +1074,7 @@ def test_replace_text_with_rpr(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run - mock_elem.firstChild = MagicMock() - mock_elem.firstChild.data = "hello world" + _attach_text_node(mock_elem, "hello world") mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1101,8 +1110,7 @@ def test_suggest_deletion_with_rpr(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run - mock_elem.firstChild = MagicMock() - mock_elem.firstChild.data = "hello world" + _attach_text_node(mock_elem, "hello world") mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1146,7 +1154,7 @@ def _get_by_tag(tag): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run - mock_elem.firstChild.data = "anchor" + _attach_text_node(mock_elem, "anchor") mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1178,8 +1186,7 @@ def test_replace_text_returns_minus_one_when_no_ins_found(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run - mock_elem.firstChild = MagicMock() - mock_elem.firstChild.data = "hello" + _attach_text_node(mock_elem, "hello") mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1208,8 +1215,7 @@ def test_suggest_deletion_returns_minus_one_when_no_del_found(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run - mock_elem.firstChild = MagicMock() - mock_elem.firstChild.data = "hello" + _attach_text_node(mock_elem, "hello") mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1238,6 +1244,7 @@ def test_insert_text_returns_minus_one_when_no_ins_found(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run + _attach_text_node(mock_elem, "anchor") mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1266,9 +1273,8 @@ def test_replace_text_text_mismatch_raises_error(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run - mock_elem.firstChild = MagicMock() # The matcher found "world" in this node, but actual content is different - mock_elem.firstChild.data = "different content" + _attach_text_node(mock_elem, "different content") mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1290,9 +1296,8 @@ def test_suggest_deletion_text_mismatch_raises_error(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run - mock_elem.firstChild = MagicMock() # The matcher found "world" in this node, but actual content is different - mock_elem.firstChild.data = "different content" + _attach_text_node(mock_elem, "different content") mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1314,8 +1319,7 @@ def test_replace_text_with_before_text_only(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run - mock_elem.firstChild = MagicMock() - mock_elem.firstChild.data = "prefix hello" # "hello" is at end + _attach_text_node(mock_elem, "prefix hello") # "hello" is at end mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1347,8 +1351,7 @@ def test_suggest_deletion_with_before_text_only(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run - mock_elem.firstChild = MagicMock() - mock_elem.firstChild.data = "prefix hello" # "hello" is at end + _attach_text_node(mock_elem, "prefix hello") # "hello" is at end mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1380,8 +1383,7 @@ def test_suggest_deletion_with_after_text_only(self): mock_elem.nodeName = "w:t" mock_elem.parentNode = mock_run - mock_elem.firstChild = MagicMock() - mock_elem.firstChild.data = "hello suffix" # "hello" is at start + _attach_text_node(mock_elem, "hello suffix") # "hello" is at start mock_editor.find_all_nodes.return_value = [mock_elem] @@ -1643,3 +1645,216 @@ def test_restore_deletion_converts_rsiddel_to_rsidr(self): assert len(r_elems) == 1 assert r_elems[0].getAttribute("w:rsidR") == "00112233" assert not r_elems[0].hasAttribute("w:rsidDel") + + +def _split_wt_text_nodes(doc, target_text, tag="w:t"): + """Reach into the DOM and split an element's TEXT_NODE into multiple siblings. + + Simulates the minidom multi-child-text-node state reported in issue #9 — + Word documents with smart quotes (U+2018/U+2019) can land in this state. + Returns the modified element. + """ + dom = doc._document_editor.dom + for wt in dom.getElementsByTagName(tag): + full = "".join(c.data for c in wt.childNodes if c.nodeType == c.TEXT_NODE) + if target_text not in full: + continue + while wt.firstChild: + wt.removeChild(wt.firstChild) + idx = full.find(target_text) + before, after = full[:idx], full[idx + len(target_text) :] + owner = wt.ownerDocument + if before: + wt.appendChild(owner.createTextNode(before)) + wt.appendChild(owner.createTextNode(target_text)) + if after: + wt.appendChild(owner.createTextNode(after)) + return wt + raise AssertionError(f"No {tag} containing {target_text!r}") + + +class TestMultiTextNodeWtElements: + """Issue #9: w:t elements with multiple TEXT_NODE children (smart-quote split).""" + + # The buggy paths in replace_text/suggest_deletion/_insert_text (lines 547, + # 650, 1389) are only reached when paragraph=None (via _get_nth_match). + # When paragraph= is passed, control routes through _replace_across_nodes + # which uses _get_node_text and avoids the bug. + + def test_set_node_text_consolidates_split_nodes(self, clean_workspace): + """Direct contract test for _set_node_text: starts from a multi-TEXT_NODE + state, ends with exactly one TEXT_NODE carrying the full new content. + Guards against future "simplifications" that would re-introduce the + firstChild.data assignment pattern.""" + doc = Document.open(clean_workspace) + wt = _split_wt_text_nodes(doc, "quick brown fox") + text_nodes_before = [c for c in wt.childNodes if c.nodeType == c.TEXT_NODE] + assert len(text_nodes_before) > 1 + + doc._revision_manager._set_node_text(wt, "consolidated") + + text_nodes_after = [c for c in wt.childNodes if c.nodeType == c.TEXT_NODE] + assert len(text_nodes_after) == 1 + assert text_nodes_after[0].data == "consolidated" + doc.close() + + def test_replace_without_paragraph_arg_succeeds(self, clean_workspace): + doc = Document.open(clean_workspace) + wt = _split_wt_text_nodes(doc, "quick brown fox") + assert len(wt.childNodes) > 1 + doc._revision_manager.replace_text("quick brown fox", "slow red turtle") + paragraphs = doc.list_paragraphs() + assert any("slow red turtle" in p for p in paragraphs) + assert not any("quick brown fox" in p for p in paragraphs) + doc.close() + + def test_delete_without_paragraph_arg_succeeds(self, clean_workspace): + doc = Document.open(clean_workspace) + _split_wt_text_nodes(doc, "quick brown fox") + doc._revision_manager.suggest_deletion("quick brown fox") + paragraphs = doc.list_paragraphs() + assert not any("quick brown fox" in p for p in paragraphs) + doc.close() + + def test_insert_without_paragraph_arg_succeeds(self, clean_workspace): + doc = Document.open(clean_workspace) + ref = find_ref(doc, "lazy dog") + doc.insert_before("lazy dog", "INS_TARGET ", paragraph=ref) + _split_wt_text_nodes(doc, "INS_TARGET") + doc._revision_manager.insert_text_before("INS_TARGET", "X_") + paragraphs = doc.list_paragraphs() + assert any("X_INS_TARGET" in p for p in paragraphs) + doc.close() + + def test_replace_inside_ins_writes_full_text(self, clean_workspace): + # Hits _replace_across_nodes' "all inside ins" path which previously + # used firstChild.data assignment. + doc = Document.open(clean_workspace) + ref = find_ref(doc, "lazy dog") + ref = doc.insert_before("lazy dog", "INS_TARGET ", paragraph=ref) + _split_wt_text_nodes(doc, "INS_TARGET") + doc.replace("INS_TARGET", "REPLACED", paragraph=ref) + paragraphs = doc.list_paragraphs() + assert any("REPLACED" in p for p in paragraphs) + assert not any("INS_TARGET" in p for p in paragraphs) + doc.close() + + def test_list_revisions_with_multi_text_node_delText(self, clean_workspace): + doc = Document.open(clean_workspace) + ref = find_ref(doc, "quick brown fox") + doc.delete("quick brown fox", paragraph=ref) + + dom = doc._document_editor.dom + del_texts = dom.getElementsByTagName("w:delText") + assert del_texts, "expected at least one w:delText after delete" + elem = del_texts[0] + full = "".join(c.data for c in elem.childNodes if c.nodeType == c.TEXT_NODE) + while elem.firstChild: + elem.removeChild(elem.firstChild) + mid = len(full) // 2 + elem.appendChild(elem.ownerDocument.createTextNode(full[:mid])) + elem.appendChild(elem.ownerDocument.createTextNode(full[mid:])) + + revisions = doc.list_revisions() + deletions = [r for r in revisions if r.type == "deletion"] + assert deletions + assert deletions[0].text == full + doc.close() + + def test_save_load_roundtrip_after_multi_node_edit(self, clean_workspace, tmp_path): + doc = Document.open(clean_workspace) + _split_wt_text_nodes(doc, "quick brown fox") + doc._revision_manager.replace_text("quick brown fox", "slow red turtle") + out = tmp_path / "edited.docx" + doc.save(out) + doc.close() + + doc2 = Document.open(out) + paragraphs = doc2.list_paragraphs() + assert any("slow red turtle" in p for p in paragraphs) + assert not any("quick brown fox" in p for p in paragraphs) + doc2.close() + + +def _build_smart_quote_docx(simple_docx, dest): + """Build a real .docx containing smart-quote text in a single . + + Repacks ``simple_docx`` with the second paragraph rewritten to contain + U+2018/U+2019 smart quotes, simulating the structure reported in + GitHub issue #9. + """ + import shutil + + from docx_editor.ooxml.pack import pack_document + from docx_editor.ooxml.unpack import unpack_document + + work = dest.parent / "_smart_quote_build" + if work.exists(): + shutil.rmtree(work) + unpack_document(simple_docx, work) + doc_xml = work / "word" / "document.xml" + xml = doc_xml.read_text(encoding="utf-8") + # Replace "The quick brown fox..." paragraph's text with one carrying + # smart quotes. The surrounding ... structure + # mirrors what Word emits. + target = "The quick brown fox jumps over the lazy dog." + replacement = "‘Library Bookshelves’ are in all Libraries." + assert target in xml, "fixture assumption broken: simple.docx changed" + xml = xml.replace(target, replacement) + doc_xml.write_text(xml, encoding="utf-8") + pack_document(work, dest) + shutil.rmtree(work) + return dest + + +class TestSmartQuoteEndToEnd: + """End-to-end: real .docx with smart quotes, full open/edit/save/reopen.""" + + def test_replace_around_smart_quotes(self, simple_docx, tmp_path): + src = tmp_path / "with_smart_quotes.docx" + _build_smart_quote_docx(simple_docx, src) + + doc = Document.open(src, force_recreate=True) + try: + # Force the multi-text-node state on the smart-quote w:t so we + # exercise the codepath issue #9 describes regardless of how + # the local minidom build represents the parsed text. + _split_wt_text_nodes(doc, "Library Bookshelves") + + doc._revision_manager.replace_text("Library Bookshelves", "Reading Rooms") + out = tmp_path / "edited.docx" + doc.save(out) + finally: + doc.close() + + doc2 = Document.open(out, force_recreate=True) + try: + paragraphs = doc2.list_paragraphs() + joined = " ".join(paragraphs) + assert "Reading Rooms" in joined + assert "Library Bookshelves" not in joined + # Smart quotes must survive the edit + assert "‘" in joined and "’" in joined + finally: + doc2.close() + + def test_delete_around_smart_quotes(self, simple_docx, tmp_path): + src = tmp_path / "with_smart_quotes.docx" + _build_smart_quote_docx(simple_docx, src) + + doc = Document.open(src, force_recreate=True) + try: + _split_wt_text_nodes(doc, "Library Bookshelves") + doc._revision_manager.suggest_deletion("Library Bookshelves") + out = tmp_path / "edited.docx" + doc.save(out) + finally: + doc.close() + + doc2 = Document.open(out, force_recreate=True) + try: + joined = " ".join(doc2.list_paragraphs()) + assert "Library Bookshelves" not in joined + assert "‘" in joined and "’" in joined + finally: + doc2.close() diff --git a/tests/test_xml_editor.py b/tests/test_xml_editor.py index 9a2365a..ded030b 100644 --- a/tests/test_xml_editor.py +++ b/tests/test_xml_editor.py @@ -245,6 +245,40 @@ def test_get_element_text_nested(self, tmp_path): text = editor._get_element_text(outer) assert text == "Hello World" + def test_get_element_text_preserves_split_whitespace(self, tmp_path): + """Issue #9: a whitespace fragment between two non-whitespace TEXT_NODEs + must be preserved (it's content, not pretty-print indentation).""" + xml_content = 'placeholder' + xml_path = tmp_path / "split.xml" + xml_path.write_text(xml_content) + editor = XMLEditor(xml_path) + item = editor.dom.getElementsByTagName("item")[0] + # Replace the single TEXT_NODE with three TEXT_NODE siblings + while item.firstChild: + item.removeChild(item.firstChild) + owner = item.ownerDocument + item.appendChild(owner.createTextNode("Library")) + item.appendChild(owner.createTextNode(" ")) + item.appendChild(owner.createTextNode("Bookshelves")) + + assert editor._get_element_text(item) == "Library Bookshelves" + + def test_get_element_text_honors_xml_space_preserve(self, tmp_path): + """A lone whitespace TEXT_NODE inside an element marked + xml:space="preserve" is significant content and must be returned. + + Without the attribute it would be treated as pretty-print indent and + stripped — matching the pre-existing behavior.""" + xml_content = ' ' + xml_path = tmp_path / "preserve.xml" + xml_path.write_text(xml_content) + editor = XMLEditor(xml_path) + + keep = editor.dom.getElementsByTagName("keep")[0] + drop = editor.dom.getElementsByTagName("drop")[0] + assert editor._get_element_text(keep) == " " + assert editor._get_element_text(drop) == "" + class TestXMLEditorGetNextRid: """Tests for XMLEditor.get_next_rid method.""" @@ -436,6 +470,30 @@ def test_inject_xml_space_for_whitespace(self, docx_xml_file): new_t = nodes[0].getElementsByTagName("w:t")[0] assert new_t.getAttribute("xml:space") == "preserve" + def test_inject_xml_space_for_split_text_nodes(self, docx_xml_file): + """Issue #9: whitespace in a non-first TEXT_NODE must still trigger preserve. + + Simulates a w:t parsed into multiple TEXT_NODE children (as happens with + smart quotes on some minidom builds): the leading non-whitespace fragment + is followed by a trailing space fragment. Pre-fix this lost the preserve + attribute because only firstChild.data was inspected. + """ + editor = DocxXMLEditor(docx_xml_file, rsid="00AABBCC", author="Test") + p_elem = editor.get_node("w:p", contains="Hello") + + nodes = editor.insert_after(p_elem, "placeholder") + new_t = nodes[0].getElementsByTagName("w:t")[0] + # Replace the single TEXT_NODE with two siblings: "word" + " " (trailing space) + while new_t.firstChild: + new_t.removeChild(new_t.firstChild) + owner = new_t.ownerDocument + new_t.appendChild(owner.createTextNode("word")) + new_t.appendChild(owner.createTextNode(" ")) + # Re-run injection on the wrapped run + editor._inject_attributes_to_nodes([nodes[0]]) + + assert new_t.getAttribute("xml:space") == "preserve" + def test_inject_tracked_change_attrs(self, docx_xml_file): """Test w:ins gets proper attributes.""" editor = DocxXMLEditor(docx_xml_file, rsid="00AABBCC", author="TestAuthor")