From 0fb579cb79fa2fe9139d603f377db8a98fba5797 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 24 Jun 2026 08:55:49 -0300 Subject: [PATCH 1/2] fix(super-editor): keep editing-mode typing inside tracked changes plain [SD-3502] Cherry-pick of the editing-mode direct-edit fix to stable. A collapsed insertion strictly inside another user's tracked insertion or deletion (track changes off) now stays plain text and splits the existing suggestion, instead of being recorded as a tracked change. Includes the slice.size mark-cleanup fix plus unit and export tests. The original commit's apps/cli/src/lib/context.ts formatting hunk was intentionally omitted: stable's version differs (it carries ACCEPTED_RUNTIME_VALUES) and is already prettier-clean, so the hunk does not apply and is unrelated to the fix. (cherry picked from commit 71fb0472f86606d63d23fe2573c92e8e3bf1e84f) --- .../Editor.track-changes-dispatch.test.js | 185 +++++++++++++-- .../src/editors/v1/core/Editor.ts | 71 ++++++ .../trackChangesRoundtrip.test.js | 214 ++++++++++++++++++ 3 files changed, 454 insertions(+), 16 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/Editor.track-changes-dispatch.test.js b/packages/super-editor/src/editors/v1/core/Editor.track-changes-dispatch.test.js index fef6dea240..b004173b6b 100644 --- a/packages/super-editor/src/editors/v1/core/Editor.track-changes-dispatch.test.js +++ b/packages/super-editor/src/editors/v1/core/Editor.track-changes-dispatch.test.js @@ -3,6 +3,7 @@ import { readFile } from 'node:fs/promises'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { TextSelection } from 'prosemirror-state'; +import { Slice, Fragment } from 'prosemirror-model'; import { initTestEditor } from '@tests/helpers/helpers.js'; import { Editor } from '@core/Editor.js'; import { CustomSelectionPluginKey } from '@core/selection-state.js'; @@ -14,6 +15,8 @@ const ALICE = { name: 'Alice Reviewer', email: 'alice@example.com' }; const BOB = { name: 'Bob Reviewer', email: 'bob@example.com' }; const FIXED_DATE = '2026-05-21T00:00:00.000Z'; const FOREIGN_INSERT_ID = 'foreign-insert'; +const FOREIGN_DELETE_ID = 'foreign-delete'; +const DELETED_TEXT = 'please remove this sentence'; const INSERTED_TEXT = 'here is my new text, do you like it?'; const INSERTED_TAIL = 'do you like it?'; const INSERTED_TEXT_AFTER_DIRECT_DELETE = INSERTED_TEXT.replace(INSERTED_TAIL, ''); @@ -115,6 +118,34 @@ const setDocumentWithSeparateRunTrackedInsertion = ( ); }; +const setDocumentWithTrackedDeletion = (editor, { author = ALICE, id = FOREIGN_DELETE_ID } = {}) => { + const { schema } = editor; + const deleteMark = schema.marks[TrackDeleteMarkName].create({ + id, + author: author.name, + authorEmail: author.email, + date: FIXED_DATE, + }); + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create( + { sdBlockId: TEST_PARAGRAPH_BLOCK_ID }, + schema.nodes.run.create({}, [ + schema.text('hello there '), + schema.text(DELETED_TEXT, [deleteMark]), + schema.text(' after'), + ]), + ), + ); + + editor.dispatch( + editor.state.tr + .replaceWith(0, editor.state.doc.content.size, doc.content) + .setMeta('skipTrackChanges', true) + .setMeta('inputType', 'test-setup'), + ); +}; + const firstBlockId = (editor) => editor.state.doc.firstChild?.attrs?.sdBlockId ?? TEST_PARAGRAPH_BLOCK_ID; const deleteText = (editor, text) => { const { from, to } = findTextRange(editor, text); @@ -491,7 +522,7 @@ describe('Editor dispatch tracked-change meta', () => { ); }); - it('mutates another user tracked insertion in place on direct insert while local track mode is off', () => { + it('inserts plain text on direct insert inside another user tracked insertion while local track mode is off', () => { ({ editor } = initTestEditor({ mode: 'text', content: '

