Skip to content

Commit e580508

Browse files
authored
fix: fall back to w:t when w:del lacks w:delText in revision parsing (#34)
* fix: fall back to w:t when w:del lacks w:delText in revision parsing Nonconforming producers sometimes leave plain w:t inside w:del, which made such deletions appear in list_revisions() with empty text. When a w:del contains no w:delText elements, extract text from w:t instead. The fallback only fires on an empty w:delText node list, so conforming documents behave identically. * docs: clarify scope of w:delText fallback comment Multi-reviewer pass confirmed the logic but flagged that the mixed- content behavior (w:delText present alongside stray w:t reads only w:delText) and the deliberate-fallback nature were documented only in tests. State both in the code comment. Skipped as out of scope: nested w:ins filtering in the fallback (doubly-malformed input only).
1 parent 72c5479 commit e580508

2 files changed

Lines changed: 58 additions & 0 deletions

File tree

‎docx_editor/track_changes.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,6 +1644,11 @@ def _parse_revision(self, elem, rev_type: Literal["insertion", "deletion"]) -> R
16441644
text_elems = elem.getElementsByTagName("w:t")
16451645
else:
16461646
text_elems = elem.getElementsByTagName("w:delText")
1647+
if not text_elems:
1648+
# Deliberate interop fallback: nonconforming producers may
1649+
# leave plain w:t inside w:del. Fires only when the w:del has
1650+
# no w:delText at all; mixed content reads only w:delText.
1651+
text_elems = elem.getElementsByTagName("w:t")
16471652

16481653
text_parts = [self._get_node_text(t_elem) for t_elem in text_elems]
16491654

‎tests/test_track_changes.py‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1647,6 +1647,59 @@ def test_restore_deletion_converts_rsiddel_to_rsidr(self):
16471647
assert not r_elems[0].hasAttribute("w:rsidDel")
16481648

16491649

1650+
class TestParseRevisionDelTextFallback:
1651+
"""Tests for w:delText -> w:t fallback when reading deletion text."""
1652+
1653+
def test_deletion_with_plain_wt_falls_back(self):
1654+
"""Test that deletion text falls back to w:t when w:delText is absent."""
1655+
import defusedxml.minidom
1656+
1657+
# Nonconforming producers may leave plain w:t inside w:del
1658+
xml = """<?xml version="1.0"?>
1659+
<w:document xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main">
1660+
<w:del w:id="1" w:author="Foreign">
1661+
<w:r>
1662+
<w:t>lost text</w:t>
1663+
</w:r>
1664+
</w:del>
1665+
</w:document>"""
1666+
1667+
mock_editor = MagicMock()
1668+
mock_editor.dom = defusedxml.minidom.parseString(xml)
1669+
1670+
manager = RevisionManager(mock_editor)
1671+
1672+
revisions = manager.list_revisions()
1673+
assert len(revisions) == 1
1674+
assert revisions[0].type == "deletion"
1675+
assert revisions[0].text == "lost text"
1676+
1677+
def test_deletion_with_deltext_unchanged(self):
1678+
"""Test that the fallback does not fire when w:delText exists."""
1679+
import defusedxml.minidom
1680+
1681+
xml = """<?xml version="1.0"?>
1682+
<w:document xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main">
1683+
<w:del w:id="1" w:author="Test">
1684+
<w:r>
1685+
<w:delText>proper</w:delText>
1686+
</w:r>
1687+
<w:r>
1688+
<w:t>stray</w:t>
1689+
</w:r>
1690+
</w:del>
1691+
</w:document>"""
1692+
1693+
mock_editor = MagicMock()
1694+
mock_editor.dom = defusedxml.minidom.parseString(xml)
1695+
1696+
manager = RevisionManager(mock_editor)
1697+
1698+
revisions = manager.list_revisions()
1699+
assert len(revisions) == 1
1700+
assert revisions[0].text == "proper"
1701+
1702+
16501703
def _split_wt_text_nodes(doc, target_text, tag="w:t"):
16511704
"""Reach into the DOM and split an element's TEXT_NODE into multiple siblings.
16521705

0 commit comments

Comments
 (0)