Skip to content

Commit 85d715d

Browse files
authored
fix: infinite headless numbering sync loop on missing list definitions (#2593)
1 parent 6fd6101 commit 85d715d

3 files changed

Lines changed: 131 additions & 10 deletions

File tree

packages/super-editor/src/editors/v1/extensions/paragraph/numberingPlugin.js

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,19 @@ export function createNumberingPlugin(editor) {
206206
}
207207
};
208208

209+
const normalizeListRendering = (listRendering) => listRendering ?? null;
210+
211+
const serializeListRendering = (listRendering) => JSON.stringify(normalizeListRendering(listRendering));
212+
213+
const updateListRenderingIfNeeded = (node, pos, nextListRendering) => {
214+
if (serializeListRendering(node?.attrs?.listRendering) === serializeListRendering(nextListRendering)) {
215+
return;
216+
}
217+
218+
tr.setNodeAttribute(pos, 'listRendering', normalizeListRendering(nextListRendering));
219+
bumpBlockRev(node, pos);
220+
};
221+
209222
// Generate new list properties
210223
numberingManager.enableCache();
211224
try {
@@ -221,9 +234,8 @@ export function createNumberingPlugin(editor) {
221234

222235
if (!definitionDetails || Object.keys(definitionDetails).length === 0) {
223236
// Treat as normal paragraph if definition is missing
224-
tr.setNodeAttribute(pos, 'listRendering', null);
225-
bumpBlockRev(node, pos);
226-
return;
237+
updateListRenderingIfNeeded(node, pos, null);
238+
return false;
227239
}
228240

229241
let { lvlText, customFormat, listNumberingType, suffix, justification, abstractId } = definitionDetails;
@@ -254,11 +266,8 @@ export function createNumberingPlugin(editor) {
254266
...(customFormat ? { customFormat } : {}),
255267
};
256268

257-
if (JSON.stringify(node.attrs.listRendering) !== JSON.stringify(newListRendering)) {
258-
// Updating rendering attrs for node view usage
259-
tr.setNodeAttribute(pos, 'listRendering', newListRendering);
260-
bumpBlockRev(node, pos);
261-
}
269+
// Updating rendering attrs for node view usage
270+
updateListRenderingIfNeeded(node, pos, newListRendering);
262271

263272
return false; // no need to descend into a paragraph
264273
});

packages/super-editor/src/editors/v1/extensions/paragraph/numberingPlugin.test.js

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,14 +259,51 @@ describe('numberingPlugin', () => {
259259
expect(result).toBe(tr);
260260
});
261261