', @@ -503,16 +534,115 @@ describe('Editor dispatch tracked-change meta', () => { const insertPos = findTextRange(editor, INSERTED_TEXT).from + 4; editor.dispatch(editor.state.tr.insertText('ZZ', insertPos).setMeta('inputType', 'insertText')); - expect(textForMarkId(editor, TrackInsertMarkName, FOREIGN_INSERT_ID)).toBe( - 'hereZZ is my new text, do you like it?', - ); + // Editing mode: the typed text stays plain and the existing suggestion is + // simply split around it, so the foreign insertion never covers the new chars. + expect(editor.state.doc.textContent).toContain('hereZZ is my new text, do you like it?'); + expect(textForMarkId(editor, TrackInsertMarkName, FOREIGN_INSERT_ID)).toBe(INSERTED_TEXT); expect( markEntries(editor, TrackInsertMarkName).filter(({ mark }) => mark.attrs?.id !== FOREIGN_INSERT_ID), ).toHaveLength(0); + expect(markEntries(editor, TrackInsertMarkName).some(({ text }) => text.includes('ZZ'))).toBe(false); expect(markEntries(editor, TrackDeleteMarkName)).toHaveLength(0); }); - it('updates the existing insertion review item on direct insert inside another user tracked insertion', () => { + it('does not over-strip the existing suggestion on an open-slice paste inside another user tracked insertion while local track mode is off', () => { + ({ editor } = initTestEditor({ + mode: 'text', + content: '

', + user: BOB, + useImmediateSetTimeout: false, + })); + setDocumentWithTrackedInsertion(editor); + + // A genuinely open slice - the shape a multi-paragraph / rich-HTML paste produces - + // has content.size > slice.size because of its open node boundaries (a slice of + // inline text within one node would be closed, content.size === size). Paste it + // collapsed strictly inside the foreign insertion. The mark cleanup must span only + // the inserted size (slice.size); using content.size would overrun and strip + // trackInsert from the suggestion text after the paste. + const { schema } = editor; + const openSlice = new Slice( + Fragment.from( + schema.nodes.paragraph.create({ sdBlockId: 'paste-src' }, schema.nodes.run.create({}, schema.text('PASTE'))), + ), + 2, + 2, + ); + expect(openSlice.content.size).toBeGreaterThan(openSlice.size); + + const insertPos = findTextRange(editor, INSERTED_TEXT).from + 4; + editor.dispatch( + editor.state.tr.replaceRange(insertPos, insertPos, openSlice).setMeta('inputType', 'insertFromPaste'), + ); + + // The paste landed at the cursor (after "here"); textContent joins blocks with no + // separator, so this holds whether the open slice merges inline or splits a block. + expect(editor.state.doc.textContent).toContain('herePASTE is my new text'); + + // The whole original insertion stays tracked (split around the now-plain paste); + // no characters after the paste point lost their trackInsert mark. + expect(textForMarkId(editor, TrackInsertMarkName, FOREIGN_INSERT_ID)).toBe(INSERTED_TEXT); + expect(markEntries(editor, TrackInsertMarkName).some(({ text }) => text.includes('PASTE'))).toBe(false); + }); + + it('inserts plain text on direct insert inside another user tracked deletion while local track mode is off', () => { + ({ editor } = initTestEditor({ + mode: 'text', + content: '

', + user: BOB, + useImmediateSetTimeout: false, + })); + setDocumentWithTrackedDeletion(editor); + + const beforeTrackedChanges = editor.doc.trackChanges.list().items; + expect(beforeTrackedChanges).toHaveLength(1); + const beforeChangeId = beforeTrackedChanges[0].id; + + const insertPos = findTextRange(editor, DELETED_TEXT).from + 6; + editor.dispatch(editor.state.tr.insertText('ZZ', insertPos).setMeta('inputType', 'insertText')); + + // The Slack bug: typing inside someone else's pending deletion must not join + // the deletion (struck through) nor become a new tracked insert. The chars + // stay plain and the deletion is preserved, split around them. + expect(editor.state.doc.textContent).toContain('pleaseZZ remove this sentence'); + expect(textForMarkId(editor, TrackDeleteMarkName, FOREIGN_DELETE_ID)).toBe(DELETED_TEXT); + expect(markEntries(editor, TrackDeleteMarkName).some(({ text }) => text.includes('ZZ'))).toBe(false); + expect(markEntries(editor, TrackInsertMarkName)).toHaveLength(0); + + const afterTrackedChanges = editor.doc.trackChanges.list().items; + expect(afterTrackedChanges).toHaveLength(1); + expect(afterTrackedChanges[0].id).toBe(beforeChangeId); + }); + + it('inserts plain text on collapsed paste inside another user tracked deletion while local track mode is off', () => { + ({ editor } = initTestEditor({ + mode: 'text', + content: '

