瀏覽代碼

Merge pull request #29 from ckeditor/t/ckeditor5-engine/1213

Internal: Simplified autoparagraphing after engine view -> model conversion refactoring. Closes https://github.com/ckeditor/ckeditor5-engine/issues/1213.
Piotr Jasiun 7 年之前
父節點
當前提交
646249baba
共有 2 個文件被更改,包括 80 次插入135 次删除
  1. 63 124
      packages/ckeditor5-paragraph/src/paragraph.js
  2. 17 11
      packages/ckeditor5-paragraph/tests/paragraph.js

+ 63 - 124
packages/ckeditor5-paragraph/src/paragraph.js

@@ -12,6 +12,9 @@ import ParagraphCommand from './paragraphcommand';
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
 import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter';
+import { SchemaContext } from '@ckeditor/ckeditor5-engine/src/model/schema';
+import Position from '@ckeditor/ckeditor5-engine/src/model/position';
+import Range from '@ckeditor/ckeditor5-engine/src/model/range';
 
 /**
  * The paragraph feature for the editor.
@@ -36,14 +39,6 @@ export default class Paragraph extends Plugin {
 		const data = editor.data;
 		const editing = editor.editing;
 
-		/**
-		 * List of empty roots and batches that made them empty.
-		 *
-		 * @private
-		 * @type {Map<module:engine/model/rootelement~RootElement,module:engine/model/batch~Batch>}
-		 */
-		this._rootsToFix = new Map();
-
 		editor.commands.add( 'paragraph', new ParagraphCommand( editor ) );
 
 		// Schema.
@@ -61,23 +56,61 @@ export default class Paragraph extends Plugin {
 
 		// Content autoparagraphing. --------------------------------------------------
 
-		// Step 1.
-		// "Second chance" converters for elements and texts which were not allowed in their original locations.
-		// They check if this element/text could be converted if it was in a paragraph.
-		// Forcefully converted items will be temporarily in an invalid context. It's going to be fixed in step 2.
+		// Handles elements not converted by plugins and checks if would be converted if
+		// we wraps them by a paragraph or changes them to a paragraph.
+		data.viewToModel.on( 'element', ( evt, data, conversionApi ) => {
+			// When element is already consumed by higher priority converters then do nothing.
+			if ( !conversionApi.consumable.test( data.viewItem, { name: data.viewItem.name } ) ) {
+				return;
+			}
+
+			// When element is paragraph-like lets try to change it into a paragraph.
+			if ( Paragraph.paragraphLikeElements.has( data.viewItem.name ) ) {
+				if ( data.viewItem.isEmpty ) {
+					return;
+				}
+
+				const paragraph = conversionApi.writer.createElement( 'paragraph' );
 
-		// Executed after converter added by a feature, but before "default" to-model-fragment converter.
-		data.viewToModel.on( 'element', convertAutoparagraphableItem, { priority: 'low' } );
-		// Executed after default text converter.
-		data.viewToModel.on( 'text', convertAutoparagraphableItem, { priority: 'lowest' } );
+				// Find allowed parent for paragraph that we are going to insert.
+				// If current parent does not allow to insert paragraph but one of the ancestors does
+				// then split nodes to allowed parent.
+				const splitResult = conversionApi.splitToAllowedParent( paragraph, data.cursorPosition );
 
-		// Step 2.
-		// After an item is "forced" to be converted by `convertAutoparagraphableItem`, we need to actually take
-		// care of adding the paragraph (assumed in `convertAutoparagraphableItem`) and wrap that item in it.
+				// When there is no split result it means that we can't insert paragraph in this position.
+				if ( !splitResult ) {
+					return;
+				}
+
+				// Insert paragraph in allowed position.
+				conversionApi.writer.insert( paragraph, splitResult.position );
+
+				// Convert children to paragraph.
+				const { modelRange } = conversionApi.convertChildren( data.viewItem, Position.createAt( paragraph ) );
 
-		// Executed after all converters (even default ones).
-		data.viewToModel.on( 'element', autoparagraphItems, { priority: 'lowest' } );
-		data.viewToModel.on( 'documentFragment', autoparagraphItems, { priority: 'lowest' } );
+				// Output range starts before paragraph but ends inside it after last child.
+				// This is because we want to keep siblings inside the same paragraph as long as it is possible.
+				// When next node won't be allowed in a paragraph it will split this paragraph anyway.
+				data.modelRange = new Range( Position.createBefore( paragraph ), modelRange.end );
+				data.cursorPosition = data.modelRange.end;
+
+			// When element is not paragraph-like lets try to wrap it by a paragraph.
+			} else if ( isParagraphable( data.viewItem, data.cursorPosition, conversionApi.schema ) ) {
+				data = Object.assign( data, wrapInParagraph( data.viewItem, data.cursorPosition, conversionApi ) );
+			}
+		}, { priority: 'low' } );
+
+		// Handles not converted text nodes and checks if would be converted if we wraps then by a paragraph.
+		data.viewToModel.on( 'text', ( evt, data, conversionApi ) => {
+			// When node is already converted then do nothing.
+			if ( data.modelRange ) {
+				return;
+			}
+
+			if ( isParagraphable( data.viewItem, data.cursorPosition, conversionApi.schema ) ) {
+				data = Object.assign( data, wrapInParagraph( data.viewItem, data.cursorPosition, conversionApi ) );
+			}
+		}, { priority: 'lowest' } );
 
 		// Empty roots autoparagraphing. -----------------------------------------------
 
@@ -162,117 +195,23 @@ Paragraph.paragraphLikeElements = new Set( [
 	'td'
 ] );
 
-// This converter forces a conversion of a non-consumed view item, if that item would be allowed by schema and converted it if was
-// inside a paragraph element. The converter checks whether conversion would be possible if there was a paragraph element
-// between `data.input` item and its parent. If the conversion would be allowed, the converter adds `"paragraph"` to the
-// context and fires conversion for `data.input` again.
-function convertAutoparagraphableItem( evt, data, consumable, conversionApi ) {
-	// If the item wasn't consumed by some of the dedicated converters...
-	if ( !consumable.test( data.input, { name: data.input.name } ) ) {
-		return;
-	}
-
-	// But would be allowed if it was in a paragraph...
-	if ( !isParagraphable( data.input, data.context, conversionApi.schema, false ) ) {
-		return;
-	}
-
-	// Convert that item in a paragraph context.
-	data.context.push( 'paragraph' );
-	const item = conversionApi.convertItem( data.input, consumable, data );
-	data.context.pop();
+function wrapInParagraph( input, position, conversionApi ) {
+	const paragraph = conversionApi.writer.createElement( 'paragraph' );
 
-	data.output = item;
+	conversionApi.writer.insert( paragraph, position );
+	return conversionApi.convertItem( input, Position.createAt( paragraph ) );
 }
 
-// This converter checks all children of an element or document fragment that has been converted and wraps
-// children in a paragraph element if it is allowed by schema.
-//
-// Basically, after an item is "forced" to be converted by `convertAutoparagraphableItem`, we need to actually take
-// care of adding the paragraph (assumed in `convertAutoparagraphableItem`) and wrap that item in it.
-function autoparagraphItems( evt, data, consumable, conversionApi ) {
-	// Autoparagraph only if the element has been converted.
-	if ( !data.output ) {
-		return;
-	}
-
-	const isParagraphLike = Paragraph.paragraphLikeElements.has( data.input.name ) && !data.output.is( 'element' );
-
-	// Keep in mind that this converter is added to all elements and document fragments.
-	// This means that we have to make a smart decision in which elements (at what level) auto-paragraph should be inserted.
-	// There are three situations when it is correct to add paragraph:
-	//   -	we are converting a view document fragment: this means that we are at the top level of conversion and we should
-	//		add paragraph elements for "bare" texts (unless converting in $clipboardHolder, but this is covered by schema),
-	//   -	we are converting an element that was converted to model element: this means that it will be represented in model
-	//		and has added its context when converting children - we should add paragraph for those items that passed
-	//		in `convertAutoparagraphableItem`, because it is correct for them to be autoparagraphed,
-	//	 -	we are converting "paragraph-like" element, which children should always be autoparagraphed (if it is allowed by schema,
-	//		so we won't end up with, i.e., paragraph inside paragraph, if paragraph was in paragraph-like element).
-	const shouldAutoparagraph =
-		( data.input.is( 'documentFragment' ) ) ||
-		( data.input.is( 'element' ) && data.output.is( 'element' ) ) ||
-		isParagraphLike;
-
-	if ( !shouldAutoparagraph ) {
-		return;
-	}
-
-	// Take care of proper context. This is important for `isParagraphable` checks.
-	const needsNewContext = data.output.is( 'element' );
-
-	if ( needsNewContext ) {
-		data.context.push( data.output );
-	}
-
-	// `paragraph` element that will wrap auto-paragraphable children.
-	let autoParagraph = null;
-
-	// Check children and wrap them in a `paragraph` element if they need to be wrapped.
-	// Be smart when wrapping children and put all auto-paragraphable siblings in one `paragraph` parent:
-	// foo<$text bold="true">bar</$text><paragraph>xxx</paragraph>baz      --->
-	// <paragraph>foo<$text bold="true">bar</$text></paragraph><paragraph>xxx</paragraph><paragraph>baz</paragraph>
-	for ( let i = 0; i < data.output.childCount; i++ ) {
-		const child = data.output.getChild( i );
-
-		if ( isParagraphable( child, data.context, conversionApi.schema, isParagraphLike ) ) {
-			// If there is no wrapping `paragraph` element, create it.
-			if ( !autoParagraph ) {
-				autoParagraph = conversionApi.writer.createElement( 'paragraph' );
-				conversionApi.writer.insert( autoParagraph, data.output, child.index );
-			}
-			// Otherwise, use existing `paragraph` and just fix iterator.
-			// Thanks to reusing `paragraph` element, multiple siblings ends up in same container.
-			else {
-				i--;
-			}
-
-			conversionApi.writer.append( child, autoParagraph );
-		} else {
-			// That was not a paragraphable children, reset `paragraph` wrapper - following auto-paragraphable children
-			// need to be placed in a new `paragraph` element.
-			autoParagraph = null;
-		}
-	}
-
-	if ( needsNewContext ) {
-		data.context.pop();
-	}
-}
-
-function isParagraphable( node, context, schema, insideParagraphLikeElement ) {
-	// Node is paragraphable if it is inside paragraph like element, or...
-	// It is not allowed at this context...
-	if ( !insideParagraphLikeElement && schema.checkChild( context, node ) ) {
-		return false;
-	}
+function isParagraphable( node, position, schema ) {
+	const context = new SchemaContext( position );
 
-	// And paragraph is allowed in this context...
+	// When paragraph is allowed in this context...
 	if ( !schema.checkChild( context, 'paragraph' ) ) {
 		return false;
 	}
 
 	// And a node would be allowed in this paragraph...
-	if ( !schema.checkChild( [ ...context, 'paragraph' ], node ) ) {
+	if ( !schema.checkChild( context.concat( 'paragraph' ), node ) ) {
 		return false;
 	}
 

+ 17 - 11
packages/ckeditor5-paragraph/tests/paragraph.js

@@ -92,7 +92,7 @@ describe( 'Paragraph feature', () => {
 			} );
 
 			it( 'should not autoparagraph text (in clipboard holder)', () => {
-				const modelFragment = editor.data.parse( 'foo', '$clipboardHolder' );
+				const modelFragment = editor.data.parse( 'foo', [ '$clipboardHolder' ] );
 
 				expect( stringifyModel( modelFragment ) )
 					.to.equal( 'foo' );
@@ -101,7 +101,7 @@ describe( 'Paragraph feature', () => {
 			it( 'should not autoparagraph text (in a context which does not allow paragraphs', () => {
 				model.schema.register( 'specialRoot' );
 
-				const modelFragment = editor.data.parse( 'foo', 'specialRoot' );
+				const modelFragment = editor.data.parse( 'foo', [ 'specialRoot' ] );
 
 				expect( stringifyModel( modelFragment ) )
 					.to.equal( '' );
@@ -125,7 +125,7 @@ describe( 'Paragraph feature', () => {
 			} );
 
 			it( 'should not autoparagraph 3 inline nodes (in clipboardHolder)', () => {
-				const modelFragment = editor.data.parse( 'foo<b>bar</b>bom', '$clipboardHolder' );
+				const modelFragment = editor.data.parse( 'foo<b>bar</b>bom', [ '$clipboardHolder' ] );
 
 				expect( stringifyModel( modelFragment ) )
 					.to.equal( 'foobarbom' );
@@ -189,8 +189,8 @@ describe( 'Paragraph feature', () => {
 
 			// This test was taken from the list package.
 			it( 'does not break when some converter returns nothing', () => {
-				editor.data.viewToModel.on( 'element:li', ( evt, data, consumable ) => {
-					consumable.consume( data.input, { name: true } );
+				editor.data.viewToModel.on( 'element:li', ( evt, data, conversionApi ) => {
+					conversionApi.consumable.consume( data.input, { name: true } );
 				}, { priority: 'highest' } );
 
 				const modelFragment = editor.data.parse( '<ul><li></li></ul>' );
@@ -243,7 +243,7 @@ describe( 'Paragraph feature', () => {
 			} );
 
 			it( 'should convert h1+h2 (in clipboard holder)', () => {
-				const modelFragment = editor.data.parse( '<h1>foo</h1><h2>bar</h2>', '$clipboardHolder' );
+				const modelFragment = editor.data.parse( '<h1>foo</h1><h2>bar</h2>', [ '$clipboardHolder' ] );
 
 				expect( stringifyModel( modelFragment ) )
 					.to.equal( '<paragraph>foo</paragraph><paragraph>bar</paragraph>' );
@@ -257,7 +257,7 @@ describe( 'Paragraph feature', () => {
 
 				buildViewConverter().for( editor.data.viewToModel ).fromElement( 'div' ).toElement( 'div' );
 
-				const modelFragment = editor.data.parse( '<h1>foo</h1><h2>bar</h2><div>bom</div>', 'specialRoot' );
+				const modelFragment = editor.data.parse( '<h1>foo</h1><h2>bar</h2><div>bom</div>', [ 'specialRoot' ] );
 
 				expect( stringifyModel( modelFragment ) )
 					.to.equal( '<div>bom</div>' );
@@ -271,7 +271,7 @@ describe( 'Paragraph feature', () => {
 			} );
 
 			it( 'should convert ul,ol>li (in clipboard holder)', () => {
-				const modelFragment = editor.data.parse( '<ul><li>a</li><li>b</li></ul><ol><li>c</li></ol>', '$clipboardHolder' );
+				const modelFragment = editor.data.parse( '<ul><li>a</li><li>b</li></ul><ol><li>c</li></ol>', [ '$clipboardHolder' ] );
 
 				expect( stringifyModel( modelFragment ) )
 					.to.equal( '<paragraph>a</paragraph><paragraph>b</paragraph><paragraph>c</paragraph>' );
@@ -287,7 +287,7 @@ describe( 'Paragraph feature', () => {
 			// "b" is not autoparagraphed because clipboard holder allows text nodes.
 			// There's a similar integrational test what's going to happen when pasting in paragraph-integration.js.
 			it( 'should convert ul>li>ul>li+li (in clipboard holder)', () => {
-				const modelFragment = editor.data.parse( '<ul><li>a<ul><li>b</li><li>c</li></ul></li></ul>', '$clipboardHolder' );
+				const modelFragment = editor.data.parse( '<ul><li>a<ul><li>b</li><li>c</li></ul></li></ul>', [ '$clipboardHolder' ] );
 
 				expect( stringifyModel( modelFragment ) )
 					.to.equal( '<paragraph>a</paragraph><paragraph>b</paragraph><paragraph>c</paragraph>' );
@@ -303,7 +303,7 @@ describe( 'Paragraph feature', () => {
 			// "b" is not autoparagraphed because clipboard holder allows text nodes.
 			// There's a similar integrational test what's going to happen when pasting in paragraph-integration.js.
 			it( 'should convert ul>li>p,text (in clipboard holder)', () => {
-				const modelFragment = editor.data.parse( '<ul><li><p>a</p>b</li></ul>', '$clipboardHolder' );
+				const modelFragment = editor.data.parse( '<ul><li><p>a</p>b</li></ul>', [ '$clipboardHolder' ] );
 
 				expect( stringifyModel( modelFragment ) )
 					.to.equal( '<paragraph>a</paragraph><paragraph>b</paragraph>' );
@@ -321,7 +321,7 @@ describe( 'Paragraph feature', () => {
 			it( 'should convert td (in clipboardHolder)', () => {
 				const modelFragment = editor.data.parse(
 					'<table><tr><td>a</td><td>b</td></tr><tr><td>c</td><td>d</td></tr></table>',
-					'$clipboardHolder'
+					[ '$clipboardHolder' ]
 				);
 
 				expect( stringifyModel( modelFragment ) )
@@ -363,6 +363,12 @@ describe( 'Paragraph feature', () => {
 				expect( modelFragment.getChild( 0 ).childCount ).to.equal( 1 );
 				expect( modelFragment.getChild( 0 ).getChild( 0 ) ).to.be.instanceOf( ModelText );
 			} );
+
+			it( 'should not convert empty elements', () => {
+				const modelFragment = editor.data.parse( '<ul><li></li><ul>' );
+
+				expect( stringifyModel( modelFragment ) ).to.equal( '' );
+			} );
 		} );
 	} );