262-
it('clears list rendering when the definition details are missing', () => {
262+
it('does not write list rendering when a missing definition is already cleared', () => {
263263
const editor = createEditor();
264264
const plugin = createNumberingPlugin(editor);
265265
const { appendTransaction } = plugin.spec;
266266

267267
const paragraph = {
268268
type: { name: 'paragraph' },
269269
attrs: {
270+
listRendering: null,
271+
paragraphProperties: {
272+
numberingProperties: { numId: 2, ilvl: 0 },
273+
},
274+
},
275+
};
276+
277+
const doc = makeDoc([{ node: paragraph, pos: 7 }]);
278+
const tr = createTransaction();
279+
const transactions = [{ docChanged: true, getMeta: vi.fn().mockReturnValue(false) }];
280+
281+
ListHelpers.getListDefinitionDetails.mockReturnValue(null);
282+
283+
const result = appendTransaction(transactions, {}, { doc, tr });
284+
285+
expect(tr.setNodeAttribute).not.toHaveBeenCalled();
286+
expect(generateOrderedListIndex).not.toHaveBeenCalled();
287+
expect(docxNumberingHelpers.normalizeLvlTextChar).not.toHaveBeenCalled();
288+
expect(numberingManager.calculateCounter).not.toHaveBeenCalled();
289+
expect(result).toBeNull();
290+
});
291+
292+
it('clears stale list rendering when the definition details are missing', () => {
293+
const editor = createEditor();
294+
const plugin = createNumberingPlugin(editor);
295+
const { appendTransaction } = plugin.spec;
296+
297+
const paragraph = {
298+
type: { name: 'paragraph' },
299+
attrs: {
300+
listRendering: {
301+
markerText: '1.',
302+
suffix: '.',
303+
justification: 'left',
304+
path: [1],
305+
numberingType: 'decimal',
306+
},
270307
paragraphProperties: {
271308
numberingProperties: { numId: 2, ilvl: 0 },
272309
},
@@ -341,7 +378,7 @@ describe('numberingPlugin', () => {
341378
expect(tr.setNodeAttribute).toHaveBeenCalledWith(3, 'sdBlockRev', 6);
342379
});
343380

344-
it('increments sdBlockRev when listRendering is cleared due to missing definition', () => {
381+
it('does not bump sdBlockRev when a missing definition is already cleared', () => {
345382
const editor = createEditor();
346383
const plugin = createNumberingPlugin(editor);
347384
const { appendTransaction } = plugin.spec;
@@ -350,6 +387,40 @@ describe('numberingPlugin', () => {
350387
type: { name: 'paragraph' },
351388
attrs: {
352389
sdBlockRev: 10,
390+
listRendering: null,
391+
paragraphProperties: {
392+
numberingProperties: { numId: 2, ilvl: 0 },
393+
},
394+
},
395+
};
396+
397+
const doc = makeDoc([{ node: paragraph, pos: 5 }]);
398+
const tr = createTransaction();
399+
const transactions = [{ docChanged: true, getMeta: vi.fn().mockReturnValue(false) }];
400+
401+
ListHelpers.getListDefinitionDetails.mockReturnValue(null);
402+
403+
appendTransaction(transactions, {}, { doc, tr });
404+
405+
expect(tr.setNodeAttribute).not.toHaveBeenCalled();
406+
});
407+
408+
it('increments sdBlockRev when stale listRendering is cleared due to a missing definition', () => {
409+
const editor = createEditor();
410+
const plugin = createNumberingPlugin(editor);
411+
const { appendTransaction } = plugin.spec;
412+
413+
const paragraph = {
414+
type: { name: 'paragraph' },
415+
attrs: {
416+
sdBlockRev: 10,
417+
listRendering: {
418+
markerText: '1.',
419+
suffix: '.',
420+
justification: 'left',
421+
path: [1],
422+
numberingType: 'decimal',
423+
},
353424
paragraphProperties: {
354425
numberingProperties: { numId: 2, ilvl: 0 },
355426
},

packages/super-editor/src/editors/v1/tests/editor/sd-1994-headless-lvltext-null.test.js

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,4 +138,45 @@ describe('SD-1994 reproduction: headless docx + markdown JSON ordered lists', ()
138138

139139
editor.destroy();
140140
}, 60_000);
141+
142+
it('does not loop when a headless docx editor encounters a missing numbering definition', async () => {
143+
const buffer = await loadDocxFixture('blank-doc.docx');
144+
const [content, , mediaFiles, fonts] = await Editor.loadXmlData(buffer, true);
145+
146+
const editor = new Editor({
147+
mode: 'docx',
148+
isHeadless: true,
149+
documentId: 'sd-2061-missing-numbering-definition',
150+
extensions: getStarterExtensions(),
151+
content,
152+
mediaFiles,
153+
fonts,
154+
jsonOverride: {
155+
type: 'doc',
156+
content: [
157+
{
158+
type: 'paragraph',
159+
attrs: {
160+
paragraphProperties: {
161+
numberingProperties: {
162+
numId: 0,
163+
ilvl: 0,
164+
},
165+
},
166+
listRendering: null,
167+
},
168+
content: [{ type: 'text', text: 'List item with missing numbering definition' }],
169+
},
170+
],
171+
},
172+
});
173+
174+
const firstParagraph = editor.getJSON()?.content?.[0];
175+
176+
expect(firstParagraph?.type).toBe('paragraph');
177+
expect(firstParagraph?.attrs?.paragraphProperties?.numberingProperties).toEqual({ numId: 0, ilvl: 0 });
178+
expect(firstParagraph?.attrs?.listRendering ?? null).toBeNull();
179+
180+
editor.destroy();
181+
}, 60_000);
141182
});

0 commit comments

Comments
 (0)