Browse Source

Merge pull request #1830 from ckeditor/i/6116

Other: Introduced support for multi-range selections. Closes ckeditor/ckeditor5#6116.
Piotrek Koszuliński 5 years ago
parent
commit
5e4341d0e5

+ 12 - 2
packages/ckeditor5-engine/src/model/schema.js

@@ -672,12 +672,22 @@ export default class Schema {
 
 		let backwardWalker, forwardWalker;
 
+		// Never leave a limit element.
+		const limitElement = position.getAncestors().reverse().find( item => this.isLimit( item ) ) || position.root;
+
 		if ( direction == 'both' || direction == 'backward' ) {
-			backwardWalker = new TreeWalker( { startPosition: position, direction: 'backward' } );
+			backwardWalker = new TreeWalker( {
+				boundaries: Range._createIn( limitElement ),
+				startPosition: position,
+				direction: 'backward'
+			} );
 		}
 
 		if ( direction == 'both' || direction == 'forward' ) {
-			forwardWalker = new TreeWalker( { startPosition: position } );
+			forwardWalker = new TreeWalker( {
+				boundaries: Range._createIn( limitElement ),
+				startPosition: position
+			} );
 		}
 
 		for ( const data of combineWalkers( backwardWalker, forwardWalker ) ) {

+ 7 - 11
packages/ckeditor5-engine/src/model/utils/deletecontent.js

@@ -48,7 +48,10 @@ import DocumentSelection from '../documentselection';
  * For example `<paragraph>x</paragraph>[<image src="foo.jpg"></image>]` will become:
  *
  * * `<paragraph>x</paragraph><paragraph>[]</paragraph>` with the option disabled (`doNotAutoparagraph == false`)
- * * `<paragraph>x[]</paragraph>` with the option enabled (`doNotAutoparagraph == true`).
+ * * `<paragraph>x</paragraph>[]` with the option enabled (`doNotAutoparagraph == true`).
+ *
+ * If you use this option you need to make sure to handle invalid selections yourself or leave
+ * them to the selection post-fixer (may not always work).
  *
  * **Note:** if there is no valid position for the selection, the paragraph will always be created:
  *
@@ -109,16 +112,9 @@ export default function deleteContent( model, selection, options = {} ) {
 
 		// 4. Add a paragraph to set selection in it.
 		// 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( schema, startPos ) ) {
-			// If auto-paragraphing is off, find the closest valid selection range and collapse the selection there.
-			// If there is no valid selection range, create paragraph anyway and set selection there.
-			const validSelectionRange = schema.getNearestSelectionRange( startPos );
-
-			if ( options.doNotAutoparagraph && validSelectionRange ) {
-				collapseSelectionAt( writer, selection, validSelectionRange );
-			} else {
-				insertParagraph( writer, startPos, selection );
-			}
+		// If autoparagraphing is off, we assume that you know what you do so we leave the selection wherever it was.
+		if ( !options.doNotAutoparagraph && shouldAutoparagraph( schema, startPos ) ) {
+			insertParagraph( writer, startPos, selection );
 		}
 
 		endPos.detach();

+ 2 - 4
packages/ckeditor5-engine/src/model/utils/insertcontent.js

@@ -50,13 +50,11 @@ export default function insertContent( model, content, selectable, placeOrOffset
 			selection = writer.createSelection( selectable, placeOrOffset );
 		}
 
-		const insertionPosition = selection.getFirstPosition();
-
 		if ( !selection.isCollapsed ) {
 			model.deleteContent( selection, { doNotAutoparagraph: true } );
 		}
 
-		const insertion = new Insertion( model, writer, insertionPosition );
+		const insertion = new Insertion( model, writer, selection.anchor );
 
 		let nodesToInsert;
 
@@ -89,7 +87,7 @@ export default function insertContent( model, content, selectable, placeOrOffset
 			// @if CK_DEBUG // console.warn( 'Cannot determine a proper selection range after insertion.' );
 		}
 
-		const affectedRange = insertion.getAffectedRange() || model.createRange( insertionPosition );
+		const affectedRange = insertion.getAffectedRange() || model.createRange( selection.anchor );
 
 		insertion.destroy();
 

+ 50 - 0
packages/ckeditor5-engine/tests/model/schema.js

@@ -1420,12 +1420,20 @@ describe( 'Schema', () => {
 				isObject: true
 			} );
 
+			// Similar to the <caption> case.
 			model.schema.register( 'blockWidget', {
 				allowIn: '$root',
 				allowContentOf: '$block',
 				isObject: true
 			} );
 
+			// Similar to the <tableCell> case.
+			model.schema.register( 'blockContainerWidget', {
+				allowIn: '$root',
+				allowContentOf: '$root',
+				isObject: true
+			} );
+
 			doc.createRoot();
 			selection = doc.selection;
 		} );
@@ -1622,6 +1630,48 @@ describe( 'Schema', () => {
 			);
 		} );
 
