浏览代码

Distinguished different kinds of selection in the Model#deleteContent() hook. Made weak tests more realistic.

Aleksander Nowodzinski 5 年之前
父节点
当前提交
9cad27cd8d

+ 27 - 12
packages/ckeditor5-table/src/tableselection.js

@@ -293,19 +293,34 @@ export default class TableSelection extends Plugin {
 	 */
 	_handleDeleteContent( event, args ) {
 		const [ selection, options ] = args;
-		const model = this.editor.model;
-		const isBackward = !options || options.direction == 'backward';
 
-		if ( this.hasMultiCellSelection && selection.rangeCount > 1 ) {
-			event.stop();
-
-			model.change( writer => {
-				clearTableCellsContents( model, this.getSelectedTableCells() );
-
-				const tableCell = isBackward ? this._endElement : this._startElement;
-
-				writer.setSelection( tableCell, 'in' );
-			} );
+		// This Model#deleteContent() hook supports multiple-cell selections only.
+		// **Note**: It executes Model#deleteContent() itself for each individual cell within
+		// the selection, so this also avoids infinite loops.
+		if ( selection.rangeCount > 1 ) {
+			const model = this.editor.model;
+			const isBackward = !options || options.direction == 'backward';
+			const selectedTableCells = [ ...this.getSelectedTableCells() ];
+
+			// There are possible multiple-range selection that have nothing to do with tables.
+			// Let's filter them out and focus on table cells only.
+			if ( selectedTableCells.length ) {
+				event.stop();
+
+				model.change( writer => {
+					clearTableCellsContents( model, selectedTableCells );
+
+					const tableCell = isBackward ? this._endElement : this._startElement;
+
+					// The insertContent() helper passes the actual DocumentSelection,
+					// while the deleteContent() helper always operates on the abstract clones.
+					if ( selection.is( 'documentSelection' ) ) {
+						writer.setSelection( tableCell, 'in' );
+					} else {
+						selection.setTo( tableCell, 'in' );
+					}
+				} );
+			}
 		}
 	}
 }

+ 8 - 6
packages/ckeditor5-table/tests/tableselection-integration.js

@@ -108,15 +108,16 @@ describe( 'table selection', () => {
 
 				viewDocument.fire( 'keydown', { keyCode: getCode( 'x' ) } );
 
-				//                                      figure       table         tbody         tr            td            span
-				const viewSpan = viewDocument.getRoot().getChild( 0 ).getChild( 1 ).getChild( 0 ).getChild( 1 ).getChild( 1 ).getChild( 0 );
+				// Mutate at the place where the document selection was put; it's more realistic
+				// than mutating at some arbitrary position.
+				const placeOfMutation = viewDocument.selection.getFirstRange().start.parent;
 
 				viewDocument.fire( 'mutations', [
 					{
 						type: 'children',
 						oldChildren: [],
 						newChildren: [ new ViewText( 'x' ) ],
-						node: viewSpan
+						node: placeOfMutation
 					}
 				] );
 
@@ -130,15 +131,16 @@ describe( 'table selection', () => {
 			it( 'should not interfere with default key handler if no table selection', () => {
 				viewDocument.fire( 'keydown', { keyCode: getCode( 'x' ) } );
 
-				//                                      figure       table         tbody         tr            td            span
-				const viewSpan = viewDocument.getRoot().getChild( 0 ).getChild( 1 ).getChild( 0 ).getChild( 0 ).getChild( 0 ).getChild( 0 );
+				// Mutate at the place where the document selection was put; it's more realistic
+				// than mutating at some arbitrary position.
+				const placeOfMutation = viewDocument.selection.getFirstRange().start.parent;
 
 				viewDocument.fire( 'mutations', [
 					{
 						type: 'children',
 						oldChildren: [],
 						newChildren: [ new ViewText( 'x' ) ],
-						node: viewSpan
+						node: placeOfMutation
 					}
 				] );