浏览代码

Merge pull request #301 from ckeditor/i/6437

Fix: Removing the last header row no longer breaks the table in the editing view. Closes ckeditor/ckeditor5#6437.
Piotrek Koszuliński 5 年之前
父节点
当前提交
3c7c721568

+ 8 - 2
packages/ckeditor5-table/src/commands/removerowcommand.js

@@ -60,7 +60,10 @@ export default class RemoveRowCommand extends Command {
 
 		const columnIndexToFocus = this.editor.plugins.get( 'TableUtils' ).getCellLocation( firstCell ).column;
 
-		model.change( writer => {
+		// Use single batch to modify table in steps but in one undo step.
+		const batch = model.createBatch();
+
+		model.enqueueChange( batch, writer => {
 			// This prevents the "model-selection-range-intersects" error, caused by removing row selected cells.
 			writer.setSelection( writer.createSelection( table, 'on' ) );
 
@@ -68,9 +71,12 @@ export default class RemoveRowCommand extends Command {
 
 			this.editor.plugins.get( 'TableUtils' ).removeRows( table, {
 				at: removedRowIndexes.first,
-				rows: rowsToRemove
+				rows: rowsToRemove,
+				batch
 			} );
+		} );
 
+		model.enqueueChange( batch, writer => {
 			const cellToFocus = getCellToFocus( table, removedRowIndexes.first, columnIndexToFocus );
 
 			writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );

+ 8 - 11
packages/ckeditor5-table/src/converters/downcast.js

@@ -209,9 +209,6 @@ export function downcastTableHeadingRowsChange( options = {} ) {
 					renameViewTableCell( tableCell, 'th', conversionApi, asWidget );
 				}
 			}
-
-			// Cleanup: this will remove any empty section from the view which may happen when moving all rows from a table section.
-			removeTableSectionIfEmpty( 'tbody', viewTable, conversionApi );
 		}
 		// The head section has shrunk so move rows from <thead> to <tbody>.
 		else {
@@ -234,11 +231,12 @@ export function downcastTableHeadingRowsChange( options = {} ) {
 			for ( const tableWalkerValue of tableWalker ) {
 				renameViewTableCellIfRequired( tableWalkerValue, tableAttributes, conversionApi, asWidget );
 			}
-
-			// Cleanup: this will remove any empty section from the view which may happen when moving all rows from a table section.
-			removeTableSectionIfEmpty( 'thead', viewTable, conversionApi );
 		}
 
+		// Cleanup: Ensure that thead & tbody sections are removed if left empty after moving rows. See #6437, #6391.
+		removeTableSectionIfEmpty( 'thead', viewTable, conversionApi );
+		removeTableSectionIfEmpty( 'tbody', viewTable, conversionApi );
+
 		function isBetween( index, lower, upper ) {
 			return index > lower && index < upper;
 		}
@@ -298,6 +296,7 @@ export function downcastRemoveRow() {
 		const viewStart = mapper.toViewPosition( data.position ).getLastMatchingPosition( value => !value.item.is( 'tr' ) );
 		const viewItem = viewStart.nodeAfter;
 		const tableSection = viewItem.parent;
+		const viewTable = tableSection.parent;
 
 		// Remove associated <tr> from the view.
 		const removeRange = viewWriter.createRangeOn( viewItem );
@@ -307,11 +306,9 @@ export function downcastRemoveRow() {
 			mapper.unbindViewElement( child );
 		}
 
-		// Check if table section has any children left - if not remove it from the view.
-		if ( !tableSection.childCount ) {
-			// No need to unbind anything as table section is not represented in the model.
-			viewWriter.remove( viewWriter.createRangeOn( tableSection ) );
-		}
+		// Cleanup: Ensure that thead & tbody sections are removed if left empty after removing rows. See #6437, #6391.
+		removeTableSectionIfEmpty( 'thead', viewTable, conversionApi );
+		removeTableSectionIfEmpty( 'tbody', viewTable, conversionApi );
 	}, { priority: 'higher' } );
 }
 

+ 3 - 2
packages/ckeditor5-table/src/tableutils.js

@@ -287,6 +287,7 @@ export default class TableUtils extends Plugin {
 		const rowsToRemove = options.rows || 1;
 		const first = options.at;
 		const last = first + rowsToRemove - 1;
+		const batch = options.batch || 'default';
 
 		// Removing rows from table requires most calculations to be done prior to changing table structure.
 
@@ -294,7 +295,7 @@ export default class TableUtils extends Plugin {
 		const { cellsToMove, cellsToTrim } = getCellsToMoveAndTrimOnRemoveRow( table, first, last );
 
 		// 2. Execution
-		model.change( writer => {
+		model.enqueueChange( batch, writer => {
 			// 2a. Move cells from removed rows that extends over a removed section - must be done before removing rows.
 			// This will fill any gaps in a rows below that previously were empty because of row-spanned cells.
 			const rowAfterRemovedSection = last + 1;
@@ -311,7 +312,7 @@ export default class TableUtils extends Plugin {
 			}
 
 			// 2d. Adjust heading rows if removed rows were in a heading section.
-			updateHeadingRows( table, first, last, model, writer.batch );
+			updateHeadingRows( table, first, last, model, batch );
 		} );
 	}
 

+ 12 - 15
packages/ckeditor5-table/tests/commands/removerowcommand.js

@@ -9,22 +9,19 @@ import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils
 
 import RemoveRowCommand from '../../src/commands/removerowcommand';
 import TableSelection from '../../src/tableselection';
-import { defaultConversion, defaultSchema, modelTable, viewTable } from '../_utils/utils';
+import { modelTable, viewTable } from '../_utils/utils';
 import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
+import TableEditing from '../../src/tableediting';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 
 describe( 'RemoveRowCommand', () => {
 	let editor, model, command;
 
-	beforeEach( () => {
-		return VirtualTestEditor.create( { plugins: [ TableSelection ] } )
-			.then( newEditor => {
-				editor = newEditor;
-				model = editor.model;
-				command = new RemoveRowCommand( editor );
+	beforeEach( async () => {
+		editor = await VirtualTestEditor.create( { plugins: [ Paragraph, TableEditing, TableSelection ] } );
 
-				defaultSchema( model.schema );
-				defaultConversion( editor.conversion );
-			} );
+		model = editor.model;
+		command = new RemoveRowCommand( editor );
 	} );
 
 	afterEach( () => {
@@ -290,11 +287,11 @@ describe( 'RemoveRowCommand', () => {
 					[ '[]40', '41' ]
 				], { headingRows: 1 } ) );
 
-				// The view should also be properly downcasted.
+				// The editing view should also be properly downcasted.
 				assertEqualMarkup( getViewData( editor.editing.view, { withoutSelection: true } ), viewTable( [
 					[ '00', '01' ],
 					[ '40', '41' ]
-				], { headingRows: 1 } ) );
+				], { headingRows: 1, asWidget: true } ) );
 			} );
 
 			it( 'should support removing mixed heading and cell rows', () => {
@@ -509,8 +506,8 @@ describe( 'RemoveRowCommand', () => {
 			setData( model, modelTable( [
 				[ { rowspan: 4, contents: '00' }, { rowspan: 3, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ],
 				[ { rowspan: 2, contents: '13' }, '14' ],
-				[ '22[]', '23', '24' ],
-				[ '30', '31', '32', '33', '34' ]
+				[ '22[]', '24' ],
+				[ '31', '32', '33', '34' ]
 			] ) );
 
 			command.execute();
@@ -518,7 +515,7 @@ describe( 'RemoveRowCommand', () => {
 			assertEqualMarkup( getData( model ), modelTable( [
 				[ { rowspan: 3, contents: '00' }, { rowspan: 2, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ],
 				[ '13', '14' ],
-				[ '30', '31', '[]32', '33', '34' ]
+				[ '31', '32', '[]33', '34' ]
 			] ) );
 		} );
 

+ 452 - 234
packages/ckeditor5-table/tests/converters/downcast.js

@@ -4,13 +4,15 @@
  */
 
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
-import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
-import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 
-import { defaultConversion, defaultSchema, modelTable, viewTable } from '../_utils/utils';
+import TableEditing from '../../src/tableediting';
 import env from '@ckeditor/ckeditor5-utils/src/env';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
+import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
+import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
+import { defaultConversion, defaultSchema, modelTable, viewTable } from '../_utils/utils';
 
 function paragraphInTableCell() {
 	return dispatcher => dispatcher.on( 'insert:paragraph', ( evt, data, conversionApi ) => {
@@ -42,24 +44,25 @@ describe( 'downcast converters', () => {
 
 	testUtils.createSinonSandbox();
 
-	beforeEach( () => {
-		// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
-		testUtils.sinon.stub( env, 'isEdge' ).get( () => false );
+	describe( 'downcastInsertTable()', () => {
+		// The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing.
+		beforeEach( () => {
+			// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
+			testUtils.sinon.stub( env, 'isEdge' ).get( () => false );
 
-		return VirtualTestEditor.create()
-			.then( newEditor => {
-				editor = newEditor;
-				model = editor.model;
-				doc = model.document;
-				root = doc.getRoot( 'main' );
-				view = editor.editing.view;
+			return VirtualTestEditor.create()
+				.then( newEditor => {
+					editor = newEditor;
+					model = editor.model;
+					doc = model.document;
+					root = doc.getRoot( 'main' );
+					view = editor.editing.view;
 
-				defaultSchema( model.schema );
-				defaultConversion( editor.conversion );
-			} );
-	} );
+					defaultSchema( model.schema );
+					defaultConversion( editor.conversion );
+				} );
+		} );
 
-	describe( 'downcastInsertTable()', () => {
 		it( 'should create table with tbody', () => {
 			setModelData( model, modelTable( [ [ '' ] ] ) );
 
@@ -355,6 +358,24 @@ describe( 'downcast converters', () => {
 	} );
 
 	describe( 'downcastInsertRow()', () => {
+		// The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing.
+		beforeEach( () => {
+			// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
+			testUtils.sinon.stub( env, 'isEdge' ).get( () => false );
+
+			return VirtualTestEditor.create()
+				.then( newEditor => {
+					editor = newEditor;
+					model = editor.model;
+					doc = model.document;
+					root = doc.getRoot( 'main' );
+					view = editor.editing.view;
+
+					defaultSchema( model.schema );
+					defaultConversion( editor.conversion );
+				} );
+		} );
+
 		it( 'should react to changed rows', () => {
 			setModelData( model, modelTable( [
 				[ '00', '01' ]
@@ -592,6 +613,24 @@ describe( 'downcast converters', () => {
 	} );
 
 	describe( 'downcastInsertCell()', () => {
+		// The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing.
+		beforeEach( () => {
+			// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
+			testUtils.sinon.stub( env, 'isEdge' ).get( () => false );
+
+			return VirtualTestEditor.create()
+				.then( newEditor => {
+					editor = newEditor;
+					model = editor.model;
+					doc = model.document;
+					root = doc.getRoot( 'main' );
+					view = editor.editing.view;
+
+					defaultSchema( model.schema );
+					defaultConversion( editor.conversion );
+				} );
+		} );
+
 		it( 'should add tableCell on proper index in tr', () => {
 			setModelData( model, modelTable( [
 				[ '00', '01' ]
@@ -736,6 +775,24 @@ describe( 'downcast converters', () => {
 	} );
 
 	describe( 'downcastTableHeadingColumnsChange()', () => {
+		// The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing.
+		beforeEach( () => {
+			// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
+			testUtils.sinon.stub( env, 'isEdge' ).get( () => false );
+
+			return VirtualTestEditor.create()
+				.then( newEditor => {
+					editor = newEditor;
+					model = editor.model;
+					doc = model.document;
+					root = doc.getRoot( 'main' );
+					view = editor.editing.view;
+
+					defaultSchema( model.schema );
+					defaultConversion( editor.conversion );
+				} );
+		} );
+
 		it( 'should work for adding heading columns', () => {
 			setModelData( model, modelTable( [
 				[ '00', '01' ],
@@ -895,7 +952,7 @@ describe( 'downcast converters', () => {
 
 				assertEqualMarkup( getViewData( view, { withoutSelection: true } ),
 					'<figure class="ck-widget ck-widget_with-selection-handle table" contenteditable="false">' +
-					'<div class="ck ck-widget__selection-handle"></div>' +
+						'<div class="ck ck-widget__selection-handle"></div>' +
 						'<table>' +
 							'<thead>' +
 								'<tr>' +
@@ -912,190 +969,201 @@ describe( 'downcast converters', () => {
 	} );
 
 	describe( 'downcastTableHeadingRowsChange()', () => {
-		it( 'should work for adding heading rows', () => {
-			setModelData( model, modelTable( [
-				[ '00', '01' ],
-				[ '10', '11' ],
-				[ '20', '21' ]
-			] ) );
-
-			const table = root.getChild( 0 );
-
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 2, table );
-			} );
+		// The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing.
+		beforeEach( () => {
+			// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
+			testUtils.sinon.stub( env, 'isEdge' ).get( () => false );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '00', '01' ],
-				[ '10', '11' ],
-				[ '20', '21' ]
-			], { headingRows: 2 } ) );
+			return VirtualTestEditor.create( { plugins: [ Paragraph, TableEditing ] } )
+				.then( newEditor => {
+					editor = newEditor;
+					model = editor.model;
+					doc = model.document;
+					root = doc.getRoot( 'main' );
+					view = editor.editing.view;
+				} );
 		} );
 
-		it( 'should work for changing number of heading rows to a bigger number', () => {
-			setModelData( model, modelTable( [
-				[ '00', '01' ],
-				[ '10', '11' ],
-				[ '20', '21' ]
-			], { headingRows: 1 } ) );
+		// The heading rows change downcast conversion is not executed in data pipeline.
+		describe( 'editing pipeline', () => {
+			it( 'should work for adding heading rows', () => {
+				setModelData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				] ) );
 
-			const table = root.getChild( 0 );
+				const table = root.getChild( 0 );
 
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 2, table );
+				model.change( writer => {
+					writer.setAttribute( 'headingRows', 2, table );
+				} );
+
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				], { headingRows: 2, asWidget: true } ) );
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '00', '01' ],
-				[ '10', '11' ],
-				[ '20', '21' ]
-			], { headingRows: 2 } ) );
-		} );
+			it( 'should work for changing number of heading rows to a bigger number', () => {
+				setModelData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				], { headingRows: 1 } ) );
 
-		it( 'should work for changing number of heading rows to a smaller number', () => {
-			setModelData( model, modelTable( [
-				[ '00', '01' ],
-				[ '10', '11' ],
-				[ '20', '21' ],
-				[ '30', '31' ]
-			], { headingRows: 3 } ) );
+				const table = root.getChild( 0 );
 
-			const table = root.getChild( 0 );
+				model.change( writer => {
+					writer.setAttribute( 'headingRows', 2, table );
+				} );
 
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 2, table );
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				], { headingRows: 2, asWidget: true } ) );
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '00', '01' ],
-				[ '10', '11' ],
-				[ '20', '21' ],
-				[ '30', '31' ]
-			], { headingRows: 2 } ) );
-		} );
+			it( 'should work for changing number of heading rows to a smaller number', () => {
+				setModelData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				], { headingRows: 3 } ) );
 
-		it( 'should work for removing heading rows', () => {
-			setModelData( model, modelTable( [
-				[ '00', '01' ],
-				[ '10', '11' ]
-			], { headingRows: 2 } ) );
+				const table = root.getChild( 0 );
 
-			const table = root.getChild( 0 );
+				model.change( writer => {
+					writer.setAttribute( 'headingRows', 2, table );
+				} );
 
-			model.change( writer => {
-				writer.removeAttribute( 'headingRows', table );
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				], { headingRows: 2, asWidget: true } ) );
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '00', '01' ],
-				[ '10', '11' ]
-			] ) );
-		} );
+			it( 'should work for removing heading rows', () => {
+				setModelData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ]
+				], { headingRows: 2 } ) );
 
-		it( 'should work for making heading rows without tbody', () => {
-			setModelData( model, modelTable( [
-				[ '00', '01' ],
-				[ '10', '11' ]
-			] ) );
+				const table = root.getChild( 0 );
 
-			const table = root.getChild( 0 );
+				model.change( writer => {
+					writer.removeAttribute( 'headingRows', table );
+				} );
 
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 2, table );
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
+					[ '00', '01' ],
+					[ '10', '11' ]
+				], { asWidget: true } ) );
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '00', '01' ],
-				[ '10', '11' ]
-			], { headingRows: 2 } ) );
-		} );
+			it( 'should work for making heading rows without tbody', () => {
+				setModelData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ]
+				] ) );
 
-		it( 'should be possible to overwrite', () => {
-			editor.conversion.attributeToAttribute( { model: 'headingRows', view: 'headingRows', converterPriority: 'high' } );
-			setModelData( model, modelTable( [ [ '00' ] ] ) );
+				const table = root.getChild( 0 );
 
-			const table = root.getChild( 0 );
+				model.change( writer => {
+					writer.setAttribute( 'headingRows', 2, table );
+				} );
 
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 1, table );
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
+					[ '00', '01' ],
+					[ '10', '11' ]
+				], { headingRows: 2, asWidget: true } ) );
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ),
-				'<figure class="table" headingRows="1">' +
-					'<table>' +
-						'<tbody>' +
-							'<tr><td>00</td></tr>' +
-						'</tbody>' +
-					'</table>' +
-				'</figure>'
-			);
-		} );
-
-		it( 'should work with adding table rows at the beginning of a table', () => {
-			setModelData( model, modelTable( [
-				[ '00', '01' ],
-				[ '10', '11' ]
-			], { headingRows: 1 } ) );
-
-			const table = root.getChild( 0 );
+			it( 'should be possible to overwrite', () => {
+				editor.conversion.attributeToAttribute( {
+					model: 'headingRows',
+					view: 'headingRows',
+					converterPriority: 'high'
+				} );
+				setModelData( model, modelTable( [ [ '00' ] ] ) );
 
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 2, table );
+				const table = root.getChild( 0 );
 
-				const tableRow = writer.createElement( 'tableRow' );
+				model.change( writer => {
+					writer.setAttribute( 'headingRows', 1, table );
+				} );
 
-				writer.insert( tableRow, table, 0 );
-				writer.insertElement( 'tableCell', tableRow, 'end' );
-				writer.insertElement( 'tableCell', tableRow, 'end' );
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ),
+					'<figure class="ck-widget ck-widget_with-selection-handle table" contenteditable="false" headingRows="1">' +
+						'<div class="ck ck-widget__selection-handle"></div>' +
+						'<table>' +
+							'<tbody>' +
+								'<tr>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">00</span>' +
+									'</td>' +
+								'</tr>' +
+							'</tbody>' +
+						'</table>' +
+					'</figure>'
+				);
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '', '' ],
-				[ '00', '01' ],
-				[ '10', '11' ]
-			], { headingRows: 2 } ) );
-		} );
+			it( 'should work with adding table rows at the beginning of a table', () => {
+				setModelData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ]
+				], { headingRows: 1 } ) );
 
-		it( 'should work with adding a table row and expanding heading', () => {
-			setModelData( model, modelTable( [
-				[ '00', '01' ],
-				[ '10', '11' ],
-				[ '20', '21' ]
-			], { headingRows: 1 } ) );
+				const table = root.getChild( 0 );
 
-			const table = root.getChild( 0 );
+				model.change( writer => {
+					writer.setAttribute( 'headingRows', 2, table );
 
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 2, table );
+					const tableRow = writer.createElement( 'tableRow' );
 
-				const tableRow = writer.createElement( 'tableRow' );
+					writer.insert( tableRow, table, 0 );
+					writer.insertElement( 'tableCell', tableRow, 'end' );
+					writer.insertElement( 'tableCell', tableRow, 'end' );
+				} );
 
-				writer.insert( tableRow, table, 1 );
-				writer.insertElement( 'tableCell', tableRow, 'end' );
-				writer.insertElement( 'tableCell', tableRow, 'end' );
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
+					[ '', '' ],
+					[ '00', '01' ],
+					[ '10', '11' ]
+				], { headingRows: 2, asWidget: true } ) );
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '00', '01' ],
-				[ '', '' ],
-				[ '10', '11' ],
-				[ '20', '21' ]
-			], { headingRows: 2 } ) );
-		} );
+			it( 'should work with adding a table row and expanding heading', () => {
+				setModelData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				], { headingRows: 1 } ) );
 
-		describe( 'options.asWidget=true', () => {
-			beforeEach( () => {
-				return VirtualTestEditor.create()
-					.then( newEditor => {
-						editor = newEditor;
-						model = editor.model;
-						doc = model.document;
-						root = doc.getRoot( 'main' );
-						view = editor.editing.view;
+				const table = root.getChild( 0 );
 
-						defaultSchema( model.schema );
-						defaultConversion( editor.conversion, true );
-					} );
+				model.change( writer => {
+					writer.setAttribute( 'headingRows', 2, table );
+
+					const tableRow = writer.createElement( 'tableRow' );
+
+					writer.insert( tableRow, table, 1 );
+					writer.insertElement( 'tableCell', tableRow, 'end' );
+					writer.insertElement( 'tableCell', tableRow, 'end' );
+				} );
+
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
+					[ '00', '01' ],
+					[ '', '' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				], { headingRows: 2, asWidget: true } ) );
 			} );
 
 			it( 'should create renamed cell as a widget', () => {
@@ -1126,107 +1194,257 @@ describe( 'downcast converters', () => {
 	} );
 
 	describe( 'downcastRemoveRow()', () => {
-		it( 'should react to removed row from the beginning of a tbody', () => {
-			setModelData( model, modelTable( [
-				[ '00[]', '01' ],
-				[ '10', '11' ]
-			] ) );
+		// The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing.
+		beforeEach( async () => {
+			// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
+			testUtils.sinon.stub( env, 'isEdge' ).get( () => false );
 
-			const table = root.getChild( 0 );
+			editor = await VirtualTestEditor.create( { plugins: [ Paragraph, TableEditing ] } );
 
-			model.change( writer => {
-				writer.remove( table.getChild( 1 ) );
+			model = editor.model;
+			root = model.document.getRoot( 'main' );
+			view = editor.editing.view;
+		} );
+
+		// The remove row downcast conversion is not executed in data pipeline.
+		describe( 'editing pipeline', () => {
+			it( 'should react to removed row from the beginning of a body rows (no heading rows)', () => {
+				setModelData( model, modelTable( [
+					[ '00[]', '01' ],
+					[ '10', '11' ]
+				] ) );
+
+				const table = root.getChild( 0 );
+
+				model.change( writer => {
+					writer.remove( table.getChild( 1 ) );
+				} );
+
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ),
+					'<figure class="ck-widget ck-widget_with-selection-handle table" contenteditable="false">' +
+						'<div class="ck ck-widget__selection-handle"></div>' +
+						'<table>' +
+							'<tbody>' +
+								'<tr>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">00</span>' +
+									'</td>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">01</span>' +
+									'</td>' +
+								'</tr>' +
+							'</tbody>' +
+						'</table>' +
+					'</figure>'
+				);
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '00', '01' ]
-			] ) );
-		} );
+			it( 'should react to removed row form the end of a body rows (no heading rows)', () => {
+				setModelData( model, modelTable( [
+					[ '00[]', '01' ],
+					[ '10', '11' ]
+				] ) );
 
-		it( 'should react to removed row form the end of a tbody', () => {
-			setModelData( model, modelTable( [
-				[ '00[]', '01' ],
-				[ '10', '11' ]
-			] ) );
+				const table = root.getChild( 0 );
 
-			const table = root.getChild( 0 );
+				model.change( writer => {
+					writer.remove( table.getChild( 0 ) );
+				} );
 
-			model.change( writer => {
-				writer.remove( table.getChild( 0 ) );
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ),
+					'<figure class="ck-widget ck-widget_with-selection-handle table" contenteditable="false">' +
+						'<div class="ck ck-widget__selection-handle"></div>' +
+						'<table>' +
+							'<tbody>' +
+								'<tr>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">10</span>' +
+									'</td>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">11</span>' +
+									'</td>' +
+								'</tr>' +
+							'</tbody>' +
+						'</table>' +
+					'</figure>'
+				);
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '10', '11' ]
-			] ) );
-		} );
+			it( 'should react to removed row from the beginning of a heading rows (no body rows)', () => {
+				setModelData( model, modelTable( [
+					[ '00[]', '01' ],
+					[ '10', '11' ]
+				], { headingRows: 2 } ) );
 
-		it( 'should react to removed row from the beginning of a thead', () => {
-			setModelData( model, modelTable( [
-				[ '00[]', '01' ],
-				[ '10', '11' ]
-			], { headingRows: 2 } ) );
+				const table = root.getChild( 0 );
 
-			const table = root.getChild( 0 );
+				model.change( writer => {
+					// Removing row from a heading section changes requires changing heading rows attribute.
+					writer.setAttribute( 'headingRows', 1, table );
+					writer.remove( table.getChild( 0 ) );
+				} );
 
-			model.change( writer => {
-				writer.remove( table.getChild( 1 ) );
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ),
+					'<figure class="ck-widget ck-widget_with-selection-handle table" contenteditable="false">' +
+						'<div class="ck ck-widget__selection-handle"></div>' +
+						'<table>' +
+							'<thead>' +
+								'<tr>' +
+									'<th class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">10</span>' +
+									'</th>' +
+									'<th class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">11</span>' +
+									'</th>' +
+								'</tr>' +
+							'</thead>' +
+						'</table>' +
+					'</figure>'
+				);
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '00', '01' ]
-			], { headingRows: 2 } ) );
-		} );
+			it( 'should react to removed row form the end of a heading rows (no body rows)', () => {
+				setModelData( model, modelTable( [
+					[ '00[]', '01' ],
+					[ '10', '11' ]
+				], { headingRows: 2 } ) );
 
-		it( 'should react to removed row form the end of a thead', () => {
-			setModelData( model, modelTable( [
-				[ '00[]', '01' ],
-				[ '10', '11' ]
-			], { headingRows: 2 } ) );
+				const table = root.getChild( 0 );
 
-			const table = root.getChild( 0 );
+				model.change( writer => {
+					// Removing row from a heading section changes requires changing heading rows attribute.
+					writer.setAttribute( 'headingRows', 1, table );
+					writer.remove( table.getChild( 1 ) );
+				} );
 
-			model.change( writer => {
-				writer.remove( table.getChild( 0 ) );
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ),
+					'<figure class="ck-widget ck-widget_with-selection-handle table" contenteditable="false">' +
+						'<div class="ck ck-widget__selection-handle"></div>' +
+						'<table>' +
+							'<thead>' +
+								'<tr>' +
+									'<th class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">00</span>' +
+									'</th>' +
+									'<th class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">01</span>' +
+									'</th>' +
+								'</tr>' +
+							'</thead>' +
+						'</table>' +
+					'</figure>'
+				);
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '10', '11' ]
-			], { headingRows: 2 } ) );
-		} );
+			it( 'should react to removed row form the end of a heading rows (first cell in body has colspan)', () => {
+				setModelData( model, modelTable( [
+					[ '00[]', '01', '02', '03' ],
+					[ { rowspan: 2, colspan: 2, contents: '10' }, '12', '13' ],
+					[ '22', '23' ]
+				], { headingRows: 1 } ) );
 
-		it( 'should remove empty thead section if a last row was removed from thead', () => {
-			setModelData( model, modelTable( [
-				[ '00[]', '01' ],
-				[ '10', '11' ]
-			], { headingRows: 1 } ) );
+				const table = root.getChild( 0 );
 
-			const table = root.getChild( 0 );
+				model.change( writer => {
+					// Removing row from a heading section changes requires changing heading rows attribute.
+					writer.remove( table.getChild( 0 ) );
+					writer.setAttribute( 'headingRows', 0, table );
+				} );
 
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 0, table );
-				writer.remove( table.getChild( 0 ) );
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ),
+					'<figure class="ck-widget ck-widget_with-selection-handle table" contenteditable="false">' +
+					'<div class="ck ck-widget__selection-handle"></div>' +
+						'<table>' +
+							'<tbody>' +
+								'<tr>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" ' +
+										'colspan="2" contenteditable="true" rowspan="2">' +
+										'<span style="display:inline-block">10</span>' +
+									'</td>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">12</span>' +
+									'</td>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">13</span>' +
+									'</td>' +
+								'</tr>' +
+								'<tr>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">22</span>' +
+									'</td>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">23</span>' +
+									'</td>' +
+								'</tr>' +
+							'</tbody>' +
+						'</table>' +
+					'</figure>'
+				);
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '10', '11' ]
-			] ) );
-		} );
+			it( 'should remove empty thead if a last row was removed from a heading rows (has heading and body)', () => {
+				setModelData( model, modelTable( [
+					[ '00[]', '01' ],
+					[ '10', '11' ]
+				], { headingRows: 1 } ) );
 
-		it( 'should remove empty tbody section if a last row was removed from tbody', () => {
-			setModelData( model, modelTable( [
-				[ '00[]', '01' ],
-				[ '10', '11' ]
-			], { headingRows: 1 } ) );
+				const table = root.getChild( 0 );
 
-			const table = root.getChild( 0 );
+				model.change( writer => {
+					// Removing row from a heading section changes requires changing heading rows attribute.
+					writer.removeAttribute( 'headingRows', table );
+					writer.remove( table.getChild( 0 ) );
+				} );
 
-			model.change( writer => {
-				writer.remove( table.getChild( 1 ) );
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ),
+					'<figure class="ck-widget ck-widget_with-selection-handle table" contenteditable="false">' +
+						'<div class="ck ck-widget__selection-handle"></div>' +
+						'<table>' +
+							'<tbody>' +
+								'<tr>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">10</span>' +
+									'</td>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">11</span>' +
+									'</td>' +
+								'</tr>' +
+							'</tbody>' +
+						'</table>' +
+					'</figure>'
+				);
 			} );
 
-			assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-				[ '00', '01' ]
-			], { headingRows: 1 } ) );
+			it( 'should remove empty tbody if a last row was removed a body rows (has heading and body)', () => {
+				setModelData( model, modelTable( [
+					[ '00[]', '01' ],
+					[ '10', '11' ]
+				], { headingRows: 1 } ) );
+
+				const table = root.getChild( 0 );
+
+				model.change( writer => {
+					writer.remove( table.getChild( 1 ) );
+				} );
+
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ),
+					'<figure class="ck-widget ck-widget_with-selection-handle table" contenteditable="false">' +
+						'<div class="ck ck-widget__selection-handle"></div>' +
+						'<table>' +
+							'<thead>' +
+								'<tr>' +
+									'<th class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">00</span>' +
+									'</th>' +
+									'<th class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block">01</span>' +
+									'</th>' +
+								'</tr>' +
+							'</thead>' +
+						'</table>' +
+					'</figure>'
+				);
+			} );
 		} );
 	} );
 

+ 4 - 2
packages/ckeditor5-table/tests/tableutils.js

@@ -1174,7 +1174,7 @@ describe( 'TableUtils', () => {
 				] ) );
 			} );
 
-			it( 'should create one undo step (1 batch)', () => {
+			it( 'should re-use batch to create one undo step', () => {
 				setData( model, modelTable( [
 					[ '00', '01' ],
 					[ '10', '11' ],
@@ -1190,7 +1190,9 @@ describe( 'TableUtils', () => {
 					createdBatches.add( operation.batch );
 				} );
 
-				tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } );
+				const batch = model.createBatch();
+
+				tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2, batch } );
 
 				expect( createdBatches.size ).to.equal( 1 );
 			} );