Merge pull request #127 from ckeditor/t/125

Fix: A table cell should always have a `<paragraph>` in the model. Closes #125.

BREAKING CHANGE: The `injectTablePostFixer()` function from  `table/converters/table-post-fixer` is now `injectTableLayoutPostFixer()`and is moved to `table/converters/table-layout-post-fixer` module.

Author: Reinmar

src/converters/table-cell-content-post-fixer.js

@@ -0,0 +1,112 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/**
+ * @module table/converters/table-cell-content-post-fixer
+ */
+
+/**
+ * Injects a table cell post-fixer into the model.
+ *
+ * The role of the table post-fixer is to ensure that the table cells have the correct content
+ * after a {@link module:engine/model/model~Model#change `change()`} block was executed.
+ *
+ * A table cells must contains at least one block as a child. The empty table cell will have empty `<paragraph>` as a child.
+ *
+ *        <table>
+ *            <tableRow>
+ *                <tableCell></tableCell>
+ *            </tableRow>
+ *        </table>
+ *
+ * Will be fixed to:
+ *
+ *        <table>
+ *            <tableRow>
+ *                <tableCell><paragraph></paragraph></tableCell>
+ *            </tableRow>
+ *        </table>
+ *
+ * @param {module:engine/model/model~Model} model
+ */
+export default function injectTableCellContentPostFixer( model ) {
+	model.document.registerPostFixer( writer => tableCellContentsPostFixer( writer, model ) );
+}
+
+// The table cell contents post-fixer.
+//
+// @param {module:engine/model/writer~Writer} writer
+// @param {module:engine/model/model~Model} model
+function tableCellContentsPostFixer( writer, model ) {
+	const changes = model.document.differ.getChanges();
+
+	let wasFixed = false;
+
+	for ( const entry of changes ) {
+		// Enforce paragraph in tableCell even after other feature remove its contents.
+		if ( entry.type == 'remove' && entry.position.parent.is( 'tableCell' ) ) {
+			wasFixed = fixTableCellContent( entry.position.parent, writer ) || wasFixed;
+		}
+
+		// Analyze table cells on insertion.
+		if ( entry.type == 'insert' ) {
+			if ( entry.name == 'table' ) {
+				wasFixed = fixTable( entry.position.nodeAfter, writer ) || wasFixed;
+			}
+
+			if ( entry.name == 'tableRow' ) {
+				wasFixed = fixTableRow( entry.position.nodeAfter, writer ) || wasFixed;
+			}
+
+			if ( entry.name == 'tableCell' ) {
+				wasFixed = fixTableCellContent( entry.position.nodeAfter, writer ) || wasFixed;
+			}
+		}
+	}
+
+	return wasFixed;
+}
+
+// Fixes all table cells in a table.
+//
+// @param {module:engine/model/element~Element} table
+// @param {module:engine/model/writer~Writer} writer
+function fixTable( table, writer ) {
+	let wasFixed = false;
+
+	for ( const row of table.getChildren() ) {
+		wasFixed = fixTableRow( row, writer ) || wasFixed;
+	}
+
+	return wasFixed;
+}
+
+// Fixes all table cells in a table row.
+//
+// @param {module:engine/model/element~Element} tableRow
+// @param {module:engine/model/writer~Writer} writer
+function fixTableRow( tableRow, writer ) {
+	let wasFixed = false;
+
+	for ( const tableCell of tableRow.getChildren() ) {
+		wasFixed = fixTableCellContent( tableCell, writer ) || wasFixed;
+	}
+
+	return wasFixed;
+}
+
+// Fixes all table cell content by adding paragraph to a table cell without any child.
+//
+// @param {module:engine/model/element~Element} table
+// @param {module:engine/model/writer~Writer} writer
+function fixTableCellContent( tableCell, writer ) {
+	if ( tableCell.childCount == 0 ) {
+		writer.insertElement( 'paragraph', tableCell );
+
+		return true;
+	}
+
+	return false;
+}

src/converters/table-post-fixer.js renamed to src/converters/table-layout-post-fixer.js

