Explorar el Código

Modified #deleteContent() to check the elements next to common ancestor element to decide if it should merge or remove them.

Kuba Niegowski hace 5 años
padre
commit
4b30f8be65

+ 30 - 20
packages/ckeditor5-engine/src/model/utils/deletecontent.js

@@ -80,7 +80,7 @@ export default function deleteContent( model, selection, options = {} ) {
 			return;
 		}
 
-		const startPos = selRange.start;
+		let startPos = selRange.start;
 		const endPos = LivePosition.fromPosition( selRange.end, 'toNext' );
 
 		// 2. Remove the content if there is any.
@@ -97,7 +97,7 @@ export default function deleteContent( model, selection, options = {} ) {
 		// as it's hard to imagine what should actually be the default behavior. Usually, specific features will
 		// want to override that behavior anyway.
 		if ( !options.leaveUnmerged ) {
-			mergeBranches( writer, startPos, endPos );
+			startPos = mergeBranches( writer, startPos, endPos ) || startPos;
 
 			// TMP this will be replaced with a postfixer.
 			// We need to check and strip disallowed attributes in all nested nodes because after merge
@@ -145,14 +145,16 @@ function mergeBranches( writer, startPos, endPos ) {
 		return;
 	}
 
-	// If first block element is empty then override it's name to make merging more user friendly.
-	// Example:
+	// If the start element on the common ancestor level is empty, and the end element on the same level is not empty
+	// then remove former one and merging is done.
 	// <heading1>[</heading1><paragraph>]foo</paragraph> -> <paragraph>[]foo</paragraph>
-	const startBlock = getParentBlock( startPos );
-	const endBlock = getParentBlock( endPos );
+	// <blockQuote><heading1>[</heading1><paragraph>]foo</paragraph> -> <blockQuote><paragraph>[]foo</paragraph></blockQuote>
+	const [ startAncestor, endAncestor ] = getElementsNextToCommonAncestor( startPos, endPos );
 
-	if ( startBlock && endBlock && startBlock.isEmpty && !endBlock.isEmpty && startBlock.name != endBlock.name ) {
-		writer.rename( startBlock, endBlock.name );
+	if ( !hasContent( startAncestor ) && hasContent( endAncestor ) ) {
+		writer.remove( startAncestor );
+
+		return endPos;
 	}
 
 	// Remember next positions to merge. For example:
@@ -192,22 +194,30 @@ function mergeBranches( writer, startPos, endPos ) {
 	mergeBranches( writer, startPos, endPos );
 }
 
-// Finds the lowest element in position's ancestors which is a block.
-// It will search until first ancestor that is a limit element.
-function getParentBlock( position ) {
-	const element = position.parent;
-	const schema = element.root.document.model.schema;
-	const ancestors = element.getAncestors( { parentFirst: true, includeSelf: true } );
+function getElementsNextToCommonAncestor( positionA, positionB ) {
+	const ancestorsA = positionA.getAncestors();
+	const ancestorsB = positionB.getAncestors();
 
-	for ( const element of ancestors ) {
-		if ( schema.isLimit( element ) ) {
-			return null;
-		}
+	let i = 0;
+
+	while ( ancestorsA[ i ] && ancestorsA[ i ] == ancestorsB[ i ] ) {
+		i++;
+	}
 
-		if ( schema.isBlock( element ) ) {
-			return element;
+	return [ ancestorsA[ i ], ancestorsB[ i ] ];
+}
+
+function hasContent( element ) {
+	const schema = element.root.document.model.schema;
+	const range = Range._createIn( element );
+
+	for ( const item of range.getItems() ) {
+		if ( item.is( 'textProxy' ) || schema.isObject( item ) ) {
+			return true;
 		}
 	}
+
+	return false;
 }
 
 function shouldAutoparagraph( schema, position ) {

+ 29 - 107
packages/ckeditor5-engine/tests/model/utils/deletecontent.js

@@ -220,7 +220,8 @@ describe( 'DataController utils', () => {
 				schema.register( 'image', { inheritAllFrom: '$text' } );
 				schema.register( 'pchild', { allowIn: 'paragraph' } );
 				schema.register( 'pparent', { allowIn: '$root' } );
-				schema.extend( '$text', { allowIn: [ 'pchild', 'pparent' ] } );
+				schema.register( 'hchild', { allowIn: 'heading1' } );
+				schema.extend( '$text', { allowIn: [ 'pchild', 'pparent', 'hchild' ] } );
 			} );
 
 			test(
@@ -344,47 +345,24 @@ describe( 'DataController utils', () => {
 					'<paragraph>x<pchild>fo[]ar</pchild>y</paragraph>'
 				);
 
-				it( 'merges elements when deep nested (3rd level)', () => {
-					const root = doc.getRoot();
-
-					// We need to use the raw API due to https://github.com/ckeditor/ckeditor5-engine/issues/905.
-					// <pparent>x<paragraph>x<pchild>fo[o</pchild></paragraph></pparent>
-					// <pparent><paragraph><pchild>b]ar</pchild>y</paragraph>y</pparent>
-
-					root._appendChild(
-						new Element( 'pparent', null, [
-							'x',
-							new Element( 'paragraph', null, [
-								'x',
-								new Element( 'pchild', null, 'foo' )
-							] )
-						] )
-					);
-
-					root._appendChild(
-						new Element( 'pparent', null, [
-							new Element( 'paragraph', null, [
-								new Element( 'pchild', null, 'bar' ),
-								'y'
-							] ),
-							'y'
-						] )
-					);
-
-					const range = new Range(
-						new Position( doc.getRoot(), [ 0, 1, 1, 2 ] ), // fo[o
-						new Position( doc.getRoot(), [ 1, 0, 0, 1 ] ) // b]ar
-					);
-
-					model.change( writer => {
-						writer.setSelection( range );
-					} );
+				test(
+					'removes block element with nested element',
+					'<paragraph><pchild>[foo</pchild></paragraph><paragraph><pchild>b]ar</pchild></paragraph>',
+					'<paragraph><pchild>[]ar</pchild></paragraph>'
+				);
 
-					deleteContent( model, doc.selection );
+				test(
+					'removes heading element',
+					'<heading1><hchild>[foo</hchild></heading1><paragraph><pchild>b]ar</pchild></paragraph>',
+					'<paragraph><pchild>[]ar</pchild></paragraph>'
+				);
 
-					expect( getData( model ) )
-						.to.equal( '<pparent>x<paragraph>x<pchild>fo[]ar</pchild>y</paragraph>y</pparent>' );
-				} );
+				test(
+					'merges elements when deep nested (3rd level)',
+					'<pparent>x<paragraph>x<pchild>fo[o</pchild></paragraph></pparent>' +
+					'<pparent><paragraph><pchild>b]ar</pchild>y</paragraph>y</pparent>',
+					'<pparent>x<paragraph>x<pchild>fo[]ar</pchild>y</paragraph>y</pparent>'
+				);
 
 				test(
 					'merges elements when left end deep nested',
@@ -398,40 +376,11 @@ describe( 'DataController utils', () => {
 					'<paragraph>x</paragraph><paragraph>fo[]ar</paragraph><paragraph>x</paragraph>'
 				);
 
-				it( 'merges elements when left end deep nested (3rd level)', () => {
-					const root = doc.getRoot();
-
-					// We need to use the raw API due to https://github.com/ckeditor/ckeditor5-engine/issues/905.
-					// <pparent>x<paragraph>foo<pchild>ba[r</pchild></paragraph></pparent><paragraph>b]om</paragraph>
-
-					root._appendChild(
-						new Element( 'pparent', null, [
-							'x',
-							new Element( 'paragraph', null, [
-								'foo',
-								new Element( 'pchild', null, 'bar' )
-							] )
-						] )
-					);
-
-					root._appendChild(
-						new Element( 'paragraph', null, 'bom' )
-					);
-
-					const range = new Range(
-						new Position( doc.getRoot(), [ 0, 1, 3, 2 ] ), // ba[r
-						new Position( doc.getRoot(), [ 1, 1 ] ) // b]om
-					);
-
-					model.change( writer => {
-						writer.setSelection( range );
-					} );
-
-					deleteContent( model, doc.selection );
-
-					expect( getData( model ) )
-						.to.equal( '<pparent>x<paragraph>foo<pchild>ba[]om</pchild></paragraph></pparent>' );
-				} );
+				test(
+					'merges elements when left end deep nested (3rd level)',
+					'<pparent>x<paragraph>foo<pchild>ba[r</pchild></paragraph></pparent><paragraph>b]om</paragraph>',
+					'<pparent>x<paragraph>foo<pchild>ba[]om</pchild></paragraph></pparent>'
+				);
 
 				test(
 					'merges elements when right end deep nested (in an empty container)',
@@ -442,41 +391,14 @@ describe( 'DataController utils', () => {
 				test(
 					'merges elements when left end deep nested (in an empty container)',
 					'<paragraph><pchild>[foo</pchild></paragraph><paragraph>b]ar</paragraph><paragraph>x</paragraph>',
-					'<paragraph><pchild>[]ar</pchild></paragraph><paragraph>x</paragraph>'
+					'<paragraph>[]ar</paragraph><paragraph>x</paragraph>'
 				);
 
-				it( 'merges elements when right end deep nested (3rd level)', () => {
-					const root = doc.getRoot();
-
-					// We need to use the raw API due to https://github.com/ckeditor/ckeditor5-engine/issues/905.
-					// <paragraph>fo[o</paragraph><pparent><paragraph><pchild>bar]</pchild></paragraph></pparent>
-
-					root._appendChild(
-						new Element( 'paragraph', null, 'foo' )
-					);
-
-					root._appendChild(
-						new Element( 'pparent', null, [
-							new Element( 'paragraph', null, [
-								new Element( 'pchild', null, 'bar' )
-							] )
-						] )
-					);
-
-					const range = new Range(
-						new Position( doc.getRoot(), [ 0, 2 ] ), // f[oo
-						new Position( doc.getRoot(), [ 1, 0, 0, 3 ] ) // bar]
-					);
-
-					model.change( writer => {
-						writer.setSelection( range );
-					} );
-
-					deleteContent( model, doc.selection );
-
-					expect( getData( model ) )
-						.to.equal( '<paragraph>fo[]</paragraph>' );
-				} );
+				test(
+					'merges elements when right end deep nested (3rd level)',
+					'<paragraph>fo[o</paragraph><pparent><paragraph><pchild>bar]</pchild></paragraph></pparent>',
+					'<paragraph>fo[]</paragraph>'
+				);
 			} );
 
 			describe( 'with object elements', () => {