Skip to content

fix(docx): read moved text, bidirectional spans and block-level custom XML - #4369

Open
L4XB wants to merge 2 commits into
docling-project:mainfrom
L4XB:fix/docx-moved-and-bidi-runs
Open

L4XB wants to merge 2 commits into
docling-project:mainfrom
L4XB:fix/docx-moved-and-bidi-runs

Conversation

@L4XB

@L4XB L4XB commented Sep 24, 2026

Copy link
Copy Markdown

The Word backend already reads the runs inside smart tags, custom XML, tracked insertions and simple fields (_iter_paragraph_content), and the paragraphs of block-level content controls (_walk_linear). It still drops three wrappers of ordinary content:

  • w:moveTo: the new position of text that was moved with track changes on. Word writes w:moveFrom at the old position and w:moveTo at the new one. w:moveFrom is correctly left out, but because w:moveTo is not read either, moved text disappears from the output altogether.
  • w:dir and w:bdo: a bidirectional embedding or override around runs, used in right-to-left text.
  • Block-level w:customXml: custom XML markup around body paragraphs and tables. Its inline form is already read.

A probe document with one marker in each place, through DocumentConverter:

marker main this PR
moved text (w:moveTo) – ✓
moved-away text (w:moveFrom), should stay out – –
run in w:dir – ✓
run in w:bdo – ✓
paragraph in block w:customXml – ✓
inline custom XML, smart tag, simple field, insertion, block content control ✓ ✓

Change:

  • moveTo, dir and bdo join the set _get_children_recursive looks through.
  • _walk_linear walks a block-level customXml the way it walks a content control's sdtContent. The branch sits next to the sdt one, before the image branches, for the reason the existing comment gives.

Tests (tests/test_backend_msword.py):

  • test_runs_in_moves_and_bidirectional_spans_are_read: a move-from/move-to pair, a w:dir span and a w:bdo span. It checks the exact text items, so the moved-away text must stay out.
  • test_paragraphs_and_tables_in_block_custom_xml_are_read: a paragraph and a two-cell table inside block-level custom XML, between two plain paragraphs.

