Merge pull request #1022 from ckeditor/t/1012

Feature: `DataController#deleteContent()` will leave a paragraph if the entire content was selected. Closes #1012.

On the occasion `$root` element has been marked as a limit element in Schema in order to simplify the checks.

Author: Reinmar

src/controller/deletecontent.js

@@ -32,17 +32,24 @@ export default function deleteContent( selection, batch, options = {} ) {
 		return;
 	}
 
-	const selRange = selection.getFirstRange();
+	// 1. Replace the entire content with paragraph.
+	// See: https://github.com/ckeditor/ckeditor5-engine/issues/1012#issuecomment-315017594.
+	if ( shouldEntireContentBeReplacedWithParagraph( batch.document.schema, selection ) ) {
+		replaceEntireContentWithParagraph( batch, selection );
+
+		return;
+	}
 
+	const selRange = selection.getFirstRange();
 	const startPos = selRange.start;
 	const endPos = LivePosition.createFromPosition( selRange.end );
 
-	// 1. Remove the content if there is any.
+	// 2. Remove the content if there is any.
 	if ( !selRange.start.isTouching( selRange.end ) ) {
 		batch.remove( selRange );
 	}
 
-	// 2. Merge elements in the right branch to the elements in the left branch.
+	// 3. Merge elements in the right branch to the elements in the left branch.
 	// The only reasonable (in terms of data and selection correctness) case in which we need to do that is:
 	//
 	// <heading type=1>Fo[</heading><paragraph>]ar</paragraph> => <heading type=1>Fo^ar</heading>
