8
0
Quellcode durchsuchen

Merge pull request #113 from ckeditor/t/ckeditor5/1263

Fix: Preserve correct order of block elements inside list items during view to model conversion. Closes ckeditor/ckeditor5#1263.
Szymon Cofalik vor 7 Jahren
Ursprung
Commit
62e53fbbcf
2 geänderte Dateien mit 292 neuen und 16 gelöschten Zeilen
  1. 62 16
      packages/ckeditor5-list/src/converters.js
  2. 230 0
      packages/ckeditor5-list/tests/listediting.js

+ 62 - 16
packages/ckeditor5-list/src/converters.js

@@ -371,22 +371,7 @@ export function viewModelConverter( evt, data, conversionApi ) {
 
 		writer.insert( listItem, splitResult.position );
 
-		// Remember position after list item, next list items will be inserted at this position.
-		let nextPosition = ModelPosition.createAfter( listItem );
-
-		// Check all children of the converted `<li>`.
-		// At this point we assume there are no "whitespace" view text nodes in view list, between view list items.
-		// This should be handled by `<ul>` and `<ol>` converters.
-		for ( const child of data.viewItem.getChildren() ) {
-			// If this is a view list element, we will convert it after last `listItem` model element.
-			if ( child.name == 'ul' || child.name == 'ol' ) {
-				nextPosition = conversionApi.convertItem( child, nextPosition ).modelCursor;
-			}
-			// If it was not a list it was a "regular" list item content. Just convert it to `listItem`.
-			else {
-				conversionApi.convertItem( child, ModelPosition.createAt( listItem, 'end' ) );
-			}
-		}
+		const nextPosition = viewToModelListItemChildrenConverter( listItem, data.viewItem.getChildren(), conversionApi );
 
 		conversionStore.indent--;
 
@@ -814,6 +799,67 @@ function generateLiInUl( modelItem, conversionApi ) {
 	return viewItem;
 }
 
+// Helper function that converts children of a given `<li>` view element into corresponding model elements.
+// The function maintains proper order of elements if model `listItem` is split during the conversion
+// due to block children conversion.
+//
+// @param {module:engine/model/element~Element} listItemModel List item model element to which converted children will be inserted.
+// @param {Iterable.<module:engine/view/node~Node>} viewChildren View elements which will be converted.
+// @param {Object} conversionApi Conversion interface to be used by the callback.
+// @returns {module:engine/model/position~Position} Position on which next elements should be inserted after children conversion.
+function viewToModelListItemChildrenConverter( listItemModel, viewChildren, conversionApi ) {
+	let lastListItem = listItemModel;
+	let nextPosition = ModelPosition.createAfter( lastListItem );
+
+	// Check all children of the converted `<li>`. At this point we assume there are no "whitespace" view text nodes
+	// in view list, between view list items. This should be handled by `<ul>` and `<ol>` converters.
+	for ( const child of viewChildren ) {
+		// If this is a view list element, we will insert its conversion result after currently handled `listItem`.
+		if ( child.name == 'ul' || child.name == 'ol' ) {
+			nextPosition = conversionApi.convertItem( child, nextPosition ).modelCursor;
+		}
+		// If it is not a view list element it is a "regular" list item content. Its conversion result will
+		// be inserted as `listItem` children (block children may split current `listItem`).
+		else {
+			const result = conversionApi.convertItem( child, ModelPosition.createAt( lastListItem, 'end' ) );
+			const convertedChild = result.modelRange.start.nodeAfter;
+
+			nextPosition = result.modelCursor;
+
+			// If there is a block element child being converted it may split the current list item, for example:
+			//
+			//		<li><p>Foo</p></li>
+			//
+			// will be converted to:
+			//
+			//		<listItem></listItem><paragraph>Foo</paragraph><listItem></listItem>
+			//
+			// so we need to update reference to `lastListItem`.
+			if ( convertedChild && convertedChild.is( 'element' ) &&
+				!conversionApi.schema.checkChild( lastListItem, convertedChild.name ) ) {
+				lastListItem = nextPosition.parent;
+
+				// Depending on the used converter for block elements, usually the position (`result.modelCursor`
+				// marked as # below) points to the second list item after conversion:
+				//
+				//		`<li><p>Foo</p></li>` -> `<listItem></listItem><paragraph>Foo</paragraph><listItem>#</listItem>`
+				//
+				// However, in some cases like autoparagraphing the position is placed on the end of the block element:
+				//
+				//		`<li><h2>Foo</h2></li>` -> `<listItem></listItem><paragraph>Foo#</paragraph><listItem></listItem>`
+				//
+				// We need to check for such cases and use proper list item and position based on it.
+				if ( !lastListItem.is( 'listItem' ) && lastListItem.nextSibling && lastListItem.nextSibling.is( 'listItem' ) ) {
+					lastListItem = lastListItem.nextSibling;
+					nextPosition = ModelPosition.createAt( lastListItem, 'end' );
+				}
+			}
+		}
+	}
+
+	return nextPosition;
+}
+
 // Helper function that seeks for a list item sibling of given model item (or position) which meets given criteria.
 // `options` object may contain one or more of given values (by default they are `false`):
 // `options.sameIndent` - whether sought sibling should have same indent (default = no),

+ 230 - 0
packages/ckeditor5-list/tests/listediting.js

@@ -365,6 +365,167 @@ describe( 'ListEditing', () => {
 
 				expect( getModelData( model, { withoutSelection: true } ) ).to.equal( expectedModelData );
 			} );
+
+			describe( 'block elements inside list items', () => {
+				describe( 'single block', () => {
+					test(
+						'single item',
+						'<ul><li><p>Foo</p></li></ul>',
+						'<p>Foo</p>'
+					);
+					test(
+						'multiple items',
+						'<ul><li><p>Foo</p></li><li><p>Bar</p></li></ul>',
+						'<p>Foo</p><p>Bar</p>'
+					);
+					test(
+						'nested items',
+						'<ul><li><p>Foo</p><ol><li><p>Bar</p></li></ol></li></ul>',
+						'<p>Foo</p><p>Bar</p>'
+					);
+				} );
+
+				describe( 'multiple blocks', () => {
+					test(
+						'single item',
+						'<ul><li><h2>Foo</h2><p>Bar</p></li></ul>',
+						'<p>Foo</p><p>Bar</p>'
+					);
+					test(
+						'multiple items',
+						'<ol><li><p>123</p></li></ol><ul><li><h2>Foo</h2><p>Bar</p></li></ul>',
+						'<p>123</p><p>Foo</p><p>Bar</p>'
+					);
+					test(
+						'nested items #2',
+						'<ol><li><p>123</p><p>456</p><ul><li><h2>Foo</h2><p>Bar</p></li></ul></li></ol>',
+						'<p>123</p><p>456</p><p>Foo</p><p>Bar</p>'
+					);
+				} );
+
+				describe.skip( 'multiple blocks', () => { // Skip due to #112 issue.
+					test(
+						'nested items #1',
+						'<ol><li><p>123</p><ul><li><h2>Foo</h2><p>Bar</p></li></ul><p>456</p></li></ol>',
+						'<p>123</p><p>Foo</p><p>Bar</p><p>456</p>'
+					);
+				} );
+
+				describe( 'inline + block', () => {
+					test(
+						'single item',
+						'<ul><li>Foo<p>Bar</p></li></ul>',
+						'<ul><li>Foo</li></ul><p>Bar</p>'
+					);
+					test(
+						'multiple items',
+						'<ul><li>Foo<p>Bar</p></li><li>Foz<p>Baz</p></li></ul>',
+						'<ul><li>Foo</li></ul><p>Bar</p><ul><li>Foz</li></ul><p>Baz</p>'
+					);
+					test(
+						'split by list items',
+						'<ul><li>Foo</li><li><p>Bar</p></li></ul>',
+						'<ul><li>Foo</li></ul><p>Bar</p>'
+					);
+					test(
+						'nested split by list items',
+						'<ul><li>Foo<ol><li><p>Bar</p></li></ol></li></ul>',
+						'<ul><li>Foo</li></ul><p>Bar</p>'
+					);
+					test(
+						'nested items #1',
+						'<ol><li>Foo<p>Bar</p><ul><li>123<h2>456</h2></li></ul></li></ol>',
+						'<ol><li>Foo</li></ol><p>Bar</p><ul><li>123</li></ul><p>456</p>'
+					);
+					test(
+						'nested items #2',
+						'<ol><li>Foo<p>Bar</p><ul><li>123<h2>456</h2></li></ul></li><li>abc<h2>def</h2></li></ol>',
+						'<ol><li>Foo</li></ol><p>Bar</p><ul><li>123</li></ul><p>456</p><ol><li>abc</li></ol><p>def</p>'
+					);
+				} );
+
+				describe( 'block + inline', () => {
+					test(
+						'single item',
+						'<ul><li><p>Foo</p>Bar</li></ul>',
+						'<p>Foo</p><ul><li>Bar</li></ul>'
+					);
+					test(
+						'multiple items',
+						'<ul><li><p>Foo</p>Bar</li><li><p>Foz</p>Baz</li></ul>',
+						'<p>Foo</p><ul><li>Bar</li></ul><p>Foz</p><ul><li>Baz</li></ul>'
+					);
+					test(
+						'split by list items',
+						'<ul><li><p>Bar</p><li>Foo</li></li></ul>',
+						'<p>Bar</p><ul><li>Foo</li></ul>'
+					);
+					test(
+						'nested split by list items',
+						'<ul><li><p>Bar</p><ol><li>Foo</li></ol></li></ul>',
+						'<p>Bar</p><ol><li>Foo</li></ol>'
+					);
+					test(
+						'nested items #1',
+						'<ol><li><p>Foo</p>Bar<ul><li><h2>123</h2>456</li></ul></li></ol>',
+						'<p>Foo</p><ol><li>Bar</li></ol><p>123</p><ul><li>456</li></ul>'
+					);
+					test(
+						'nested items #2',
+						'<ol><li><p>Foo</p>Bar<ul><li><h2>123</h2>456</li></ul></li><li><h2>abc</h2>def</li></ol>',
+						'<p>Foo</p><ol><li>Bar</li></ol><p>123</p><ul><li>456</li></ul><p>abc</p><ol><li>def</li></ol>'
+					);
+				} );
+
+				describe( 'complex', () => {
+					test(
+						'single item with inline block inline',
+						'<ul><li>Foo<p>Bar</p>Baz</li></ul>',
+						'<ul><li>Foo</li></ul><p>Bar</p><ul><li>Baz</li></ul>'
+					);
+					test(
+						'single item with inline block block',
+						'<ul><li>Text<p>Foo</p><p>Bar</p></li></ul>',
+						'<ul><li>Text</li></ul><p>Foo</p><p>Bar</p>'
+					);
+					test(
+						'single item with block block inline',
+						'<ul><li><p>Foo</p><p>Bar</p>Text</li></ul>',
+						'<p>Foo</p><p>Bar</p><ul><li>Text</li></ul>'
+					);
+					test(
+						'single item with block block block',
+						'<ul><li><p>Foo</p><p>Bar</p><p>Baz</p></li></ul>',
+						'<p>Foo</p><p>Bar</p><p>Baz</p>'
+					);
+					test(
+						'item inline + item block and inline',
+						'<ul><li>Foo</li><li><p>Bar</p>Baz</li></ul>',
+						'<ul><li>Foo</li></ul><p>Bar</p><ul><li>Baz</li></ul>'
+					);
+					test(
+						'item inline and block + item inline',
+						'<ul><li>Foo<p>Bar</p></li><li>Baz</li></ul>',
+						'<ul><li>Foo</li></ul><p>Bar</p><ul><li>Baz</li></ul>'
+					);
+					test(
+						'multiple items inline/block mix',
+						'<ul><li>Text<p>Foo</p></li><li>Bar<p>Baz</p>123</li></ul>',
+						'<ul><li>Text</li></ul><p>Foo</p><ul><li>Bar</li></ul><p>Baz</p><ul><li>123</li></ul>'
+					);
+					test(
+						'nested items',
+						'<ul><li>Foo<p>Bar</p></li><li>Baz<p>123</p>456<ol><li>ABC<p>DEF</p></li><li>GHI</li></ol></li></ul>',
+						'<ul><li>Foo</li></ul><p>Bar</p><ul><li>Baz</li></ul><p>123</p><ul><li>456<ol><li>ABC</li></ol></li></ul>' +
+							'<p>DEF</p><ol><li>GHI</li></ol>'
+					);
+					test(
+						'list with empty inline element',
+						'<ul><li><span></span>Foo<p>Bar</p></li></ul>',
+						'<ul><li>Foo</li></ul><p>Bar</p>'
+					);
+				} );
+			} );
 		} );
 
 		describe( 'position mapping', () => {
@@ -3571,6 +3732,75 @@ describe( 'ListEditing', () => {
 				'<paragraph>Bar</paragraph>'
 			);
 		} );
+
+		it( 'should handle block elements inside pasted list #1', () => {
+			setModelData( model,
+				'<listItem listType="bulleted" listIndent="0">A</listItem>' +
+				'<listItem listType="bulleted" listIndent="1">B[]</listItem>' +
+				'<listItem listType="bulleted" listIndent="2">C</listItem>'
+			);
+
+			const clipboard = editor.plugins.get( 'Clipboard' );
+
+			clipboard.fire( 'inputTransformation', {
+				content: parseView( '<ul><li>W<ul><li>X<p>Y</p>Z</li></ul></li></ul>' )
+			} );
+
+			expect( getModelData( model ) ).to.equal(
+				'<listItem listIndent="0" listType="bulleted">A</listItem>' +
+				'<listItem listIndent="1" listType="bulleted">BW</listItem>' +
+				'<listItem listIndent="2" listType="bulleted">X</listItem>' +
+				'<paragraph>Y</paragraph>' +
+				'<listItem listIndent="0" listType="bulleted">Z[]</listItem>' +
+				'<listItem listIndent="1" listType="bulleted">C</listItem>'
+			);
+		} );
+
+		it( 'should handle block elements inside pasted list #2', () => {
+			setModelData( model,
+				'<listItem listType="bulleted" listIndent="0">A[]</listItem>' +
+				'<listItem listType="bulleted" listIndent="1">B</listItem>' +
+				'<listItem listType="bulleted" listIndent="2">C</listItem>'
+			);
+
+			const clipboard = editor.plugins.get( 'Clipboard' );
+
+			clipboard.fire( 'inputTransformation', {
+				content: parseView( '<ul><li>W<ul><li>X<p>Y</p>Z</li></ul></li></ul>' )
+			} );
+
+			expect( getModelData( model ) ).to.equal(
+				'<listItem listIndent="0" listType="bulleted">AW</listItem>' +
+				'<listItem listIndent="1" listType="bulleted">X</listItem>' +
+				'<paragraph>Y</paragraph>' +
+				'<listItem listIndent="0" listType="bulleted">Z[]</listItem>' +
+				'<listItem listIndent="0" listType="bulleted">B</listItem>' +
+				'<listItem listIndent="1" listType="bulleted">C</listItem>'
+			);
+		} );
+
+		it( 'should handle block elements inside pasted list #3', () => {
+			setModelData( model,
+				'<listItem listType="bulleted" listIndent="0">A[]</listItem>' +
+				'<listItem listType="bulleted" listIndent="1">B</listItem>' +
+				'<listItem listType="bulleted" listIndent="2">C</listItem>'
+			);
+
+			const clipboard = editor.plugins.get( 'Clipboard' );
+
+			clipboard.fire( 'inputTransformation', {
+				content: parseView( '<ul><li><p>W</p><p>X</p><p>Y</p></li><li>Z</li></ul>' )
+			} );
+
+			expect( getModelData( model ) ).to.equal(
+				'<listItem listIndent="0" listType="bulleted">AW</listItem>' +
+				'<paragraph>X</paragraph>' +
+				'<paragraph>Y</paragraph>' +
+				'<listItem listIndent="0" listType="bulleted">Z[]</listItem>' +
+				'<listItem listIndent="1" listType="bulleted">B</listItem>' +
+				'<listItem listIndent="2" listType="bulleted">C</listItem>'
+			);
+		} );
 	} );
 
 	describe( 'other', () => {