diff --git a/packages/docx-compare/src/tagged/nativeMoveParagraphParity.test.ts b/packages/docx-compare/src/tagged/nativeMoveParagraphParity.test.ts new file mode 100644 index 00000000..4aff7a1d --- /dev/null +++ b/packages/docx-compare/src/tagged/nativeMoveParagraphParity.test.ts @@ -0,0 +1,135 @@ +import { describe, expect } from 'vitest'; +import { testAllure } from '../testing/allure-test.js'; +import { parseXml, serializeXml, acceptChanges, rejectChanges, DocxArchive, buildSyntheticDocx } from '@usejunior/docx-core'; +import { compareDocuments } from '../index.js'; +import { acceptAllChanges, rejectAllChanges, extractTextWithParagraphs } from './trackChangesAcceptorAst.js'; + +const test = testAllure.epic('Document Comparison').withLabels({ feature: 'Native move paragraph parity' }) + .conformance({ spec: 'ECMA-376', edition: 5, part: 1, section: '17.13.5.22' }); +const W = 'http://schemas.openxmlformats.org/wordprocessingml/2006/main'; +const attrs = 'w:author="Comparator" w:date="2026-09-17T00:00:00Z"'; +const source = `` + + `` + + `Moved paragraph`; +const destination = `` + + `` + + `Moved paragraph`; +const stable = 'Stable paragraph'; +const wrap = (body: string) => `${body}`; +const texts = (xml: string) => Array.from(parseXml(xml).getElementsByTagNameNS(W, 'p')).map(p => + Array.from(p.getElementsByTagNameNS(W, 't')).map(t => t.textContent).join('')); + +describe('complete move ranges through native paragraph-mark resolution', () => { + for (const [name, body] of [ + ['middle', source + destination + stable], + ['terminal destination', source + stable + destination], + ['terminal source', destination + stable + source], + ['table cell', '' + source + destination + stable + ''], + ['terminal table cell', '' + source + stable + destination + ''], + ]) { + for (const [operation, nativeProject, astProject] of [ + ['Accept', acceptChanges, acceptAllChanges], ['Reject', rejectChanges, rejectAllChanges], + ] as const) { + test(`${operation}: ${name} agrees with AST without empty moved containers`, () => { + const input = wrap(body!); + const doc = parseXml(input); + nativeProject(doc); + const output = serializeXml(doc); + expect(texts(output)).toEqual(texts(astProject(input))); + expect(texts(output).filter(t => t === 'Moved paragraph')).toHaveLength(1); + expect(texts(output).filter(t => t === '')).toHaveLength(0); + expect(output).not.toContain('moveFrom'); + expect(output).not.toContain('moveTo'); + }); + } + } + + test('selective native resolution leaves foreign complete move endpoints and boundaries untouched', () => { + const input = wrap(source + stable + destination); + for (const project of [acceptChanges, rejectChanges]) { + const doc = parseXml(input); + const before = serializeXml(doc); + project(doc, { filter: e => e.getAttributeNS(W, 'author') === 'AI' }); + expect(serializeXml(doc)).toBe(before); + } + }); + + test('content-only move wrappers retain untracked paragraph containers', () => { + const input = wrap((source + stable + destination).replace(/.*?<\/w:pPr>/g, '')); + for (const [nativeProject, astProject] of [[acceptChanges, acceptAllChanges], [rejectChanges, rejectAllChanges]] as const) { + const doc = parseXml(input); + nativeProject(doc); + expect(texts(serializeXml(doc))).toEqual(texts(astProject(input))); + expect(doc.getElementsByTagNameNS(W, 'p').length).toBe(3); + } + }); + + for (const movedContent of [false, true]) { + test(`Reject preserves local bookmarks around ${movedContent ? 'mixed moved and' : 'only'} untracked surviving content`, () => { + const surviving = 'Untracked survivor'; + const to = (movedContent ? destination : destination.replace(//, '')) + .replace('', `${surviving}`); + const input = wrap(source + to + stable); + for (const project of [(xml: string) => { const doc = parseXml(xml); rejectChanges(doc); return serializeXml(doc); }, rejectAllChanges]) { + const output = parseXml(project(input)); + expect(Array.from(output.getElementsByTagNameNS(W, 'bookmarkStart')).map(b => b.getAttributeNS(W, 'name'))).toEqual(['keepme']); + expect(Array.from(output.getElementsByTagNameNS(W, 'bookmarkEnd')).map(b => b.getAttributeNS(W, 'id'))).toEqual(['90']); + expect(texts(serializeXml(output))).toEqual(['Moved paragraph', 'Untracked survivorStable paragraph']); + } + }); + } + + test('selective resolution resolves one author while preserving another complete move', () => { + const foreign = (source + stable + destination).replaceAll('Comparator', 'Human') + .replaceAll('move1', 'move2').replace(/w:id="(\d+)"/g, (_, id: string) => `w:id="${Number(id) + 10}"`); + for (const project of [acceptChanges, rejectChanges]) { + const doc = parseXml(wrap(source + stable + destination + foreign)); + const before = Array.from(doc.getElementsByTagNameNS(W, 'p')).slice(3).map(p => p.toString()); + project(doc, { filter: e => e.getAttributeNS(W, 'author') === 'Comparator' }); + const remaining = Array.from(doc.getElementsByTagNameNS(W, 'p')); + expect(remaining.slice(-3).map(p => p.toString())).toEqual(before); + expect(remaining.length).toBe(5); + expect(Array.from(doc.getElementsByTagNameNS(W, 'moveTo')).map(e => e.getAttributeNS(W, 'author'))).toEqual(['Human', 'Human']); + expect(Array.from(doc.getElementsByTagNameNS(W, 'moveFrom')).map(e => e.getAttributeNS(W, 'author'))).toEqual(['Human', 'Human']); + } + }); + + for (const terminal of [false, true]) { + test(`resolves local endpoint bookmarks without relocating removed ${terminal ? 'terminal' : 'middle'} pairs`, () => { + const bookmarked = (xml: string, id: number) => xml.replace('', ``) + .replace('', ``); + const from = bookmarked(source, 21); + const to = bookmarked(destination, 22); + const input = wrap(terminal ? from + stable + to : from + to + stable); + for (const [project, astProject, expectedId] of [[acceptChanges, acceptAllChanges, '22'], [rejectChanges, rejectAllChanges, '21']] as const) { + const doc = parseXml(input); + project(doc); + const ast = parseXml(astProject(input)); + for (const kind of ['bookmarkStart', 'bookmarkEnd']) { + const ids = (d: Document) => Array.from(d.getElementsByTagNameNS(W, kind)).map(e => e.getAttributeNS(W, 'id')); + expect(ids(doc)).toEqual([expectedId]); + expect(ids(doc)).toEqual(ids(ast)); + } + expect(texts(serializeXml(doc))).toEqual(texts(serializeXml(ast))); + } + }); + } + + test('native Accept/Reject resolve actual comparison-authored terminal moves to source paragraphs', async () => { + const moving = 'the complete movable paragraph changes its position in the document'; + const anchored = 'the long stable anchor paragraph remains unchanged in its position'; + const finish = 'the second stable anchor paragraph remains unchanged throughout'; + const build = (values: string[]) => buildSyntheticDocx({ paragraphs: values }); + const original = await build([moving, anchored, finish]); + const revised = await build([anchored, finish, moving]); + const result = await compareDocuments(original, revised, { detectMoves: true }); + const xml = await (await DocxArchive.load(result.document)).getDocumentXml(); + expect(xml).toContain('moveToRangeStart'); + for (const [project, control] of [[acceptChanges, revised], [rejectChanges, original]] as const) { + const doc = parseXml(xml); + project(doc); + expect(extractTextWithParagraphs(serializeXml(doc))) + .toBe(extractTextWithParagraphs(await (await DocxArchive.load(control)).getDocumentXml())); + } + }); +}); diff --git a/packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts b/packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts index b671fb27..d2dc1ebb 100644 --- a/packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts +++ b/packages/docx-compare/src/tagged/trackChangesAcceptorAst.ts @@ -725,6 +725,8 @@ export function rejectAllChanges(documentXml: string): string { for (const paragraph of markInsertedParagraphs) { if (!paragraphHasParaMarker(paragraph, 'w:moveTo')) continue; const direct = childElements(paragraph); + const substantive = direct.filter(child => !['w:pPr', 'w:bookmarkStart', 'w:bookmarkEnd'].includes(child.tagName)); + if (substantive.length === 0 || !substantive.every(child => ['w:ins', 'w:moveTo'].includes(child.tagName))) continue; const starts = direct.filter((child) => child.tagName === 'w:bookmarkStart'); const ends = direct.filter((child) => child.tagName === 'w:bookmarkEnd'); for (const start of starts) { diff --git a/packages/docx-core/src/primitives/accept_changes.ts b/packages/docx-core/src/primitives/accept_changes.ts index da247899..18398fdf 100644 --- a/packages/docx-core/src/primitives/accept_changes.ts +++ b/packages/docx-core/src/primitives/accept_changes.ts @@ -363,7 +363,7 @@ export function acceptChanges( }; } - // Phase A — Identify paragraphs whose MARK is a tracked deletion + // Phase A — Identify deleted or moved-from paragraph marks. const markDeletedParagraphs: Element[] = []; const allParagraphs = collectByLocalName(root, 'p'); // Capture selected direct mark/property histories before other phases remove @@ -382,7 +382,7 @@ export function acceptChanges( // accept. safe-docx's deleted paragraphs always carry the mark now, so the // mark-based rule suffices and is Word-faithful. (Mirrors acceptAllChanges and the // reject-side rule.) - if (paragraphHasParaMarker(p, 'del', filter)) { + if (paragraphHasParaMarker(p, 'del', filter) || paragraphHasParaMarker(p, 'moveFrom', filter)) { markDeletedParagraphs.push(p); } } @@ -392,7 +392,7 @@ export function acceptChanges( // text. Accepting the deletion must remove that live original-side endpoint // as well; otherwise a cross-paragraph range becomes orphaned. const deletedBookmarkIds = new Set(); - for (const deletion of collectByLocalName(root, 'del').filter(filter)) { + for (const deletion of [...collectByLocalName(root, 'del'), ...collectByLocalName(root, 'moveFrom')].filter(filter)) { for (const localName of ['bookmarkStart', 'bookmarkEnd']) { for (const boundary of collectByLocalName(deletion, localName)) { const id = boundary.getAttributeNS(W_NS, 'id') ?? boundary.getAttribute('w:id'); @@ -405,7 +405,8 @@ export function acceptChanges( .filter((child): child is Element => child.nodeType === 1); const substantive = direct.filter((child) => !isW(child, 'pPr') && !isW(child, 'bookmarkStart') && !isW(child, 'bookmarkEnd')); - if (substantive.length === 0 || !substantive.every((child) => isW(child, 'del') && filter(child))) { + if (substantive.length === 0 || !substantive.every((child) => + (isW(child, 'del') || isW(child, 'moveFrom')) && filter(child))) { continue; } for (const boundary of direct.filter((child) => diff --git a/packages/docx-core/src/primitives/native_move_paragraph_marks.test.ts b/packages/docx-core/src/primitives/native_move_paragraph_marks.test.ts new file mode 100644 index 00000000..4525b62a --- /dev/null +++ b/packages/docx-core/src/primitives/native_move_paragraph_marks.test.ts @@ -0,0 +1,113 @@ +import { describe, expect } from 'vitest'; +import { testAllure } from '../testing/allure-test.js'; +import { parseXml } from './xml.js'; +import { acceptChanges } from './accept_changes.js'; +import { rejectChanges } from './reject_changes.js'; + +const W = 'http://schemas.openxmlformats.org/wordprocessingml/2006/main'; +const test = testAllure.epic('Document Comparison').withLabels({ feature: 'Native move paragraph marks' }) + .conformance( + { spec: 'ECMA-376', edition: 5, part: 1, section: '17.13.5.21' }, + { spec: 'ECMA-376', edition: 5, part: 1, section: '17.13.5.22' }, + { spec: 'ECMA-376', edition: 5, part: 1, section: '17.13.5.25' }, + { spec: 'ECMA-376', edition: 5, part: 1, section: '17.13.5.26' }, + ); +// Complete range/mark/content shapes are the subjects of these consumer tests. +const endpoint = (kind: 'moveFrom' | 'moveTo', base: number, body: string, bookmark = true) => + '' + + '' + + (bookmark ? '' : '') + + body + + (bookmark ? '' : '') + + ''; +const content = (kind: 'moveFrom' | 'moveTo', id: number, author = 'AI') => + 'MOVED'; +const stable = 'STABLE'; +const document = (body: string) => parseXml('' + body + ''); +const texts = (doc: Document) => Array.from(doc.getElementsByTagNameNS(W, 'p')).map(p => + Array.from(p.getElementsByTagNameNS(W, 't')).map(t => t.textContent).join('')); + +describe('native move-mark resolution without comparison coverage indirection', () => { + for (const terminal of [false, true]) { + for (const [name, resolve, expectedBookmark] of [ + ['accept', acceptChanges, '12'], ['reject', rejectChanges, '3'], + ] as const) { + test(name + ' resolves ' + (terminal ? 'terminal' : 'middle') + ' move marks and local bookmark pairs', () => { + const from = endpoint('moveFrom', 1, content('moveFrom', 4)); + const to = endpoint('moveTo', 10, content('moveTo', 13)); + const doc = document(terminal ? from + stable + to : from + to + stable); + resolve(doc); + expect(texts(doc)).toEqual(name === 'accept' && terminal ? ['STABLE', 'MOVED'] : ['MOVED', 'STABLE']); + for (const kind of ['bookmarkStart', 'bookmarkEnd']) { + expect(Array.from(doc.getElementsByTagNameNS(W, kind)).map(n => n.getAttributeNS(W, 'id'))) + .toEqual([expectedBookmark]); + } + for (const kind of ['moveFrom', 'moveTo', 'moveFromRangeStart', 'moveFromRangeEnd', 'moveToRangeStart', 'moveToRangeEnd']) { + expect(doc.getElementsByTagNameNS(W, kind)).toHaveLength(0); + } + }); + } + } + for (const [name, body] of [ + ['empty', ''], + ['untracked', 'KEPT'], + ['foreign move', content('moveTo', 13, 'Human')], + ['mixed surviving content', content('moveTo', 13) + 'KEPT'], + ]) { + test('reject preserves local bookmarks around ' + name + ' content', () => { + const doc = document(endpoint('moveTo', 10, body!) + stable); + // Select id-only ends explicitly; author-only orphan ends remain in #941. + rejectChanges(doc, { filter: e => e.getAttributeNS(W, 'author') === 'AI' || e.localName.endsWith('RangeEnd') }); + expect(Array.from(doc.getElementsByTagNameNS(W, 'bookmarkStart')).map(n => n.getAttributeNS(W, 'id'))).toEqual(['12']); + expect(Array.from(doc.getElementsByTagNameNS(W, 'bookmarkEnd')).map(n => n.getAttributeNS(W, 'id'))).toEqual(['12']); + expect(texts(doc)).toEqual([name === 'foreign move' ? 'MOVEDSTABLE' : name === 'empty' ? 'STABLE' : 'KEPTSTABLE']); + expect(doc.getElementsByTagNameNS(W, 'moveTo')).toHaveLength(name === 'foreign move' ? 1 : 0); + }); + } + test('reject consumes local bookmark pairs around fully selected insertion content at a moved break', () => { + const doc = document(endpoint('moveTo', 10, 'INSERTED') + stable); + rejectChanges(doc); + expect(texts(doc)).toEqual(['STABLE']); + expect(doc.getElementsByTagNameNS(W, 'bookmarkStart')).toHaveLength(0); + expect(doc.getElementsByTagNameNS(W, 'bookmarkEnd')).toHaveLength(0); + }); + for (const [name, body] of [ + ['empty', ''], + ['untracked', 'KEPT'], + ['foreign move', content('moveFrom', 4, 'Human')], + ['mixed surviving content', content('moveFrom', 4) + 'KEPT'], + ]) { + test('accept preserves local bookmarks around ' + name + ' content', () => { + const doc = document(endpoint('moveFrom', 1, body!) + stable); + // Select id-only ends explicitly; author-only orphan ends remain in #941. + acceptChanges(doc, { filter: e => e.getAttributeNS(W, 'author') === 'AI' || e.localName.endsWith('RangeEnd') }); + for (const kind of ['bookmarkStart', 'bookmarkEnd']) { + expect(Array.from(doc.getElementsByTagNameNS(W, kind)).map(n => n.getAttributeNS(W, 'id'))).toEqual(['3']); + } + expect(texts(doc)).toEqual([name === 'foreign move' ? 'MOVEDSTABLE' : name === 'empty' ? 'STABLE' : 'KEPTSTABLE']); + expect(doc.getElementsByTagNameNS(W, 'moveFrom')).toHaveLength(name === 'foreign move' ? 1 : 0); + }); + } + for (const [kind, resolve] of [['moveFrom', acceptChanges], ['moveTo', rejectChanges]] as const) { + test('consumes a wrapper-nested ' + kind + ' bookmark and its live counterpart', () => { + const nested = '' + + 'MOVED'; + const following = 'STABLE'; + const doc = document(endpoint(kind, 1, nested, false) + following); + resolve(doc); + expect(texts(doc)).toEqual(['STABLE']); + expect(doc.getElementsByTagNameNS(W, 'bookmarkStart')).toHaveLength(0); + expect(doc.getElementsByTagNameNS(W, 'bookmarkEnd')).toHaveLength(0); + }); + } + test('reject does not harvest an untracked spanning start as a local moveTo pair', () => { + const body = '' + content('moveTo', 13); + const following = 'STABLE'; + const doc = document(endpoint('moveTo', 10, body) + following); + rejectChanges(doc); + expect(texts(doc)).toEqual(['STABLE']); + for (const kind of ['bookmarkStart', 'bookmarkEnd']) { + expect(Array.from(doc.getElementsByTagNameNS(W, kind)).map(n => n.getAttributeNS(W, 'id'))).toEqual(['77']); + } + }); +}); diff --git a/packages/docx-core/src/primitives/reject_changes.ts b/packages/docx-core/src/primitives/reject_changes.ts index 4353de28..bf96b5d6 100644 --- a/packages/docx-core/src/primitives/reject_changes.ts +++ b/packages/docx-core/src/primitives/reject_changes.ts @@ -458,8 +458,9 @@ export function rejectChanges( }; } - // Phase A — Identify paragraphs whose MARK is a tracked insertion + // Phase A — Identify inserted or moved-to paragraph marks. const markInsertedParagraphs = new Set(); + const removedMoveToBookmarkIds = new Set(); const allParagraphs = collectByLocalName(root, 'p'); const resolvedMarkProperties = allParagraphs.filter(p => ['ins', 'del', 'moveFrom', 'moveTo', 'rPrChange'].some(kind => paragraphHasParaMarker(p, kind, filter))); @@ -474,9 +475,22 @@ export function rejectChanges( // inserted into a pre-existing paragraph, which Word/LibreOffice keep (empty) on // reject. safe-docx's inserted paragraphs always carry the mark now, so the // mark-based rule suffices and is Word-faithful. (Mirrors rejectAllChanges.) - if (paragraphHasParaMarker(p, 'ins', filter)) { + if (paragraphHasParaMarker(p, 'ins', filter) || paragraphHasParaMarker(p, 'moveTo', filter)) { markInsertedParagraphs.add(p); } + if (paragraphHasParaMarker(p, 'moveTo', filter)) { + const direct = Array.from(p.childNodes).filter((n): n is Element => n.nodeType === 1); + const substantive = direct.filter(n => !isW(n, 'pPr') && !isW(n, 'bookmarkStart') && !isW(n, 'bookmarkEnd')); + // A moved break does not imply that untracked or foreign content (and + // the bookmarks around it) disappears when the move is rejected. + if (substantive.length === 0 || !substantive.every(n => + (isW(n, 'ins') || isW(n, 'moveTo')) && filter(n))) continue; + const endIds = new Set(direct.filter(n => isW(n, 'bookmarkEnd')).map(n => n.getAttributeNS(W_NS, 'id'))); + for (const start of direct.filter(n => isW(n, 'bookmarkStart'))) { + const id = start.getAttributeNS(W_NS, 'id'); + if (id && endIds.has(id)) removedMoveToBookmarkIds.add(id); + } + } } // Phase B — Preserve cross-paragraph bookmark boundaries. Direct-child @@ -492,8 +506,8 @@ export function rejectChanges( // a selected insertion can have a live counterpart outside the wrapper so // the combined redline visibly brackets inserted text. Rejecting the // insertion must remove that revised-side counterpart as well. - const insertedBookmarkIds = new Set(); - for (const insertion of collectByLocalName(root, 'ins').filter(filter)) { + const insertedBookmarkIds = new Set(removedMoveToBookmarkIds); + for (const insertion of [...collectByLocalName(root, 'ins'), ...collectByLocalName(root, 'moveTo')].filter(filter)) { for (const localName of ['bookmarkStart', 'bookmarkEnd']) { for (const boundary of collectByLocalName(insertion, localName)) { const id = boundary.getAttributeNS(W_NS, 'id') ?? boundary.getAttribute('w:id');