Skip to content

Commit 1d3af54

Browse files
committed
fix(comments): order replyLst before txBody per [MS-PPTX] CT_Comment — replies now render (issue #25)
Follow-up to ca4da3f. After the four parent-binding fixes, PowerPoint rendered parent comment threads but SILENTLY DROPPED every reply (`interceptor macos find "Merging"` → 0; only empty Reply boxes). The advisor (Rule 2) correctly flagged reply-rendering as a logically independent concern none of the four parent fixes touched. Root cause is authoritative, not inference: the [MS-PPTX] CT_Comment schema (learn.microsoft.com/openspecs MS-PPTX, 2018/8/main) defines <xsd:complexType name="CT_Comment"><xsd:sequence> <xsd:group ref="EG_CommentAnchor"/> <!-- pc:sldMkLst --> <xsd:element name="pos" minOccurs="0"/> <xsd:element name="replyLst" type="CT_CommentReplyList" minOccurs="0"/> <xsd:group ref="EG_CommentProperties"/> <!-- txBody, extLst --> </xsd:sequence></xsd:complexType> replyLst MUST precede txBody. Our CT_ThreadedComment ZeroOrOne successors emitted txBody first, replyLst after — out of sequence, so PowerPoint validated the parent and discarded the misplaced replyLst. The reply element itself (CT_CommentReply = EG_CommentProperties only) was already correct; this was purely a child-ordering defect. Fix: reorder the successor tuples in src/pptx/oxml/comments.py so the emitted order is pc:sldMkLst, [pos], p188:replyLst, p188:txBody, [p188:extLst] — matching the schema sequence exactly. No API change. Two new regression assertions lock it (it_places_replyLst_before_txBody_per_ms_pptx_sequence, it_keeps_reply_txBody_with_bodyPr_after_reorder); red->green proven by stashing oxml/comments.py (the order test fails pre-fix). Verification: pytest 3761 passed; ruff check All checks passed; ruff format clean; behave 1062 scenarios / 3215 steps 0 failed. Regenerated UAT deck: replyLst-before-txBody True, reply texts present. Opened in real PowerPoint via Interceptor — AX tree now exposes the reply node "Comment from Alex Reviewer. Merging now." plus a "View 1 more reply" control (PowerPoint parsed the full reply list); pre-fix that count was zero. uat/FIXED_replies_rendering.png. Issue #25 now fully resolves: parent comments AND replies render in PowerPoint's Comments pane. UAT script runs clean — pending maintainer visual signoff in PowerPoint/Keynote. No push/PR per repo §6a. Refs #25.
1 parent ca4da3f commit 1d3af54

2 files changed

Lines changed: 58 additions & 3 deletions

File tree

src/pptx/oxml/comments.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -334,11 +334,23 @@ class CT_ThreadedComment(BaseOxmlElement):
334334
empty-Comments-pane root cause). Replies do not carry their own marker.
335335
"""
336336

337-
sldMkLst = ZeroOrOne("pc:sldMkLst", successors=("p188:txBody", "p188:replyLst", "p188:extLst"))
338-
txBody = ZeroOrOne("p188:txBody", successors=("p188:replyLst", "p188:extLst"))
337+
# Child order is FIXED by [MS-PPTX] CT_Comment (authoritative spec, the
338+
# contract PowerPoint implements):
339+
# <xsd:sequence>
340+
# EG_CommentAnchor (pc:sldMkLst) minOccurs=1
341+
# pos (a:CT_Point2D) minOccurs=0
342+
# replyLst (CT_CommentReplyList) minOccurs=0
343+
# EG_CommentProperties (txBody, extLst) minOccurs=1
344+
# </xsd:sequence>
345+
# i.e. replyLst MUST precede txBody. Emitting <p188:replyLst> AFTER
346+
# <p188:txBody> (the pre-2026-05-17 order) is out-of-sequence: PowerPoint
347+
# renders the parent comment but SILENTLY DROPS every reply. The successor
348+
# tuples below encode this exact order (pos omitted — optional, unused).
349+
sldMkLst = ZeroOrOne("pc:sldMkLst", successors=("p188:replyLst", "p188:txBody", "p188:extLst"))
339350
replyLst: "CT_ThreadedCommentReplyList | None" = ZeroOrOne( # pyright: ignore
340-
"p188:replyLst", successors=("p188:extLst",)
351+
"p188:replyLst", successors=("p188:txBody", "p188:extLst")
341352
)
353+
txBody = ZeroOrOne("p188:txBody", successors=("p188:extLst",))
342354

343355
id: str = RequiredAttribute( # pyright: ignore[reportAssignmentType]
344356
"id", XsdString

tests/test_issue25_comments.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,49 @@ def it_keeps_exactly_one_sldMkLst_when_set_twice(self):
758758
"set_slide_marker must be idempotent (exactly one <pc:sldMkLst>)"
759759
)
760760

761+
def it_places_replyLst_before_txBody_per_ms_pptx_sequence(self):
762+
# [MS-PPTX] CT_Comment sequence: EG_CommentAnchor, pos?, replyLst?,
763+
# EG_CommentProperties(txBody, extLst). replyLst MUST precede txBody.
764+
# Emitting replyLst AFTER txBody makes PowerPoint render the parent
765+
# comment but SILENTLY DROP every reply (issue #25, the reply defect).
766+
prs = Presentation()
767+
s = prs.slides.add_slide(prs.slide_layouts[1])
768+
c = s.comments.add("parent body", author="Rev")
769+
c.replies.add("a reply body", author="Rev2")
770+
buf = io.BytesIO()
771+
prs.save(buf)
772+
buf.seek(0)
773+
import zipfile
774+
775+
z = zipfile.ZipFile(buf)
776+
name = next(n for n in z.namelist() if "modernComment" in n)
777+
xml = z.read(name).decode()
778+
assert "replyLst" in xml, "expected a <p188:replyLst> for the reply"
779+
assert "a reply body" in xml, "reply text must serialize"
780+
assert xml.index("replyLst") < xml.index("txBody"), (
781+
"<p188:replyLst> must precede <p188:txBody> per [MS-PPTX] "
782+
"CT_Comment sequence, or PowerPoint drops every reply"
783+
)
784+
785+
def it_keeps_reply_txBody_with_bodyPr_after_reorder(self):
786+
# Guard the reorder didn't regress the reply body shape.
787+
prs = Presentation()
788+
s = prs.slides.add_slide(prs.slide_layouts[1])
789+
c = s.comments.add("p", author="Rev")
790+
c.replies.add("r", author="Rev2")
791+
buf = io.BytesIO()
792+
prs.save(buf)
793+
buf.seek(0)
794+
import re
795+
import zipfile
796+
797+
z = zipfile.ZipFile(buf)
798+
name = next(n for n in z.namelist() if "modernComment" in n)
799+
xml = z.read(name).decode()
800+
reply = re.search(r"<p188:reply\b.*?</p188:reply>", xml, re.S).group(0)
801+
assert "bodyPr" in reply
802+
assert reply.index("bodyPr") < reply.index("<a:p")
803+
761804

762805
if __name__ == "__main__":
763806
raise SystemExit(pytest.main([__file__, "-q"]))

0 commit comments

Comments
 (0)