Results:

  • Both tests fail on main. With the change, the four test_backend_msword*.py files pass: 76 passed, 1 skipped.
  • Five single mutations each fail one of the tests:
    • moveTo, dir or bdo dropped from the set;
    • the customXml branch removed;
    • moveFrom added to the set.
  • No DOCX under tests/data contains w:moveTo, w:dir, w:bdo or w:customXml; I checked all 275 word/*.xml parts. So no reference output changes.
  • ruff check and ruff format --check (0.15.12) are clean. ty check on msword_backend.py reports the same 12 diagnostics as on main.

Checklist:

  • Documentation has been updated, if necessary. Not needed.
  • Examples have been added, if necessary. Not needed.
  • Tests have been added, if necessary.

…m XML

The Word backend already reads runs inside smart tags, custom XML,
tracked insertions and simple fields, and the paragraphs of block-level
content controls. It dropped three more wrappers of ordinary content:

- w:moveTo, the new position of text moved with track changes on, so
  moved text disappeared from the output altogether (the old position,
  w:moveFrom, is correctly left out);
- w:dir and w:bdo, bidirectional embeddings and overrides around runs;
- block-level w:customXml around body paragraphs and tables.

The run recursion now includes moveTo, dir and bdo, and block-level
custom XML is walked like the content of a block-level content control.

Signed-off-by: L4XB <lukas.buck@e-mail.de>
@github-actions

Copy link
Copy Markdown
Contributor

✅ DCO Check Passed

Thanks @L4XB, all your commits are properly signed off. 🎉

@mergify

mergify Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@chrikrah chrikrah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply correct. Here is the falsification evidence the body does not carry.

Your tests pin the change, and they also catch a plausible wrong variant. Reverting production and keeping the tests fails exactly the two new ones:

2 failed, 65 passed, 1 skipped

And adding "moveFrom" to the skip set, which would resurrect moved-away text, fails test_runs_in_moves_and_bidirectional_spans_are_read. That is the half most likely to be got wrong, and it is pinned.

No committed ground truth needs regenerating. No .docx under tests/data contains any affected tag:

customXml -> 0 files     moveTo -> 0 files     moveFrom -> 0 files
w:bdo     -> 0 files     smartTag -> 0 files
w:dir     -> 1 file: tests/data/docx/sources/textbox.docx    [false positive]

That last hit is the attribute w:directFormattingOnRuns="1", not a <w:dir> element.

Seven probes at head, checking for leakage, double emission, nesting, and that deletions stay dropped:

customXmlPr leak       -> ['Before', 'In custom XML', 'After']   no placeholder or attribute leak
nested block customXml -> ['Before', 'deep']
moveTo nested in ins   -> ['A B']
w:del dropped          -> ['keep  tail']
empty block customXml  -> ['Before', 'After']

One hole left, and it is the same one-line pattern rather than something this introduced. w:customXml wrapping a w:tr or a w:tc still drops the whole row or cell, identically at base and head:

customXml around w:tr -> tables: [['R2C1', 'R2C2']]      row R1C1/R1C2 gone
customXml around w:tc -> tables: [['plain cell']]        'CX cell' gone

_walk_linear is what got the fix. _handle_tables iterates w:tr and w:tc directly and never sees through the wrapper. Since the title says block-level content, it is worth deciding whether that belongs here or in its own change.

ruff check and ruff format --check are clean at head.

@el-belgounetos

Copy link
Copy Markdown

HI,

Verified on a patched copy: body moveTo is fixed (a Word-made move, 12_move_word_real.docx, gives the text once). moveTo inside a table cell is not fixed by this PR alone (11_moveto_incell.docx), because plain cells go through _Cell.text and not _get_children_recursive. It is fixed only combined with #4455.

12_move_word_real.docx
11b_moveto_body.docx
11_moveto_incell.docx

@L4XB

L4XB commented Oct 5, 2026

Copy link
Copy Markdown
Author

@chrikrah thanks for the probes and the mutation check. On the hole you found: you've since filed it as #4382, and #4393 fixes it, row and cell wrappers alike. So I've kept it out of this PR, which stays on run- and paragraph-level content.

@el-belgounetos thanks for checking with real Word files. Your reading is right. _handle_tables takes a plain cell's text from cell.text. python-docx builds that from each paragraph's direct w:r / w:hyperlink children only, so a w:moveTo in a cell never reaches _get_children_recursive, and neither do the other wrappers. #4455 sends plain cells through _iter_paragraph_content, and that is where the moveTo / dir / bdo entries from this PR apply. With both PRs, a cell's moved text is read, as your 11_moveto_incell.docx shows.

The two changes touch different code and merge cleanly in either order. Once one of them lands, I'm happy to add a cell-level moveTo case to whichever is still open.

…di-runs

Signed-off-by: L4XB <lukas.buck@e-mail.de>

# Conflicts:
#	docling/backend/msword_backend.py
@L4XB

L4XB commented Oct 17, 2026

Copy link
Copy Markdown
Author

Merged main (6b4d22e). main now walks an sdt through _walk_optional_container, so block-level customXml shares that branch and that helper instead of keeping its own elif. As a separate branch it would have pushed _walk_linear to C901 31 > 30 after #4615.

The four test_backend_msword* files pass (293 passed, 1 skipped), and ruff 0.16.10 check and format are clean. Dropping "customXml" from the branch again fails test_paragraphs_and_tables_in_block_custom_xml_are_read.

@github-actions github-actions Bot added ai:ci-safe AI triage found no CI-safety concern (advisory) bug Something isn't working docx issue related to docx backend rtl-language right-to-left languages labels Oct 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

AI triage (advisory)

Checked commit 6b4d22e686. The analysis read the diff only. It did not run code from this pull request.

The PR makes the Word backend read text in w:moveTo, w:dir and w:bdo wrappers. It also makes the backend walk block-level w:customXml around body paragraphs and tables. Two new tests cover both changes.

Topic labels added: bug, docx, rtl-language.

CI safety: no concern found. A maintainer must still approve CI.

1 related pull request(s) in the same code
PR State Signal Assessment
#4587 open changes the same code related: Both PRs change _iter_paragraph_content in msword_backend.py, but they solve different problems. PR 4587 splits paragraphs at page and column breaks. This PR reads more wrapper elements.

The ai:* labels are advisory. They do not approve CI and they do not add tests:full. A new push runs this check again.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI first review (advisory): no blocker found

This review covers the full diff at 6b4d22e686. It read the code and did not run it.

The run-level change (moveTo, dir, bdo) is small and its test pins the moveFrom exclusion. The block-level customXml branch skips the textbox guard that sdt and tbl have. A textbox in custom XML is therefore read before the other paragraphs in the same wrapper. Check this point before merge.

Findings outside the diff lines

  • suggestion docling/backend/msword_backend.py:1188: Block customXml is not in the textbox skip set, so textbox text can move out of reading order
    The comment above this guard says that container elements that walk their own children must be skipped. Otherwise the descendant textbox XPath extracts the textboxes before the container's children are walked. This PR makes customXml such a container (elif tag_name in ("sdt", "customXml")), but the guard still lists only tbl and sdt.

    Trigger: a block-level w:customXml that holds a paragraph, then a paragraph with a textbox.

    Result: the XPath runs on the customXml element first. It emits the textbox content, then the children are walked. The textbox text lands before the earlier paragraph of the wrapper instead of after it. The processed_textbox_elements check prevents duplicates, but it does not keep the order.

    Fix: add "customXml" to the set at this line. Add a small test with a textbox in custom XML if you want to pin it. The two new tests do not cover textboxes.

A maintainer makes the merge decision. This review never approves the pull request.

@github-actions github-actions Bot added the ai:review-lgtm AI first review found no blocker or major issue (advisory) label Oct 17, 2026

@chrikrah chrikrah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@L4XB blocking: block-level customXml now walks its children but is missing from the textbox skip set at msword_backend.py:1188, so a textbox inside the wrapper comes out ahead of the paragraph before it. The github-actions review on this head flagged it, and it reproduces:

# 6b4d22e exported; body = START, [A, paragraph holding a w:txbxContent "TB", C], END
$ PYTHONPATH=$PWD python probe.py plain p_plain.docx
plain ['START', 'A', 'TB', 'C', 'END']
$ PYTHONPATH=$PWD python probe.py cx p_cx.docx      # same three wrapped in w:customXml
cx ['START', 'TB', 'A', 'C', 'END']
# with {"tbl", "sdt", "customXml"} at line 1188
cx ['START', 'A', 'TB', 'C', 'END']

# your diff (0e54294..6b4d22e) applied to main d01c9c0
$ PYTHONPATH=$PWD python -m pytest tests/test_backend_msword*.py -q
1 failed, 296 passed in 79.84s
# the failure is test_e2e_docx_conversions on docx_list_east_asian_num_fmt.docx; plain d01c9c0 fails it too
"moveTo" removed from the descend set -> test_runs_in_moves_and_bidirectional_spans_are_read FAILED

Everything else holds, and the diff still applies cleanly after the 20 commits main gained. Could you add "customXml" to that set with a textbox-in-wrapper test? I'd approve on that push, and then dolfim-ibm could take it into the next docx batch.

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

ai:ci-safe AI triage found no CI-safety concern (advisory) ai:review-lgtm AI first review found no blocker or major issue (advisory) bug Something isn't working docx issue related to docx backend rtl-language right-to-left languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants