Explorar el Código

Merge branch 'master' into i/7542

Aleksander Nowodzinski hace 5 años
padre
commit
77addc9ecd

+ 3 - 3
docs/framework/guides/contributing/git-commit-message-convention.md

@@ -24,8 +24,8 @@ Type (another-package-name): If the change affects more than one package, it's p
 
 Optional description.
 
-BREAKING CHANGE (scope): If any breaking changes were done, they need to be listed here.
-BREAKING CHANGE (scope): Another breaking change if needed. Closes #ZZZ.
+BREAKING CHANGE (package-name): If any breaking changes were done, they need to be listed here.
+BREAKING CHANGE (package-name): Another breaking change if needed. Closes #ZZZ.
 ```
 
 ### Commit types
@@ -50,7 +50,7 @@ If any change contains the `MAJOR BREAKING CHANGE` note, the next release will b
 
 For reference on how to identify minor or major breaking change see the {@link framework/guides/support/versioning-policy versioning policy guide}.
 
-Each `BREAKING CHANGE` note must be followed by the scope of changes.
+Each `BREAKING CHANGE` note must be followed by the package name.
 
 ### Package name
 

+ 1 - 1
packages/ckeditor5-core/tests/_utils-tests/assertions/attribute.js

@@ -54,7 +54,7 @@ describe( 'attribute chai assertion', () => {
 				hasAttribute: () => true,
 				getAttribute: () => 'bar'
 			} ).to.have.attribute( 'foo', 'baz' );
-		} ).to.throw( 'expected { Object (hasAttribute, getAttribute) } to have attribute \'foo\' of \'bar\', but got \'baz\'' );
+		} ).to.throw( 'expected { Object (hasAttribute, getAttribute) } to have attribute \'foo\' of \'baz\', but got \'bar\'' );
 	} );
 
 	it( 'negated, should assert for the given type the \'target.getAttribute\' returns a value different than the given one', () => {

+ 2 - 2
packages/ckeditor5-core/tests/_utils/assertions/attribute.js

@@ -54,8 +54,8 @@ chai.Assertion.addMethod( 'attribute', function attributeAssertion( key, value,
 			attributeValue === value,
 			`expected #{this} to have attribute '${ key }' of #{exp}, but got #{act}`,
 			`expected #{this} to not have attribute '${ key }' of #{exp}`,
-			attributeValue,
-			value
+			value,
+			attributeValue
 		);
 	}
 } );

+ 1 - 1
packages/ckeditor5-engine/src/view/domconverter.js

@@ -82,7 +82,7 @@ export default class DomConverter {
 		 * @readonly
 		 * @member {Array.<String>} module:engine/view/domconverter~DomConverter#blockElements
 		 */
-		this.blockElements = [ 'p', 'div', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'li', 'dd', 'dt', 'figcaption' ];
+		this.blockElements = [ 'p', 'div', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'li', 'dd', 'dt', 'figcaption', 'td', 'th' ];
 
 		/**
 		 * Block {@link module:engine/view/filler filler} creator, which is used to create all block fillers during the

+ 1 - 1
packages/ckeditor5-table/src/commands/mergecellcommand.js

@@ -109,7 +109,7 @@ export default class MergeCellCommand extends Command {
 			const table = findAncestor( 'table', removedTableCellRow );
 
 			// Remove empty rows and columns after merging.
-			removeEmptyRowsColumns( table, tableUtils, writer.batch );
+			removeEmptyRowsColumns( table, tableUtils );
 		} );
 	}
 

+ 1 - 1
packages/ckeditor5-table/src/commands/mergecellscommand.js

@@ -60,7 +60,7 @@ export default class MergeCellsCommand extends Command {
 			const table = findAncestor( 'table', firstTableCell );
 
 			// Remove rows and columns that become empty (have no anchored cells).
-			removeEmptyRowsColumns( table, tableUtils, writer.batch );
+			removeEmptyRowsColumns( table, tableUtils );
 
 			writer.setSelection( firstTableCell, 'in' );
 		} );

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

@@ -60,20 +60,14 @@ export default class RemoveRowCommand extends Command {
 
 		const columnIndexToFocus = this.editor.plugins.get( 'TableUtils' ).getCellLocation( firstCell ).column;
 
-		// Use single batch to modify table in steps but in one undo step.
-		const batch = model.createBatch();
-
-		model.enqueueChange( batch, () => {
+		model.change( writer => {
 			const rowsToRemove = removedRowIndexes.last - removedRowIndexes.first + 1;
 
 			this.editor.plugins.get( 'TableUtils' ).removeRows( table, {
 				at: removedRowIndexes.first,
-				rows: rowsToRemove,
-				batch
+				rows: rowsToRemove
 			} );
-		} );
 
-		model.enqueueChange( batch, writer => {
 			const cellToFocus = getCellToFocus( table, removedRowIndexes.first, columnIndexToFocus );
 
 			writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );

+ 0 - 100
packages/ckeditor5-table/src/converters/downcast.js

@@ -174,79 +174,6 @@ export function downcastInsertCell() {
 }
 
 /**
- * Conversion helper that acts on heading row table attribute change.
- *
- * This converter will:
- *
- * * Rename `<td>` to `<th>` elements or vice versa depending on headings.
- * * Create `<thead>` or `<tbody>` elements if needed.
- * * Remove empty `<thead>` or `<tbody>` if needed.
- *
- * @returns {Function} Conversion helper.
- */
-export function downcastTableHeadingRowsChange() {
-	return dispatcher => dispatcher.on( 'attribute:headingRows:table', ( evt, data, conversionApi ) => {
-		const table = data.item;
-
-		if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
-			return;
-		}
-
-		const figureElement = conversionApi.mapper.toViewElement( table );
-		const viewTable = getViewTable( figureElement );
-
-		const oldRows = data.attributeOldValue;
-		const newRows = data.attributeNewValue;
-
-		// The head section has grown so move rows from <tbody> to <thead>.
-		if ( newRows > oldRows ) {
-			// Filter out only those rows that are in wrong section.
-			const rowsToMove = Array.from( table.getChildren() ).filter( ( { index } ) => isBetween( index, oldRows - 1, newRows ) );
-
-			const viewTableHead = getOrCreateTableSection( 'thead', viewTable, conversionApi );
-			moveViewRowsToTableSection( rowsToMove, viewTableHead, conversionApi, 'end' );
-
-			// Rename all table cells from moved rows to 'th' as they lands in <thead>.
-			for ( const tableRow of rowsToMove ) {
-				for ( const tableCell of tableRow.getChildren() ) {
-					renameViewTableCell( tableCell, 'th', conversionApi );
-				}
-			}
-		}
-		// The head section has shrunk so move rows from <thead> to <tbody>.
-		else {
-			// Filter out only those rows that are in wrong section.
-			const rowsToMove = Array.from( table.getChildren() )
-				.filter( ( { index } ) => isBetween( index, newRows - 1, oldRows ) )
-				.reverse(); // The rows will be moved from <thead> to <tbody> in reverse order at the beginning of a <tbody>.
-
-			const viewTableBody = getOrCreateTableSection( 'tbody', viewTable, conversionApi );
-			moveViewRowsToTableSection( rowsToMove, viewTableBody, conversionApi, 0 );
-
-			// Check if cells moved from <thead> to <tbody> requires renaming to <td> as this depends on current heading columns attribute.
-			const tableWalker = new TableWalker( table, { startRow: newRows ? newRows - 1 : newRows, endRow: oldRows - 1 } );
-
-			const tableAttributes = {
-				headingRows: table.getAttribute( 'headingRows' ) || 0,
-				headingColumns: table.getAttribute( 'headingColumns' ) || 0
-			};
-
-			for ( const tableSlot of tableWalker ) {
-				renameViewTableCellIfRequired( tableSlot, tableAttributes, 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;
-		}
-	} );
-}
-
-/**
  * Conversion helper that acts on heading column table attribute change.
  *
  * Depending on changed attributes this converter will rename `<td` to `<th>` elements or vice versa depending on the cell column index.
@@ -333,11 +260,6 @@ function renameViewTableCell( tableCell, desiredCellElementName, conversionApi )
 	const viewWriter = conversionApi.writer;
 	const viewCell = conversionApi.mapper.toViewElement( tableCell );
 
-	// View cell might be not yet converted - skip it as it will be properly created by cell converter later on.
-	if ( !viewCell ) {
-		return;
-	}
-
 	const editable = viewWriter.createEditableElement( desiredCellElementName, viewCell.getAttributes() );
 	const renamedCell = toWidgetEditable( editable, viewWriter );
 
@@ -545,28 +467,6 @@ function removeTableSectionIfEmpty( sectionName, tableElement, conversionApi ) {
 	}
 }
 
-// Moves view table rows associated with passed model rows to the provided table section element.
-//
-// **Note**: This method will skip not converted table rows.
-//
-// @param {Array.<module:engine/model/element~Element>} rowsToMove
-// @param {module:engine/view/element~Element} viewTableSection
-// @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi
-// @param {Number|'end'|'before'|'after'} offset Offset or one of the flags.
-function moveViewRowsToTableSection( rowsToMove, viewTableSection, conversionApi, offset ) {
-	for ( const tableRow of rowsToMove ) {
-		const viewTableRow = conversionApi.mapper.toViewElement( tableRow );
-
-		// View table row might be not yet converted - skip it as it will be properly created by cell converter later on.
-		if ( viewTableRow ) {
-			conversionApi.writer.move(
-				conversionApi.writer.createRangeOn( viewTableRow ),
-				conversionApi.writer.createPositionAt( viewTableSection, offset )
-			);
-		}
-	}
-}
-
 // Finds a '<table>' element inside the `<figure>` widget.
 //
 // @param {module:engine/view/element~Element} viewFigure

+ 54 - 0
packages/ckeditor5-table/src/converters/table-heading-rows-refresh-post-fixer.js

@@ -0,0 +1,54 @@
+/**
+ * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+/**
+ * @module table/converters/table-heading-rows-refresh-post-fixer
+ */
+
+/**
+ * Injects a table post-fixer into the model which marks the table in the differ to have it re-rendered.
+ *
+ * Table heading rows are represented in the model by a `headingRows` attribute. However, in the view, it's represented as separate
+ * sections of the table (`<thead>` or `<tbody>`) and changing `headingRows` attribute requires moving table rows between two sections.
+ * This causes problems with structural changes in a table (like adding and removing rows) thus atomic converters cannot be used.
+ *
+ * When table `headingRows` attribute changes, the entire table is re-rendered.
+ *
+ * @param {module:engine/model/model~Model} model
+ */
+export default function injectTableHeadingRowsRefreshPostFixer( model ) {
+	model.document.registerPostFixer( () => tableHeadingRowsRefreshPostFixer( model ) );
+}
+
+function tableHeadingRowsRefreshPostFixer( model ) {
+	const differ = model.document.differ;
+
+	// Stores tables to be refreshed so the table will be refreshed once for multiple changes.
+	const tablesToRefresh = new Set();
+
+	for ( const change of differ.getChanges() ) {
+		if ( change.type != 'attribute' ) {
+			continue;
+		}
+
+		const element = change.range.start.nodeAfter;
+
+		if ( element && element.is( 'table' ) && change.attributeKey == 'headingRows' ) {
+			tablesToRefresh.add( element );
+		}
+	}
+
+	if ( tablesToRefresh.size ) {
+		// @if CK_DEBUG_TABLE // console.log( `Post-fixing table: refreshing heading rows (${ tablesToRefresh.size }).` );
+
+		for ( const table of tablesToRefresh.values() ) {
+			differ.refreshItem( table );
+		}
+
+		return true;
+	}
+
+	return false;
+}

+ 2 - 4
packages/ckeditor5-table/src/tableclipboard.js

@@ -205,7 +205,7 @@ function prepareTableForPasting( selectedTableCells, pastedDimensions, writer, t
 		selection.lastRow += pastedDimensions.height - 1;
 		selection.lastColumn += pastedDimensions.width - 1;
 
-		expandTableSize( selectedTable, selection.lastRow + 1, selection.lastColumn + 1, writer, tableUtils );
+		expandTableSize( selectedTable, selection.lastRow + 1, selection.lastColumn + 1, tableUtils );
 	}
 
 	// In case of expanding selection we do not reset the selection so in this case we will always try to fix selection
@@ -320,13 +320,12 @@ function replaceSelectedCellsWithPasted( pastedTable, pastedDimensions, selected
 }
 
 // Expand table (in place) to expected size.
-function expandTableSize( table, expectedHeight, expectedWidth, writer, tableUtils ) {
+function expandTableSize( table, expectedHeight, expectedWidth, tableUtils ) {
 	const tableWidth = tableUtils.getColumns( table );
 	const tableHeight = tableUtils.getRows( table );
 
 	if ( expectedWidth > tableWidth ) {
 		tableUtils.insertColumns( table, {
-			batch: writer.batch,
 			at: tableWidth,
 			columns: expectedWidth - tableWidth
 		} );
@@ -334,7 +333,6 @@ function expandTableSize( table, expectedHeight, expectedWidth, writer, tableUti
 
 	if ( expectedHeight > tableHeight ) {
 		tableUtils.insertRows( table, {
-			batch: writer.batch,
 			at: tableHeight,
 			rows: expectedHeight - tableHeight
 		} );

+ 4 - 4
packages/ckeditor5-table/src/tableediting.js

@@ -15,8 +15,7 @@ import {
 	downcastInsertRow,
 	downcastInsertTable,
 	downcastRemoveRow,
-	downcastTableHeadingColumnsChange,
-	downcastTableHeadingRowsChange
+	downcastTableHeadingColumnsChange
 } from './converters/downcast';
 
 import InsertTableCommand from './commands/inserttablecommand';
@@ -36,6 +35,7 @@ import TableUtils from '../src/tableutils';
 import injectTableLayoutPostFixer from './converters/table-layout-post-fixer';
 import injectTableCellParagraphPostFixer from './converters/table-cell-paragraph-post-fixer';
 import injectTableCellRefreshPostFixer from './converters/table-cell-refresh-post-fixer';
+import injectTableHeadingRowsRefreshPostFixer from './converters/table-heading-rows-refresh-post-fixer';
 
 import '../theme/tableediting.css';
 
@@ -113,9 +113,8 @@ export default class TableEditing extends Plugin {
 		conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 		conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
 
-		// Table heading rows and columns conversion.
+		// Table heading columns conversion (change of heading rows requires reconversion of the whole table).
 		conversion.for( 'editingDowncast' ).add( downcastTableHeadingColumnsChange() );
-		conversion.for( 'editingDowncast' ).add( downcastTableHeadingRowsChange() );
 
 		// Define all the commands.
 		editor.commands.add( 'insertTable', new InsertTableCommand( editor ) );
@@ -143,6 +142,7 @@ export default class TableEditing extends Plugin {
 		editor.commands.add( 'selectTableRow', new SelectRowCommand( editor ) );
 		editor.commands.add( 'selectTableColumn', new SelectColumnCommand( editor ) );
 
+		injectTableHeadingRowsRefreshPostFixer( model );
 		injectTableLayoutPostFixer( model );
 		injectTableCellRefreshPostFixer( model );
 		injectTableCellParagraphPostFixer( model );

+ 23 - 23
packages/ckeditor5-table/src/tableutils.js

@@ -136,7 +136,7 @@ export default class TableUtils extends Plugin {
 
 			// Inserting rows inside heading section requires to update `headingRows` attribute as the heading section will grow.
 			if ( headingRows > insertAt ) {
-				writer.setAttribute( 'headingRows', headingRows + rowsToInsert, table );
+				updateNumericAttribute( 'headingRows', headingRows + rowsToInsert, table, writer, 0 );
 			}
 
 			// Inserting at the end or at the beginning of a table doesn't require to calculate anything special.
@@ -309,9 +309,8 @@ 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';
 
-		model.enqueueChange( batch, writer => {
+		model.change( writer => {
 			// Removing rows from the table require that most calculations to be done prior to changing table structure.
 			// Preparations must be done in the same enqueueChange callback to use the current table structure.
 
@@ -337,11 +336,15 @@ export default class TableUtils extends Plugin {
 				updateNumericAttribute( 'rowspan', rowspan, cell, writer );
 			}
 
-			// 2d. Remove empty columns (without anchored cells) if there are any.
-			removeEmptyColumns( table, this );
+			// 2d. Adjust heading rows if removed rows were in a heading section.
+			updateHeadingRows( table, first, last, writer );
 
-			// 2e. Adjust heading rows if removed rows were in a heading section.
-			updateHeadingRows( table, first, last, model, batch );
+			// 2e. Remove empty columns (without anchored cells) if there are any.
+			if ( !removeEmptyColumns( table, this ) ) {
+				// If there wasn't any empty columns then we still need to check if this wasn't called
+				// because of cleaning empty rows and we only removed one of them.
+				removeEmptyRows( table, this );
+			}
 		} );
 	}
 
@@ -396,7 +399,11 @@ export default class TableUtils extends Plugin {
 			}
 
 			// Remove empty rows that could appear after removing columns.
-			removeEmptyRows( table, this, writer.batch );
+			if ( !removeEmptyRows( table, this ) ) {
+				// If there wasn't any empty rows then we still need to check if this wasn't called
+				// because of cleaning empty columns and we only removed one of them.
+				removeEmptyColumns( table, this );
+			}
 		} );
 	}
 
@@ -776,21 +783,14 @@ function adjustHeadingColumns( table, removedColumnIndexes, writer ) {
 }
 
 // Calculates a new heading rows value for removing rows from heading section.
-function updateHeadingRows( table, first, last, model, batch ) {
-	// Must be done after the changes in table structure (removing rows).
-	// Otherwise the downcast converter for headingRows attribute will fail.
-	// See https://github.com/ckeditor/ckeditor5/issues/6391.
-	//
-	// Must be completely wrapped in enqueueChange to get the current table state (after applying other enqueued changes).
-	model.enqueueChange( batch, writer => {
-		const headingRows = table.getAttribute( 'headingRows' ) || 0;
-
-		if ( first < headingRows ) {
-			const newRows = last < headingRows ? headingRows - ( last - first + 1 ) : first;
-
-			updateNumericAttribute( 'headingRows', newRows, table, writer, 0 );
-		}
-	} );
+function updateHeadingRows( table, first, last, writer ) {
+	const headingRows = table.getAttribute( 'headingRows' ) || 0;
+
+	if ( first < headingRows ) {
+		const newRows = last < headingRows ? headingRows - ( last - first + 1 ) : first;
+
+		updateNumericAttribute( 'headingRows', newRows, table, writer, 0 );
+	}
 }
 
 // Finds cells that will be:

+ 21 - 15
packages/ckeditor5-table/src/utils/structure.js

@@ -342,13 +342,17 @@ export function removeEmptyColumns( table, tableUtils ) {
 		return cellsCount ? result : [ ...result, column ];
 	}, [] );
 
-	// @if CK_DEBUG_TABLE // emptyColumns.length > 0 && console.log( `Removing empty columns: ${ emptyColumns.join( ', ' ) }.` );
+	if ( emptyColumns.length > 0 ) {
+		// Remove only last empty column because it will recurrently trigger removing empty rows.
+		const emptyColumn = emptyColumns[ emptyColumns.length - 1 ];
 
-	emptyColumns.reverse().forEach( column => {
-		tableUtils.removeColumns( table, { at: column } );
-	} );
+		// @if CK_DEBUG_TABLE // console.log( `Removing empty column: ${ emptyColumn }.` );
+		tableUtils.removeColumns( table, { at: emptyColumn } );
 
-	return emptyColumns.length > 0;
+		return true;
+	}
+
+	return false;
 }
 
 /**
@@ -380,10 +384,9 @@ export function removeEmptyColumns( table, tableUtils ) {
  * @protected
  * @param {module:engine/model/element~Element} table
  * @param {module:table/tableutils~TableUtils} tableUtils
- * @param {module:engine/model/batch~Batch|null} [batch] Batch that should be used for removing empty rows.
  * @returns {Boolean} True if removed some rows.
  */
-export function removeEmptyRows( table, tableUtils, batch ) {
+export function removeEmptyRows( table, tableUtils ) {
 	const emptyRows = [];
 
 	for ( let rowIndex = 0; rowIndex < table.childCount; rowIndex++ ) {
@@ -394,13 +397,17 @@ export function removeEmptyRows( table, tableUtils, batch ) {
 		}
 	}
 
-	// @if CK_DEBUG_TABLE // emptyRows.length > 0 && console.log( `Removing empty rows: ${ emptyRows.join( ', ' ) }.` );
+	if ( emptyRows.length > 0 ) {
+		// Remove only last empty row because it will recurrently trigger removing empty columns.
+		const emptyRow = emptyRows[ emptyRows.length - 1 ];
 
-	emptyRows.reverse().forEach( row => {
-		tableUtils.removeRows( table, { at: row, batch } );
-	} );
+		// @if CK_DEBUG_TABLE // console.log( `Removing empty row: ${ emptyRow }.` );
+		tableUtils.removeRows( table, { at: emptyRow } );
+
+		return true;
+	}
 
-	return emptyRows.length > 0;
+	return false;
 }
 
 /**
@@ -428,14 +435,13 @@ export function removeEmptyRows( table, tableUtils, batch ) {
  * @protected
  * @param {module:engine/model/element~Element} table
  * @param {module:table/tableutils~TableUtils} tableUtils
- * @param {module:engine/model/batch~Batch|null} [batch] Batch that should be used for removing empty rows.
  */
-export function removeEmptyRowsColumns( table, tableUtils, batch ) {
+export function removeEmptyRowsColumns( table, tableUtils ) {
 	const removedColumns = removeEmptyColumns( table, tableUtils );
 
 	// If there was some columns removed then cleaning empty rows was already triggered.
 	if ( !removedColumns ) {
-		removeEmptyRows( table, tableUtils, batch );
+		removeEmptyRows( table, tableUtils );
 	}
 }
 

+ 47 - 0
packages/ckeditor5-table/tests/commands/mergecellscommand.js

@@ -706,6 +706,53 @@ describe( 'MergeCellsCommand', () => {
 					]
 				] ) );
 			} );
+
+			it( 'should remove all empty rows and columns', () => {
+				setData( model, modelTable( [
+					[ '00', '01', '02' ],
+					[ '10', '11', '12' ],
+					[ '20', '21', '22' ]
+				] ) );
+
+				tableSelection.setCellSelection(
+					root.getNodeByPath( [ 0, 0, 0 ] ),
+					root.getNodeByPath( [ 0, 2, 2 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[
+						'<paragraph>[00</paragraph><paragraph>01</paragraph><paragraph>02</paragraph>' +
+						'<paragraph>10</paragraph><paragraph>11</paragraph><paragraph>12</paragraph>' +
+						'<paragraph>20</paragraph><paragraph>21</paragraph><paragraph>22]</paragraph>'
+					]
+				] ) );
+			} );
+
+			it( 'should remove all empty rows and columns (asymmetrical case)', () => {
+				setData( model, modelTable( [
+					[ '00', '01', { contents: '02', rowspan: 3 } ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				] ) );
+
+				tableSelection.setCellSelection(
+					root.getNodeByPath( [ 0, 0, 0 ] ),
+					root.getNodeByPath( [ 0, 2, 1 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[
+						'<paragraph>[00</paragraph><paragraph>01</paragraph>' +
+						'<paragraph>10</paragraph><paragraph>11</paragraph>' +
+						'<paragraph>20</paragraph><paragraph>21]</paragraph>',
+						'02'
+					]
+				] ) );
+			} );
 		} );
 	} );
 

+ 33 - 32
packages/ckeditor5-table/tests/converters/downcast.js

@@ -5,6 +5,7 @@
 
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
 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';
@@ -19,7 +20,7 @@ describe( 'downcast converters', () => {
 	testUtils.createSinonSandbox();
 
 	beforeEach( async () => {
-		editor = await VirtualTestEditor.create( { plugins: [ Paragraph, TableEditing ] } );
+		editor = await VirtualTestEditor.create( { plugins: [ Paragraph, TableEditing, UndoEditing ] } );
 
 		model = editor.model;
 		root = model.document.getRoot( 'main' );
@@ -840,8 +841,10 @@ describe( 'downcast converters', () => {
 		} );
 	} );
 
-	describe( 'downcastTableHeadingRowsChange()', () => {
+	describe( 'downcastTableHeadingRowsChange', () => {
 		// The heading rows change downcast conversion is not executed in data pipeline.
+		// Note that headingRows table attribute triggers whole table downcast.
+
 		describe( 'editing pipeline', () => {
 			it( 'should work for adding heading rows', () => {
 				setModelData( model, modelTable( [
@@ -941,36 +944,6 @@ describe( 'downcast converters', () => {
 				], { headingRows: 2, asWidget: true } ) );
 			} );
 
-			it( 'should be possible to overwrite', () => {
-				editor.conversion.attributeToAttribute( {
-					model: 'headingRows',
-					view: 'headingRows',
-					converterPriority: 'high'
-				} );
-				setModelData( model, modelTable( [ [ '00' ] ] ) );
-
-				const table = root.getChild( 0 );
-
-				model.change( writer => {
-					writer.setAttribute( 'headingRows', 1, table );
-				} );
-
-				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>'
-				);
-			} );
-
 			it( 'should work with adding table rows at the beginning of a table', () => {
 				setModelData( model, modelTable( [
 					[ '00', '01' ],
@@ -1047,6 +1020,34 @@ describe( 'downcast converters', () => {
 					'</figure>'
 				);
 			} );
+
+			it( 'should properly integrate with undo', () => {
+				setModelData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				], { headingRows: 1 } ) );
+
+				const table = root.getChild( 0 );
+
+				model.change( writer => {
+					writer.setAttribute( 'headingRows', 2, table );
+				} );
+
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				], { headingRows: 2, asWidget: true } ) );
+
+				editor.execute( 'undo' );
+
+				assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				], { headingRows: 1, asWidget: true } ) );
+			} );
 		} );
 	} );
 

+ 28 - 2
packages/ckeditor5-table/tests/tableclipboard-paste.js

@@ -3553,6 +3553,32 @@ describe( 'table clipboard', () => {
 				[ '02', '21', '22' ]
 			] ) );
 		} );
+
+		it( 'removes block fillers from empty cells (both td and th)', async () => {
+			await createEditor();
+
+			setModelData( model, modelTable( [
+				[ '00', '01', '02' ],
+				[ '01', '11', '12' ],
+				[ '02', '21', '22' ]
+			] ) );
+
+			tableSelection.setCellSelection(
+				modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+				modelRoot.getNodeByPath( [ 0, 1, 1 ] )
+			);
+
+			pasteTable( [
+				[ '&nbsp;', '&nbsp;' ],
+				[ '&nbsp;', '&nbsp;' ]
+			], { headingRows: 1 } );
+
+			assertEqualMarkup( getModelData( model, { withoutSelection: true } ), modelTable( [
+				[ '', '', '02' ],
+				[ '', '', '12' ],
+				[ '02', '21', '22' ]
+			] ) );
+		} );
 	} );
 
 	async function createEditor( extraPlugins = [] ) {
@@ -3566,13 +3592,13 @@ describe( 'table clipboard', () => {
 		tableSelection = editor.plugins.get( 'TableSelection' );
 	}
 
-	function pasteTable( tableData ) {
+	function pasteTable( tableData, attributes = {} ) {
 		const data = {
 			dataTransfer: createDataTransfer(),
 			preventDefault: sinon.spy(),
 			stopPropagation: sinon.spy()
 		};
-		data.dataTransfer.setData( 'text/html', viewTable( tableData ) );
+		data.dataTransfer.setData( 'text/html', viewTable( tableData, attributes ) );
 		viewDocument.fire( 'paste', data );
 
 		return data;

+ 15 - 0
packages/ckeditor5-table/tests/tableutils.js

@@ -959,6 +959,21 @@ describe( 'TableUtils', () => {
 				], { headingRows: 1 } ) );
 			} );
 
+			it( 'should change heading rows if removing a heading row (and cell below is row-spanned)', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', { contents: '11', rowspan: 2 } ],
+					[ '20' ]
+				], { headingRows: 1 } ) );
+
+				tableUtils.removeRows( root.getChild( 0 ), { at: 0 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ '10', { contents: '11', rowspan: 2 } ],
+					[ '20' ]
+				] ) );
+			} );
+
 			it( 'should decrease rowspan of table cells from previous rows', () => {
 				// +----+----+----+----+----+
 				// | 00 | 01 | 02 | 03 | 04 |