+		describe( 'in case of other types of objects', () => {
+			test(
+				'should return null when cannot leave a limit element',
+				'<blockContainerWidget>[]</blockContainerWidget>',
+				'both',
+				null
+			);
+
+			test(
+				'should return null when cannot leave a limit element (surrounded with paragraphs)',
+				'<paragraph>x</paragraph><blockContainerWidget>[]</blockContainerWidget><paragraph>x</paragraph>',
+				'both',
+				null
+			);
+
+			test(
+				'should return null when cannot leave a limit element (surrounded by other widgets)',
+
+				'<blockContainerWidget><paragraph>x</paragraph></blockContainerWidget>' +
+				'<blockContainerWidget>[]</blockContainerWidget>' +
+				'<blockContainerWidget><paragraph>x</paragraph></blockContainerWidget>',
+
+				'both',
+
+				null
+			);
+
+			test(
+				'should keep scanning even though encountering a limit in one direction (left)',
+				'<paragraph>x</paragraph><blockContainerWidget>[]<paragraph>x</paragraph></blockContainerWidget><paragraph>x</paragraph>',
+				'both',
+				'<paragraph>x</paragraph><blockContainerWidget><paragraph>[]x</paragraph></blockContainerWidget><paragraph>x</paragraph>'
+			);
+
+			test(
+				'should keep scanning even though encountering a limit in one direction (right)',
+				'<paragraph>x</paragraph><blockContainerWidget><paragraph>x</paragraph>[]</blockContainerWidget><paragraph>x</paragraph>',
+				'both',
+				'<paragraph>x</paragraph><blockContainerWidget><paragraph>x[]</paragraph></blockContainerWidget><paragraph>x</paragraph>'
+			);
+		} );
+
 		function test( testName, data, direction, expected ) {
 			it( testName, () => {
 				let range;

+ 8 - 4
packages/ckeditor5-engine/tests/model/utils/deletecontent.js

@@ -10,6 +10,7 @@ import Selection from '../../../src/model/selection';
 import Element from '../../../src/model/element';
 import deleteContent from '../../../src/model/utils/deletecontent';
 import { setData, getData } from '../../../src/dev-utils/model';
+import { stringify } from '../../../src/dev-utils/view';
 
 describe( 'DataController utils', () => {
 	let model, doc;
@@ -444,7 +445,7 @@ describe( 'DataController utils', () => {
 					'<paragraph><pchild>[]ar</pchild></paragraph><paragraph>x</paragraph>'
 				);
 
-				it( 'merges elements when left end deep nested (3rd level)', () => {
+				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.
@@ -615,7 +616,7 @@ describe( 'DataController utils', () => {
 					.to.equal( 'x[]z' );
 			} );
 
-			it( 'creates a paragraph when text is not allowed (custom selection)', () => {
+			it( 'moves the (custom) selection to the nearest paragraph', () => {
 				setData(
 					model,
 					'<paragraph>[x]</paragraph><paragraph>yyy</paragraph><paragraph>z</paragraph>',
@@ -633,6 +634,9 @@ describe( 'DataController utils', () => {
 
 				expect( getData( model, { rootName: 'bodyRoot' } ) )
 					.to.equal( '<paragraph>[x]</paragraph><paragraph></paragraph><paragraph>z</paragraph>' );
+
+				expect( stringify( root, selection ) )
+					.to.equal( '<$root><paragraph>x</paragraph><paragraph>[]</paragraph><paragraph>z</paragraph></$root>' );
 			} );
 
 			it( 'creates a paragraph when text is not allowed (block widget selected)', () => {
@@ -725,7 +729,7 @@ describe( 'DataController utils', () => {
 					.to.equal( '<paragraph>x[]</paragraph><paragraph>z</paragraph>' );
 			} );
 
-			it( 'creates paragraph when after deletion there is no valid selection range', () => {
+			it( 'does not create a paragraph when after deletion there is no valid selection range (empty root)', () => {
 				setData(
 					model,
 					'[<blockWidget></blockWidget>]',
@@ -735,7 +739,7 @@ describe( 'DataController utils', () => {
 				deleteContent( model, doc.selection, { doNotAutoparagraph: true } );
 
 				expect( getData( model, { rootName: 'bodyRoot' } ) )
-					.to.equal( '<paragraph>[]</paragraph>' );
+					.to.equal( '[]' );
 			} );
 		} );
 

+ 46 - 2
packages/ckeditor5-engine/tests/model/utils/insertcontent.js

@@ -223,6 +223,24 @@ describe( 'DataController utils', () => {
 			expect( doc.getRoot().getChild( 0 ).getChild( 1 ) ).to.equal( content );
 		} );
 
+		it( 'should use the selection set by deleteContent()', () => {
+			model.on( 'deleteContent', evt => {
+				evt.stop();
+
+				model.change( writer => {
+					writer.setSelection( root.getChild( 0 ), 'end' );
+				} );
+			}, { priority: 'high' } );
+
+			model.schema.register( 'paragraph', { inheritAllFrom: '$block' } );
+
+			setData( model, '<paragraph>[fo]o</paragraph>' );
+
+			insertHelper( 'xyz' );
+
+			expect( getData( model ) ).to.equal( '<paragraph>fooxyz[]</paragraph>' );
+		} );
+
 		describe( 'in simple scenarios', () => {
 			beforeEach( () => {
 				model = new Model();
@@ -697,6 +715,32 @@ describe( 'DataController utils', () => {
 						'<listItem>]o</listItem>'
 					);
 				} );
+
+				// See ckeditor5#2010.
+				it( 'should handle bQ+p over bQ+p insertion', () => {
+					model.schema.register( 'blockQuote', {
+						allowWhere: '$block',
+						allowContentOf: '$root'
+					} );
+
+					setData( model, '<blockQuote><paragraph>[foo</paragraph></blockQuote><paragraph>bar]</paragraph>' );
+
+					const affectedRange = insertHelper( '<blockQuote><paragraph>xxx</paragraph></blockQuote><paragraph>yyy</paragraph>' );
+
+					expect( getData( model ) ).to.equal(
+						'<blockQuote>' +
+							'<paragraph>xxx</paragraph>' +
+						'</blockQuote>' +
+						'<paragraph>yyy[]</paragraph>'
+					);
+
+					expect( stringify( root, affectedRange ) ).to.equal(
+						'[<blockQuote>' +
+							'<paragraph>xxx</paragraph>' +
+						'</blockQuote>' +
+						'<paragraph>yyy</paragraph>]'
+					);
+				} );
 			} );
 
 			describe( 'mixed content to block', () => {
@@ -1275,11 +1319,11 @@ describe( 'DataController utils', () => {
 			it( 'should not remove empty elements when not-allowed element is paste', () => {
 				setData( model, '<wrapper><limit><paragraph>[]</paragraph></limit></wrapper>' );
 
-				// pasted content is forbidden in current selection
+				// Pasted content is forbidden in current selection.
 				const affectedRange = insertHelper( '<wrapper><limit><paragraph>foo</paragraph></limit></wrapper>' );
 
 				expect( getData( model ) ).to.equal( '<wrapper><limit>[]<paragraph></paragraph></limit></wrapper>' );
-				expect( stringify( root, affectedRange ) ).to.equal( '<wrapper><limit><paragraph>[]</paragraph></limit></wrapper>' );
+				expect( stringify( root, affectedRange ) ).to.equal( '<wrapper><limit>[]<paragraph></paragraph></limit></wrapper>' );
 			} );
 
 			it( 'should correctly paste allowed nodes', () => {

+ 2 - 2
packages/ckeditor5-engine/tests/model/utils/selection-post-fixer.js

@@ -1242,10 +1242,10 @@ describe( 'Selection post-fixer', () => {
 				} );
 
 				expect( getModelData( model ) ).to.equal(
-					'<paragraph>foo[]</paragraph>' +
+					'<paragraph>foo</paragraph>' +
 					'<table>' +
 						'<tableRow>' +
-							'<tableCell><paragraph>aaa</paragraph></tableCell>' +
+							'[<tableCell><paragraph>aaa</paragraph></tableCell>]' +
 							'<tableCell><paragraph>bbb</paragraph></tableCell>' +
 						'</tableRow>' +
 					'</table>' +