diff --git a/packages/super-editor/src/extensions/paragraph/numberingPlugin.js b/packages/super-editor/src/extensions/paragraph/numberingPlugin.js index b19e830bc4..d031db79f2 100644 --- a/packages/super-editor/src/extensions/paragraph/numberingPlugin.js +++ b/packages/super-editor/src/extensions/paragraph/numberingPlugin.js @@ -208,58 +208,62 @@ export function createNumberingPlugin(editor) { // Generate new list properties numberingManager.enableCache(); - newState.doc.descendants((node, pos) => { - let resolvedProps = calculateResolvedParagraphProperties(editor, node, newState.doc.resolve(pos)); - if (node.type.name !== 'paragraph' || !resolvedProps.numberingProperties) { - return; - } + try { + newState.doc.descendants((node, pos) => { + let resolvedProps = calculateResolvedParagraphProperties(editor, node, newState.doc.resolve(pos)); + if (node.type.name !== 'paragraph' || !resolvedProps.numberingProperties) { + return; + } - // Retrieving numbering definition from docx - const { numId, ilvl: level = 0 } = resolvedProps.numberingProperties; - const definitionDetails = ListHelpers.getListDefinitionDetails({ numId, level, editor }); + // Retrieving numbering definition from docx + const { numId, ilvl: level = 0 } = resolvedProps.numberingProperties; + const definitionDetails = ListHelpers.getListDefinitionDetails({ numId, level, editor }); - if (!definitionDetails || Object.keys(definitionDetails).length === 0) { - // Treat as normal paragraph if definition is missing - tr.setNodeAttribute(pos, 'listRendering', null); - bumpBlockRev(node, pos); - return; - } + if (!definitionDetails || Object.keys(definitionDetails).length === 0) { + // Treat as normal paragraph if definition is missing + tr.setNodeAttribute(pos, 'listRendering', null); + bumpBlockRev(node, pos); + return; + } - let { lvlText, customFormat, listNumberingType, suffix, justification, abstractId } = definitionDetails; - // Defining the list marker - let markerText = ''; - listNumberingType = listNumberingType || 'decimal'; - const count = numberingManager.calculateCounter(numId, level, pos, abstractId); - numberingManager.setCounter(numId, level, pos, count, abstractId); - const path = numberingManager.calculatePath(numId, level, pos); - if (listNumberingType !== 'bullet') { - markerText = generateOrderedListIndex({ - listLevel: path, - lvlText: lvlText, - listNumberingType, - customFormat, - }); - } else { - markerText = docxNumberingHelpers.normalizeLvlTextChar(lvlText); - } + let { lvlText, customFormat, listNumberingType, suffix, justification, abstractId } = definitionDetails; + // Defining the list marker + let markerText = ''; + listNumberingType = listNumberingType || 'decimal'; + const count = numberingManager.calculateCounter(numId, level, pos, abstractId); + numberingManager.setCounter(numId, level, pos, count, abstractId); + const path = numberingManager.calculatePath(numId, level, pos); + if (listNumberingType !== 'bullet') { + markerText = + generateOrderedListIndex({ + listLevel: path, + lvlText: lvlText, + listNumberingType, + customFormat, + }) ?? ''; + } else { + markerText = docxNumberingHelpers.normalizeLvlTextChar(lvlText) ?? ''; + } - const newListRendering = { - markerText, - suffix, - justification, - path, - numberingType: listNumberingType, - }; + const newListRendering = { + markerText, + suffix, + justification, + path, + numberingType: listNumberingType, + }; - if (JSON.stringify(node.attrs.listRendering) !== JSON.stringify(newListRendering)) { - // Updating rendering attrs for node view usage - tr.setNodeAttribute(pos, 'listRendering', newListRendering); - bumpBlockRev(node, pos); - } + if (JSON.stringify(node.attrs.listRendering) !== JSON.stringify(newListRendering)) { + // Updating rendering attrs for node view usage + tr.setNodeAttribute(pos, 'listRendering', newListRendering); + bumpBlockRev(node, pos); + } - return false; // no need to descend into a paragraph - }); - numberingManager.disableCache(); + return false; // no need to descend into a paragraph + }); + } finally { + numberingManager.disableCache(); + } return tr.docChanged ? tr : null; }, }); diff --git a/packages/super-editor/src/extensions/paragraph/numberingPlugin.test.js b/packages/super-editor/src/extensions/paragraph/numberingPlugin.test.js index a47213466f..029f14b7b5 100644 --- a/packages/super-editor/src/extensions/paragraph/numberingPlugin.test.js +++ b/packages/super-editor/src/extensions/paragraph/numberingPlugin.test.js @@ -487,4 +487,111 @@ describe('numberingPlugin', () => { expect(tr.setNodeAttribute).not.toHaveBeenCalledWith(6, 'sdBlockRev', expect.anything()); }); }); + + describe('null lvlText crash prevention', () => { + it('does not crash when ordered list has null lvlText', () => { + const editor = createEditor(); + const plugin = createNumberingPlugin(editor); + const { appendTransaction } = plugin.spec; + + const paragraph = { + type: { name: 'paragraph' }, + attrs: { + listRendering: null, + paragraphProperties: { + numberingProperties: { numId: 1, ilvl: 0 }, + }, + }, + }; + + const doc = makeDoc([{ node: paragraph, pos: 5 }]); + const tr = createTransaction(); + const transactions = [{ docChanged: true, getMeta: vi.fn().mockReturnValue(false) }]; + + numberingManager.calculateCounter.mockReturnValue(1); + numberingManager.calculatePath.mockReturnValue([1]); + generateOrderedListIndex.mockReturnValue(null); + ListHelpers.getListDefinitionDetails.mockReturnValue({ + lvlText: null, + customFormat: null, + listNumberingType: 'decimal', + suffix: '.', + justification: 'left', + abstractId: 'a1', + }); + + const result = appendTransaction(transactions, {}, { doc, tr }); + + expect(tr.setNodeAttribute).toHaveBeenCalledWith(5, 'listRendering', { + markerText: '', + suffix: '.', + justification: 'left', + path: [1], + numberingType: 'decimal', + }); + expect(result).toBe(tr); + }); + + it('produces empty marker for bullet list with null lvlText', () => { + const editor = createEditor(); + const plugin = createNumberingPlugin(editor); + const { appendTransaction } = plugin.spec; + + const paragraph = { + type: { name: 'paragraph' }, + attrs: { + listRendering: null, + paragraphProperties: { + numberingProperties: { numId: 1, ilvl: 0 }, + }, + }, + }; + + const doc = makeDoc([{ node: paragraph, pos: 5 }]); + const tr = createTransaction(); + const transactions = [{ docChanged: true, getMeta: vi.fn().mockReturnValue(false) }]; + + numberingManager.calculateCounter.mockReturnValue(1); + numberingManager.calculatePath.mockReturnValue([1]); + docxNumberingHelpers.normalizeLvlTextChar.mockReturnValue(undefined); + ListHelpers.getListDefinitionDetails.mockReturnValue({ + lvlText: null, + customFormat: null, + listNumberingType: 'bullet', + suffix: '\t', + justification: 'left', + abstractId: 'a1', + }); + + const result = appendTransaction(transactions, {}, { doc, tr }); + + expect(tr.setNodeAttribute).toHaveBeenCalledWith(5, 'listRendering', { + markerText: '', + suffix: '\t', + justification: 'left', + path: [1], + numberingType: 'bullet', + }); + expect(result).toBe(tr); + }); + + it('ensures disableCache runs even if descendants scan throws', () => { + const editor = createEditor(); + const plugin = createNumberingPlugin(editor); + const { appendTransaction } = plugin.spec; + + const doc = { + descendants: vi.fn(() => { + throw new Error('simulated crash'); + }), + resolve: vi.fn(), + }; + const tr = createTransaction(); + const transactions = [{ docChanged: true, getMeta: vi.fn().mockReturnValue(false) }]; + + expect(() => appendTransaction(transactions, {}, { doc, tr })).toThrow('simulated crash'); + expect(numberingManager.enableCache).toHaveBeenCalled(); + expect(numberingManager.disableCache).toHaveBeenCalled(); + }); + }); }); diff --git a/shared/common/list-numbering/index.test.ts b/shared/common/list-numbering/index.test.ts index 3cfa0373a8..3076ebcd50 100644 --- a/shared/common/list-numbering/index.test.ts +++ b/shared/common/list-numbering/index.test.ts @@ -73,6 +73,54 @@ describe('generateOrderedListIndex', () => { }); expect(result).toBeNull(); }); + + describe('malformed lvlText', () => { + it('returns null when lvlText is null', () => { + const result = generateOrderedListIndex({ + listLevel: [1], + lvlText: null, + listNumberingType: 'decimal', + }); + expect(result).toBeNull(); + }); + + it('returns null when lvlText is undefined', () => { + const result = generateOrderedListIndex({ + listLevel: [1], + lvlText: undefined, + listNumberingType: 'decimal', + }); + expect(result).toBeNull(); + }); + + it('returns null when lvlText is a non-string type', () => { + const result = generateOrderedListIndex({ + listLevel: [1], + lvlText: 42 as any, + listNumberingType: 'decimal', + }); + expect(result).toBeNull(); + }); + + it('still formats correctly with valid lvlText after guard', () => { + const result = generateOrderedListIndex({ + listLevel: [3], + lvlText: '%1.', + listNumberingType: 'decimal', + }); + expect(result).toBe('3.'); + }); + }); + + it('handles undefined customFormat for custom numbering type', () => { + const result = generateOrderedListIndex({ + listLevel: [5], + lvlText: '%1)', + listNumberingType: 'custom', + customFormat: undefined, + }); + expect(result).toBe('5)'); + }); }); describe('normalizeLvlTextChar', () => { diff --git a/shared/common/list-numbering/index.ts b/shared/common/list-numbering/index.ts index a066f304d4..05d3f601e8 100644 --- a/shared/common/list-numbering/index.ts +++ b/shared/common/list-numbering/index.ts @@ -34,7 +34,7 @@ const listIndexMap: Record = { export interface GenerateOrderedListIndexOptions { listLevel: number[]; - lvlText: string; + lvlText: string | null | undefined; listNumberingType?: string; customFormat?: string; } @@ -45,11 +45,13 @@ export const generateOrderedListIndex = ({ listNumberingType, customFormat, }: GenerateOrderedListIndexOptions): string | null => { + if (typeof lvlText !== 'string') return null; const handler = listIndexMap[listNumberingType as string]; return handler ? handler(listLevel, lvlText, customFormat) : null; }; const createNumbering = (values: string[], lvlText: string): string => { + if (typeof lvlText !== 'string') return ''; return values.reduce((acc, value, index) => { return Number(value) > 9 ? acc.replace(/^0/, '').replace(`%${index + 1}`, value) @@ -75,6 +77,9 @@ const decimalZeroFormatter: NumberFormatter = (value, idx) => { }; const generateFromCustom = (path: number[], lvlText: string, customFormat: string): string => { + if (typeof customFormat !== 'string') { + return generateNumbering(path, lvlText, numberToStringFormatter); + } if (customFormat.match(/(?:[0]+\d,\s){3}\.{3}/) == null) { return generateNumbering(path, lvlText, numberToStringFormatter); }