diff --git a/devtools/visual-testing/tests/interactions/stories/formatting/clear-format-undo.ts b/devtools/visual-testing/tests/interactions/stories/formatting/clear-format-undo.ts new file mode 100644 index 0000000000..c75ba72dbb --- /dev/null +++ b/devtools/visual-testing/tests/interactions/stories/formatting/clear-format-undo.ts @@ -0,0 +1,59 @@ +import { defineStory } from '@superdoc-testing/helpers'; + +const WAIT_MS = 300; +const FONT_NAME = 'Courier New'; + +export default defineStory({ + name: 'clear-format-undo', + description: 'Clear formatting on styled text, then undo to verify marks are fully restored.', + tickets: ['SD-1771'], + startDocument: null, + layout: true, + toolbar: 'full', + waitForFonts: true, + + async run(_page, helpers): Promise { + const { step, type, newLine, bold, italic, selectAll, undo, focus, executeCommand, waitForStable, milestone } = + helpers; + + await step('Type and format text', async () => { + await focus(); + + // Type three lines with different formatting + await bold(); + await type('Bold text here.'); + await bold(); + await newLine(); + + await italic(); + await type('Italic text here.'); + await italic(); + await newLine(); + + await type('Custom font text.'); + await waitForStable(WAIT_MS); + + // Apply a custom font to the last line + await selectAll(); + await executeCommand('setFontFamily', FONT_NAME); + await waitForStable(WAIT_MS); + + await milestone('formatted', 'Text with bold, italic, and Courier New applied.'); + }); + + await step('Clear formatting', async () => { + await selectAll(); + await executeCommand('clearFormat'); + await waitForStable(WAIT_MS); + + await milestone('cleared', 'All formatting cleared from text.'); + }); + + await step('Undo clear format', async () => { + await undo(); + await waitForStable(WAIT_MS); + + await milestone('after-undo', 'Undo restored formatting — bold, italic, and Courier New should reappear.'); + }); + }, +}); diff --git a/packages/super-editor/src/core/commands/unsetAllMarks.js b/packages/super-editor/src/core/commands/unsetAllMarks.js index ba9cc2b9dc..8f02995acb 100644 --- a/packages/super-editor/src/core/commands/unsetAllMarks.js +++ b/packages/super-editor/src/core/commands/unsetAllMarks.js @@ -1,5 +1,14 @@ /** * Remove all marks in the current selection. + * + * We collect marks only from leaf/atom nodes (text nodes) and remove each + * explicitly via `tr.removeMark(from, to, mark)`. This avoids a ProseMirror + * asymmetry: `RemoveMarkStep` strips marks from ALL inline nodes (including + * non-atom containers like `run`), but its inverse `AddMarkStep` only adds + * marks to atom nodes. When a container carries a mark with different attrs + * (e.g. textStyle with all-null attrs on a run node), undo would overwrite + * the correct mark on the text node. By scoping removal to leaf-node marks + * only, the undo path restores the exact marks that were visible to the user. */ //prettier-ignore export const unsetAllMarks = () => ({ tr, dispatch, editor }) => { @@ -12,7 +21,26 @@ export const unsetAllMarks = () => ({ tr, dispatch, editor }) => { if (dispatch) { if (!empty) { ranges.forEach((range) => { - tr.removeMark(range.$from.pos, range.$to.pos); + const from = range.$from.pos; + const to = range.$to.pos; + + // Collect unique marks from leaf/atom nodes only (not inline containers) + const seen = new Set(); + const marksToRemove = []; + tr.doc.nodesBetween(from, to, (node) => { + if (!node.isInline || !node.isLeaf) return; + for (const mark of node.marks) { + const key = mark.type.name + '\0' + JSON.stringify(mark.attrs); + if (!seen.has(key)) { + seen.add(key); + marksToRemove.push(mark); + } + } + }); + + for (const mark of marksToRemove) { + tr.removeMark(from, to, mark); + } }); } // Clear stored marks to prevent formatting from being inherited by newly typed content diff --git a/packages/super-editor/src/extensions/format-commands/clear-format-undo.test.js b/packages/super-editor/src/extensions/format-commands/clear-format-undo.test.js new file mode 100644 index 0000000000..8a004baa52 --- /dev/null +++ b/packages/super-editor/src/extensions/format-commands/clear-format-undo.test.js @@ -0,0 +1,127 @@ +import { describe, expect, it } from 'vitest'; +import { TextSelection } from 'prosemirror-state'; +import { initTestEditor, loadTestDataForEditorTests } from '@tests/helpers/helpers.js'; + +/** + * SD-1771: Formatting - Undo clear formatting leaves mixed formatting + * + * Root cause: run nodes (non-atom inline containers) can carry a textStyle + * mark with all-null attrs. ProseMirror's `tr.removeMark(from, to)` creates + * RemoveMarkStep for marks on ALL inline nodes (including the run's + * null-attrs textStyle). On undo, AddMarkStep only applies to atom/text + * nodes, so the run's null-attrs textStyle overwrites the text's correct + * textStyle mark. + * + * Fix: unsetAllMarks now collects marks only from leaf/atom nodes and removes + * each explicitly, avoiding RemoveMarkSteps for container-node marks. + */ +describe('SD-1771: Clear format + undo mark restoration', () => { + it('should restore textStyle mark attrs after clear format + undo (simple doc)', () => { + const { editor } = initTestEditor({ + loadFromSchema: true, + content: { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { + type: 'run', + attrs: { + runProperties: { + bold: true, + fontFamily: { ascii: 'Roboto', hAnsi: 'Roboto', cs: 'Roboto' }, + fontSize: 44, + color: { val: '000000' }, + }, + }, + content: [ + { + type: 'text', + text: 'Hello World', + marks: [ + { type: 'bold' }, + { + type: 'textStyle', + attrs: { color: '#000000', fontFamily: 'Roboto, sans-serif', fontSize: '22pt' }, + }, + ], + }, + ], + }, + ], + }, + ], + }, + }); + + const from = 2; + const to = 2 + 'Hello World'.length; + editor.view.dispatch(editor.state.tr.setSelection(TextSelection.create(editor.state.doc, from, to))); + editor.commands.clearFormat(); + editor.commands.undo(); + + const marks = []; + editor.state.doc.descendants((node) => { + if (node.isText) marks.push(...node.marks.map((m) => ({ type: m.type.name, attrs: { ...m.attrs } }))); + }); + const textStyle = marks.find((m) => m.type === 'textStyle'); + expect(textStyle).toBeDefined(); + expect(textStyle.attrs.color).toBe('#000000'); + expect(textStyle.attrs.fontFamily).toBe('Roboto, sans-serif'); + expect(textStyle.attrs.fontSize).toBe('22pt'); + }); + + it('should restore textStyle marks correctly after clear format + undo (real DOCX)', async () => { + const { docx, media, mediaFiles, fonts } = await loadTestDataForEditorTests('sdpr.docx'); + const { editor } = initTestEditor({ content: docx, media, mediaFiles, fonts }); + + // Find first paragraph with styled text (textStyle marks with real values) + let targetFrom = null; + let targetTo = null; + let expectedColor = null; + let expectedFontFamily = null; + let expectedFontSize = null; + + editor.state.doc.descendants((node, pos) => { + if (targetFrom !== null) return false; + if (node.type.name === 'paragraph' && node.textContent.length > 5) { + node.descendants((child) => { + if (targetFrom !== null) return false; + if (child.isText) { + const ts = child.marks.find((m) => m.type.name === 'textStyle'); + if (ts && (ts.attrs.fontFamily || ts.attrs.fontSize || ts.attrs.color)) { + targetFrom = pos + 1; + targetTo = pos + node.nodeSize - 1; + expectedColor = ts.attrs.color; + expectedFontFamily = ts.attrs.fontFamily; + expectedFontSize = ts.attrs.fontSize; + return false; + } + } + }); + } + }); + + expect(targetFrom).not.toBeNull(); + + editor.view.dispatch(editor.state.tr.setSelection(TextSelection.create(editor.state.doc, targetFrom, targetTo))); + + // Clear formatting then undo + editor.commands.clearFormat(); + editor.commands.undo(); + + // After undo, the first text node's textStyle should have the original attrs + let textStyleAfterUndo = null; + editor.state.doc.nodesBetween(targetFrom, Math.min(targetTo, editor.state.doc.content.size), (node) => { + if (!textStyleAfterUndo && node.isText) { + textStyleAfterUndo = node.marks.find((m) => m.type.name === 'textStyle'); + } + }); + + expect(textStyleAfterUndo).toBeDefined(); + expect(textStyleAfterUndo.attrs.color).toBe(expectedColor); + expect(textStyleAfterUndo.attrs.fontFamily).toBe(expectedFontFamily); + expect(textStyleAfterUndo.attrs.fontSize).toBe(expectedFontSize); + }); +}); diff --git a/packages/super-editor/src/tests/data/sdpr.docx b/packages/super-editor/src/tests/data/sdpr.docx new file mode 100644 index 0000000000..b3b56b8fda Binary files /dev/null and b/packages/super-editor/src/tests/data/sdpr.docx differ