@@ -4,23 +4,24 @@
  */
 
 /**
- * @module table/converters/table-post-fixer
+ * @module table/converters/table-layout-post-fixer
  */
 
 import Position from '@ckeditor/ckeditor5-engine/src/model/position';
 import { createEmptyTableCell, findAncestor, updateNumericAttribute } from './../commands/utils';
 import TableWalker from './../tablewalker';
 
 /**
- * Injects a table post-fixer into the model.
+ * Injects a table layout post-fixer into the model.
  *
- * The role of the table post-fixer is to ensure that the table rows have the correct structure
+ * The role of the table layout post-fixer is to ensure that the table rows have the correct structure
  * after a {@link module:engine/model/model~Model#change `change()`} block was executed.
  *
  * The correct structure means that:
  *
  * * All table rows have the same size.
  * * None of a table cells that extend vertically beyond their section (either header or body).
+ * * A table cell has always at least one element as child.
  *
  * If the table structure is not correct, the post-fixer will automatically correct it in two steps:
  *
@@ -35,12 +36,12 @@ import TableWalker from './../tablewalker';
  *
  *		<table headingRows="1">
  *			<tableRow>
- *				<tableCell rowspan="2">FOO</tableCell>
- *				<tableCell colspan="2">BAR</tableCell>
+ *				<tableCell rowspan="2"><paragraph>FOO</paragraph></tableCell>
+ *				<tableCell colspan="2"><paragraph>BAR</paragraph></tableCell>
  *			</tableRow>
  *			<tableRow>
- *				<tableCell>BAZ</tableCell>
- *				<tableCell>XYZ</tableCell>
+ *				<tableCell><paragraph>BAZ</paragraph></tableCell>
+ *				<tableCell><paragraph>XYZ</paragraph></tableCell>
  *			</tableRow>
  *		</table>
  *
@@ -55,8 +56,8 @@ import TableWalker from './../tablewalker';
  *			</thead>
  *			<tbody>
  *				<tr>
- *					<td>BAZ<td>
- *					<td>XYZ<td>
+ *					<td>BAZ</td>
+ *					<td>XYZ</td>
  *				</tr>
  *			</tbody>
  *		</table>
@@ -90,8 +91,8 @@ import TableWalker from './../tablewalker';
  *			</thead>
  *			<tbody>
  *				<tr>
- *					<td>BAZ<td>
- *					<td>XYZ<td>
+ *					<td>BAZ</td>
+ *					<td>XYZ</td>
  *				</tr>
  *			</tbody>
  *		</table>
@@ -113,8 +114,8 @@ import TableWalker from './../tablewalker';
  *					<td>12</td>
  *				</tr>
  *				<tr>
- *					<td>21<td>
- *					<td>22<td>
+ *					<td>21</td>
+ *					<td>22</td>
  *				</tr>
  *			</tbody>
  *		</table>
@@ -156,8 +157,8 @@ import TableWalker from './../tablewalker';
  *					<td>(empty, inserted by A)</td>
  *				</tr>
  *				<tr>
- *					<td>21<td>
- *					<td>22<td>
+ *					<td>21</td>
+ *					<td>22</td>
  *					<td>(empty, inserted by A)</td>
  *				</tr>
  *				<tr>
@@ -197,32 +198,31 @@ import TableWalker from './../tablewalker';
  *				<tr>
  *					<td>11</td>
  *					<td>12</td>
- *					<td>(empty, inserted by a post-fixer after undo)<td>
+ *					<td>(empty, inserted by a post-fixer after undo)</td>
  *				</tr>
  *				<tr>
- *					<td>21<td>
- *					<td>22<td>
- *					<td>(empty, inserted by a post-fixer after undo)<td>
+ *					<td>21</td>
+ *					<td>22</td>
+ *					<td>(empty, inserted by a post-fixer after undo)</td>
  *				</tr>
  *				<tr>
- *					<td>(empty, inserted by B)<td>
- *					<td>(empty, inserted by B)<td>
- *					<td>(empty, inserted by a post-fixer)<td>
+ *					<td>(empty, inserted by B)</td>
+ *					<td>(empty, inserted by B)</td>
+ *					<td>(empty, inserted by a post-fixer)</td>
  *				</tr>
  *			</tbody>
  *		</table>
- *
  * @param {module:engine/model/model~Model} model
  */
-export default function injectTablePostFixer( model ) {
-	model.document.registerPostFixer( writer => tablePostFixer( writer, model ) );
+export default function injectTableLayoutPostFixer( model ) {
+	model.document.registerPostFixer( writer => tableLayoutPostFixer( writer, model ) );
 }
 
-// The table post-fixer.
+// The table layout post-fixer.
 //
 // @param {module:engine/model/writer~Writer} writer
 // @param {module:engine/model/model~Model} model
-function tablePostFixer( writer, model ) {
+function tableLayoutPostFixer( writer, model ) {
 	const changes = model.document.differ.getChanges();
 
 	let wasFixed = false;
@@ -233,7 +233,6 @@ function tablePostFixer( writer, model ) {
 	for ( const entry of changes ) {
 		let table;
 
-		// Fix table on table insert.
 		if ( entry.name == 'table' && entry.type == 'insert' ) {
 			table = entry.position.nodeAfter;
 		}

src/tableediting.js

@@ -33,7 +33,8 @@ import SetHeaderColumnCommand from './commands/setheadercolumncommand';
 import { findAncestor } from './commands/utils';
 import TableUtils from '../src/tableutils';
 
-import injectTablePostFixer from './converters/table-post-fixer';
+import injectTableLayoutPostFixer from './converters/table-layout-post-fixer';
+import injectTableCellContentsPostFixer from './converters/table-cell-content-post-fixer';
 import injectTableCellPostFixer from './converters/tablecell-post-fixer';
 
 import '../theme/tableediting.css';
@@ -145,7 +146,8 @@ export default class TableEditing extends Plugin {
 		editor.commands.add( 'setTableColumnHeader', new SetHeaderColumnCommand( editor ) );
 		editor.commands.add( 'setTableRowHeader', new SetHeaderRowCommand( editor ) );
 
-		injectTablePostFixer( model );
+		injectTableLayoutPostFixer( model );
+		injectTableCellContentsPostFixer( model );
 
 		// Handle tab key navigation.
 		this.editor.keystrokes.set( 'Tab', ( ...args ) => this._handleTabOnSelectedTable( ...args ), { priority: 'low' } );