Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Skip already visited list elements during reconversion and post fixing #17564

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Skip already visited list elements during reconversion and post fixing #17564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions packages/ckeditor5-list/src/list/converters.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,32 +138,33 @@ export function reconvertItemsOnDataChange(
const changes = model.document.differ.getChanges();
const itemsToRefresh = [];
const itemToListHead = new Map<ListElement, ListElement>();
const visited = new Set<Element>();
const changedItems = new Set<Node>();

for ( const entry of changes ) {
if ( entry.type == 'insert' && entry.name != '$text' ) {
findAndAddListHeadToMap( entry.position, itemToListHead );
findAndAddListHeadToMap( entry.position, itemToListHead, visited );

// Insert of a non-list item.
if ( !entry.attributes.has( 'listItemId' ) ) {
findAndAddListHeadToMap( entry.position.getShiftedBy( entry.length ), itemToListHead );
findAndAddListHeadToMap( entry.position.getShiftedBy( entry.length ), itemToListHead, visited );
} else {
changedItems.add( entry.position.nodeAfter! );
}
}
// Removed list item.
else if ( entry.type == 'remove' && entry.attributes.has( 'listItemId' ) ) {
findAndAddListHeadToMap( entry.position, itemToListHead );
findAndAddListHeadToMap( entry.position, itemToListHead, visited );
}
// Changed list attribute.
else if ( entry.type == 'attribute' ) {
const item = entry.range.start.nodeAfter!;

if ( attributeNames.includes( entry.attributeKey ) ) {
findAndAddListHeadToMap( entry.range.start, itemToListHead );
findAndAddListHeadToMap( entry.range.start, itemToListHead, visited );

if ( entry.attributeNewValue === null ) {
findAndAddListHeadToMap( entry.range.start.getShiftedBy( 1 ), itemToListHead );
findAndAddListHeadToMap( entry.range.start.getShiftedBy( 1 ), itemToListHead, visited );

// Check if paragraph should be converted from bogus to plain paragraph.
if ( doesItemBlockRequiresRefresh( item as Element ) ) {
Expand Down
13 changes: 7 additions & 6 deletions packages/ckeditor5-list/src/list/listediting.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,7 @@ function modelChangePostFixer(
listEditing: ListEditing
) {
const changes = model.document.differ.getChanges();
const visited = new Set<Element>();
const itemToListHead = new Map<ListElement, ListElement>();
const multiBlock = listEditing.editor.config.get( 'list.multiBlock' );

Expand All @@ -775,30 +776,30 @@ function modelChangePostFixer(
}
}

findAndAddListHeadToMap( entry.position, itemToListHead );
findAndAddListHeadToMap( entry.position, itemToListHead, visited );

// Insert of a non-list item - check if there is a list after it.
if ( !entry.attributes.has( 'listItemId' ) ) {
findAndAddListHeadToMap( entry.position.getShiftedBy( entry.length ), itemToListHead );
findAndAddListHeadToMap( entry.position.getShiftedBy( entry.length ), itemToListHead, visited );
}

// Check if there is no nested list.
for ( const { item: innerItem, previousPosition } of model.createRangeIn( item as Element ) ) {
if ( isListItemBlock( innerItem ) ) {
findAndAddListHeadToMap( previousPosition, itemToListHead );
findAndAddListHeadToMap( previousPosition, itemToListHead, visited );
}
}
}
// Removed list item or block adjacent to a list.
else if ( entry.type == 'remove' ) {
findAndAddListHeadToMap( entry.position, itemToListHead );
findAndAddListHeadToMap( entry.position, itemToListHead, visited );
}
// Changed list item indent or type.
else if ( entry.type == 'attribute' && attributeNames.includes( entry.attributeKey ) ) {
findAndAddListHeadToMap( entry.range.start, itemToListHead );
findAndAddListHeadToMap( entry.range.start, itemToListHead, visited );

if ( entry.attributeNewValue === null ) {
findAndAddListHeadToMap( entry.range.start.getShiftedBy( 1 ), itemToListHead );
findAndAddListHeadToMap( entry.range.start.getShiftedBy( 1 ), itemToListHead, visited );
}
}

Expand Down
12 changes: 10 additions & 2 deletions packages/ckeditor5-list/src/list/utils/postfixers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @module list/list/utils/postfixers
*/

import type { Position, Writer } from 'ckeditor5/src/engine.js';
import type { Element, Position, Writer } from 'ckeditor5/src/engine.js';
import { SiblingListBlocksIterator, type ListIteratorValue } from './listwalker.js';
import { getListItemBlocks, isListItemBlock, ListItemUid, type ListElement } from './model.js';

Expand All @@ -17,10 +17,12 @@ import { getListItemBlocks, isListItemBlock, ListItemUid, type ListElement } fro
* @internal
* @param position The search starting position.
* @param itemToListHead The map from list item element to the list head element.
* @param visited A set of elements that were already visited.
*/
export function findAndAddListHeadToMap(
position: Position,
itemToListHead: Map<ListElement, ListElement>
itemToListHead: Map<ListElement, ListElement>,
visited: Set<Element>
): void {
const previousNode = position.nodeBefore;

Expand All @@ -42,6 +44,12 @@ export function findAndAddListHeadToMap(
for ( const { node } of new SiblingListBlocksIterator( listHead, 'backward' ) ) {
listHead = node;

if ( visited.has( listHead ) ) {
return;
}

visited.add( listHead );

if ( itemToListHead.has( listHead ) ) {
return;
}
Expand Down
24 changes: 16 additions & 8 deletions packages/ckeditor5-list/tests/list/utils/postfixers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ describe( 'List - utils - postfixers', () => {
const fragment = parseModel( input, schema );
const position = model.createPositionAt( fragment, 1 );
const itemToListHead = new Map();
const visited = new Set();

findAndAddListHeadToMap( position, itemToListHead );
findAndAddListHeadToMap( position, itemToListHead, visited );

const heads = Array.from( itemToListHead.values() );

Expand All @@ -62,8 +63,9 @@ describe( 'List - utils - postfixers', () => {
const fragment = parseModel( input, schema );
const position = model.createPositionAt( fragment, 2 );
const itemToListHead = new Map();
const visited = new Set();

findAndAddListHeadToMap( position, itemToListHead );
findAndAddListHeadToMap( position, itemToListHead, visited );

const heads = Array.from( itemToListHead.values() );

Expand All @@ -81,8 +83,9 @@ describe( 'List - utils - postfixers', () => {
const fragment = parseModel( input, schema );
const position = model.createPositionAt( fragment, 3 );
const itemToListHead = new Map();
const visited = new Set();

findAndAddListHeadToMap( position, itemToListHead );
findAndAddListHeadToMap( position, itemToListHead, visited );

const heads = Array.from( itemToListHead.values() );

Expand All @@ -101,10 +104,11 @@ describe( 'List - utils - postfixers', () => {
const fragment = parseModel( input, schema );
const position = model.createPositionAt( fragment, 3 );
const itemToListHead = new Map();
const visited = new Set();

itemToListHead.set( fragment.getChild( 2 ), fragment.getChild( 1 ) );

findAndAddListHeadToMap( position, itemToListHead );
findAndAddListHeadToMap( position, itemToListHead, visited );

const heads = Array.from( itemToListHead.values() );

Expand All @@ -123,10 +127,11 @@ describe( 'List - utils - postfixers', () => {
const fragment = parseModel( input, schema );
const position = model.createPositionAt( fragment, 4 );
const itemToListHead = new Map();
const visited = new Set();

itemToListHead.set( fragment.getChild( 2 ), fragment.getChild( 1 ) );

findAndAddListHeadToMap( position, itemToListHead );
findAndAddListHeadToMap( position, itemToListHead, visited );

const heads = Array.from( itemToListHead.values() );

Expand All @@ -145,8 +150,9 @@ describe( 'List - utils - postfixers', () => {
const fragment = parseModel( input, schema );
const position = model.createPositionAt( fragment, 4 );
const itemToListHead = new Map();
const visited = new Set();

findAndAddListHeadToMap( position, itemToListHead );
findAndAddListHeadToMap( position, itemToListHead, visited );

const heads = Array.from( itemToListHead.values() );

Expand All @@ -166,8 +172,9 @@ describe( 'List - utils - postfixers', () => {
const fragment = parseModel( input, schema );
const position = model.createPositionAt( fragment, 5 );
const itemToListHead = new Map();
const visited = new Set();

findAndAddListHeadToMap( position, itemToListHead );
findAndAddListHeadToMap( position, itemToListHead, visited );

const heads = Array.from( itemToListHead.values() );

Expand All @@ -188,8 +195,9 @@ describe( 'List - utils - postfixers', () => {
const fragment = parseModel( input, schema );
const position = model.createPositionAt( fragment, 3 );
const itemToListHead = new Map();
const visited = new Set();

findAndAddListHeadToMap( position, itemToListHead );
findAndAddListHeadToMap( position, itemToListHead, visited );

const heads = Array.from( itemToListHead.values() );

Expand Down