', + user: BOB, + useImmediateSetTimeout: false, + })); + setDocumentWithTrackedDeletion(editor); + + const beforeTrackedChanges = editor.doc.trackChanges.list().items; + expect(beforeTrackedChanges).toHaveLength(1); + + // A paste with a collapsed selection arrives as a single collapsed ReplaceStep + // carrying a slice (here a plain text node), the same shape the fix keys on. + const insertPos = findTextRange(editor, DELETED_TEXT).from + 6; + editor.dispatch( + editor.state.tr.insert(insertPos, editor.schema.text('PASTED')).setMeta('inputType', 'insertFromPaste'), + ); + + expect(editor.state.doc.textContent).toContain('pleasePASTED remove this sentence'); + expect(markEntries(editor, TrackInsertMarkName)).toHaveLength(0); + expect(markEntries(editor, TrackDeleteMarkName).some(({ text }) => text.includes('PASTED'))).toBe(false); + expect(textForMarkId(editor, TrackDeleteMarkName, FOREIGN_DELETE_ID)).toBe(DELETED_TEXT); + + const afterTrackedChanges = editor.doc.trackChanges.list().items; + expect(afterTrackedChanges).toHaveLength(1); + }); + + it('does not expand the existing insertion review item on direct insert inside another user tracked insertion', () => { ({ editor } = initTestEditor({ mode: 'text', content: '

', @@ -525,14 +655,17 @@ describe('Editor dispatch tracked-change meta', () => { const insertPos = findTextRange(editor, INSERTED_TEXT).from + 4; editor.dispatch(editor.state.tr.insertText('ZZ', insertPos).setMeta('inputType', 'insertText')); + // The plain typed text is not part of any review item, so the existing + // insertion comment keeps its original text and no new child item appears. expect(editor.doc.comments.list().items).toEqual( expect.arrayContaining([ expect.objectContaining({ trackedChangeType: 'insert', - trackedChangeText: 'hereZZ is my new text, do you like it?', + trackedChangeText: INSERTED_TEXT, }), ]), ); + expect(editor.doc.comments.list().items.some((item) => item.trackedChangeText?.includes('ZZ'))).toBe(false); const addChildInsertionEvent = emitSpy.mock.calls.find( ([eventName, payload]) => @@ -544,9 +677,20 @@ describe('Editor dispatch tracked-change meta', () => { )?.[1]; expect(addChildInsertionEvent).toBeUndefined(); + + // No commentsUpdate event of any kind (add or update) should reference the + // plain typed text - the suggestion was split around it, not mutated, so the + // stale direct-insertion mutation event must be suppressed. + const anyEventMentionsTypedText = emitSpy.mock.calls.some( + ([eventName, payload]) => + eventName === 'commentsUpdate' && + typeof payload?.trackedChangeText === 'string' && + payload.trackedChangeText.includes('ZZ'), + ); + expect(anyEventMentionsTypedText).toBe(false); }); - it('keeps the tracked-change id and linked review comment id stable on direct insert inside a live tracked insertion', () => { + it('keeps a live tracked insertion intact and inserts plain text on direct insert inside it', () => { ({ editor } = initTestEditor({ mode: 'text', content: '

', @@ -583,19 +727,22 @@ describe('Editor dispatch tracked-change meta', () => { ); expect(receipt.success).toBe(true); + expect(editor.state.doc.textContent).toContain('live-reZZview-comment'); const afterTrackedChanges = editor.doc.trackChanges.list().items; const afterComments = editor.doc.comments.list().items; + // Direct insert stays plain: the suggestion is split around it, so its id, + // linked comment, and text all stay exactly as before (no ZZ absorbed). expect(afterTrackedChanges).toHaveLength(1); expect(afterComments).toHaveLength(1); expect(afterTrackedChanges[0].id).toBe(beforeChangeId); expect(afterComments[0].commentId).toBe(beforeCommentId); - expect(afterTrackedChanges[0].insertedText).toBe('live-reZZview-comment'); - expect(afterComments[0].trackedChangeText).toBe('live-reZZview-comment'); + expect(afterTrackedChanges[0].insertedText).toBe('live-review-comment'); + expect(afterComments[0].trackedChangeText).toBe('live-review-comment'); }); - it('mutates an imported-style separate-run tracked insertion in place from document-api direct insert', () => { + it('inserts plain text inside an imported-style separate-run tracked insertion from document-api direct insert', () => { ({ editor } = initTestEditor({ mode: 'text', content: '

