fix(comments): don't drop DOCX comments containing bookmark nodes (#3828)

packages/superdoc/src/stores/comment-normalize.js

@@ -0,0 +1,106 @@
+/**
+ * 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<string>} 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),
+  );
+
+/**
+ * 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<string>} [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. 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) {
+    return Array.isArray(node.content)
+      ? node.content
+          .map((child) => normalizeCommentForEditor(child, supportedNodeNames))
+          .flat()
+          .filter(Boolean)
+      : 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,
+  };
+};

packages/superdoc/src/stores/comment-normalize.test.js

@@ -0,0 +1,141 @@
+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('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');
+  });
+});

packages/superdoc/src/stores/comments-store.bookmark.test.js

@@ -0,0 +1,128 @@
+import { describe, it, expect, beforeEach, 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' }];
+  });
+
+  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');
+  });
+});