diff --git a/packages/editor/src/extensions/markdown/Lists/index.ts b/packages/editor/src/extensions/markdown/Lists/index.ts index d7095538e..1da1cfbfe 100644 --- a/packages/editor/src/extensions/markdown/Lists/index.ts +++ b/packages/editor/src/extensions/markdown/Lists/index.ts @@ -50,8 +50,8 @@ export const Lists: ExtensionAuto = (builder, opts) => { builder.use(ListsInputRulesExtension, {bulletListInputRule: opts?.ulInputRules}); + // Order matters: merge must run before collapse (see CollapseListsPlugin.ts) builder.addPlugin(mergeListsPlugin); - builder.addPlugin(collapseListsPlugin); builder diff --git a/packages/editor/src/extensions/markdown/Lists/plugins/CollapseListsPlugin.test.ts b/packages/editor/src/extensions/markdown/Lists/plugins/CollapseListsPlugin.test.ts index 5bdb02a85..81350b1d6 100644 --- a/packages/editor/src/extensions/markdown/Lists/plugins/CollapseListsPlugin.test.ts +++ b/packages/editor/src/extensions/markdown/Lists/plugins/CollapseListsPlugin.test.ts @@ -1,4 +1,5 @@ -import {EditorState, TextSelection} from 'prosemirror-state'; +import {type Node, Slice} from 'prosemirror-model'; +import {AllSelection, EditorState, TextSelection} from 'prosemirror-state'; import {builders} from 'prosemirror-test-builder'; import {EditorView} from 'prosemirror-view'; @@ -7,9 +8,10 @@ import {BaseNode, BaseSchemaSpecs} from '../../../base/BaseSchema/BaseSchemaSpec import {ListsSpecs} from '../ListsSpecs'; import {ListNode} from '../const'; -import {collapseListsPlugin} from './CollapseListsPlugin'; +import {collapseAllNestedListItems, collapseListsPlugin} from './CollapseListsPlugin'; +import {mergeListsPlugin} from './MergeListsPlugin'; -const {schema} = new ExtensionsManager({ +const {schema, markupParser: parser} = new ExtensionsManager({ extensions: (builder) => builder.use(BaseSchemaSpecs, {}).use(ListsSpecs), }).buildDeps(); @@ -21,7 +23,7 @@ const {doc, p, li, ul} = builders<'doc' | 'p' | 'li' | 'ul'>(schema, { }); describe('CollapseListsPlugin', () => { - it('should collapse nested bullet list without remaining content and move selection to the end of the first text node', () => { + it('should collapse nested bullet list without remaining content and place cursor on text', () => { const view = new EditorView(null, { state: EditorState.create({schema, plugins: [collapseListsPlugin()]}), }); @@ -34,10 +36,9 @@ describe('CollapseListsPlugin', () => { expect(view.state.doc).toMatchNode(doc(ul(li(p('Nested item'))))); - const textStartPos = view.state.doc.resolve(3); - const textEndPos = textStartPos.pos + textStartPos.nodeAfter!.nodeSize; - - expect(view.state.selection.from).toBe(textEndPos); + const sel = view.state.selection; + const $from = view.state.doc.resolve(sel.from); + expect($from.parent.type.name).toBe('paragraph'); }); it('should collapse nested bullet list with remaining content', () => { @@ -115,8 +116,8 @@ describe('CollapseListsPlugin', () => { ), ); - const textStartPos = view.state.doc.resolve(38); - expect(view.state.selection.from).toBe(textStartPos.pos); + const $from = view.state.doc.resolve(view.state.selection.from); + expect($from.parent.type.name).toBe('paragraph'); }); it('should not collapse list item without nested bullet list and not change selection if no collapse happened', () => { @@ -139,4 +140,193 @@ describe('CollapseListsPlugin', () => { expect(view.state.selection.from).toBe(selectionPos.pos); }); + + it('should not crash on a large bullet list with 3-level nesting from "N. M." items', () => { + const markdown = [ + '- 1. Replied in the original ticket.', + '- 2. Fixed the macro processing for table of contents.', + '- 3. The heading is already H2 after import, everything is correct.', + '- 4. Added support for this macro.', + '- 5. Fixed processing of such quotes.', + '- 6. Fixed processing of em dashes.', + '- 7. 8. Replied in the original ticket.', + '- 9. Could not reproduce, apparently the screenshot shows a placeholder for a magic link logo.', + '- 10. In the source data items are presented as text not a list. In markdown this is considered a numbered list and indentation is automatically added. The inner list is also formatted as a first-level indented list because it is not a second-level list at the markup level. Accordingly the indentation is the same. In this case this is expected behavior.', + '- 11. Email highlighting is standard editor behavior.', + '- 12. Added escaping of backslashes.', + '- 13. The code contains links as anchor tags and such links are not mapped. Only internal Confluence links are mapped.', + '- 14. 15. Replied in the original ticket.', + '- 16. This is a feature of how the editor works. Possibly a bug, will discuss with the team.', + '- 17. Duplicates of previous errors.', + '- 18. The image was inserted by link and either was not found or is not accessible.', + '- 19. 20. Expected behavior and link highlighting.', + ].join('\n'); + + const parsedDoc = parser.parse(markdown); + + const view = new EditorView(null, { + state: EditorState.create({schema, plugins: [collapseListsPlugin()]}), + }); + + expect(() => { + view.dispatch( + view.state.tr + .setSelection(new AllSelection(view.state.doc)) + .replaceSelection(new Slice(parsedDoc.content, 0, 0)), + ); + }).not.toThrow(); + + const resultDoc = view.state.doc; + expect(hasRedundantNesting(resultDoc)).toBe(false); + + let listItemCount = 0; + resultDoc.descendants((node) => { + if (node.type.name === ListNode.ListItem) listItemCount++; + return true; + }); + expect(listItemCount).toBe(17); + }); + + it('collapseAllNestedListItems should return null when no collapsible items exist', () => { + const view = new EditorView(null, { + state: EditorState.create({schema, plugins: [collapseListsPlugin()]}), + }); + + const initialDoc = doc(ul(li(p('Plain item')), li(p('Another item')))); + view.dispatch( + view.state.tr.replaceWith(0, view.state.doc.nodeSize - 2, initialDoc.content), + ); + + const {tr} = view.state; + expect(collapseAllNestedListItems(tr)).toBeNull(); + expect(tr.docChanged).toBe(false); + }); + + it('should correctly collapse adjacent wrapped siblings', () => { + const view = new EditorView(null, { + state: EditorState.create({schema, plugins: [collapseListsPlugin()]}), + }); + + const initialDoc = doc( + ul( + li(ul(li(p('First wrapped')))), + li(ul(li(p('Second wrapped')))), + li(ul(li(p('Third wrapped')))), + ), + ); + + view.dispatch( + view.state.tr.replaceWith(0, view.state.doc.nodeSize - 2, initialDoc.content), + ); + + expect(view.state.doc).toMatchNode( + doc(ul(li(p('First wrapped')), li(p('Second wrapped')), li(p('Third wrapped')))), + ); + }); + + it('should correctly collapse adjacent wrapped items with remaining content', () => { + const view = new EditorView(null, { + state: EditorState.create({schema, plugins: [collapseListsPlugin()]}), + }); + + const initialDoc = doc( + ul(li(ul(li(p('A'))), p('A-extra')), li(ul(li(p('B'))), p('B-extra'))), + ); + + view.dispatch( + view.state.tr.replaceWith(0, view.state.doc.nodeSize - 2, initialDoc.content), + ); + + expect(view.state.doc).toMatchNode( + doc(ul(li(p('A')), li(p('A-extra')), li(p('B')), li(p('B-extra')))), + ); + }); }); + +describe('CollapseListsPlugin + MergeListsPlugin integration', () => { + it('should collapse wrapped nesting and merge resulting adjacent lists', () => { + const markdown = [ + '- 1. First item', + '- 2. Second item', + '', + 'Some text between', + '', + '- 3. Third item', + ].join('\n'); + + const parsedDoc = parser.parse(markdown); + + // register merge BEFORE collapse — same order as production (index.ts) + const view = new EditorView(null, { + state: EditorState.create({ + schema, + plugins: [mergeListsPlugin(), collapseListsPlugin()], + }), + }); + + view.dispatch( + view.state.tr + .setSelection(new AllSelection(view.state.doc)) + .replaceSelection(new Slice(parsedDoc.content, 0, 0)), + ); + + const resultDoc = view.state.doc; + + expect(hasRedundantNesting(resultDoc)).toBe(false); + + // count top-level bullet_list nodes — the first two items should + // be in one list (merged), the third after the paragraph in another + let bulletListCount = 0; + resultDoc.forEach((child) => { + if (child.type.name === ListNode.BulletList) bulletListCount++; + }); + expect(bulletListCount).toBe(2); + }); + + it('should merge adjacent same-type lists produced by collapse', () => { + // two separate bullet lists that each contain redundant nesting + // after collapse, the lists are adjacent and should be merged + const view = new EditorView(null, { + state: EditorState.create({ + schema, + plugins: [mergeListsPlugin(), collapseListsPlugin()], + }), + }); + + // two separate top-level bullet lists, each with redundant nesting + const initialDoc = doc(ul(li(ul(li(p('A'))))), ul(li(ul(li(p('B')))))); + + view.dispatch( + view.state.tr.replaceWith(0, view.state.doc.nodeSize - 2, initialDoc.content), + ); + + const resultDoc = view.state.doc; + expect(hasRedundantNesting(resultDoc)).toBe(false); + + // after collapse both items are flat, and merge should + // combine the two adjacent bullet_lists into one + let bulletListCount = 0; + resultDoc.forEach((child) => { + if (child.type.name === ListNode.BulletList) bulletListCount++; + }); + expect(bulletListCount).toBe(1); + + expect(resultDoc).toMatchNode(doc(ul(li(p('A')), li(p('B'))))); + }); +}); + +function hasRedundantNesting(node: Node): boolean { + let found = false; + node.descendants((child) => { + if (found) return false; + if (child.type.name === ListNode.ListItem && child.firstChild) { + const fc = child.firstChild; + if (fc.type.name === ListNode.BulletList || fc.type.name === ListNode.OrderedList) { + found = true; + return false; + } + } + return true; + }); + return found; +} diff --git a/packages/editor/src/extensions/markdown/Lists/plugins/CollapseListsPlugin.ts b/packages/editor/src/extensions/markdown/Lists/plugins/CollapseListsPlugin.ts index b6da0a9c1..88c96d55b 100644 --- a/packages/editor/src/extensions/markdown/Lists/plugins/CollapseListsPlugin.ts +++ b/packages/editor/src/extensions/markdown/Lists/plugins/CollapseListsPlugin.ts @@ -1,96 +1,147 @@ -import {Fragment, type Node} from 'prosemirror-model'; -import {Plugin, TextSelection, type Transaction} from 'prosemirror-state'; +import {Fragment, type Node, type Schema} from 'prosemirror-model'; +import {Plugin, Selection, type Transaction} from 'prosemirror-state'; // @ts-ignore // TODO: fix cjs build -import {findChildren, hasParentNode} from 'prosemirror-utils'; +import {hasParentNode} from 'prosemirror-utils'; -import {getChildrenOfNode} from '../../../../utils'; import {isListItemNode, isListNode, liType} from '../utils'; +const MAX_COLLAPSE_DEPTH = 100; + +/** + * Collapses redundant nested lists inside list items + * + * Converts structures where a list item starts with + * a nested list into a flat list structure + * + * IMPORTANT: must be registered AFTER MergeListsPlugin. + * Collapsing may create adjacent lists that require merging + */ export const collapseListsPlugin = () => new Plugin({ appendTransaction(trs, oldState, newState) { + // early exit if document unchanged const docChanged = trs.some((tr) => tr.docChanged); - if (!docChanged) return null; + if (!docChanged) { + return null; + } + // early exit if not inside a list const hasParentList = hasParentNode(isListNode)(newState.selection) || hasParentNode(isListNode)(oldState.selection); - if (!hasParentList) return null; + if (!hasParentList) { + return null; + } const {tr} = newState; - let prevStepsCount = -1; - let currentStepsCount = 0; - - // execute until there are no nested lists. - while (prevStepsCount !== currentStepsCount) { - const listNodes = findChildren(tr.doc, isListNode, true); - prevStepsCount = currentStepsCount; - currentStepsCount = collapseEmptyListItems(tr, listNodes); + + // collapse of redundant nested lists + const lastCollapsePos = collapseAllNestedListItems(tr); + + // restore cursor position + if (lastCollapsePos !== null) { + const clampedPos = Math.min(lastCollapsePos, tr.doc.content.size); + tr.setSelection(Selection.near(tr.doc.resolve(clampedPos))); } return tr.docChanged ? tr : null; }, }); -export function collapseEmptyListItems( - tr: Transaction, - nodes: ReturnType, -): number { - const stepsCountBefore = tr.steps.length; - // TODO: fix cjs build - nodes.reverse().forEach((list: {node: Node; pos: number}) => { - const listNode = list.node; - const listPos = list.pos; - const childrenOfList = getChildrenOfNode(listNode).reverse(); - - childrenOfList.forEach(({node: itemNode, offset}) => { - if (isListItemNode(itemNode)) { - const {firstChild} = itemNode; - const listItemNodePos = listPos + 1 + offset; - - // if the first child of a list element is a list, - // then collapse is required - if (firstChild && isListNode(firstChild)) { - const nestedList = firstChild.content; - - // nodes at the same level as the list - const remainingNodes = itemNode.content.content.slice(1); - - const listItems = remainingNodes.length - ? nestedList.append( - Fragment.from( - liType(tr.doc.type.schema).create(null, remainingNodes), - ), - ) - : nestedList; - - const mappedStart = tr.mapping.map(listItemNodePos); - const mappedEnd = tr.mapping.map(listItemNodePos + itemNode.nodeSize); - - tr.replaceWith(mappedStart, mappedEnd, listItems); - - const closestTextNodePos = findClosestTextNodePos( - tr.doc, - mappedStart + nestedList.size, - ); - if (closestTextNodePos) { - tr.setSelection(TextSelection.create(tr.doc, closestTextNodePos)); - } - } - } +interface Replacement { + from: number; + to: number; + content: Fragment; +} + +/** + * Finds list items with redundant nesting, collapses them recursively, + * and applies replacements in reverse document order + */ +export function collapseAllNestedListItems(tr: Transaction): number | null { + const replacements: Replacement[] = []; + const schema = tr.doc.type.schema; + + // skipUntil prevents re-processing children that were already collapsed + // by the recursive call in collapseListItemContent + let skipUntil = -1; + + tr.doc.descendants((node, pos) => { + if (pos < skipUntil || node.isTextblock) { + return false; + } + + if (!isListItemNode(node)) { + return true; + } + + const {firstChild} = node; + if (!firstChild || !isListNode(firstChild)) { + return true; + } + + const collapsedContent = collapseListItemContent(node, schema, 0); + if (collapsedContent === null) { + return true; + } + + replacements.push({ + from: pos, + to: pos + node.nodeSize, + content: collapsedContent, }); + skipUntil = pos + node.nodeSize; + + return false; }); - return tr.steps.length - stepsCountBefore; + if (replacements.length === 0) { + return null; + } + + // apply from end to start so earlier positions stay valid + for (let i = replacements.length - 1; i >= 0; i--) { + const {from, to, content} = replacements[i]; + tr.replaceWith(from, to, content); + } + + // map the original end of the last replacement to its post-edit position + const last = replacements[replacements.length - 1]; + return tr.mapping.map(last.to); } -function findClosestTextNodePos(doc: Node, pos: number): number | null { - while (pos < doc.content.size) { - const node = doc.nodeAt(pos); - if (node && node.isText) { - return pos; +/** + * Recursively collapses a list item with redundant nesting + * into a Fragment of flat list items + * + * A redundantly nested list item has a list as its first child + * (instead of a paragraph) + */ +function collapseListItemContent(itemNode: Node, schema: Schema, depth: number): Fragment | null { + const {firstChild} = itemNode; + if (!firstChild || !isListNode(firstChild)) { + return null; + } + if (depth >= MAX_COLLAPSE_DEPTH) { + return null; + } + + // .slice(1) = all children except firstChild + const remaining = itemNode.content.content.slice(1); + + let result = Fragment.empty; + firstChild.forEach((child) => { + if (isListItemNode(child)) { + const collapsed = collapseListItemContent(child, schema, depth + 1); + result = result.append(collapsed ?? Fragment.from(child)); + } else { + result = result.append(Fragment.from(child)); } - pos++; + }); + + if (remaining.length) { + result = result.append(Fragment.from(liType(schema).create(null, remaining))); } - return null; + + return result; }