Skip to content

Keep every child of a text box that replacement rewrites - #180

Open
hadim wants to merge 2 commits into
tensorbee:mainfrom
hadim:fix/text-box-rewrite-keeps-all-children
Open

hadim wants to merge 2 commits into
tensorbee:mainfrom
hadim:fix/text-box-rewrite-keeps-all-children

Conversation

@hadim

@hadim hadim commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Keep every child of a text box that replacement rewrites.
    rewrite_text_boxes (rdocx-oxml placeholder.rs) now handles each w:p
    of a w:txbxContent where it stands. A paragraph that the edit changes is
    re-serialised in place. A paragraph without a hit is copied through as read.
    Every other child is copied through verbatim with
    raw_xml::capture_element, or written as read when it is an empty element,
    whitespace, a comment or a processing instruction. The same loop now closes
    the text box with the end tag it read, and it returns an error when the part
    ends inside a text box.
  • Re-record the archive measurements of rdocx-oxml and rdocx, dated
    2026-09-27.

Part of #160. I found this defect while checking the acceptance of section 1
of #160 (every walker reaches content controls at every level). The issue does
not report it. Sections 1 to 4 of #160 remain open for other PRs.

Why

Text-box replacement works on the raw XML of the document part and of each
header and footer. The walker parsed only the w:p children of every
w:txbxContent and wrote only those paragraphs back. It consumed and dropped
every other child element, and it also dropped empty elements, whitespace and
comments. The document layer stores the rewritten part as soon as one text box
in it has a hit. So a single hit deleted the tables, block content controls,
bookmarks and empty paragraphs of every text box in that part, including the
text boxes without a hit, and reported success. try_replace_text,
replace_all, replace_regex, render_template and rdocx replace all go
through this walker, for the DrawingML shape and for its VML copy in
mc:Fallback.

