Table caption feature

packages/ckeditor5-table/src/converters/table-caption-post-fixer.js

@@ -45,7 +45,7 @@ function tableCaptionPostFixer( writer, model ) {
 			const firstCaption = captionsToMerge.shift();
 
 			if ( !firstCaption ) {
-				return;
+				continue;
 			}
 
 			// Move all the contents of the captions to the first one.

packages/ckeditor5-table/src/converters/table-layout-post-fixer.js

@@ -385,8 +385,8 @@ function getChildrenLengths( table ) {
 	// TableWalker will not provide items for the empty rows, we need to pre-fill this array.
 	const lengths = new Array( table.childCount ).fill( 0 );
 
-	for ( const { row } of new TableWalker( table, { includeAllSlots: true } ) ) {
-		lengths[ row ]++;
+	for ( const { rowIndex } of new TableWalker( table, { includeAllSlots: true } ) ) {
+		lengths[ rowIndex ]++;
 	}
 
 	return lengths;

packages/ckeditor5-table/src/tableediting.js

@@ -84,13 +84,6 @@ export default class TableEditing extends Plugin {
 		// Allow all $block content inside a table cell.
 		schema.extend( '$block', { allowIn: 'tableCell' } );
 
-		// Disallow a table in a table.
-		schema.addChildCheck( ( context, childDefinition ) => {
-			if ( childDefinition.name == 'table' && Array.from( context.getNames() ).includes( 'table' ) ) {
-				return false;
-			}
-		} );
-
 		// Figure conversion.
 		conversion.for( 'upcast' ).add( upcastTableFigure() );
 

packages/ckeditor5-table/src/tableutils.js

@@ -7,6 +7,7 @@
  * @module table/tableutils
  */
 
+import { CKEditorError } from 'ckeditor5/src/utils';
 import { Plugin } from 'ckeditor5/src/core';
 
 import TableWalker from './tablewalker';
@@ -153,6 +154,19 @@ export default class TableUtils extends Plugin {
 		const rows = this.getRows( table );
 		const columns = this.getColumns( table );
 
+		if ( insertAt > rows ) {
+			/**
+			 * The `options.at` points at a row position that does not exist.
+			 *
+			 * @error tableutils-insertrows-insert-out-of-range
+			 */
+			throw new CKEditorError(
+				'tableutils-insertrows-insert-out-of-range',
+				this,
+				{ options }
+			);
+		}
+
 		model.change( writer => {
 			const headingRows = table.getAttribute( 'headingRows' ) || 0;
 
@@ -334,8 +348,11 @@ export default class TableUtils extends Plugin {
 		const model = this.editor.model;
 
 		const rowsToRemove = options.rows || 1;
+		const rowCount = this.getRows( table );
 		const first = options.at;
-		const last = first + rowsToRemove - 1;
+
+		// Make sure we are not removing too much, e.g. non-row elements at the end of the table.
+		const last = Math.min( first + rowsToRemove - 1, rowCount - 1 );
 
 		model.change( writer => {
 			// Removing rows from the table require that most calculations to be done prior to changing table structure.
@@ -479,6 +496,10 @@ export default class TableUtils extends Plugin {
 	 * @param {Number} numberOfCells
 	 */
 	splitCellVertically( tableCell, numberOfCells = 2 ) {
+		if ( !tableCell || !tableCell.is( 'element', 'tableCell' ) ) {
+			return;
+		}
+
 		const model = this.editor.model;
 		const tableRow = tableCell.parent;
 		const table = tableRow.parent;
@@ -615,6 +636,10 @@ export default class TableUtils extends Plugin {
 	 * @param {Number} numberOfCells
 	 */
 	splitCellHorizontally( tableCell, numberOfCells = 2 ) {
+		if ( !tableCell || !tableCell.is( 'element', 'tableCell' ) ) {
+			return;
+		}
+
 		const model = this.editor.model;
 
 		const tableRow = tableCell.parent;
@@ -723,7 +748,8 @@ export default class TableUtils extends Plugin {
 	 */
 	getColumns( table ) {
 		// Analyze first row only as all the rows should have the same width.
-		// We are taking first row without checking because we expect that table will have only tableRow models at the beginning.
+		// Using the first row without checking if it's a tableRow because we expect
+		// that table will have only tableRow model elements at the beginning.
 		const row = table.getChild( 0 );
 
 		return [ ...row.getChildren() ].reduce( ( columns, row ) => {

packages/ckeditor5-table/src/tablewalker.js

@@ -159,6 +159,14 @@ export default class TableWalker {
 		 */
 		this._row = 0;
 
+		/**
+		 * The index of the current row element in the table.
+		 *
+		 * @type {Number}
+		 * @protected
+		 */
+		this._rowIndex = 0;
+
 		/**
 		 * The current column index.
 		 *
@@ -209,15 +217,21 @@ export default class TableWalker {
 	 * @returns {module:table/tablewalker~TableSlot} The next table walker's value.
 	 */
 	next() {
-		const row = this._table.getChild( this._row );
+		const row = this._table.getChild( this._rowIndex );
 
 		// Iterator is done when there's no row (table ended) or the row is after `endRow` limit.
 		if ( !row || this._isOverEndRow() ) {
 			return { done: true };
 		}
 
-		// We step over current row when it is not a tableRow instance or we reached the end of it.
-		if ( !row.is( 'element', 'tableRow' ) || this._isOverEndColumn() ) {
+		// We step over current element when it is not a tableRow instance.
+		if ( !row.is( 'element', 'tableRow' ) ) {
+			this._rowIndex++;
+
+			return this.next();
+		}
+
+		if ( this._isOverEndColumn() ) {
 			return this._advanceToNextRow();
 		}
 
@@ -281,6 +295,7 @@ export default class TableWalker {
 	 */
 	_advanceToNextRow() {
 		this._row++;
+		this._rowIndex++;
 		this._column = 0;
 		this._cellIndex = 0;
 		this._nextCellAtColumn = -1;
@@ -466,6 +481,15 @@ class TableSlot {
 		 */
 		this._cellIndex = tableWalker._cellIndex;
 
+		/**
+		 * The index of the current row element in the table.
+		 *
+		 * @readonly
+		 * @member {Number}
+		 * @private
+		 */
+		this._rowIndex = tableWalker._rowIndex;
+
 		/**
 		 * The table element.
 		 *
@@ -506,6 +530,16 @@ class TableSlot {
 		return parseInt( this.cell.getAttribute( 'rowspan' ) || 1 );
 	}
 
+	/**
+	 * The index of the current row element in the table.
+	 *
+	 * @readonly
+	 * @returns {Number}
+	 */
+	get rowIndex() {
+		return this._rowIndex;
+	}
+
 	/**
 	 * Returns the {@link module:engine/model/position~Position} before the table slot.
 	 *

packages/ckeditor5-table/src/utils/structure.js

@@ -17,9 +17,9 @@ import { createEmptyTableCell, updateNumericAttribute } from './common';
  *
  *		const croppedTable = cropTableToDimensions( table, {
  *			startRow: 1,
- *			endRow: 1,
- *			startColumn: 3,
- *			endColumn: 3
+ *			endRow: 3,
+ *			startColumn: 1,
+ *			endColumn: 3done
  *		}, writer );
  *
  * Calling the code above for the table below:

packages/ckeditor5-table/tests/commands/mergecellcommand.js

@@ -607,6 +607,30 @@ describe( 'MergeCellCommand', () => {
 				expect( command.value ).to.be.undefined;
 			} );
 
+			it( 'should be undefined if in last row - ignore non-row elements', () => {
+				model.schema.register( 'foo', {
+					allowIn: 'table',
+					allowContentOf: '$block',
+					isLimit: true
+				} );
+
+				setData( model,
+					'<table>' +
+						'<tableRow>' +
+							'<tableCell><paragraph>00</paragraph></tableCell>' +
+							'<tableCell><paragraph>10</paragraph></tableCell>' +
+						'</tableRow>' +
+						'<tableRow>' +
+							'<tableCell><paragraph>10[]</paragraph></tableCell>' +
+							'<tableCell><paragraph>11</paragraph></tableCell>' +
+						'</tableRow>' +
+						'<foo>An extra element</foo>' +
+					'</table>'
+				);
+
+				expect( command.value ).to.be.undefined;
+			} );
+
 			it( 'should be set to mergeable cell with the same rowspan', () => {
 				setData( model, modelTable( [
 					[ { colspan: 2, contents: '00[]' }, '02' ],

packages/ckeditor5-table/tests/commands/removerowcommand.js

@@ -487,6 +487,45 @@ describe( 'RemoveRowCommand', () => {
 			] ) );
 		} );
 
+		it( 'should remove last row - ignore non-row elements', () => {
+			model.schema.register( 'foo', {
+				allowIn: 'table',
+				allowContentOf: '$block',
+				isLimit: true
+			} );
+
+			editor.conversion.elementToElement( {
+				view: 'foo',
+				model: 'foo'
+			} );
+
+			setData( model,
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>00</paragraph></tableCell>' +
+						'<tableCell><paragraph>01</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>[]10</paragraph></tableCell>' +
+						'<tableCell><paragraph>11</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<foo>An extra element</foo>' +
+				'</table>'
+			);
+
+			command.execute();
+
+			assertEqualMarkup( getData( model ),
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>[]00</paragraph></tableCell>' +
+						'<tableCell><paragraph>01</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<foo>An extra element</foo>' +
+				'</table>'
+			);
+		} );
+
 		it( 'should change heading rows if removing a heading row', () => {
 			setData( model, modelTable( [
 				[ '00', '01' ],

packages/ckeditor5-table/tests/converters/table-caption-post-fixer.js

@@ -6,10 +6,11 @@
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
 import { getData as getModelData, parse, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
 
 import TableEditing from '../../src/tableediting';
 import TableCaptionEditing from '../../src/tablecaption/tablecaptionediting';
-import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
+import TableWalker from '../../src/tablewalker';
 
 describe( 'Table caption post-fixer', () => {
 	let editor, model, root;
@@ -120,6 +121,71 @@ describe( 'Table caption post-fixer', () => {
 			);
 		} );
 
+		it( 'should merge all captions in between the rows (and TableWalker should still provide valid rows)', () => {
+			const modelTable =
+				'<table>' +
+					'<caption>caption 0</caption>' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph>00</paragraph>' +
+						'</tableCell>' +
+						'<tableCell>' +
+							'<paragraph>01</paragraph>' +
+						'</tableCell>' +
+					'</tableRow>' +
+					'<caption>caption 1</caption>' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph>10</paragraph>' +
+						'</tableCell>' +
+						'<tableCell>' +
+							'<paragraph>11</paragraph>' +
+						'</tableCell>' +
+					'</tableRow>' +
+					'<caption>caption 2</caption>' +
+				'</table>';
+			const parsed = parse( modelTable, model.schema );
+
+			model.change( writer => {
+				writer.remove( writer.createRangeIn( root ) );
+				writer.insert( parsed, root );
+
+				const slots = Array.from( new TableWalker( root.getChild( 0 ) ) );
+
+				expect( slots.length ).to.equal( 4 );
+				expect( slots[ 0 ].row ).to.equal( 0 );
+				expect( slots[ 0 ].column ).to.equal( 0 );
+				expect( slots[ 1 ].row ).to.equal( 0 );
+				expect( slots[ 1 ].column ).to.equal( 1 );
+				expect( slots[ 2 ].row ).to.equal( 1 );
+				expect( slots[ 2 ].column ).to.equal( 0 );
+				expect( slots[ 3 ].row ).to.equal( 1 );
+				expect( slots[ 3 ].column ).to.equal( 1 );
+			} );
+
+			assertEqualMarkup( getModelData( model, { withoutSelection: true } ),
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph>00</paragraph>' +
+						'</tableCell>' +
+						'<tableCell>' +
+							'<paragraph>01</paragraph>' +
+						'</tableCell>' +
+					'</tableRow>' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph>10</paragraph>' +
+						'</tableCell>' +
+						'<tableCell>' +
+							'<paragraph>11</paragraph>' +
+						'</tableCell>' +
+					'</tableRow>' +
+					'<caption>caption 0caption 1caption 2</caption>' +
+				'</table>'
+			);
+		} );
+
 		it( 'should move final caption at the end of the table', () => {
 			const modelTable =
 				'<table>' +