Skip to content

Fix story splices into a main part with a root default namespace - #182

Open
hadim wants to merge 3 commits into
tensorbee:mainfrom
hadim:fix/story-splice-namespace-facts
Open

hadim wants to merge 3 commits into
tensorbee:mainfrom
hadim:fix/story-splice-namespace-facts

Conversation

@hadim

@hadim hadim commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • rdocx: before set_story_source_xml publishes a story splice into the
    main part, it runs unsafe_serializer_namespace_prefix, the check of the
    save path, on the root default, the root w, r and mc declarations and
    the w:body declarations of the part it replaces. A used root default, or
    one whose use cannot be classified, is refused with cannot serialize a modified document with a shadowed `default` namespace. A root w, r or
    mc prefix bound to another URI, and any declaration on w:body, are
    refused with the same error naming that prefix. An unused default, like the
    Google Docs task namespace, passes (157-3).
  • rdocx: after a main-part splice, set_story_source_xml refreshes
    root_namespace_declarations, body_namespace_declarations and
    body_namespace_bindings from the bytes it publishes, as
    flush_document_to_package already does (157-1).
  • Tests: three regression tests next to the F-X091 root default tests in
    regression_test.rs, and one Python test in test_core.py that builds the
    issue's file with zipfile only. HLD 04 now says which bindings a story
    splice checks, and that a splice refreshes the namespace facts.
  • The rdocx archive row is re-recorded.

Part of #157. The reported failure is fixed. Python add_picture now succeeds
on the issue's file and on fixture-report.docx from #158. What remains is
the add_picture column of the producer-traits matrix (157-4). It lands with
the acceptance matrices of #158 and #160. I ran that column locally on this
branch and every row passes, including "default namespace on the root" and
"inline content control on first runs".

Why

Python Document.add_picture calls insert_picture_to_story. For the body,
the story source is self.document.to_xml(), which never writes a root
xmlns. The picture is spliced into that canonical XML and published through
set_story_source_xml. That function replaced the part and the typed
document, but it kept the namespace facts of the part it had replaced. The
staged preparation then serializes the typed body again and parses the result
(canonicalize_drawing_ids). A content control does not come back unchanged
from that round trip, so the next flush saw a modified document. It checked
the stale facts, which still record xmlns, against the spliced bytes, which
no longer declare it. root_default_namespace_is_used returns None for that
mismatch, and the check refused. Without a content control the typed document
survives the round trip, so the check never runs. That is why the issue needs
both traits, and why Google Docs exports hit it.

The refresh alone would have opened a silent failure. The canonical story
source drops the root default and every w:body declaration, and binds the
root w, r and mc prefixes to their canonical URIs (CT_Document::from_xml
never records them in extra_namespaces). The save path refuses all three
cases for a modified document, but the splice path never ran that check. On
main, without a content control, every main-part splice on such a file is
accepted. <producer/> under xmlns="urn:used-default" ends up in no
namespace, <r:producer/> under xmlns:r="urn:not-relationships" ends up in
the relationships namespace, and <x:producer/> inside a run under
<w:body xmlns:x="urn:x"> is written with an unbound prefix, so the part is
not namespace well-formed. With a content control, the picture and append
paths were refused only by the stale facts, and insert_content was still
accepted. That is why 157-3 lands with 157-1, and why it comes first.

