Fix Pandoc subdocument XML namespace handling - #658
Conversation
yangfan-yf-yf
left a comment
There was a problem hiding this comment.
Requesting changes for two blocking issues:
-
Subdoc._get_xml() now serializes each direct body child independently. lxml writes every inherited namespace declaration onto every child, including namespaces that are not used by that child or are already available from the destination document. In a CPython 3.12.3 / lxml 6.1.1 benchmark with a 10,000-paragraph dynamic subdocument, the intermediate fragment grew from 340,000 bytes at 1f143fb to 11,800,000 bytes at 62e0075; median render() time across three fresh runs grew from 0.083 s to 2.400 s. The same path affects every subdocument, not only Pandoc input. Please preserve only the missing or conflicting declarations needed by the inserted content and add a scale regression.
-
tests/pandoc_subdoc.py is not exercised by the current test job. The job invokes python tests/runtests.py from the repository root, but the runner globs [A-Za-z]*.py relative to the current directory, so it runs setup.py rather than the scripts under tests. It also ignores each subprocess.call() return code. Against 1f143fb, the new regression script exits 1 with the reported namespace error, while runtests.py still exits 0. Please discover tests relative to file and propagate child-process failures, or invoke this regression directly from the test job.
The namespace fix itself handles the reported document correctly: the Pandoc 3.9 reproduction fails at the base with the unbound a prefix and succeeds at this head with the image relationship, a/pic elements, nested section properties, and one final body sectPr intact. Custom default namespaces, prefixed attributes, and conflicting prefix mappings also remained resolved.
|
Thanks for the detailed reproduction. Both requested changes are addressed in
The follow-up also passes the related subdocument/header-footer checks, |
Why
Pandoc-generated DOCX files can declare DrawingML namespaces on the document
root rather than on each body child. Removing the serialized
w:bodywrapperthen leaves unbound prefixes in the inserted fragment, so Word-compatible
parsers cannot reopen the rendered document.
Root cause
The original implementation serialized the whole body and removed the wrapper,
including declarations required by descendants. The first fix serialized every
child independently, which preserved correctness but repeated inherited
namespace declarations on every child and caused severe fragment growth.
Change
conflicting declarations on inserted content
sectPrremoval behaviorsys.executable, propagate child failures, and emit a deterministicSHA-256 RUN/PASS ledger
Validation
regression with an 11,800,000-byte fragment
1,000,000-byte structural bound
relationship, DrawingML/PictureML nodes, namespace conflicts, and final
sectPrplacementsubdocument/header-footer checks, flake8, wheel/sdist builds, and archive
audits in both review phases
without a repair prompt and confirmed the image renders
Scope and risks
This changes body-fragment serialization and the tracked test launcher only.
It does not change public APIs, template syntax, dependencies, relationships,
fixtures, headers, footers, or footnotes. The launcher change ensures this PR's
regression is a real CI-discovered test; broader nested-command cleanup remains
separate from this issue. Remote GitHub Actions still require repository-side
approval to run.
Closes #620