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/6] 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/6] 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/6] 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/6] 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 9998cf01f2defc93227a77cea98dd9af53beafa1 Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Sat, 28 Feb 2026 20:38:28 +0530 Subject: [PATCH 5/6] test: add unit tests for track format shared IDs and empty-mark removal Cover the three fixes from this PR: - Shared TrackFormat ID across nodes in a single AddMarkStep - TrackFormat removal when toggling a tracked addition back (empty after + empty before) - TrackFormat retention when before is non-empty (real tracked removal) --- .../trackChangesHelpers/addMarkStep.test.js | 169 ++++++++++++++++++ 1 file changed, 169 insertions(+) create mode 100644 packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.test.js diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.test.js new file mode 100644 index 0000000000..fa60113a92 --- /dev/null +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.test.js @@ -0,0 +1,169 @@ +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest'; +import { EditorState, TextSelection } from 'prosemirror-state'; +import { AddMarkStep, RemoveMarkStep } from 'prosemirror-transform'; +import { trackedTransaction } from './index.js'; +import { TrackFormatMarkName } from '../constants.js'; +import { initTestEditor } from '@tests/helpers/helpers.js'; + +describe('trackChangesHelpers addMarkStep / removeMarkStep (track format)', () => { + let editor; + let schema; + let basePlugins; + + const user = { name: 'Track Tester', email: 'track@example.com' }; + + beforeEach(() => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + schema = editor.schema; + basePlugins = editor.state.plugins; + }); + + afterEach(() => { + vi.restoreAllMocks(); + editor?.destroy(); + editor = null; + }); + + const createState = (doc) => + EditorState.create({ + schema, + doc, + plugins: basePlugins, + }); + + /** + * Collect all TrackFormat marks from a document. + * Returns an array of { id, before, after, from, to } objects. + */ + const getTrackFormatMarks = (docNode) => { + const results = []; + docNode.descendants((node, pos) => { + if (!node.isInline) return; + const tfMark = node.marks.find((m) => m.type.name === TrackFormatMarkName); + if (tfMark) { + results.push({ + id: tfMark.attrs.id, + before: tfMark.attrs.before, + after: tfMark.attrs.after, + from: pos, + to: pos + node.nodeSize, + }); + } + }); + return results; + }; + + it('shares one TrackFormat ID across two text nodes when a single AddMarkStep spans both', () => { + // Create a paragraph with two adjacent text nodes inside a run: + // "Hello" (plain) + "World" (italic). A single AddMarkStep across both + // should produce TrackFormat marks that share one ID. + const italicMark = schema.marks.italic.create(); + const run = schema.nodes.run.create({}, [schema.text('Hello'), schema.text('World', [italicMark])]); + const doc = schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, run)); + let state = createState(doc); + + // Find positions of both text nodes + let helloPos = null; + let worldEnd = null; + state.doc.descendants((node, pos) => { + if (node.isText && node.text === 'Hello') helloPos = pos; + if (node.isText && node.text === 'World') worldEnd = pos + node.nodeSize; + }); + expect(helloPos).toBeTypeOf('number'); + expect(worldEnd).toBeTypeOf('number'); + + // Use a single AddMarkStep to ensure both nodes are processed in one call + const boldMark = schema.marks.bold.create(); + let tr = state.tr; + tr.step(new AddMarkStep(helloPos, worldEnd, boldMark)); + tr.setMeta('inputType', 'programmatic'); + const tracked = trackedTransaction({ tr, state, user }); + state = state.apply(tracked); + + // Both text nodes should have TrackFormat marks with the same ID + const tfMarks = getTrackFormatMarks(state.doc); + expect(tfMarks.length).toBeGreaterThanOrEqual(2); + + const ids = new Set(tfMarks.map((m) => m.id)); + expect(ids.size).toBe(1); + + // The "after" array should include bold + for (const tf of tfMarks) { + expect(tf.after.some((s) => s.type === 'bold')).toBe(true); + } + }); + + it('removes TrackFormat mark when toggling bold off immediately after adding it', () => { + // Create a paragraph with plain text "Hello" + const run = schema.nodes.run.create({}, [schema.text('Hello')]); + const doc = schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, run)); + let state = createState(doc); + + let helloPos = null; + let helloEnd = null; + state.doc.descendants((node, pos) => { + if (node.isText && node.text === 'Hello') { + helloPos = pos; + helloEnd = pos + node.nodeSize; + } + }); + expect(helloPos).toBeTypeOf('number'); + + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, helloPos, helloEnd))); + + // Step 1: Add bold (tracked) + const boldMark = schema.marks.bold.create(); + let tr = state.tr.addMark(helloPos, helloEnd, boldMark); + tr.setMeta('inputType', 'programmatic'); + let tracked = trackedTransaction({ tr, state, user }); + state = state.apply(tracked); + + // Verify TrackFormat exists after adding bold + let tfMarks = getTrackFormatMarks(state.doc); + expect(tfMarks.length).toBeGreaterThanOrEqual(1); + expect(tfMarks[0].after.some((s) => s.type === 'bold')).toBe(true); + + // Step 2: Remove bold (tracked) — this reverses the tracked addition + tr = state.tr.removeMark(helloPos, helloEnd, boldMark); + tr.setMeta('inputType', 'programmatic'); + tracked = trackedTransaction({ tr, state, user }); + state = state.apply(tracked); + + // TrackFormat should be completely removed (both before and after are empty) + tfMarks = getTrackFormatMarks(state.doc); + expect(tfMarks.length).toBe(0); + }); + + it('keeps TrackFormat when removing bold reveals a tracked removal (before is non-empty)', () => { + // Create text that already has bold — removing bold in track mode creates + // a TrackFormat with before=[bold], after=[]. This should persist because + // it represents a real tracked removal. + const boldMark = schema.marks.bold.create(); + const run = schema.nodes.run.create({}, [schema.text('Hello', [boldMark])]); + const doc = schema.nodes.doc.create({}, schema.nodes.paragraph.create({}, run)); + let state = createState(doc); + + let helloPos = null; + let helloEnd = null; + state.doc.descendants((node, pos) => { + if (node.isText && node.text === 'Hello') { + helloPos = pos; + helloEnd = pos + node.nodeSize; + } + }); + expect(helloPos).toBeTypeOf('number'); + + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, helloPos, helloEnd))); + + // Remove bold (tracked) + let tr = state.tr.removeMark(helloPos, helloEnd, boldMark); + tr.setMeta('inputType', 'programmatic'); + const tracked = trackedTransaction({ tr, state, user }); + state = state.apply(tracked); + + // TrackFormat should persist with before=[bold] + const tfMarks = getTrackFormatMarks(state.doc); + expect(tfMarks.length).toBeGreaterThanOrEqual(1); + expect(tfMarks[0].before.some((s) => s.type === 'bold')).toBe(true); + }); +}); From b123b31ce63717da0bde1bded4a94011420fd2e4 Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Sun, 1 Mar 2026 17:55:39 +0530 Subject: [PATCH 6/6] =?UTF-8?q?fix:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20remove=20unused=20import=20and=20add=20range-clampi?= =?UTF-8?q?ng=20assertion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../track-changes/trackChangesHelpers/addMarkStep.test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.test.js index fa60113a92..0405b6ec7b 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.test.js @@ -1,6 +1,6 @@ import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest'; import { EditorState, TextSelection } from 'prosemirror-state'; -import { AddMarkStep, RemoveMarkStep } from 'prosemirror-transform'; +import { AddMarkStep } from 'prosemirror-transform'; import { trackedTransaction } from './index.js'; import { TrackFormatMarkName } from '../constants.js'; import { initTestEditor } from '@tests/helpers/helpers.js'; @@ -87,6 +87,9 @@ describe('trackChangesHelpers addMarkStep / removeMarkStep (track format)', () = const ids = new Set(tfMarks.map((m) => m.id)); expect(ids.size).toBe(1); + // Ranges should be clamped to per-node boundaries (no overlap between marks) + expect(tfMarks[0].to).toBeLessThanOrEqual(tfMarks[1].from); + // The "after" array should include bold for (const tf of tfMarks) { expect(tf.after.some((s) => s.type === 'bold')).toBe(true);