Notes

  • Commit order: the check first, then the refresh. No intermediate commit
    accepts a splice that main refuses. The check only adds refusals, and
    every refusal the refresh lifts is either the reported false positive or a
    case the check now refuses before publishing.
  • The fix is a local refresh in
    set_story_source_xml. I did not change flush_document_to_package to
    derive its inputs from the part bytes, because that flush runs behind every
    save. I did not route the body case through the typed body either, because
    that would leave table-cell and text-box splices broken and change the
    bytes of every Python add_picture output.
  • Scope of the root check. It covers exactly the root bindings the canonical
    source drops or rewrites: the default, w, r and mc. I considered the
    full root branch of unsafe_serializer_namespace_prefix, which typed saves
    use, and took the narrower set. The canonical root replays the producer's
    wp, a, pic, c, w14 and w15 values unchanged, and a spliced
    drawing declares its own namespaces, so a splice cannot rebind those
    prefixes. I checked this: with xmlns:a="urn:not-a" on the root, a picture
    splice writes <a:graphic xmlns:a="...drawingml/2006/main"> and the raw
    <a:producer/> keeps the producer binding. The full check would refuse
    those safe splices, which main accepts.
  • Body-level declarations are refused, the same way a modified save and
    insert_html_fragment into the body already refuse them. Leaving them out
    would also have been defensible. The canonical writer carries a
    body declaration onto some retained elements, such as a direct raw child of
    the body, but not onto a raw element nested in a paragraph or run. Review
    found that the refresh would otherwise have accepted a splice beside a
    content control on such a file and written <x:foo/> with no binding in
    scope, where main refused it through the stale facts. The cost is that a
    splice on a body-declared document whose output would have been correct
    (a direct raw child, or an attribute on w:p or w:r) is now refused too.
    None of the Production readiness for editing real docx and pptx files: an acceptance contract, two realistic fixtures and two matrices #158 fixtures or matrix rows declares anything on w:body. If
    such documents should keep accepting splices, the check would need to
    classify where each body binding is used, which is a larger change for both
    paths.
  • Behaviour change: a document whose root uses its default namespace, binds
    w, r or mc to another URI, or declares a namespace on w:body, now
    refuses every story splice into the main part (insert_content,
    import_fragment, remove_content_at, clone_content, move_content,
    insert_picture_to_story, and the appends of append_fragment_to_story
    such as add_hyperlink_to_story into a table cell), with the error typed
    edits already raise. On main these splices were accepted and changed the
    namespace of retained content, or left a prefix unbound, except the picture
    and append paths beside a content control, which the stale facts refused.
    insert_html_fragment is not in this list. Into the body or a main-part
    table cell it edits the typed body, and the typed flush already refused
    these documents on main. Its splice branch only serves headers and
    footers.
  • This includes a document that writes WordprocessingML itself unprefixed
    under a root default namespace. On main, insert_content and
    insert_picture_to_story were accepted on such a file even with a content
    control in the body. The saved part then held an unprefixed <sdt> under a
    root with no default namespace, so the content control lost its namespace.
    Such a file refused typed edits already. It now refuses splices too. If
    these documents should accept edits, that is a change to the classification
    of both paths, not to this one.
  • Same-class follow-ups, left alone because they fail the same way on main
    and are not touched by this change:
    • DocumentFragment::from_range slices the canonical story_sources() XML
      without this check. Exporting a range from a document with a used root
      default, then importing it, writes <producer/> in no namespace. The
      same check could run in from_range before it slices, but that changes a
      read-only export API.
    • A splice drops paragraph-level and run-level declarations.
      story_sources builds the main-part source with self.document.to_xml()
      and never runs replay_nested_namespace_declarations, which the save
      path does. With <w:p xmlns:x="urn:x"><w:r><x:foo/>... in the body,
      insert_picture_to_story writes <x:foo/> with no binding in scope,
      with or without a content control. Replaying the nested owners in the
      story source, or refusing when an owner would be lost, is its own change.
  • Left to a separate PR: 157-2, the whitespace growth of content controls
    under the indenting writer of CT_Document::to_xml. It is why a content
    control makes the typed document look modified after a round trip. With
    this PR the issue's file saves correctly, but whitespace inside
    w:sdt, w:sdtPr and w:sdtContent still grows on each rewrite. The fix
    depends on the Producer traits: a matrix over every operation, and what still fails in it and around it #160 section 2 decision about the indenting writer.
  • No public API changes. No binding or stub changes. The Python test helper
    _replace_document_body gained an optional root_default argument.

Tests

Added:

  • story_picture_splice_beside_content_control_ignores_unused_root_default
    (regression). It covers three cases: unused root default with a block
    goog_rdk_0 control, then the two controls (no default, no content
    control). Each case inserts a picture after the control (or after the
    matching paragraph), appends a picture and a hyperlink to a table-cell
    story, and appends a picture to the body. It asserts the exact body order
    before and after a save and reopen, with the control's tag and text intact.
    On main the first case fails with the issue's error.
  • rewritten_root_namespaces_block_story_splices_atomically (regression).
    Three roots: <producer/> under xmlns="urn:used-default",
    <r:producer/> under xmlns:r="urn:not-relationships" and
    <mc:producer/> under xmlns:mc="urn:not-compatibility", each with and
    without a block control. A body picture, a table-cell picture and an
    insert_content all fail with the shadowed error for that prefix, and the
    saved bytes are unchanged. On main every case without a control succeeds
    silently, and insert_content succeeds with a control too.
  • body_declarations_and_a_rebound_root_w_block_story_splices_atomically
    (regression). Three documents, each with and without a block control:
    <x:producer/> inside a run under <w:body xmlns:x="urn:x">, the same
    element as a direct child of the paragraph, and a q:-prefixed document
    whose root binds w to urn:producer around a <w:producer/>. A body
    picture and an insert_content fail with shadowed x or w, and the
    saved bytes are unchanged. I ran the same inputs against origin/main:
    without a control both splices succeed and the saved part loses the
    producer binding, and with a control the picture is refused while
    insert_content succeeds.
  • test_add_picture_beside_a_content_control_ignores_an_unused_root_default
    (test_core.py). It covers the after= form and the append form, with a
    paragraph after the control, and checks that the first picture lands
    before that paragraph and the second after it. It fails on main with the
    issue's error.

The test helpers use an issue_157_ prefix, so they do not read as F-157.

