Skip to content

Diff inside an unchanged content control or custom-XML wrapper instead of replacing it whole - #1148

Merged
JSv4 merged 11 commits into
mainfrom
fix/issue-1143
Oct 16, 2026
Merged

JSv4 merged 11 commits into
mainfrom
fix/issue-1143

Conversation

@JSv4

@JSv4 JSv4 commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Fixes #1143

Why

A block content control (w:sdt) or block-level w:customXml element is read as one block holding its child blocks. A comparison used to treat the whole wrapper as a single unit, so a one-word edit to a paragraph inside a contract clause wrapped in a content control produced the entire old wrapper struck through and the entire new wrapper inserted. The reviewer cannot see what actually changed, and every revision inside is reported as one big replacement.

How it works

  • When a wrapper is diffed in place. Each wrapper now carries a digest of its own markup (the "shell"): for a content control its w:sdtPr and w:sdtEndPr, for custom XML its attributes and w:customXmlPr. When the aligner pairs two wrappers of the same kind whose shells match, the edit-script builder aligns their child blocks exactly as it already does for the paragraphs inside a table cell, and stores the resulting child ops on the wrapper's ModifyBlock as a new optional WrapperOps list (wrapperOps in the edit-script JSON). If the shell changed, nothing new happens: the wrapper is still deleted and re-inserted whole, with Word's paired range markers.

  • When it is not. Two kinds of content are not carried by the child blocks alone, and a wrapper holding either is still replaced whole, as before, so that each copy keeps it:

    • Bookmarks or comment ranges that sit between blocks (a w:bookmarkStart before the first paragraph and its end after the last, say). They are not children of any paragraph, so rendering only the child ops would drop them. This covers a marker directly in the wrapper or in a table row or cell inside it (IrSdtBlock.HasBlockLevelMarkers). A bookmark inside a paragraph does not block the in-place diff.
    • A complex field that opens in one of the wrapper's paragraphs and closes in another (IrSdtBlock.HasParagraphSpanningField). Every table of contents Word generates is exactly this: a content control around entry paragraphs, with one TOC field whose begin character sits in the first entry and whose end sits paragraphs later. Field characters are zero-width scaffold in the paragraph token diff, so when an entry at the edge of that span is inserted or deleted, the begin or end character that moved into a neighbouring paragraph would be emitted without a mark, and accepting or rejecting the redline would leave the field with too many or too few end characters. Replacing the control whole keeps each copy's field intact. This is the behaviour before this PR for such controls; the in-place diff is simply not offered to them.

    The reader sets both flags in one pass over the wrapper's content (IrReader.ScanWrapperContent). It scans tables through to their cells but does not enter a nested block wrapper: that wrapper has flags of its own and is kept or replaced as one child of the outer one, so a bookmarked clause control inside a control around a whole contract no longer forces the outer control to be replaced whole. The builder diffs in place only when IrSdtBlock.CanDiffInPlace (same shell, neither flag on either side). The aligner still pairs on the shell alone (SameShell): two same-shell wrappers are the same control, and SemanticDiff compares their children even when the redline has to draw the pair as a whole replacement.

  • Rendering. The markup renderer writes one copy of the revised wrapper's shell, unmarked (it is identical on both sides), and renders the child ops inside it with the same routine table cells use. So inserted, deleted, moved and edited paragraphs and tables inside get their normal marks, and nested wrappers recurse. If either side's source element is unavailable it falls back to whole replacement.

  • Alignment. Two edited wrappers used to pair only when their content hash matched or they were the sole change in a region. Same-shell wrappers are now scored by the overlap of their content's word tokens, and leftover same-shell wrappers in a gap pair in document order (the same rule tables already use), so a heavily edited wrapper still diffs in place.

  • Other consumers. GetRevisions reports the children's revisions; SemanticDiff projects the child changes and reports a content_control.envelope / custom_xml.envelope change only when the shell changes (previously any edit inside a content control reported one). Every place that walks nested op lists (move-id renumbering, footnote-reference pairing, relocation pairing, numbering import, inserted-style collection, the revision source map) now also walks WrapperOps.

  • Consolidate. One reviewer editing inside a wrapper renders exactly like the two-way case. Two reviewers editing inside the same wrapper is still a block-level conflict (the children are not composed across reviewers, unlike table cells); a test covers it under every conflict policy and checks reject restores the base. Each competitor of such a conflict now carries the wrapper's resulting text (DocxDiffConflictCompetitor.ResultText), as a paragraph's or table's competitor does; it was empty.

  • Move names in consolidate (also a fix for an existing bug). Each reviewer's comparison numbers its moves independently, and consolidate re-numbered only moves of whole body blocks. Moves nested inside table cells (already, before this change), and now inside wrappers and notes, could therefore get the same w:name from two reviewers, which Word reads as one move with two sources. PlanMoves now walks every nested scope recursively (RewriteNestedMoveIds: table rows and cells, text boxes, wrapper children, notes) and gives each nested move a globally unique id; the walk copies nothing unless an id actually changes.