Notes

  • Merge order with fix/toc-rebuild-unbound-word-prefix. On main, and still on
    this branch, a text-box run that carries a w: attribute fails its
    paragraph parse. Word writes w:rsidR and w:rsidRPr on runs by default.
    CT_P::from_xml names w in its default scope without binding it, so the
    parse returns root attribute prefix w is unbound. The walker then fails
    for the whole part, and the document layer ignores that error (if let Ok
    in replace_in_xml_parts and replace_regex_in_xml_parts). Every text box
    of that part is silently left unreplaced, even when the hit is in a sibling
    text box without such a run. So the data loss fixed here reached mostly text
    boxes without those attributes, as written by python-docx, Google Docs and
    LibreOffice. The TOC-prefix PR fixes that parse failure. Once it lands, the
    old walker would reach Word-authored text boxes too and delete their other
    children. It should therefore land together with this PR or after it,
    never alone before it.
  • The walker streams. Collecting an ordered list of paragraphs and raw
    children, then handing the paragraphs to the edit callback as a batch, would
    also work. The walker instead hands each paragraph to the callback when it
    reaches it and writes the result in place. Both callers were per-paragraph
    sums (replace_in_paragraphs, replace_regex_in_paragraphs) with no state
    across paragraphs. For replace_many_in_xml_part, each paragraph still sees
    the pairs in the same order, so the results are the same. The callback of
    the private rewrite_text_boxes becomes FnMut(&mut CT_P) -> usize. There
    is no public API change.
  • A paragraph is re-serialised only when the callback returns a non-zero
    count. replace_in_paragraph and replace_regex_in_paragraph touch nothing
    before their first match, so a zero count means an unchanged paragraph, and
    its captured bytes are written instead. This was suggested in review. It
    keeps the start-tag attributes that CT_P does not model (w:rsidR,
    w14:paraId, w14:textId, local namespace declarations) on every
    paragraph without a hit, in the edited text box and in its siblings.
  • Behaviour changes, limited to the parts (document, header, footer) in which
    a replacement hits at least one text box. Every text box of such a part is
    walked, with or without a hit of its own:
    • Tables, content controls, bookmarks, empty paragraphs, comments and any
      other child now survive, byte for byte and in document order.
    • Paragraphs without a hit now come back byte for byte. Before, they were
      re-serialised and lost their start-tag attributes.
    • Whitespace between the children of a pretty-printed text box is now kept.
      It was dropped before.
    • The end tag is the one read from the part. The old code always wrote
      </w:txbxContent>, which did not match a start tag under another prefix.
      Edited paragraphs are still written with the w prefix, though. So a
      text box in a part that binds WordprocessingML only as the default
      namespace stays ill-formed after an edit, as it is on main. That is left
      to the namespace prefix work.
    • A part that ends inside a text box is now an error. quick-xml 0.41 returns
      Eof again and again with elements still open, and the old inner loop
      ignored it, so the call never returned. The new loop needs an Eof arm in
      any case. The document layer already ignores an error from this walker and
      leaves the part untouched.
    • A text box nested inside another one is kept as it is and not edited.
      Before, one nested in a table or a content control of the outer text box
      was deleted with it, and one nested in a paragraph was not edited either.
  • Still as on main: every paragraph is parsed with CT_P::from_xml, and an
    edited one is written with CT_P::to_xml. The walker reads past the w:p
    start tag before parsing, so an edited paragraph loses its start-tag
    attributes (rsid, w14:paraId, w14:textId). No PR of this batch
    addresses that. How an edited paragraph serialises otherwise is unchanged
    here (xml:space is in fix/compare-producer-noise).
  • Known adjacent defect, not fixed here. Once a paragraph has a hit,
    replace_in_paragraph and replace_regex_in_paragraph drop every run whose
    modelled content is empty. A run whose only child is a text box in
    mc:AlternateContent or a w:pict shape keeps that child in extra_xml,
    so it is dropped too. A hit in a body paragraph therefore deletes a text box
    anchored in the same paragraph, and a hit in a text-box paragraph deletes a
    shape run next to it. main behaves the same. This belongs with the run
    removal of the parallel PR on fix/replace-reaches-content-controls, which
    should remove only the runs that the replacement itself emptied and that
    carry no raw children (extra_xml, alt_drawings).
  • Deliberately left out: text inside a table or a block content control of a
    text box is still not replaced. It is now kept instead of deleted. Reaching
    it needs a typed parse that re-serialises the table or the control, which
    belongs with replacement inside content controls (section 1 of Producer traits: a matrix over every operation, and what still fails in it and around it #160) and
    with typed replacement of block content in headers and text boxes. No
    content-control replacement falls out of this change for free. An inline
    content control in a text-box paragraph behaves as it does in the body,
    which is the gap that section 1 of Producer traits: a matrix over every operation, and what still fails in it and around it #160 describes.
  • The code change stays inside placeholder.rs, away from the text.rs change
    of fix/toc-rebuild-unbound-word-prefix. fix/compare-producer-noise also
    edits placeholder.rs, in the run-splitting functions and at another spot
    of the test module.
  • The two re-measured archive rows are dated 2026-09-27 through
    ARCHIVE_REMEASUREMENT_DATES, as the latest re-measurements on main do.
    The rdocx row is also re-measured by the other PRs of this batch that grow
    rdocx, so it needs one fresh measurement when they are integrated.
  • The hash harness matches 49 of 49. No sample has a text box that a
    replacement hits.

Tests

Added:

  • rdocx-oxml placeholder.rs unit tests:
    • replace_in_textbox_keeps_every_other_child_in_order: a VML text box
      whose two placeholder paragraphs sit among a paragraph without a hit, a
      table, a block content control, a bookmark pair around an empty
      paragraph, an XML comment and whitespace, then a second text box with a
      table and a paragraph but no placeholder. Both paragraphs without a hit
      carry w:rsidR, w14:paraId and w14:textId. Both the plain and the
      regex walker must return the input part with only the placeholders
      replaced, so the second text box and the attributes come back unchanged.
    • replace_in_textbox_closes_it_with_its_own_prefix: a q:txbxContent
      comes back well formed and otherwise unchanged.
    • replace_in_unterminated_textbox_is_an_error: two truncated parts return
      an error instead of hanging.
  • rdocx regression_test.rs: mod text_box_replacement_keeps_every_child,
    placed next to the replacement termination tests. It runs
    try_replace_text, replace_regex and render_template over two forms: the
    DrawingML shape in mc:Choice with its VML copy in mc:Fallback, and a
    bare wp:anchor. Each text box holds a paragraph with the placeholder, a
    table, a block content control and a bookmark pair around an empty
    paragraph. Each test checks the count, and that every saved text box holds
    the edited paragraph and every other child, byte for byte and in order.

Each new test that can run on 9a7ed71 failed there without the fix: the
three regression tests and the first two unit tests. The truncated-input unit
test was not run there, since the old loop never returns. With the fix in
place but paragraphs without a hit re-serialised as before, the first unit
test fails on the dropped attributes.

End to end, with the debug CLI: a python-docx script builds both text box
forms with the same children, plus a paragraph without a hit that carries
w:rsidR and w14:paraId, and runs rdocx replace -p {{name}} -v Ada. On
the old walker, every saved text box held the edited paragraph alone. With
this branch, both text boxes keep every child, the attributed paragraph comes
back byte for byte, and python-docx reopens the output. A probe on this
branch confirms the parse failure described in the first note: a text box
run with w:rsidR makes replace_in_xml_part return the unbound prefix
error, also when the hit is in a sibling text box.

Run on macOS arm64, debug build, at the head of the branch:

  • cargo fmt --all --check and
    cargo clippy -p rdocx-oxml -p rdocx --all-targets --all-features -- -D warnings:
    clean.
  • cargo test -p rdocx-oxml: 546 passed, plus 1 doctest.
  • cargo test -p rdocx --no-fail-fast: regression 564 passed, doctests 2
    passed. Lib and integration have only environment failures (listed below).
  • cargo test -p rdocx-cli: 19 passed.
  • python3 scripts/hash_harness.py --check: 49 entries match.
  • python3 scripts/prose_check.py: 0 violations.
  • readme_doctests.validate_inventory(): 27 READMEs and 22 package
    inventories validated with the re-recorded rows.
  • No Python binding or stub changed, so the Python gate was not run.

Environment failures seen here:

  • rdocx lib: large_word_and_presentation_pdfs_preserve_logical_reading_order
    and word_and_powerpoint_chart_pixels_are_identical. These need the pinned
    Poppler.

  • rdocx integration, pinned LibreOffice 26.2.5.2 against 26.8.0.3 installed
    here:

    • odt_reader_matches_pinned_libreoffice_structure
    • public_authored_theme_and_fonts_match_pinned_word_resolution
    • every_conditional_table_region_matches_word
    • section_page_semantics_match_pinned_libreoffice_render

    I confirmed that the last two fail the same way on origin/main.

  • rdocx integration: sanitized_public_authoring_fixture_passes_every_conformance_stage,
    which needs an offline cargo run.

Replacement in text boxes walks the raw XML of the document part and
of each header and footer, parses the w:p children of every
w:txbxContent and writes those paragraphs back. Every other child was
skipped on read and never written. The document layer stores the
rewritten part as soon as one text box in it has a hit, so a single
hit deleted the tables, block content controls, bookmarks and empty
paragraphs of every text box in that part. try_replace_text,
replace_all, replace_regex and render_template all go through this
walker, for the DrawingML and the VML copy of a text box alike.

The walker now edits each paragraph in place and copies every other
child through verbatim, so each text box keeps its content in its
order. Only a paragraph that the edit changed is re-serialised. The
others are copied as read, so they also keep the start-tag attributes
that CT_P does not model, such as w:rsidR and w14:paraId. Two defects
of the same loop are fixed with it. The closing tag is the one read
from the part instead of a fixed w:txbxContent, which did not match a
start tag under another prefix. A part that ends inside a text box is
now an error instead of an endless read.

GitHub issue tensorbee#160.
The text box fix grows the rdocx-oxml sources and the rdocx regression
tests, and both are packaged, so the crates.io archive rows of the two
READMEs and their ARCHIVE_MEASUREMENTS entries are re-measured. Both
rows are dated 2026-09-27 through ARCHIVE_REMEASUREMENT_DATES.

GitHub issue tensorbee#160.

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