Merge pull request #55 from ckeditor/t/54

Internal: List postfixer now looks at the first/last item of the list when fixing list item type. Closes #54.
ckeditor · Mar 29, 2017 · ad722de · ad722de
2 parents eb1b05b + 347443a
commit ad722de
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 17 deletions.
diff --git a/src/converters.js b/src/converters.js
@@ -662,26 +662,24 @@ function _fixItemsIndent( changePosition, document, batch ) {
 function _fixItemsType( changePosition, fixPrevious, document, batch ) {
 	let item = changePosition[ fixPrevious ? 'nodeBefore' : 'nodeAfter' ];
 
-	if ( !item ) {
-		// May happen if last item got removed.
+	if ( !item || !item.is( 'listItem' ) || item.getAttribute( 'indent' ) === 0 ) {
+		// !item - when last item got removed.
+		// !item.is( 'listItem' ) - when first element to fix is not a list item already.
+		// indent === 0 - do not fix if changes are done on top level lists.
 		return;
 	}
 
-	const refItem = getSiblingListItem( item, { checkAllSiblings: true, getNext: fixPrevious, sameIndent: true } );
+	const refItem = _getBoundaryItemOfSameList( item, !fixPrevious );
 
-	if ( !refItem ) {
-		// May happen if first list item is inserted.
+	if ( !refItem || refItem == item ) {
+		// !refItem - happens if first list item is inserted.
+		// refItem == item - happens if last item is inserted.
 		return;
 	}
 
 	const refIndent = refItem.getAttribute( 'indent' );
 	const refType = refItem.getAttribute( 'type' );
 
-	if ( refIndent === 0 ) {
-		// Happens if changes are done on top level lists.
-		return;
-	}
-
 	document.enqueueChanges( () => {
 		while ( item && item.is( 'listItem' ) && item.getAttribute( 'indent' ) >= refIndent ) {
 			if ( item.getAttribute( 'type' ) != refType && item.getAttribute( 'indent' ) == refIndent ) {
@@ -839,7 +837,7 @@ function injectViewList( modelItem, injectedItem, mapper, removePosition ) {
 		// The broken ("lower") part will be moved as nested children of the inserted view item.
 		const sourceStart = ViewPosition.createBefore( viewItem.parent );
 
-		const lastModelItem = _getModelLastItem( nextItem );
+		const lastModelItem = _getBoundaryItemOfSameList( nextItem, false );
 		const lastViewItem = mapper.toViewElement( lastModelItem );
 		const sourceEnd = viewWriter.breakContainer( ViewPosition.createAfter( lastViewItem ) );
 		const sourceRange = new ViewRange( sourceStart, sourceEnd );
@@ -945,15 +943,15 @@ function hoistNestedLists( nextIndent, modelRemoveStartPosition, viewRemoveStart
 	}
 }
 
-// Helper function to obtain the last model list item that is a forward sibling of given model list item that has
-// same or bigger indent. In other words, it looks for the last model item that is a nested item of the same item
-// that given item.
-function _getModelLastItem( item ) {
+// Helper function to obtain the first or the last model list item which is in on the same indent level as given `item`.
+function _getBoundaryItemOfSameList( item, getFirst ) {
 	const indent = item.getAttribute( 'indent' );
+	const direction = getFirst ? 'previousSibling' : 'nextSibling';
+
 	let result = item;
 
-	while ( item.nextSibling && item.nextSibling.is( 'listItem' ) && item.nextSibling.getAttribute( 'indent' ) >= indent ) {
-		item = item.nextSibling;
+	while ( item[ direction ] && item[ direction ].is( 'listItem' ) && item[ direction ].getAttribute( 'indent' ) >= indent ) {
+		item = item[ direction ];
 
 		if ( item.getAttribute( 'indent' ) == indent ) {
 			result = item;

diff --git a/tests/listengine.js b/tests/listengine.js
@@ -1514,6 +1514,35 @@ describe( 'ListEngine', () => {
 
 					false
 				);
+
+				_test(
+					'two list items with mismatched types inserted in one batch',
+
+					'<listItem indent="0" type="bulleted">a</listItem>' +
+					'<listItem indent="1" type="bulleted">b</listItem>[]',
+
+					'<ul>' +
+						'<li>' +
+							'a' +
+							'<ul>' +
+								'<li>b</li>' +
+								'<li>c</li>' +
+								'<li>d</li>' +
+							'</ul>' +
+						'</li>' +
+					'</ul>',
+
+					() => {
+						const item1 = '<listItem indent="1" type="numbered">c</listItem>';
+						const item2 = '<listItem indent="1" type="bulleted">d</listItem>';
+
+						modelDoc.enqueueChanges( () => {
+							modelDoc.batch()
+								.insert( ModelPosition.createAt( modelRoot, 'end' ), parseModel( item1, modelDoc.schema ) )
+								.insert( ModelPosition.createAt( modelRoot, 'end' ), parseModel( item2, modelDoc.schema ) );
+						} );
+					}
+				);
 			} );
 
 			describe( 'remove', () => {
@@ -2730,6 +2759,31 @@ describe( 'ListEngine', () => {
 				'<listItem indent="2" type="bulleted">c</listItem>' +
 				'<listItem indent="1" type="bulleted">x</listItem>'
 			);
+
+			it( 'two list items with mismatched types inserted in one batch', () => {
+				const input =
+					'<listItem indent="0" type="bulleted">a</listItem>' +
+					'<listItem indent="1" type="bulleted">b</listItem>';
+
+				const output =
+					'<listItem indent="0" type="bulleted">a</listItem>' +
+					'<listItem indent="1" type="bulleted">b</listItem>' +
+					'<listItem indent="1" type="bulleted">c</listItem>' +
+					'<listItem indent="1" type="bulleted">d</listItem>';
+
+				setModelData( modelDoc, input );
+
+				const item1 = '<listItem indent="1" type="numbered">c</listItem>';
+				const item2 = '<listItem indent="1" type="bulleted">d</listItem>';
+
+				modelDoc.enqueueChanges( () => {
+					modelDoc.batch()
+						.insert( ModelPosition.createAt( modelRoot, 'end' ), parseModel( item1, modelDoc.schema ) )
+						.insert( ModelPosition.createAt( modelRoot, 'end' ), parseModel( item2, modelDoc.schema ) );
+				} );
+
+				expect( getModelData( modelDoc, { withoutSelection: true } ) ).to.equal( output );
+			} );
 		} );
 
 		describe( 'remove', () => {