Validation

  • 87 test cases, from 21 parameterized tests, in BlockWrapperNestedDiffTests (each for both w:customXml and w:sdt), plus 2 in DocxDiffNoteReferencePairingTests. They check that accept equals the revised document and reject equals the original, block for block (each wrapper's exact properties, each paragraph's properties and formatted text), and cover: the issue's one-word case; a changed shell still replacing the whole wrapper; nested wrappers (including an inner wrapper changed inside an unchanged outer one); first/middle/last child inserted or deleted; a table inside a wrapper; two adjacent edited wrappers; a paragraph inserted before an edited wrapper; moves inside a wrapper and from the body into one; list numbering and new paragraph styles inside a wrapper; footnote references inside a wrapper; GetRevisions; the edit-script JSON round trip; SemanticDiff; consolidate with one reviewer, with two reviewers in different wrappers, two reviewers in the same wrapper under every policy, and two reviewers moving paragraphs in wrappers in different table cells, in different footnotes, and directly in different table cells; and unique move names across the body and a wrapper in a table cell for a single comparison; a bookmark and a comment range bracketing paragraphs between blocks, placed directly in the wrapper, in a nested wrapper and in a table cell inside it, each staying in the redline around its paragraphs (the comment-range cases check only the reject side of the round trip, see Accepting a whole-wrapper replacement moves the comment reference out of the wrapper #1157 under "Known limits"); a bookmark inside a paragraph still diffing in place; a bookmark between a nested wrapper's blocks leaving the outer wrapper diffed in place, whether the edit is beside the nested wrapper or inside it; a table of contents (Word's shape: hyperlinked entries with PAGEREF fields, the TOC field's end either in its own trailing paragraph or in the last entry) with an entry inserted first or last, deleted first, middle or last, or edited, 12 cases, each checking that the redline adds no validation error and that accept and reject each keep the same field begin, separate and end characters as the revised and original documents; the real fixture TestFiles/HC022-Table-Of-Contents.docx with an entry removed and another edited, under the same checks; and the competitor text of a same-wrapper conflict.
  • Each piece of the fix was reverted on its own to confirm its tests fail without it: builder, markup renderer, revision renderer, aligner, note-reference pairing, relocation pairing, numbering walk, style walk, consolidate move-id globalization, the note merge reading the rewritten scripts, and the between-block marker guard (all 12 marker cases fail without it; checking only the wrapper's direct children fails the 4 table-cell cases). The two "shell changed" tests pass before the fix as well; they pin existing behaviour. The table-of-contents tests were written against the branch before the field guard: 5 of the 12 synthetic cases and the real fixture failed (an unbalanced field in the reject view, or a tab moved inside a PAGEREF field in the accept view); the 6 nested-marker tests and the 2 conflict-text tests failed before their changes.
  • Full .NET suite passes; Release library build has 0 warnings; corpus baselines unchanged.
  • Allocation benchmark (scripts/perf-run.sh origin/main), no case above base:
case alloc base alloc head Δ
compare/churn 364.9 MiB 364.9 MiB -0.0%
compare/heavy 254.2 MiB 253.7 MiB -0.2%
compare/rewrite 499.6 MiB 499.6 MiB +0.0%
consolidate 586.7 MiB 586.7 MiB +0.0%
DocxDiff edit script + semantic changes 668.7 MiB 668.3 MiB -0.1%
redline -> HTML with tracked-change markup 307.7 MiB 292.5 MiB -4.9%

(All other cases are within ±0.1%.)

Adversarial review

Two rounds. The second, on the opened PR, changed:

  • Table-of-contents controls. A probe comparing a Word-shaped table of contents with an entry added or removed showed the in-place diff leaving the TOC field unbalanced after accept or reject, where the whole replacement before this PR kept it balanced. Such a wrapper (a field spanning its paragraphs) is now replaced whole again; see "When it is not". The same shapes fail identically at body level, outside any control, on main; that pre-existing defect is DocxDiff leaves a paragraph-spanning field unbalanced when the paragraph holding its end is inserted or deleted, and moves a tab into a PAGEREF field when a hyperlinked entry is edited #1156 and is not changed here.
  • Markers in a nested wrapper no longer force the enclosing wrapper to be replaced whole; the nested wrapper is kept or replaced as one child with its own markers.
  • A same-wrapper conflict's competitors carried empty text.
  • Stale comments in the consolidate merger (the move plan's map also holds nested groups; the note merge reads the planned scripts).
  • The test file was consolidated to shrink the diff: the three acceptance tests became one that runs both compare front doors and consolidate; the bookmark and comment-range tests became one parameterized test; the move tests became two, one for a move within or into a wrapper and one for two reviewers moving paragraphs in different scopes; the two-wrapper and two-cell fixtures share builders. Coverage is unchanged except that the comment-range cases gained a reject round-trip check, and trying the accept side as well found Accepting a whole-wrapper replacement moves the comment reference out of the wrapper #1157.

The first round, on the full diff before opening, changed:

  • A test found that two reviewers' moves inside wrappers shared a move name; that led to the recursive move-id globalization above, which also fixes the same collision for moves directly inside table cells and for wrappers inside notes.
  • The numbering import and the inserted-style collection did not walk wrapper children: kept list items inside a wrapper whose list had a different numId in the revised document were given a new list, and a new style used only inside a wrapper could be dropped from a paired paragraph. Both fixed, with tests.
  • SemanticDiff still reported an envelope change for any edit inside a content control; it now compares the shell, as it already did for custom XML.
  • Diffing in place dropped bookmarks and comment ranges between a wrapper's blocks, which the whole-wrapper copy keeps. Such a wrapper now keeps the whole replacement (see "When it is not" above).

Known limits

  • Two reviewers editing inside the same wrapper is a block conflict, not a per-child composition.
  • Accepting a redline that replaces a wrapper whole while a comment range sits between its blocks moves the comment reference to a paragraph before the wrapper and leaves an empty paragraph behind; rejecting is exact. This is how main behaves too (the comment's markers are collapsed to single survivors that land in different copies of the wrapper); tracked in Accepting a whole-wrapper replacement moves the comment reference out of the wrapper #1157.
  • A wrapper with a bookmark or comment range between its blocks, or with a field spanning its paragraphs (every Word table of contents), gets no finer diff than before: it is still replaced whole. A finer diff of a table of contents waits on DocxDiff leaves a paragraph-spanning field unbalanced when the paragraph holding its end is inserted or deleted, and moves a tab into a PAGEREF field when a hyperlinked entry is edited #1156.
  • Separately, and not changed here: a block-level bookmark is dropped between body blocks, and in a table cell it is kept but written before the cell's paragraphs; accepting revisions drops them too, which also means the DocxCompare front door (it accepts its inputs first) loses them before comparing. Tracked in Comparison drops bookmarks and comment ranges that sit between blocks #1149. The new marker tests therefore check the redline produced by DocxDiff.Compare.
  • Leftover same-shell custom-XML wrappers with identical shells pair by position, as tables do. Two unrelated clauses in identical custom-XML shells therefore render as one wrapper with deleted and inserted children rather than as two wrappers; both shapes accept and reject to the right documents, and a content control's shell (its id and tag) is its identity, so no similarity floor is applied.
  • The shell digest includes a control's w:id and w:showingPlcHdr, so a control whose id differs between the inputs, or whose placeholder text was replaced by typed content, is still replaced whole. Excluding them would need the renderer to emit the revised w:sdtPr while reject restores the original one, which Word has no markup for.

JSv4 added 8 commits October 15, 2026 08:53
…ng it whole (#1143)

A block content control (w:sdt) or block-level w:customXml element whose own
markup is unchanged is now diffed in place: the edit script's ModifyBlock
carries the wrappers' aligned child ops (WrapperOps, serialized as
wrapperOps), the markup renderer draws them inside one copy of the wrapper,
GetRevisions reports the children's revisions, and SemanticDiff projects
them. A changed wrapper (sdtPr/sdtEndPr, or customXml attributes and
customXmlPr) is still replaced whole. The aligner pairs same-shell wrappers
by content similarity and as a gap residue, and consolidate gives moves
inside wrappers global ids so two reviewers' moves keep distinct names.
… own markup changes (#1143)

Also covers two reviewers editing inside the same wrapper under every
conflict policy.
Moves inside table cells, textboxes, block wrappers and notes kept the id
each reviewer's comparison gave them, so two reviewers' nested moves could
share one w:name. PlanMoves now walks every nested scope recursively and
ApplyMovePlan rewrites them, and the note merge reads the rewritten scripts.
…1143)

The numbering import and the inserted-paragraph style collection did not
look inside block wrappers, so kept list items in a wrapper could start a
new list and a new style used only there could be dropped.
…ks (#1143)

The reader drops bookmarks and comment ranges that sit between blocks, so
diffing such a wrapper in place lost them, while the whole-wrapper copy
keeps them. The reader now records them per wrapper, at any depth outside
a paragraph, and the builder diffs in place only when neither side has
any.
JSv4 added 3 commits October 15, 2026 21:43
)

A block content control or customXml wrapper whose paragraphs are bridged by
one complex field (every table of contents Word generates) is replaced whole
again instead of diffed in place. Field characters are zero-width scaffold in
the paragraph token diff, so an entry inserted or deleted at the edge of the
span left the accept or reject view with an unbalanced field.

IrReader.ScanWrapperContent sets HasParagraphSpanningField and
HasBlockLevelMarkers in one pass over the wrapper's content: it reads tables
through to their cells and stops at a nested block wrapper, which answers for
its own markers, so a bookmarked clause control no longer forces the control
around it to be replaced whole. IrSdtBlock.CanDiffInPlace checks both flags
on both sides; the aligner still pairs on the shell alone so the semantic diff
compares the children.
…1143)

The competitors of a block conflict on a wrapper carried an empty ResultText
because BlockResultText and ContestedBlockText knew paragraphs and tables
only. AppendBlockText walks a wrapper's children, and the cells of a table
inside it, the same way. Also corrects the MovePlan and MergeNoteScopes
comments, which described the pre-globalization move map and the raw scripts.
…markers and conflict text

Merges the three acceptance tests into one that runs both compare front doors
and consolidate, the bookmark and comment-range tests into one parameterized
test, and the move tests into two; the two-wrapper and two-cell fixtures share
builders. Adds the table-of-contents cases (Word's shape, 12 synthetic cases
and the HC022 fixture, each checking that the redline adds no validator error
and that accept and reject keep every field character), markers inside a
nested wrapper, and the competitor text of a same-wrapper conflict.

The comment-range cases check only the reject round trip: accepting a
whole-wrapper replacement displaces the comment reference (issue #1157).
@JSv4
JSv4 merged commit f1802cc into main Oct 16, 2026
17 checks passed
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.

Diff inside an unchanged block wrapper (w:customXml / w:sdt) instead of replacing it whole

1 participant