', @@ -622,18 +769,20 @@ describe('Editor dispatch tracked-change meta', () => { ); expect(receipt.success).toBe(true); - expect(textForMarkId(editor, TrackInsertMarkName, FOREIGN_INSERT_ID)).toBe( - IMPORTED_TRACKED_INSERTION_TEXT_AFTER_DIRECT_INSERT, - ); + // The imported suggestion is split around the plain text; its text and the + // tracked change stay unchanged, and the new chars carry no review mark. + expect(editor.state.doc.textContent).toContain(IMPORTED_TRACKED_INSERTION_TEXT_AFTER_DIRECT_INSERT); + expect(textForMarkId(editor, TrackInsertMarkName, FOREIGN_INSERT_ID)).toBe(IMPORTED_TRACKED_INSERTION_TEXT); + expect(markEntries(editor, TrackInsertMarkName).some(({ text }) => text.includes(DIRECT_INSERT_TEXT))).toBe(false); expect(markEntries(editor, TrackDeleteMarkName)).toHaveLength(0); const afterTrackedChanges = editor.doc.trackChanges.list().items; expect(afterTrackedChanges).toHaveLength(1); expect(afterTrackedChanges[0].id).toBe(beforeChangeId); - expect(afterTrackedChanges[0].insertedText).toBe(IMPORTED_TRACKED_INSERTION_TEXT_AFTER_DIRECT_INSERT); + expect(afterTrackedChanges[0].insertedText).toBe(IMPORTED_TRACKED_INSERTION_TEXT); }); - it('keeps imported tracked insertions semantically updated after document-api direct insert inside the imported text', async () => { + it('keeps imported tracked insertions intact and inserts plain text on document-api direct insert inside the imported text', async () => { const fixture = await readFile(BASIC_TEXT_INSERTION_OPEN_FIXTURE); editor = await Editor.open(fixture, { isHeadless: true, @@ -665,10 +814,14 @@ describe('Editor dispatch tracked-change meta', () => { ); expect(receipt.success).toBe(true); + // The plain text splits the imported suggestion without joining it, so the + // tracked change keeps its id and original text. + expect(editor.state.doc.textContent).toContain(IMPORTED_TRACKED_INSERTION_TEXT_AFTER_DIRECT_INSERT); + expect(markEntries(editor, TrackInsertMarkName).some(({ text }) => text.includes(DIRECT_INSERT_TEXT))).toBe(false); const afterTrackedChanges = editor.doc.trackChanges.list().items; expect(afterTrackedChanges).toHaveLength(1); expect(afterTrackedChanges[0].id).toBe(beforeChangeId); - expect(afterTrackedChanges[0].insertedText).toBe(IMPORTED_TRACKED_INSERTION_TEXT_AFTER_DIRECT_INSERT); + expect(afterTrackedChanges[0].insertedText).toBe(IMPORTED_TRACKED_INSERTION_TEXT); }); it('protects an imported-style separate-run tracked insertion from direct doc.replace', () => { diff --git a/packages/super-editor/src/editors/v1/core/Editor.ts b/packages/super-editor/src/editors/v1/core/Editor.ts index 10dc866d57..ceb2e6fc4d 100644 --- a/packages/super-editor/src/editors/v1/core/Editor.ts +++ b/packages/super-editor/src/editors/v1/core/Editor.ts @@ -380,6 +380,22 @@ const stepTouchesTrackedReviewState = (step: unknown, doc: PmNode): boolean => { ) { return false; } + // Editing mode (tracking off): a collapsed direct insertion strictly + // inside ANY tracked review span (insertion or deletion) stays direct. + // Typing destroys nothing - the existing suggestion is just split around + // the new text - so it must not be force-rerouted into a tracked change. + // The insertion-interior case already returns false above; this also + // covers the deletion-interior case. Safe in suggesting mode because + // shouldTrackForComposition is already true there, so the protect path is + // irrelevant. The inherited mark is stripped from the new text in + // #dispatchTransaction so the typed chars stay plain. + if ( + step.from === step.to && + step.slice.content.size > 0 && + collapsedPositionIsInsideTrackedReviewMark(doc, step.from) + ) { + return false; + } if (rangeHasTrackedReviewMark(doc, step.from, step.to)) return true; if (step.from === step.to && step.slice.content.size > 0) { return ( @@ -409,6 +425,35 @@ const transactionTouchesTrackedReviewState = (state: EditorState, tr: Transactio const docs = (tr as unknown as { docs?: PmNode[] }).docs ?? []; return tr.steps.some((step, index) => stepTouchesTrackedReviewState(step, docs[index] ?? state.doc)); }; + +// Editing-mode collapsed insert strictly inside a tracked review span: returns +// the inserted range (in the post-step doc) so the inherited suggestion mark can +// be stripped from the new text. Marks are inclusive:false, so a strictly-interior +// insert still inherits the neighbour's trackInsert/trackDelete via PM mark +// inheritance; without the strip the typed chars would silently join the +// suggestion (struck-through inside a deletion / counted as someone else's insert). +const collapsedInsertInsideTrackedReviewMarkRange = ( + state: EditorState, + tr: Transaction, +): { from: number; to: number } | null => { + if (!tr.docChanged || tr.steps.length !== 1) return null; + + const [step] = tr.steps; + if (!(step instanceof ReplaceStep)) return null; + if (step.from !== step.to || step.slice.content.size === 0) return null; + + const docs = (tr as unknown as { docs?: PmNode[] }).docs ?? []; + const docBeforeStep = docs[0] ?? state.doc; + if (!collapsedPositionIsInsideTrackedReviewMark(docBeforeStep, step.from)) return null; + + // Use slice.size (content.size - openStart - openEnd) - the size ProseMirror + // actually maps the inserted range to (ReplaceStep.getMap uses slice.size). For + // an open slice (a multi-paragraph / rich-HTML collapsed paste) content.size + // overshoots, so the mark cleanup would strip trackInsert/trackDelete from the + // existing suggestion text after the paste. + return { from: step.from, to: step.from + step.slice.size }; +}; + // Best-effort heuristic for labeling a content-control event's `source`. A // click sets `uiEvent: 'click'` on its selection transaction (the precise // signal); this window is the fallback for selection changes that don't carry @@ -3423,13 +3468,39 @@ export class Editor extends EventEmitter { const shouldTrackForComposition = (isTrackChangesActive || forceTrackChanges) && !skipTrackChanges; const shouldTrack = shouldTrackForComposition || protectsExistingTrackedReviewState; + // Editing-mode collapsed insert strictly inside a tracked review span: the + // typed text is made plain below, so it does NOT mutate the surrounding + // suggestion. Resolve it up front so the direct-insertion comment-mutation + // event is suppressed for this case (otherwise it would emit a stale + // commentsUpdate for an insertion that was not actually expanded). A + // delete-range edit still fires that event - plainInsertRange is null for + // it, since it is not a collapsed insert. + const plainInsertRange = !shouldTrack + ? collapsedInsertInsideTrackedReviewMarkRange(prevState, transactionToApply) + : null; if ( !shouldTrack && + !plainInsertRange && directInsertionMutationCommentMeta && !transactionToApply.getMeta(TrackChangesBasePluginKey) ) { transactionToApply.setMeta(TrackChangesBasePluginKey, directInsertionMutationCommentMeta); } + // PM mark inheritance (inclusive:false) still copies the neighbour + // suggestion's trackInsert/trackDelete/trackFormat onto a strictly-interior + // insert. Strip those marks from the inserted range (and clear them from + // stored marks) so the typed chars stay plain and the existing suggestion + // is simply split around them. Composition-meta-agnostic: deferral never + // runs in editing mode, so this normalizes both IME and non-IME inserts. + if (plainInsertRange) { + const schemaMarks = prevState.schema.marks; + for (const markName of [TrackInsertMarkName, TrackDeleteMarkName, TrackFormatMarkName]) { + const markType = schemaMarks[markName]; + if (!markType) continue; + transactionToApply.removeMark(plainInsertRange.from, plainInsertRange.to, markType); + transactionToApply.removeStoredMark(markType); + } + } if (shouldTrack && forceTrackChanges && !this.options.user) { throw new Error('forceTrackChanges requires a user to be configured on the editor instance.'); } diff --git a/packages/super-editor/src/editors/v1/tests/import-export/trackChangesRoundtrip.test.js b/packages/super-editor/src/editors/v1/tests/import-export/trackChangesRoundtrip.test.js index b8fd800d7d..e308575577 100644 --- a/packages/super-editor/src/editors/v1/tests/import-export/trackChangesRoundtrip.test.js +++ b/packages/super-editor/src/editors/v1/tests/import-export/trackChangesRoundtrip.test.js @@ -347,3 +347,217 @@ describe('tracked format import/export round trip', () => { } }); }); + +describe('editing-mode direct insert inside a tracked span export', () => { + const EDITING_INSERT_TEXT = 'PLAIN'; + + const createTrackedInsertionDoc = () => ({ + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { + type: 'run', + content: [{ type: 'text', text: 'before ' }], + }, + { + type: 'run', + content: [ + { + type: 'text', + text: 'suggested text', + marks: [ + { + type: 'trackInsert', + attrs: { + id: 'editing-insert-1', + author: 'Alice Reviewer', + authorEmail: 'alice@example.com', + date: '2026-01-07T20:24:39Z', + }, + }, + ], + }, + ], + }, + { + type: 'run', + content: [{ type: 'text', text: ' after' }], + }, + ], + }, + ], + }); + + const createTrackedDeletionDoc = () => ({ + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { + type: 'run', + content: [{ type: 'text', text: 'before ' }], + }, + { + type: 'run', + content: [ + { + type: 'text', + text: 'deleted text', + marks: [ + { + type: 'trackDelete', + attrs: { + id: 'editing-delete-1', + author: 'Alice Reviewer', + authorEmail: 'alice@example.com', + date: '2026-01-07T20:24:39Z', + }, + }, + ], + }, + ], + }, + { + type: 'run', + content: [{ type: 'text', text: ' after' }], + }, + ], + }, + ], + }); + + // Finds a cursor position strictly inside the tracked span so the collapsed + // insert inherits the neighboring revision mark via PM mark inheritance. + const findInteriorPositionOfTrackedMark = (editor, markName) => { + let found = null; + editor.state.doc.descendants((node, pos) => { + if (found != null || !node.isText || !node.text) return; + const hasMark = node.marks?.some((mark) => mark.type.name === markName); + if (hasMark && node.text.length >= 2) found = pos + 1; + }); + if (found == null) throw new Error(`Could not find a position inside ${markName}`); + return found; + }; + + const collectTextsWithMarkName = (editor, markName, needle) => { + const texts = []; + editor.state.doc.descendants((node) => { + if (!node.isText || !node.text) return; + if (node.marks?.some((mark) => mark.type.name === markName) && node.text.includes(needle)) { + texts.push(node.text); + } + }); + return texts; + }; + + // Walks the exported XML and records whether each text node sits under a + // revision ancestor. + const collectTextRunSegments = (node, context, segments) => { + if (!node || typeof node !== 'object') return; + const nextContext = { + insideIns: context.insideIns || node.name === 'w:ins', + insideDel: context.insideDel || node.name === 'w:del', + }; + if (node.name === 'w:t' || node.name === 'w:delText') { + const text = (node.elements ?? []) + .filter((child) => child.type === 'text') + .map((child) => child.text ?? '') + .join(''); + segments.push({ text, name: node.name, ...nextContext }); + return; + } + if (Array.isArray(node.elements)) { + node.elements.forEach((child) => collectTextRunSegments(child, nextContext, segments)); + } + }; + + it('exports editing-mode text typed inside a tracked insertion as plain w:t, not inside w:ins', async () => { + const { docx, media, mediaFiles, fonts } = await loadTestDataForEditorTests('blank-doc.docx'); + const { editor } = await initTestEditor({ + content: docx, + media, + mediaFiles, + fonts, + isHeadless: true, + user: { name: 'Bob Reviewer', email: 'bob@example.com' }, + }); + + try { + replaceEditorDocumentContent(editor, createTrackedInsertionDoc()); + + // Editing mode (tracking off) is the initTestEditor default. A collapsed + // insert strictly inside the suggestion must stay plain text. + const insertPos = findInteriorPositionOfTrackedMark(editor, 'trackInsert'); + editor.dispatch(editor.state.tr.insertText(EDITING_INSERT_TEXT, insertPos).setMeta('inputType', 'insertText')); + + expect(collectTextsWithMarkName(editor, 'trackInsert', EDITING_INSERT_TEXT)).toHaveLength(0); + + const exportedXml = await editor.exportDocx({ exportXmlOnly: true, isFinalDoc: false }); + expect(typeof exportedXml).toBe('string'); + + const documentJson = parseXmlToJson(exportedXml); + const documentNode = documentJson.elements?.find((el) => el.name === 'w:document'); + const body = documentNode?.elements?.find((el) => el.name === 'w:body'); + expect(body).toBeDefined(); + + const segments = []; + collectTextRunSegments(body, { insideIns: false, insideDel: false }, segments); + + const plainSegment = segments.find((segment) => segment.text.includes(EDITING_INSERT_TEXT)); + expect(plainSegment).toBeDefined(); + // The new text exports as a normal run, never wrapped in a w:ins revision. + expect(plainSegment.name).toBe('w:t'); + expect(plainSegment.insideIns).toBe(false); + expect(segments.some((segment) => segment.insideIns && segment.text.includes(EDITING_INSERT_TEXT))).toBe(false); + } finally { + editor.destroy(); + } + }); + + it('exports editing-mode text typed inside a tracked deletion as plain w:t, not inside w:del', async () => { + const { docx, media, mediaFiles, fonts } = await loadTestDataForEditorTests('blank-doc.docx'); + const { editor } = await initTestEditor({ + content: docx, + media, + mediaFiles, + fonts, + isHeadless: true, + user: { name: 'Bob Reviewer', email: 'bob@example.com' }, + }); + + try { + replaceEditorDocumentContent(editor, createTrackedDeletionDoc()); + + const insertPos = findInteriorPositionOfTrackedMark(editor, 'trackDelete'); + editor.dispatch(editor.state.tr.insertText(EDITING_INSERT_TEXT, insertPos).setMeta('inputType', 'insertText')); + + expect(collectTextsWithMarkName(editor, 'trackDelete', EDITING_INSERT_TEXT)).toHaveLength(0); + + const exportedXml = await editor.exportDocx({ exportXmlOnly: true, isFinalDoc: false }); + expect(typeof exportedXml).toBe('string'); + + const documentJson = parseXmlToJson(exportedXml); + const documentNode = documentJson.elements?.find((el) => el.name === 'w:document'); + const body = documentNode?.elements?.find((el) => el.name === 'w:body'); + expect(body).toBeDefined(); + + const segments = []; + collectTextRunSegments(body, { insideIns: false, insideDel: false }, segments); + + const plainSegment = segments.find((segment) => segment.text.includes(EDITING_INSERT_TEXT)); + expect(plainSegment).toBeDefined(); + expect(plainSegment.name).toBe('w:t'); + expect(plainSegment.insideIns).toBe(false); + expect(plainSegment.insideDel).toBe(false); + expect(segments.some((segment) => segment.insideDel && segment.text.includes(EDITING_INSERT_TEXT))).toBe(false); + expect( + segments.some((segment) => segment.name === 'w:delText' && segment.text.includes(EDITING_INSERT_TEXT)), + ).toBe(false); + } finally { + editor.destroy(); + } + }); +}); From 95358a79238c24d2ea4af5e680b3e64b84833bb8 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Wed, 24 Jun 2026 10:38:36 -0300 Subject: [PATCH 2/2] fix(super-editor): strip only inherited marks on editing-mode insert, keep pasted review marks The editing-mode mark cleanup used removeMark by TYPE across the whole inserted range, so pasting a SuperDoc slice that already carries trackInsert/trackDelete into the interior of another suggestion silently dropped the pasted review marks (and exported them as plain text). Strip only the specific ENCLOSING mark instances inherited from the neighbour (matched by type + attrs); marks the pasted slice carries in (a different review id) are preserved. Adds a paste-with-marks regression. Addresses review on PR #3771. --- .../Editor.track-changes-dispatch.test.js | 42 ++++++++++++++ .../src/editors/v1/core/Editor.ts | 55 ++++++++++++++----- 2 files changed, 82 insertions(+), 15 deletions(-) diff --git a/packages/super-editor/src/editors/v1/core/Editor.track-changes-dispatch.test.js b/packages/super-editor/src/editors/v1/core/Editor.track-changes-dispatch.test.js index b004173b6b..ea037f63f9 100644 --- a/packages/super-editor/src/editors/v1/core/Editor.track-changes-dispatch.test.js +++ b/packages/super-editor/src/editors/v1/core/Editor.track-changes-dispatch.test.js @@ -585,6 +585,48 @@ describe('Editor dispatch tracked-change meta', () => { expect(markEntries(editor, TrackInsertMarkName).some(({ text }) => text.includes('PASTE'))).toBe(false); }); + it('preserves review marks carried by a pasted slice inside another user tracked insertion while local track mode is off', () => { + ({ editor } = initTestEditor({ + mode: 'text', + content: '