@@ -56,13 +63,10 @@ export default function deleteContent( selection, batch, options = {} ) {
 
 	selection.collapse( startPos );
 
-	// 3. Autoparagraphing.
+	// 4. Autoparagraphing.
 	// Check if a text is allowed in the new container. If not, try to create a new paragraph (if it's allowed here).
 	if ( shouldAutoparagraph( batch.document, startPos ) ) {
-		const paragraph = new Element( 'paragraph' );
-		batch.insert( startPos, paragraph );
-
-		selection.collapse( paragraph );
+		insertParagraph( batch, startPos, selection );
 	}
 
 	endPos.detach();
@@ -163,3 +167,61 @@ function checkCanBeMerged( leftPos, rightPos ) {
 
 	return true;
 }
+
+// Returns the lowest limit element defined in `Schema.limits` for passed selection.
+function getLimitElement( schema, selection ) {
+	let element = selection.getFirstRange().getCommonAncestor();
+
+	while ( !schema.limits.has( element.name ) ) {
+		if ( element.parent ) {
+			element = element.parent;
+		} else {
+			break;
+		}
+	}
+
+	return element;
+}
+
+function insertParagraph( batch, position, selection ) {
+	const paragraph = new Element( 'paragraph' );
+	batch.insert( position, paragraph );
+
+	selection.collapse( paragraph );
+}
+
+function replaceEntireContentWithParagraph( batch, selection ) {
+	const limitElement = getLimitElement( batch.document.schema, selection );
+
+	batch.remove( Range.createIn( limitElement ) );
+	insertParagraph( batch, Position.createAt( limitElement ), selection );
+}
+
+// We want to replace the entire content with a paragraph when:
+// * the entire content is selected,
+// * selection contains at least two elements,
+// * whether the paragraph is allowed in schema in the common ancestor.
+function shouldEntireContentBeReplacedWithParagraph( schema, selection ) {
+	const limitElement = getLimitElement( schema, selection );
+	const limitStartPosition = Position.createAt( limitElement );
+	const limitEndPosition = Position.createAt( limitElement, 'end' );
+
+	if (
+		!limitStartPosition.isTouching( selection.getFirstPosition() ) ||
+		!limitEndPosition.isTouching( selection.getLastPosition() )
+	) {
+		return false;
+	}
+
+	const range = selection.getFirstRange();
+
+	if ( range.start.parent == range.end.parent ) {
+		return false;
+	}
+
+	if ( !schema.check( { name: 'paragraph', inside: limitElement.name } ) ) {
+		return false;
+	}
+
+	return true;
+}

src/model/schema.js

@@ -83,6 +83,8 @@ export default class Schema {
 		this.allow( { name: '$block', inside: '$root' } );
 		this.allow( { name: '$inline', inside: '$block' } );
 
+		this.limits.add( '$root' );
+
 		// TMP!
 		// Create an "all allowed" context in the schema for processing the pasted content.
 		// Read: https://github.com/ckeditor/ckeditor5-engine/issues/638#issuecomment-255086588

tests/controller/deletecontent.js

@@ -235,8 +235,8 @@ describe( 'DataController', () => {
 
 			test(
 				'leaves just one element when all selected',
-				'<heading1>[x</heading1><paragraph>foo</paragraph><paragraph>y]</paragraph>',
-				'<heading1>[]</heading1>'
+				'<heading1>[x</heading1><paragraph>foo</paragraph><paragraph>y]bar</paragraph>',
+				'<heading1>[]bar</heading1>'
 			);
 
 			it( 'uses remove delta instead of merge delta if merged element is empty', () => {
@@ -450,6 +450,8 @@ describe( 'DataController', () => {
 
 				const schema = doc.schema;
 
+				schema.limits.add( 'restrictedRoot' );
+
 				schema.registerItem( 'image', '$inline' );
 				schema.registerItem( 'paragraph', '$block' );
 				schema.registerItem( 'heading1', '$block' );
@@ -465,6 +467,8 @@ describe( 'DataController', () => {
 			// See also "in simple scenarios => deletes an element".
 
 			it( 'deletes two inline elements', () => {
+				doc.schema.limits.add( 'paragraph' );
+
 				setData(
 					doc,
 					'x[<image></image><image></image>]z',
@@ -659,6 +663,94 @@ describe( 'DataController', () => {
 			);
 		} );
 
+		describe( 'should leave a paragraph if the entire content was selected', () => {
+			beforeEach( () => {
+				doc = new Document();
+				doc.createRoot();
+
+				const schema = doc.schema;
+
+				schema.registerItem( 'div', '$block' );
+				schema.limits.add( 'div' );
+
+				schema.registerItem( 'article', '$block' );
+				schema.limits.add( 'article' );
+
+				schema.registerItem( 'image', '$inline' );
+				schema.objects.add( 'image' );
+
+				schema.registerItem( 'paragraph', '$block' );
+				schema.registerItem( 'heading1', '$block' );
+				schema.registerItem( 'heading2', '$block' );
+
+				schema.allow( { name: '$text', inside: '$root' } );
+
+				schema.allow( { name: 'image', inside: '$root' } );
+				schema.allow( { name: 'image', inside: 'heading1' } );
+				schema.allow( { name: 'heading1', inside: 'div' } );
+				schema.allow( { name: 'paragraph', inside: 'div' } );
+				schema.allow( { name: 'heading1', inside: 'article' } );
+				schema.allow( { name: 'heading2', inside: 'article' } );
+			} );
+
+			test(
+				'but not if only one block was selected',
+				'<heading1>[xx]</heading1>',
+				'<heading1>[]</heading1>'
+			);
+
+			test(
+				'when the entire heading and paragraph were selected',
+				'<heading1>[xx</heading1><paragraph>yy]</paragraph>',
+				'<paragraph>[]</paragraph>'
+			);
+
+			test(
+				'when the entire content was selected',
+				'<heading1>[x</heading1><paragraph>foo</paragraph><paragraph>y]</paragraph>',
+				'<paragraph>[]</paragraph>'
+			);
+
+			test(
+				'inside the limit element when the entire heading and paragraph were inside',
+				'<div><heading1>[xx</heading1><paragraph>yy]</paragraph></div>',
+				'<div><paragraph>[]</paragraph></div>'
+			);
+
+			test(
+				'but not if schema does not accept paragraph in limit element',
+				'<article><heading1>[xx</heading1><heading2>yy]</heading2></article>',
+				'<article><heading1>[]</heading1></article>'
+			);
+
+			test(
+				'but not if selection is not containing the whole content',
+				'<image></image><heading1>[xx</heading1><paragraph>yy]</paragraph>',
+				'<image></image><heading1>[]</heading1>'
+			);
+
+			test(
+				'but not if only single element is selected',
+				'<heading1>[<image></image>xx]</heading1>',
+				'<heading1>[]</heading1>'
+			);
+
+			it( 'when root element was not added as Schema.limits works fine as well', () => {
+				doc.createRoot( 'paragraph', 'paragraphRoot' );
+
+				setData(
+					doc,
+					'x[<image></image><image></image>]z',
+					{ rootName: 'paragraphRoot' }
+				);
+
+				deleteContent( doc.selection, doc.batch() );
+
+				expect( getData( doc, { rootName: 'paragraphRoot' } ) )
+					.to.equal( 'x[]z' );
+			} );
+		} );
+
 		function test( title, input, output, options ) {
 			it( title, () => {
 				setData( doc, input );

tests/model/schema/schema.js

@@ -49,6 +49,10 @@ describe( 'Schema', () => {
 			expect( schema.limits ).to.be.instanceOf( Set );
 		} );
 
+		it( 'should mark $root as a limit element', () => {
+			expect( schema.limits.has( '$root' ) ).to.be.true;
+		} );
+
 		describe( '$clipboardHolder', () => {
 			it( 'should allow $block', () => {
 				expect( schema.check( { name: '$block', inside: [ '$clipboardHolder' ] } ) ).to.be.true;