Repository navigation
Diff inside an unchanged content control or custom-XML wrapper instead of replacing it whole - #1148
Merged
Merged
Conversation
…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.
This was referenced Oct 16, 2026
) 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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1143
Why
A block content control (
w:sdt) or block-levelw:customXmlelement 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:sdtPrandw:sdtEndPr, for custom XML its attributes andw: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'sModifyBlockas a new optionalWrapperOpslist (wrapperOpsin 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:
w:bookmarkStartbefore 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.IrSdtBlock.HasParagraphSpanningField). Every table of contents Word generates is exactly this: a content control around entry paragraphs, with oneTOCfield 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 whenIrSdtBlock.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, andSemanticDiffcompares 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.
GetRevisionsreports the children's revisions;SemanticDiffprojects the child changes and reports acontent_control.envelope/custom_xml.envelopechange 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 walksWrapperOps.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:namefrom two reviewers, which Word reads as one move with two sources.PlanMovesnow 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
BlockWrapperNestedDiffTests(each for bothw:customXmlandw:sdt), plus 2 inDocxDiffNoteReferencePairingTests. 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 withPAGEREFfields, theTOCfield'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 fixtureTestFiles/HC022-Table-Of-Contents.docxwith an entry removed and another edited, under the same checks; and the competitor text of a same-wrapper conflict.PAGEREFfield in the accept view); the 6 nested-marker tests and the 2 conflict-text tests failed before their changes.scripts/perf-run.sh origin/main), no case above base:compare/churncompare/heavycompare/rewriteconsolidateDocxDiff edit script + semantic changesredline -> HTML with tracked-change markup(All other cases are within ±0.1%.)
Adversarial review
Two rounds. The second, on the opened PR, changed:
TOCfield 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, onmain; 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.The first round, on the full diff before opening, changed:
numIdin 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.SemanticDiffstill reported an envelope change for any edit inside a content control; it now compares the shell, as it already did for custom XML.Known limits
mainbehaves 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.DocxComparefront 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 byDocxDiff.Compare.w:idandw: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 revisedw:sdtPrwhile reject restores the original one, which Word has no markup for.