Run on this branch:

  • cargo fmt --all --check: clean.
  • cargo clippy -p rdocx --all-targets --all-features -- -D warnings and the
    same for rdocx-py: clean.
  • cargo test -p rdocx --no-fail-fast: regression_test 564 passed and 7
    ignored. The lib has 465 passed with 2 environment failures.
    integration_test has 314 passed with 5 environment failures. Doc-tests
    pass. cargo test -p rdocx-cli: passed.
  • The first commit alone passes the root default tests,
    rewritten_root_namespaces_block_story_splices_atomically and
    body_declarations_and_a_rebound_root_w_block_story_splices_atomically.
  • cargo test -p rdocx-py, with the interpreter's library on
    DYLD_LIBRARY_PATH: passed.
  • Python gate (maturin develop, Python 3.12, python-docx 1.2.0): 66 passed and
    2 environment failures in crates/rdocx-py/tests.
    mypy --strict on typing_smoke.py and the package, and
    mypy.stubtest rdocx, are clean.
  • python3 scripts/hash_harness.py --check: 49 entries match.
  • python3 scripts/prose_check.py: 0 violations.
  • python3 scripts/readme_doctests.py: passes after the re-record.
  • The issue's script: add_picture ok on both rows. On
    fixture-report.docx, add_picture plus save and reopen succeeds. The
    add_picture column of the producer-traits matrix passes on every row.

Environment-only failures, all pinned tool versions:

  • rdocx lib: large_word_and_presentation_pdfs_preserve_logical_reading_order
    and word_and_powerpoint_chart_pixels_are_identical (pdftotext and
    rasterizer 26.01 pinned, 26.09 here).
  • rdocx integration_test: odt_reader_matches_pinned_libreoffice_structure,
    public_authored_theme_and_fonts_match_pinned_word_resolution,
    sanitized_public_authoring_fixture_passes_every_conformance_stage, and
    also f269_section_page_semantics::section_page_semantics_match_pinned_libreoffice_render
    and f267_table_style_conditional_tests::every_conditional_table_region_matches_word.
    The last two assert LibreOffice 26.2.5.2, and this machine has 26.8.0.3.
    Both fail the same way on origin/main (9a7ed71).
  • Python: test_rendering_threads.py, both tests, which pin pdfinfo 26.01.0.

A story splice into the main part, such as insert_content or
insert_picture_to_story, edits the canonical XML that story_sources
builds with CT_Document::to_xml. That XML never declares a root default
namespace or a namespace on w:body, and always binds the root w, r and
mc prefixes to their canonical URIs. The splice therefore published the
main part without the producer's root default and body declarations,
and with those prefixes rebound. The save path refuses all three cases,
but the splice path never ran that check. Without a content control in
the body, every splice was accepted. An element such as <producer/>
under xmlns="urn:used-default", or <r:producer/> under a non-canonical
xmlns:r, silently changed namespace, and an <x:producer/> nested in a
run under <w:body xmlns:x="urn:x"> was written with an unbound prefix.
With a content control, the picture and append paths refused only by
accident, through stale namespace facts, and insert_content was still
accepted.

set_story_source_xml now runs unsafe_serializer_namespace_prefix, the
check of the save path, on the root default, the root w, r and mc
declarations and the w:body declarations of the part it replaces,
before it publishes a main-part splice. An unused default, like the
task namespace Google Docs declares, still passes. The other fixed root
prefixes stay out of the check because the canonical root replays their
producer values unchanged. Any w:body declaration refuses, as it does
for a modified save, because the canonical writer does not carry it onto
a raw element nested in a paragraph or run.

GitHub issue tensorbee#157.
Python Document.add_picture failed with "cannot serialize a modified
document with a shadowed `default` namespace" on a Google Docs export,
whose document.xml root declares an unused default namespace and whose
body holds a content control. add_picture calls insert_picture_to_story,
which splices the picture into canonical main-part XML without the root
default and publishes it through set_story_source_xml. That function
replaced the part and the typed document but kept the root and body
namespace facts of the part it replaced. The staged package preparation
then re-serializes the typed body, and a content control does not
survive that round trip unchanged, so the flush that follows ran its
namespace check with the stale facts against the spliced bytes. The
recorded default was missing from those bytes, a case the check cannot
classify, so it refused.

set_story_source_xml now refreshes the three facts from the bytes it
publishes, as flush_document_to_package does after a canonical
publication. Body, table-cell and text-box splices into the main part
all go through it. The previous commit refuses a used root default, a
root w, r or mc prefix bound to another URI and any w:body declaration
before a splice publishes, so the refresh cannot turn their accidental
refusal into a silent namespace change.

GitHub issue tensorbee#157.
The fail-closed check and the namespace facts refresh in
set_story_source_xml, with their regression tests, grow the rdocx
package, so the README archive row and its ARCHIVE_MEASUREMENTS entry
are re-measured.

GitHub issue tensorbee#157.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant