From acc9ad563ae862acc7fd4d62ada1a08075c5828d Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Sat, 28 Feb 2026 01:40:18 +0530 Subject: [PATCH 1/7] fix: remove persisting yellow underline after canceling track-format suggestion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs caused the gold/yellow TrackFormat underline to persist after reversing a tracked format change: 1. addMarkStep applied TrackFormat using the full step range instead of clamped per-node boundaries, leaking wrong before/after arrays onto adjacent text nodes. 2. removeMarkStep didn't clean up fully-reversed format changes — when a tracked addition was toggled back (after=[] but before had items), the TrackFormat mark persisted instead of being removed. --- .../track-changes/trackChangesHelpers/addMarkStep.js | 6 +----- .../track-changes/trackChangesHelpers/removeMarkStep.js | 4 ++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js index 6fd16bebb1..10e1a66cc9 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js @@ -93,11 +93,7 @@ export const addMarkStep = ({ state, step, newTr, doc, user, date }) => { before, after, }); - newTr.addMark( - step.from, // Math.max(step.from, pos) - step.to, // Math.min(step.to, pos + node.nodeSize), - newFormatMark, - ); + newTr.addMark(Math.max(step.from, pos), Math.min(step.to, pos + node.nodeSize), newFormatMark); meta.formatMark = newFormatMark; meta.step = step; diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js index 0011e05edb..3a2776c815 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js @@ -52,6 +52,10 @@ export const removeMarkStep = ({ state, step, newTr, doc, user, date }) => { after = [ ...formatChangeMark.attrs.after.filter((mark) => !markSnapshotMatchesStepMark(mark, step.mark, true)), ]; + if (after.length === 0) { + newTr.removeMark(Math.max(step.from, pos), Math.min(step.to, pos + node.nodeSize), formatChangeMark); + return; + } before = [...formatChangeMark.attrs.before]; } else { after = [...formatChangeMark.attrs.after]; From 201e7c9a7ac41a93c8eb0fe05f0b7a06b1f54312 Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Sat, 28 Feb 2026 02:04:16 +0530 Subject: [PATCH 2/7] fix: only remove TrackFormat when both before and after are empty Narrowed the early-exit condition so TrackFormat is only removed when both arrays are empty (true no-op). Keeps the mark when before has legitimate tracked removals (e.g. before=[italic], after=[]). --- .../track-changes/trackChangesHelpers/removeMarkStep.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js index 3a2776c815..57e6fd16e4 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js @@ -52,7 +52,7 @@ export const removeMarkStep = ({ state, step, newTr, doc, user, date }) => { after = [ ...formatChangeMark.attrs.after.filter((mark) => !markSnapshotMatchesStepMark(mark, step.mark, true)), ]; - if (after.length === 0) { + if (after.length === 0 && formatChangeMark.attrs.before.length === 0) { newTr.removeMark(Math.max(step.from, pos), Math.min(step.to, pos + node.nodeSize), formatChangeMark); return; } From a62b325a22791015187079f0b2ae89d2e30e950a Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Sat, 28 Feb 2026 02:18:00 +0530 Subject: [PATCH 3/7] fix: share TrackFormat ID across nodes in the same addMarkStep Hoist a sharedWid variable so all inline nodes touched by a single addMarkStep reuse the same TrackFormat ID. With per-node clamped ranges, getLiveInlineMarksInRange no longer sees the previous node's TrackFormat, causing each node to generate a separate UUID. This broke ID-based reject/accept flows for multi-node selections. --- .../track-changes/trackChangesHelpers/addMarkStep.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js index 10e1a66cc9..1511520702 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js @@ -18,6 +18,7 @@ import { getLiveInlineMarksInRange } from './getLiveInlineMarksInRange.js'; */ export const addMarkStep = ({ state, step, newTr, doc, user, date }) => { const meta = {}; + let sharedWid = null; doc.nodesBetween(step.from, step.to, (node, pos) => { if (!node.isInline || node.type.name === 'run') { @@ -38,7 +39,7 @@ export const addMarkStep = ({ state, step, newTr, doc, user, date }) => { const existingChangeMark = liveMarks.find((mark) => [TrackDeleteMarkName, TrackFormatMarkName].includes(mark.type.name), ); - const wid = existingChangeMark ? existingChangeMark.attrs.id : uuidv4(); + const wid = existingChangeMark ? existingChangeMark.attrs.id : (sharedWid ?? (sharedWid = uuidv4())); newTr.addMark(Math.max(step.from, pos), Math.min(step.to, pos + node.nodeSize), step.mark); const allowedMarks = ['bold', 'italic', 'strike', 'underline', 'textStyle', 'highlight']; From 00fc798f68fb47d329c83908b63fbaff729d2d3e Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Sat, 28 Feb 2026 02:25:20 +0530 Subject: [PATCH 4/7] fix: share TrackFormat ID across nodes in removeMarkStep Mirror the sharedWid pattern from addMarkStep. When an existing formatChangeMark is present, reuse its ID to preserve change grouping. When creating new TrackFormat marks across multiple nodes, share a single generated ID so reject/accept treats them as one change. --- .../track-changes/trackChangesHelpers/removeMarkStep.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js index 57e6fd16e4..69d89128e2 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js @@ -16,6 +16,7 @@ import { getLiveInlineMarksInRange } from './getLiveInlineMarksInRange.js'; */ export const removeMarkStep = ({ state, step, newTr, doc, user, date }) => { const meta = {}; + let sharedWid = null; doc.nodesBetween(step.from, step.to, (node, pos) => { if (!node.isInline || node.type.name === 'run') { @@ -81,7 +82,7 @@ export const removeMarkStep = ({ state, step, newTr, doc, user, date }) => { if (after.length || before.length) { const newFormatMark = state.schema.marks[TrackFormatMarkName].create({ - id: uuidv4(), + id: formatChangeMark ? formatChangeMark.attrs.id : (sharedWid ?? (sharedWid = uuidv4())), author: user.name, authorEmail: user.email, authorImage: user.image, From 76980786c723660a9bece8a74fd3fc357ec83e3b Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Mon, 2 Mar 2026 06:36:33 -0300 Subject: [PATCH 5/7] fix(track-changes): clean up TrackFormat when cancel is a no-op on nodes with pre-existing marks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When bold is toggled on then off across multi-node selections where some nodes have pre-existing marks (e.g., italic), the removeMarkStep cancel path only cleaned up when both `before` and `after` arrays were empty. This missed the case where `before` had pre-existing marks that were never actually removed — leaving a ghost TrackFormat mark showing "Format: removed italic" even though italic was untouched. The fix checks whether all marks in `before` still exist on the node. If they do, the tracked change is a no-op and the TrackFormat is removed. --- .../trackChangesHelpers/addMarkStep.js | 1 + .../trackChangesHelpers/removeMarkStep.js | 19 +- .../removeMarkStep.test.js | 174 ++++++++++++++++++ 3 files changed, 191 insertions(+), 3 deletions(-) create mode 100644 packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js index 1511520702..57a5f16e5e 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js @@ -31,6 +31,7 @@ export const addMarkStep = ({ state, step, newTr, doc, user, date }) => { const rangeFrom = Math.max(step.from, pos); const rangeTo = Math.min(step.to, pos + node.nodeSize); + const liveMarks = getLiveInlineMarksInRange({ doc: newTr.doc, from: rangeFrom, diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js index 69d89128e2..2041606dde 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js @@ -53,9 +53,22 @@ export const removeMarkStep = ({ state, step, newTr, doc, user, date }) => { after = [ ...formatChangeMark.attrs.after.filter((mark) => !markSnapshotMatchesStepMark(mark, step.mark, true)), ]; - if (after.length === 0 && formatChangeMark.attrs.before.length === 0) { - newTr.removeMark(Math.max(step.from, pos), Math.min(step.to, pos + node.nodeSize), formatChangeMark); - return; + if (after.length === 0) { + // All additions were canceled. Check if any marks in `before` were + // actually removed from the node. If they all still exist, the + // tracked change is a no-op — clean it up. + const remainingFormatMarks = liveMarksBeforeRemove.filter( + (m) => + ![TrackDeleteMarkName, TrackFormatMarkName].includes(m.type.name) && + m.type.name !== step.mark.type.name, + ); + const isNoop = formatChangeMark.attrs.before.every((snapshot) => + remainingFormatMarks.some((m) => m.type.name === snapshot.type), + ); + if (isNoop) { + newTr.removeMark(Math.max(step.from, pos), Math.min(step.to, pos + node.nodeSize), formatChangeMark); + return; + } } before = [...formatChangeMark.attrs.before]; } else { diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js new file mode 100644 index 0000000000..6d7aa4e5ff --- /dev/null +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js @@ -0,0 +1,174 @@ +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest'; +import { EditorState } from 'prosemirror-state'; +import { AddMarkStep, RemoveMarkStep } from 'prosemirror-transform'; +import { TrackFormatMarkName } from '../constants.js'; +import { TrackChangesBasePluginKey } from '../plugins/trackChangesBasePlugin.js'; +import { addMarkStep } from './addMarkStep.js'; +import { removeMarkStep } from './removeMarkStep.js'; +import { initTestEditor } from '@tests/helpers/helpers.js'; + +describe('removeMarkStep cancel logic', () => { + let editor; + let schema; + let basePlugins; + + const user = { name: 'Track Tester', email: 'track@example.com' }; + const date = '2024-01-01T00:00:00.000Z'; + + beforeEach(() => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + schema = editor.schema; + basePlugins = editor.state.plugins; + }); + + afterEach(() => { + vi.restoreAllMocks(); + editor?.destroy(); + editor = null; + }); + + const createDocWithRuns = (runs) => { + const runNodes = runs.map(({ text, marks = [] }) => schema.nodes.run.create({}, schema.text(text, marks))); + return schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, runNodes)); + }; + + const createState = (doc) => + EditorState.create({ + schema, + doc, + plugins: basePlugins, + }); + + /** + * Simulate add-then-remove of a mark and return the resulting doc. + * Applies addMarkStep to get the TrackFormat mark, then removeMarkStep. + */ + const addThenRemoveMark = ({ state, mark, from, to }) => { + const addStep = new AddMarkStep(from, to, mark); + const addTr = state.tr; + addMarkStep({ state, step: addStep, newTr: addTr, doc: state.doc, user, date }); + const stateAfterAdd = state.apply(addTr); + + const removeStep = new RemoveMarkStep(from, to, mark); + const removeTr = stateAfterAdd.tr; + removeMarkStep({ state: stateAfterAdd, step: removeStep, newTr: removeTr, doc: stateAfterAdd.doc, user, date }); + return { stateAfterRemove: stateAfterAdd.apply(removeTr), removeTr }; + }; + + const hasTrackFormatMark = (doc) => { + let found = false; + doc.descendants((node) => { + if (node.marks?.some((m) => m.type.name === TrackFormatMarkName)) { + found = true; + } + }); + return found; + }; + + it('removes TrackFormat when bold is toggled on then off (no pre-existing marks)', () => { + const doc = createDocWithRuns([{ text: 'Hello' }]); + const state = createState(doc); + + const { stateAfterRemove } = addThenRemoveMark({ + state, + mark: schema.marks.bold.create(), + from: 2, + to: 7, + }); + + expect(hasTrackFormatMark(stateAfterRemove.doc)).toBe(false); + }); + + it('removes TrackFormat when bold is toggled on then off (node has pre-existing italic)', () => { + const italic = schema.marks.italic.create(); + const doc = createDocWithRuns([{ text: 'world', marks: [italic] }]); + const state = createState(doc); + + const { stateAfterRemove } = addThenRemoveMark({ + state, + mark: schema.marks.bold.create(), + from: 2, + to: 7, + }); + + // The bug: before this fix, a ghost TrackFormat with before=[italic], after=[] + // would persist, showing "Format: removed italic" even though italic was never touched. + expect(hasTrackFormatMark(stateAfterRemove.doc)).toBe(false); + }); + + it('keeps TrackFormat when before marks were actually removed from the node', () => { + // Setup: "world" has italic + bold + const italic = schema.marks.italic.create(); + const bold = schema.marks.bold.create(); + const doc = createDocWithRuns([{ text: 'world', marks: [italic, bold] }]); + const state = createState(doc); + + // Step 1: Remove italic (creates TrackFormat with before=[italic], after=[]) + const removeItalicStep = new RemoveMarkStep(2, 7, italic); + const removeItalicTr = state.tr; + removeMarkStep({ state, step: removeItalicStep, newTr: removeItalicTr, doc: state.doc, user, date }); + const stateAfterRemoveItalic = state.apply(removeItalicTr); + + // Step 2: Add bold (bold already exists, so this is a no-op for addMarkStep's + // hasMatchingMark check — bold is already live). Instead, simulate adding underline + // then removing it to test the cancel logic with a pre-existing removal. + const addUnderlineStep = new AddMarkStep(2, 7, schema.marks.underline.create()); + const addUnderlineTr = stateAfterRemoveItalic.tr; + addMarkStep({ + state: stateAfterRemoveItalic, + step: addUnderlineStep, + newTr: addUnderlineTr, + doc: stateAfterRemoveItalic.doc, + user, + date, + }); + const stateAfterAddUnderline = stateAfterRemoveItalic.apply(addUnderlineTr); + + // Step 3: Remove underline (cancel the addition). The TrackFormat should remain + // because italic was genuinely removed and is no longer on the node. + const removeUnderlineStep = new RemoveMarkStep(2, 7, schema.marks.underline.create()); + const removeUnderlineTr = stateAfterAddUnderline.tr; + removeMarkStep({ + state: stateAfterAddUnderline, + step: removeUnderlineStep, + newTr: removeUnderlineTr, + doc: stateAfterAddUnderline.doc, + user, + date, + }); + const stateAfterRemoveUnderline = stateAfterAddUnderline.apply(removeUnderlineTr); + + // TrackFormat should persist because italic was actually removed + expect(hasTrackFormatMark(stateAfterRemoveUnderline.doc)).toBe(true); + + // Verify the remaining TrackFormat records the italic removal + let trackFormatAttrs = null; + stateAfterRemoveUnderline.doc.descendants((node) => { + const mark = node.marks?.find((m) => m.type.name === TrackFormatMarkName); + if (mark) trackFormatAttrs = mark.attrs; + }); + expect(trackFormatAttrs.before).toEqual(expect.arrayContaining([expect.objectContaining({ type: 'italic' })])); + }); + + it('removes TrackFormat for multi-node bold toggle (Hello plain + world italic)', () => { + const italic = schema.marks.italic.create(); + const doc = createDocWithRuns([{ text: 'Hello ' }, { text: 'world', marks: [italic] }]); + const state = createState(doc); + const bold = schema.marks.bold.create(); + + // Add bold across both nodes + const addStep = new AddMarkStep(2, 13, bold); + const addTr = state.tr; + addMarkStep({ state, step: addStep, newTr: addTr, doc: state.doc, user, date }); + const stateAfterAdd = state.apply(addTr); + + // Remove bold across both nodes + const removeStep = new RemoveMarkStep(2, 13, bold); + const removeTr = stateAfterAdd.tr; + removeMarkStep({ state: stateAfterAdd, step: removeStep, newTr: removeTr, doc: stateAfterAdd.doc, user, date }); + const stateAfterRemove = stateAfterAdd.apply(removeTr); + + // Neither node should have a TrackFormat mark + expect(hasTrackFormatMark(stateAfterRemove.doc)).toBe(false); + }); +}); From 2fb149a06e3c9e26a28e50b0b19e6ec89dcd2b9f Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Mon, 2 Mar 2026 07:02:35 -0300 Subject: [PATCH 6/7] fix(track-changes): use attribute-aware comparison in isNoop check The isNoop check compared mark snapshots by type name only, ignoring attributes. For textStyle/highlight marks with differing attrs, this could incorrectly treat a real change as a no-op. Use the existing markSnapshotMatchesStepMark helper for full type+attrs comparison. Also simplifies multi-node test to use the addThenRemoveMark helper. --- .../trackChangesHelpers/removeMarkStep.js | 2 +- .../trackChangesHelpers/removeMarkStep.test.js | 18 ++++++------------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js index 2041606dde..017c7070eb 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js @@ -63,7 +63,7 @@ export const removeMarkStep = ({ state, step, newTr, doc, user, date }) => { m.type.name !== step.mark.type.name, ); const isNoop = formatChangeMark.attrs.before.every((snapshot) => - remainingFormatMarks.some((m) => m.type.name === snapshot.type), + remainingFormatMarks.some((m) => markSnapshotMatchesStepMark(snapshot, m, true)), ); if (isNoop) { newTr.removeMark(Math.max(step.from, pos), Math.min(step.to, pos + node.nodeSize), formatChangeMark); diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js index 6d7aa4e5ff..a2c86a1051 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js @@ -154,19 +154,13 @@ describe('removeMarkStep cancel logic', () => { const italic = schema.marks.italic.create(); const doc = createDocWithRuns([{ text: 'Hello ' }, { text: 'world', marks: [italic] }]); const state = createState(doc); - const bold = schema.marks.bold.create(); - - // Add bold across both nodes - const addStep = new AddMarkStep(2, 13, bold); - const addTr = state.tr; - addMarkStep({ state, step: addStep, newTr: addTr, doc: state.doc, user, date }); - const stateAfterAdd = state.apply(addTr); - // Remove bold across both nodes - const removeStep = new RemoveMarkStep(2, 13, bold); - const removeTr = stateAfterAdd.tr; - removeMarkStep({ state: stateAfterAdd, step: removeStep, newTr: removeTr, doc: stateAfterAdd.doc, user, date }); - const stateAfterRemove = stateAfterAdd.apply(removeTr); + const { stateAfterRemove } = addThenRemoveMark({ + state, + mark: schema.marks.bold.create(), + from: 2, + to: 13, + }); // Neither node should have a TrackFormat mark expect(hasTrackFormatMark(stateAfterRemove.doc)).toBe(false); From 15ad0624876288dd611fc2791cfd0a2c09698847 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Mon, 2 Mar 2026 07:29:37 -0300 Subject: [PATCH 7/7] test(track-changes): add multi-node format cancel tests - Unit tests for sharedWid across runs and range clamping - Behavior test for toggling bold on/off across mixed-mark nodes --- .../removeMarkStep.test.js | 61 ++++++++++++++++++ ...cancel-multinode-format-suggestion.spec.ts | 63 +++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 tests/behavior/tests/comments/cancel-multinode-format-suggestion.spec.ts diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js index a2c86a1051..f66cb52a71 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.test.js @@ -150,6 +150,67 @@ describe('removeMarkStep cancel logic', () => { expect(trackFormatAttrs.before).toEqual(expect.arrayContaining([expect.objectContaining({ type: 'italic' })])); }); + it('shares TrackFormat ID across nodes when addMarkStep spans multiple runs', () => { + const italic = schema.marks.italic.create(); + const doc = createDocWithRuns([{ text: 'Hello ' }, { text: 'world', marks: [italic] }]); + const state = createState(doc); + + const bold = schema.marks.bold.create(); + const step = new AddMarkStep(2, 13, bold); + const tr = state.tr; + addMarkStep({ state, step, newTr: tr, doc: state.doc, user, date }); + const result = state.apply(tr); + + // Collect TrackFormat mark IDs from all inline nodes + const ids = []; + result.doc.descendants((node) => { + const mark = node.marks?.find((m) => m.type.name === TrackFormatMarkName); + if (mark) ids.push(mark.attrs.id); + }); + + expect(ids.length).toBe(2); + expect(ids[0]).toBe(ids[1]); + }); + + it('clamps TrackFormat mark to node boundaries, not step range', () => { + // doc > paragraph > run_1("Hello " at pos 2-7) + run_2("world" at pos 10-14) + // Run open/close tags add +1 offset each, so run_2 content starts at pos 10. + const doc = createDocWithRuns([{ text: 'Hello ' }, { text: 'world' }]); + const state = createState(doc); + + // Apply bold from pos 5-11. Clamped per-node: + // run_1 text [2,8): [max(5,2), min(11,8)] = [5,8] → "lo " (3 chars) + // run_2 text [10,15): [max(5,10), min(11,15)] = [10,11] → "w" (1 char) + const bold = schema.marks.bold.create(); + const step = new AddMarkStep(5, 11, bold); + const tr = state.tr; + addMarkStep({ state, step, newTr: tr, doc: state.doc, user, date }); + const result = state.apply(tr); + + // Collect TrackFormat marks — each should be scoped to its text portion + const trackMarks = []; + result.doc.descendants((node, pos) => { + const mark = node.marks?.find((m) => m.type.name === TrackFormatMarkName); + if (mark) trackMarks.push({ pos, size: node.nodeSize, text: node.text, id: mark.attrs.id }); + }); + + // Two TrackFormat marks: one on "lo " (from first run) and one on "w" (from second run) + expect(trackMarks.length).toBe(2); + expect(trackMarks[0].text).toBe('lo '); + expect(trackMarks[1].text).toBe('w'); + // Both should share the same ID (sharedWid) + expect(trackMarks[0].id).toBe(trackMarks[1].id); + // "Hel" and "orld" should NOT have TrackFormat (they were outside the step range) + const nonTrackedTexts = []; + result.doc.descendants((node) => { + if (node.isText && !node.marks?.some((m) => m.type.name === TrackFormatMarkName)) { + nonTrackedTexts.push(node.text); + } + }); + expect(nonTrackedTexts).toContain('Hel'); + expect(nonTrackedTexts).toContain('orld'); + }); + it('removes TrackFormat for multi-node bold toggle (Hello plain + world italic)', () => { const italic = schema.marks.italic.create(); const doc = createDocWithRuns([{ text: 'Hello ' }, { text: 'world', marks: [italic] }]); diff --git a/tests/behavior/tests/comments/cancel-multinode-format-suggestion.spec.ts b/tests/behavior/tests/comments/cancel-multinode-format-suggestion.spec.ts new file mode 100644 index 0000000000..a1378321eb --- /dev/null +++ b/tests/behavior/tests/comments/cancel-multinode-format-suggestion.spec.ts @@ -0,0 +1,63 @@ +import { test, expect } from '../../fixtures/superdoc.js'; + +test.use({ config: { toolbar: 'full', comments: 'panel', trackChanges: true } }); + +/** + * Regression test for the multi-node cancel scenario: + * When bold is toggled on then off across nodes with different pre-existing marks, + * no ghost TrackFormat marks should remain. + */ +test('no ghost TrackFormat after toggling bold on then off across mixed-mark nodes', async ({ superdoc }) => { + // Type two words — we'll make only the second one italic to create mixed marks + await superdoc.type('Hello world'); + await superdoc.waitForStable(); + + // Select "world" and make it italic in editing mode + await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const { doc } = editor.state; + // Find "world" position (after "Hello ") + let worldFrom = 0; + let worldTo = 0; + doc.descendants((node: any, pos: number) => { + if (node.isText && node.text?.includes('world')) { + const offset = node.text.indexOf('world'); + worldFrom = pos + offset; + worldTo = worldFrom + 'world'.length; + } + }); + editor.commands.setTextSelection({ from: worldFrom, to: worldTo }); + editor.commands.toggleItalic(); + }); + await superdoc.waitForStable(); + + // Switch to suggesting mode + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + // Select all and toggle bold ON + await superdoc.selectAll(); + await superdoc.page.evaluate(() => (window as any).editor.commands.toggleBold()); + await superdoc.waitForStable(); + + // Verify a format tracked change was created + await superdoc.assertTrackedChangeExists('format'); + + // Toggle bold OFF (cancel the suggestion) + await superdoc.selectAll(); + await superdoc.page.evaluate(() => (window as any).editor.commands.toggleBold()); + await superdoc.waitForStable(); + + // No track-format decorations should remain — the cancel was a no-op + await expect(superdoc.page.locator('.track-format-dec')).toHaveCount(0); + + // Text should be unchanged + await superdoc.assertTextContent('Hello world'); + + // "world" should still have italic (it was never touched) + await superdoc.assertTextHasMarks('world', ['italic']); + + // Neither word should have bold + await superdoc.assertTextLacksMarks('Hello', ['bold']); + await superdoc.assertTextLacksMarks('world', ['bold']); +});