diff --git a/packages/superdoc/src/stores/comment-normalize.js b/packages/superdoc/src/stores/comment-normalize.js new file mode 100644 index 0000000000..2784ba72d4 --- /dev/null +++ b/packages/superdoc/src/stores/comment-normalize.js @@ -0,0 +1,120 @@ +/** + * Node names the reduced rich-text schema registers, derived from the extension + * objects themselves (each carries `.type === 'node'` and `.name`). ProseMirror adds + * no implicit nodes, so this set equals the schema's node set. Used to strip nodes + * that would make `nodeFromJSON` throw when rendering comment HTML. (#3828) + * + * @param {Array<{ type?: string, name?: string }>} [extensions] Editor extensions. + * @returns {Set} Registered node type names. + */ +export const getRichTextSupportedNodeNames = (extensions = []) => + new Set( + (Array.isArray(extensions) ? extensions : []) + .filter((ext) => ext?.type === 'node') + .map((ext) => ext?.name) + .filter(Boolean), + ); + +/** + * Visible whitespace-only leaf nodes that the reduced rich-text schema can't + * represent. Rather than dropping them (which would silently join surrounding + * words), map them to text so the separation survives in the rendered HTML. + */ +const VISIBLE_LEAF_NODE_TEXT = { + lineBreak: '\n', + hardBreak: '\n', + tab: '\t', +}; + +/** + * Normalize imported DOCX comment JSON into content the reduced rich-text schema can + * load. Unwraps `run` nodes, strips font attrs from `textStyle` marks, and drops any + * node the rich-text schema does not register (e.g. `bookmarkStart`/`bookmarkEnd`), + * preserving inline content those nodes wrap so the visible text survives. The caller's + * original `docxCommentJSON` is untouched, so exported metadata (bookmarks, etc.) is + * retained. + * + * When `supportedNodeNames` is empty/undefined (schema unavailable) it falls back to + * stripping only the invisible bookmark boundary nodes. (#3828) + * + * @param {*} node A ProseMirror JSON node, array of nodes, or primitive. + * @param {Set} [supportedNodeNames] Node names the target schema supports. + * @returns {*} Normalized node(s), or `null` for a dropped leaf node. + */ +export const normalizeCommentForEditor = (node, supportedNodeNames) => { + if (Array.isArray(node)) { + return node + .map((child) => normalizeCommentForEditor(child, supportedNodeNames)) + .flat() + .filter(Boolean); + } + + if (!node || typeof node !== 'object') return node; + + // Drop invisible bookmark boundary nodes and any node absent from the rich-text + // schema used to render comment HTML, preserving inline content they wrap so the + // visible text survives. Visible whitespace leaves (line breaks, tabs) are mapped + // to text so the separation is not lost. Falls back to stripping only bookmark + // boundary nodes when the supported-node set can't be determined (size 0). (#3828) + const isBoundaryNode = node.type === 'bookmarkStart' || node.type === 'bookmarkEnd'; + const isUnsupported = supportedNodeNames && supportedNodeNames.size > 0 && !supportedNodeNames.has(node.type); + if (isBoundaryNode || isUnsupported) { + if (Array.isArray(node.content)) { + return node.content + .map((child) => normalizeCommentForEditor(child, supportedNodeNames)) + .flat() + .filter(Boolean); + } + const visibleText = VISIBLE_LEAF_NODE_TEXT[node.type]; + return visibleText !== undefined ? { type: 'text', text: visibleText } : null; + } + + const stripTextStyleAttrs = (attrs) => { + if (!attrs) return attrs; + const rest = { ...attrs }; + delete rest.fontSize; + delete rest.fontFamily; + delete rest.eastAsiaFontFamily; + return Object.keys(rest).length ? rest : undefined; + }; + + const normalizeMark = (mark) => { + if (!mark) return mark; + const typeName = typeof mark.type === 'string' ? mark.type : mark.type?.name; + const attrs = mark?.attrs ? { ...mark.attrs } : undefined; + if (typeName === 'textStyle' && attrs) { + return { ...mark, attrs: stripTextStyleAttrs(attrs) }; + } + return { ...mark, attrs }; + }; + + const cloneMarks = (marks) => + Array.isArray(marks) ? marks.filter(Boolean).map((mark) => normalizeMark(mark)) : undefined; + + const cloneAttrs = (attrs) => (attrs && typeof attrs === 'object' ? { ...attrs } : undefined); + + if (!Array.isArray(node.content)) { + return { + type: node.type, + ...(node.text !== undefined ? { text: node.text } : {}), + ...(node.attrs ? { attrs: cloneAttrs(node.attrs) } : {}), + ...(node.marks ? { marks: cloneMarks(node.marks) } : {}), + }; + } + + const normalizedChildren = node.content + .map((child) => normalizeCommentForEditor(child, supportedNodeNames)) + .flat() + .filter(Boolean); + + if (node.type === 'run') { + return normalizedChildren; + } + + return { + type: node.type, + ...(node.attrs ? { attrs: cloneAttrs(node.attrs) } : {}), + ...(node.marks ? { marks: cloneMarks(node.marks) } : {}), + content: normalizedChildren, + }; +}; diff --git a/packages/superdoc/src/stores/comment-normalize.test.js b/packages/superdoc/src/stores/comment-normalize.test.js new file mode 100644 index 0000000000..b73a2cc5dd --- /dev/null +++ b/packages/superdoc/src/stores/comment-normalize.test.js @@ -0,0 +1,158 @@ +import { describe, it, expect } from 'vitest'; +import { normalizeCommentForEditor, getRichTextSupportedNodeNames } from './comment-normalize.js'; + +// Synthetic rich-text extension set (shape mirrors real Node/Mark extensions, which +// carry `.type` and `.name`). Kept local so this unit test stays pure and independent +// of the super-editor/font-system import chain. +const richTextExtensions = [ + { type: 'node', name: 'doc' }, + { type: 'node', name: 'paragraph' }, + { type: 'node', name: 'text' }, + { type: 'node', name: 'mention' }, + { type: 'mark', name: 'bold' }, + { type: 'mark', name: 'textStyle' }, + { type: 'extension', name: 'history' }, +]; +const getRichTextExtensions = () => richTextExtensions; + +const collectTypes = (nodes) => { + const types = []; + const walk = (node) => { + if (Array.isArray(node)) return node.forEach(walk); + if (!node || typeof node !== 'object') return; + types.push(node.type); + if (Array.isArray(node.content)) node.content.forEach(walk); + }; + walk(nodes); + return types; +}; + +// Mirrors the issue's minimal shape: an invisible bookmark pair around visible text. +const bookmarkParagraph = () => ({ + type: 'paragraph', + content: [ + { type: 'run', content: [{ type: 'text', text: 'Before mention ' }] }, + { type: 'bookmarkStart', attrs: { id: '42', name: '_mention' } }, + { type: 'run', content: [{ type: 'text', text: '@Person' }] }, + { type: 'bookmarkEnd', attrs: { id: '42' } }, + ], +}); + +describe('getRichTextSupportedNodeNames', () => { + it('derives node names from `.type === "node"` extensions and excludes bookmarks/marks', () => { + const names = getRichTextSupportedNodeNames(getRichTextExtensions()); + expect(names.has('paragraph')).toBe(true); + expect(names.has('text')).toBe(true); + expect(names.has('mention')).toBe(true); + // Bookmarks and marks are not registered as rich-text nodes. + expect(names.has('bookmarkStart')).toBe(false); + expect(names.has('bookmarkEnd')).toBe(false); + expect(names.has('bold')).toBe(false); + }); + + it('returns an empty set for empty/invalid input', () => { + expect(getRichTextSupportedNodeNames().size).toBe(0); + expect(getRichTextSupportedNodeNames(null).size).toBe(0); + expect(getRichTextSupportedNodeNames([{ type: 'mark', name: 'bold' }]).size).toBe(0); + }); +}); + +describe('normalizeCommentForEditor', () => { + const supported = getRichTextSupportedNodeNames(getRichTextExtensions()); + + it('drops bookmark boundary nodes while preserving visible text', () => { + const result = normalizeCommentForEditor([bookmarkParagraph()], supported); + const types = collectTypes(result); + expect(types).not.toContain('bookmarkStart'); + expect(types).not.toContain('bookmarkEnd'); + + const paragraph = result[0]; + expect(paragraph.type).toBe('paragraph'); + const text = paragraph.content.map((n) => n.text).join(''); + expect(text).toBe('Before mention @Person'); + }); + + it('unwraps an unsupported node, keeping its inline children', () => { + const node = { + type: 'paragraph', + content: [ + { + type: 'someUnknownWrapper', + content: [{ type: 'text', text: 'kept text' }], + }, + ], + }; + const [paragraph] = normalizeCommentForEditor([node], supported); + expect(collectTypes(paragraph)).not.toContain('someUnknownWrapper'); + expect(paragraph.content).toEqual([{ type: 'text', text: 'kept text' }]); + }); + + it('drops an unsupported leaf node entirely', () => { + const node = { + type: 'paragraph', + content: [ + { type: 'text', text: 'a' }, + { type: 'someUnknownLeaf', attrs: { x: 1 } }, + { type: 'text', text: 'b' }, + ], + }; + const [paragraph] = normalizeCommentForEditor([node], supported); + expect(paragraph.content).toEqual([ + { type: 'text', text: 'a' }, + { type: 'text', text: 'b' }, + ]); + }); + + it('maps unsupported visible whitespace leaves (line breaks, tabs) to text', () => { + const node = { + type: 'paragraph', + content: [ + { type: 'text', text: 'One' }, + { type: 'lineBreak' }, + { type: 'text', text: 'Two' }, + { type: 'tab' }, + { type: 'text', text: 'Three' }, + ], + }; + const [paragraph] = normalizeCommentForEditor([node], supported); + expect(collectTypes(paragraph)).not.toContain('lineBreak'); + expect(collectTypes(paragraph)).not.toContain('tab'); + expect(paragraph.content.map((n) => n.text).join('')).toBe('One\nTwo\tThree'); + }); + + it('passes supported nodes through and strips font attrs from textStyle marks', () => { + const node = { + type: 'paragraph', + content: [ + { + type: 'run', + content: [ + { + type: 'text', + text: 'styled', + marks: [{ type: 'textStyle', attrs: { fontSize: '12pt', color: '#f00' } }], + }, + ], + }, + ], + }; + const [paragraph] = normalizeCommentForEditor([node], supported); + expect(paragraph.type).toBe('paragraph'); + const [text] = paragraph.content; + expect(text).toMatchObject({ type: 'text', text: 'styled' }); + expect(text.marks[0].attrs).toEqual({ color: '#f00' }); + expect(text.marks[0].attrs).not.toHaveProperty('fontSize'); + }); + + it('falls back to stripping only bookmark nodes when the supported set is empty', () => { + const result = normalizeCommentForEditor([bookmarkParagraph()], new Set()); + const types = collectTypes(result); + // Bookmarks always stripped... + expect(types).not.toContain('bookmarkStart'); + expect(types).not.toContain('bookmarkEnd'); + // ...but supported/other nodes are not aggressively removed in fallback mode. + expect(types).toContain('paragraph'); + const text = result[0].content.map((n) => n.text).join(''); + expect(text).toBe('Before mention @Person'); + }); +}); diff --git a/packages/superdoc/src/stores/comments-store.bookmark.test.js b/packages/superdoc/src/stores/comments-store.bookmark.test.js new file mode 100644 index 0000000000..f1ad1a4a99 --- /dev/null +++ b/packages/superdoc/src/stores/comments-store.bookmark.test.js @@ -0,0 +1,132 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { createPinia, setActivePinia, defineStore } from 'pinia'; +import { ref, reactive } from 'vue'; + +// End-to-end regression test for #3828. Unlike comments-store.test.js, this file does +// NOT mock `@superdoc/super-editor` — it exercises the real headless Editor and the real +// `getRichTextExtensions()` schema so the `Unknown node type: bookmarkStart` throw is +// actually reproduced. Only the peripheral store dependencies are mocked. + +vi.mock('./superdoc-store.js', () => { + const documents = ref([]); + const user = reactive({ name: 'Alice', email: 'alice@example.com' }); + const activeSelection = reactive({ documentId: 'doc-1', selectionBounds: {} }); + const selectionPosition = reactive({ source: null }); + const getDocument = (id) => documents.value.find((doc) => doc.id === id); + + const useMockStore = defineStore('superdoc', () => ({ + documents, + user, + activeSelection, + selectionPosition, + getDocument, + })); + + return { + useSuperdocStore: useMockStore, + __mockSuperdoc: { + documents, + user, + activeSelection, + selectionPosition, + emit: vi.fn(), + config: { isInternal: false }, + }, + }; +}); + +vi.mock('@superdoc/components/CommentsLayer/use-comment', () => ({ + default: vi.fn((params = {}) => { + const selection = params.selection || { source: 'mock', selectionBounds: {} }; + return { + ...params, + selection, + getValues: () => ({ ...params, selection }), + setText: vi.fn(), + }; + }), +})); + +vi.mock('../core/collaboration/helpers.js', () => ({ + syncCommentsToClients: vi.fn(), +})); + +vi.mock('../helpers/group-changes.js', () => ({ + groupChanges: vi.fn(() => []), +})); + +import { useCommentsStore } from './comments-store.js'; +import { __mockSuperdoc } from './superdoc-store.js'; + +// Mirrors the issue's minimal shape: an invisible bookmark pair around visible text. +const bookmarkComment = () => ({ + commentId: 'c-bookmark', + creatorName: 'Imported Author', + creatorEmail: 'imported@example.com', + createdTime: 123, + elements: [ + { + type: 'paragraph', + content: [ + { type: 'run', content: [{ type: 'text', text: 'Before mention ' }] }, + { type: 'bookmarkStart', attrs: { id: '42', name: '_mention' } }, + { type: 'run', content: [{ type: 'text', text: '@Person' }] }, + { type: 'bookmarkEnd', attrs: { id: '42' } }, + ], + }, + ], +}); + +const collectTypes = (nodes) => { + const types = []; + const walk = (node) => { + if (Array.isArray(node)) return node.forEach(walk); + if (!node || typeof node !== 'object') return; + if (node.type) types.push(node.type); + if (Array.isArray(node.content)) node.content.forEach(walk); + }; + walk(nodes); + return types; +}; + +describe('processLoadedDocxComments — bookmark nodes (#3828)', () => { + let store; + const editor = { + converter: { commentThreadingProfile: null }, + state: {}, + options: { documentId: 'doc-1' }, + }; + + beforeEach(() => { + vi.useFakeTimers(); + vi.clearAllMocks(); + setActivePinia(createPinia()); + store = useCommentsStore(); + __mockSuperdoc.documents.value = [{ id: 'doc-1', type: 'docx' }]; + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('imports a comment containing bookmark boundary nodes instead of dropping it', () => { + store.processLoadedDocxComments({ + superdoc: __mockSuperdoc, + editor, + comments: [bookmarkComment()], + documentId: 'doc-1', + }); + + const comment = store.commentsList.find((c) => c.commentId === 'c-bookmark'); + expect(comment).toBeDefined(); + + // Visible text (and the mention) survive in the rendered HTML. + expect(comment.commentText).toContain('Before mention'); + expect(comment.commentText).toContain('@Person'); + + // The original DOCX JSON — including the bookmark metadata — is preserved for export. + const preservedTypes = collectTypes(comment.docxCommentJSON); + expect(preservedTypes).toContain('bookmarkStart'); + expect(preservedTypes).toContain('bookmarkEnd'); + }); +}); diff --git a/packages/superdoc/src/stores/comments-store.js b/packages/superdoc/src/stores/comments-store.js index da9644c694..4dd9df253a 100644 --- a/packages/superdoc/src/stores/comments-store.js +++ b/packages/superdoc/src/stores/comments-store.js @@ -17,6 +17,7 @@ import { import useComment from '@superdoc/components/CommentsLayer/use-comment'; import { groupChanges } from '../helpers/group-changes.js'; import { buildFloatingCommentInstances } from './helpers/floating-comment-instances.js'; +import { normalizeCommentForEditor, getRichTextSupportedNodeNames } from './comment-normalize.js'; export const useCommentsStore = defineStore('comments', () => { const BODY_TRACKED_CHANGE_STORY = { kind: 'story', storyType: 'body' }; @@ -2511,66 +2512,6 @@ export const useCommentsStore = defineStore('comments', () => { * @param {Object} commentTextJson The comment text JSON * @returns {string} The HTML content */ - const normalizeCommentForEditor = (node) => { - if (Array.isArray(node)) { - return node - .map((child) => normalizeCommentForEditor(child)) - .flat() - .filter(Boolean); - } - - if (!node || typeof node !== 'object') return node; - - const stripTextStyleAttrs = (attrs) => { - if (!attrs) return attrs; - const rest = { ...attrs }; - delete rest.fontSize; - delete rest.fontFamily; - delete rest.eastAsiaFontFamily; - return Object.keys(rest).length ? rest : undefined; - }; - - const normalizeMark = (mark) => { - if (!mark) return mark; - const typeName = typeof mark.type === 'string' ? mark.type : mark.type?.name; - const attrs = mark?.attrs ? { ...mark.attrs } : undefined; - if (typeName === 'textStyle' && attrs) { - return { ...mark, attrs: stripTextStyleAttrs(attrs) }; - } - return { ...mark, attrs }; - }; - - const cloneMarks = (marks) => - Array.isArray(marks) ? marks.filter(Boolean).map((mark) => normalizeMark(mark)) : undefined; - - const cloneAttrs = (attrs) => (attrs && typeof attrs === 'object' ? { ...attrs } : undefined); - - if (!Array.isArray(node.content)) { - return { - type: node.type, - ...(node.text !== undefined ? { text: node.text } : {}), - ...(node.attrs ? { attrs: cloneAttrs(node.attrs) } : {}), - ...(node.marks ? { marks: cloneMarks(node.marks) } : {}), - }; - } - - const normalizedChildren = node.content - .map((child) => normalizeCommentForEditor(child)) - .flat() - .filter(Boolean); - - if (node.type === 'run') { - return normalizedChildren; - } - - return { - type: node.type, - ...(node.attrs ? { attrs: cloneAttrs(node.attrs) } : {}), - ...(node.marks ? { marks: cloneMarks(node.marks) } : {}), - content: normalizedChildren, - }; - }; - const getHtmlFromComment = (commentTextElements) => { // If no content, we can't convert and its not a valid comment const elementsArray = Array.isArray(commentTextElements) @@ -2582,7 +2523,8 @@ export const useCommentsStore = defineStore('comments', () => { if (!hasContent) return; try { - const normalizedContent = normalizeCommentForEditor(elementsArray); + const supportedNodeNames = getRichTextSupportedNodeNames(getRichTextExtensions()); + const normalizedContent = normalizeCommentForEditor(elementsArray, supportedNodeNames); const contentArray = Array.isArray(normalizedContent) ? normalizedContent : normalizedContent