', + user: BOB, + useImmediateSetTimeout: false, + })); + setDocumentWithTrackedInsertion(editor); + + // Pasting a SuperDoc slice that already carries its own tracked-review marks + // (a copied suggestion) into the interior of another suggestion must keep the + // pasted marks. Only the ENCLOSING mark inherited from the neighbour is stripped; + // a blanket strip-by-type would silently drop the pasted review metadata. + const { schema } = editor; + const pastedMark = schema.marks[TrackInsertMarkName].create({ + id: 'pasted-insert', + author: 'Carol Author', + authorEmail: 'carol@example.com', + date: FIXED_DATE, + }); + const openSlice = new Slice( + Fragment.from( + schema.nodes.paragraph.create( + { sdBlockId: 'paste-src' }, + schema.nodes.run.create({}, schema.text('COPIED', [pastedMark])), + ), + ), + 2, + 2, + ); + + const insertPos = findTextRange(editor, INSERTED_TEXT).from + 4; + editor.dispatch( + editor.state.tr.replaceRange(insertPos, insertPos, openSlice).setMeta('inputType', 'insertFromPaste'), + ); + + // The pasted suggestion keeps its own review id (it is not the enclosing mark). + expect(textForMarkId(editor, TrackInsertMarkName, 'pasted-insert')).toBe('COPIED'); + // The enclosing foreign insertion did not absorb the pasted text into its id. + expect(textForMarkId(editor, TrackInsertMarkName, FOREIGN_INSERT_ID)).toBe(INSERTED_TEXT); + }); + it('inserts plain text on direct insert inside another user tracked deletion while local track mode is off', () => { ({ editor } = initTestEditor({ mode: 'text', diff --git a/packages/super-editor/src/editors/v1/core/Editor.ts b/packages/super-editor/src/editors/v1/core/Editor.ts index ceb2e6fc4d..6b96ce2999 100644 --- a/packages/super-editor/src/editors/v1/core/Editor.ts +++ b/packages/super-editor/src/editors/v1/core/Editor.ts @@ -426,16 +426,35 @@ const transactionTouchesTrackedReviewState = (state: EditorState, tr: Transactio return tr.steps.some((step, index) => stepTouchesTrackedReviewState(step, docs[index] ?? state.doc)); }; +// The tracked-review mark instances that ENCLOSE a collapsed position (present on +// both nodeBefore and nodeAfter). These are exactly the marks PM mark-inheritance +// copies onto text inserted strictly inside the span. Returning the specific +// instances (matched by type + attrs) lets the cleanup strip only the inherited +// marks - marks a pasted slice carries in (a different review id) are left intact. +const enclosingTrackedReviewMarksAtCollapsedPosition = (doc: PmNode, pos: number): PmMark[] => { + const boundedPos = Math.max(0, Math.min(doc.content.size, pos)); + const $pos = doc.resolve(boundedPos); + const afterKeys = trackedReviewMarkKeysForNode($pos.nodeAfter); + if (!afterKeys.size) return []; + const enclosing: PmMark[] = []; + for (const mark of $pos.nodeBefore?.marks ?? []) { + const key = trackedReviewMarkKey(mark); + if (key && afterKeys.has(key)) enclosing.push(mark); + } + return enclosing; +}; + // Editing-mode collapsed insert strictly inside a tracked review span: returns -// the inserted range (in the post-step doc) so the inherited suggestion mark can -// be stripped from the new text. Marks are inclusive:false, so a strictly-interior -// insert still inherits the neighbour's trackInsert/trackDelete via PM mark -// inheritance; without the strip the typed chars would silently join the -// suggestion (struck-through inside a deletion / counted as someone else's insert). +// the inserted range (in the post-step doc) plus the enclosing mark instances so +// the inherited suggestion mark can be stripped from the new text. Marks are +// inclusive:false, so a strictly-interior insert still inherits the neighbour's +// trackInsert/trackDelete via PM mark inheritance; without the strip the typed +// chars would silently join the suggestion (struck-through inside a deletion / +// counted as someone else's insert). const collapsedInsertInsideTrackedReviewMarkRange = ( state: EditorState, tr: Transaction, -): { from: number; to: number } | null => { +): { from: number; to: number; marks: PmMark[] } | null => { if (!tr.docChanged || tr.steps.length !== 1) return null; const [step] = tr.steps; @@ -444,14 +463,15 @@ const collapsedInsertInsideTrackedReviewMarkRange = ( const docs = (tr as unknown as { docs?: PmNode[] }).docs ?? []; const docBeforeStep = docs[0] ?? state.doc; - if (!collapsedPositionIsInsideTrackedReviewMark(docBeforeStep, step.from)) return null; + const marks = enclosingTrackedReviewMarksAtCollapsedPosition(docBeforeStep, step.from); + if (!marks.length) return null; // Use slice.size (content.size - openStart - openEnd) - the size ProseMirror // actually maps the inserted range to (ReplaceStep.getMap uses slice.size). For // an open slice (a multi-paragraph / rich-HTML collapsed paste) content.size // overshoots, so the mark cleanup would strip trackInsert/trackDelete from the // existing suggestion text after the paste. - return { from: step.from, to: step.from + step.slice.size }; + return { from: step.from, to: step.from + step.slice.size, marks }; }; // Best-effort heuristic for labeling a content-control event's `source`. A @@ -3488,17 +3508,22 @@ export class Editor extends EventEmitter { } // PM mark inheritance (inclusive:false) still copies the neighbour // suggestion's trackInsert/trackDelete/trackFormat onto a strictly-interior - // insert. Strip those marks from the inserted range (and clear them from - // stored marks) so the typed chars stay plain and the existing suggestion - // is simply split around them. Composition-meta-agnostic: deferral never - // runs in editing mode, so this normalizes both IME and non-IME inserts. + // insert. Strip ONLY those enclosing mark instances from the inserted range + // (matched by type + attrs) so the typed chars stay plain and the existing + // suggestion is simply split around them. Removing by instance - not by type - + // preserves review marks a pasted SuperDoc slice carries in (a different + // review id), which a blanket type-strip would silently drop. Stored marks are + // cleared by type so continued typing stays plain; that only affects the next + // typed char, never the already-inserted content. Composition-meta-agnostic: + // deferral never runs in editing mode, so this normalizes IME and non-IME inserts. if (plainInsertRange) { + for (const mark of plainInsertRange.marks) { + transactionToApply.removeMark(plainInsertRange.from, plainInsertRange.to, mark); + } const schemaMarks = prevState.schema.marks; for (const markName of [TrackInsertMarkName, TrackDeleteMarkName, TrackFormatMarkName]) { const markType = schemaMarks[markName]; - if (!markType) continue; - transactionToApply.removeMark(plainInsertRange.from, plainInsertRange.to, markType); - transactionToApply.removeStoredMark(markType); + if (markType) transactionToApply.removeStoredMark(markType); } } if (shouldTrack && forceTrackChanges && !this.options.user) {