Changed view table cell post-fixer to a model table cell post-fixer

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

@@ -4,16 +4,13 @@
  */
 
 /**
- * @module table/converters/table-cell-content-post-fixer
+ * @module table/converters/table-cell-paragraph-post-fixer
  */
 
 /**
- * Injects a table cell post-fixer into the model.
+ * Injects a table cell post-fixer into the model which inserts `paragraph` element into empty table cells.
  *
- * 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.
+ * A table cell must contain at least one block element as a child. An empty table cell will have empty `paragraph` as a child.
  *
  *		<table>
  *			<tableRow>
@@ -31,7 +28,7 @@
  *
  * @param {module:engine/model/model~Model} model
  */
-export default function injectTableCellContentPostFixer( model ) {
+export default function injectTableCellParagraphPostFixer( model ) {
 	model.document.registerPostFixer( writer => tableCellContentsPostFixer( writer, model ) );
 }
 
@@ -45,11 +42,6 @@ function tableCellContentsPostFixer( writer, model ) {
 	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' ) {
@@ -112,7 +104,7 @@ function fixTableCellContent( tableCell, writer ) {
 		return true;
 	}
 
-	// Check table cell children for directly placed $text nodes.
+	// Check table cell children for directly placed text nodes.
 	// Temporary solution. See https://github.com/ckeditor/ckeditor5/issues/1464.
 	const textNodes = Array.from( tableCell.getChildren() ).filter( child => child.is( 'text' ) );
 

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

@@ -0,0 +1,41 @@
+/**
+ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+/**
+ * @module table/converters/table-cell-refresh-post-fixer
+ */
+
+/**
+ * Injects a table cell post-fixer into the model which marks the table cell in the differ to have it re-rendered.
+ *
+ * Model `paragraph` inside a table cell can be rendered as `<span>` or `<p>`. It is rendered as `<span>` if this is the only block
+ * element in that table cell and it doesn't have any attributes. It is rendered as `<p>` otherwise.
+ *
+ * When table cell content changes, for example a second `paragraph` element is added we need to ensure that the first `paragraph` is
+ * re-rendered so it changes to `<p>` from `<span>`. The easiest way to do it is to re-render whole table cell.
+ *
+ * @param {module:engine/model/model~Model} model
+ */
+export default function injectTableCellRefreshPostFixer( model ) {
+	model.document.registerPostFixer( () => tableCellRefreshPostFixer( model ) );
+}
+
+function tableCellRefreshPostFixer( model ) {
+	const differ = model.document.differ;
+
+	let fixed = false;
+
+	for ( const change of differ.getChanges() ) {
+		const parent = change.type == 'insert' || change.type == 'remove' ? change.position.parent : change.range.start.parent;
+
+		if ( parent.is( 'tableCell' ) ) {
+			differ.refreshItem( parent );
+
+			fixed = true;
+		}
+	}
+
+	return fixed;
+}

src/tableediting.js

@@ -32,8 +32,8 @@ import { findAncestor } from './commands/utils';
 import TableUtils from '../src/tableutils';
 
 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 injectTableCellParagraphPostFixer from './converters/table-cell-paragraph-post-fixer';
+import injectTableCellRefreshPostFixer from './converters/table-cell-refresh-post-fixer';
 
 import '../theme/tableediting.css';
 
@@ -111,8 +111,6 @@ export default class TableEditing extends Plugin {
 		conversion.for( 'editingDowncast' ).add( downcastTableHeadingRowsChange( { asWidget: true } ) );
 		conversion.for( 'dataDowncast' ).add( downcastTableHeadingRowsChange() );
 
-		injectTableCellPostFixer( editor.model, editor.editing );
-
 		// Define all the commands.
 		editor.commands.add( 'insertTable', new InsertTableCommand( editor ) );
 		editor.commands.add( 'insertTableRowAbove', new InsertRowCommand( editor, { order: 'above' } ) );
@@ -135,7 +133,8 @@ export default class TableEditing extends Plugin {
 		editor.commands.add( 'setTableRowHeader', new SetHeaderRowCommand( editor ) );
 
 		injectTableLayoutPostFixer( model );
-		injectTableCellContentsPostFixer( model );
+		injectTableCellRefreshPostFixer( model );
+		injectTableCellParagraphPostFixer( model );
 
 		// Handle tab key navigation.
 		this.editor.keystrokes.set( 'Tab', ( ...args ) => this._handleTabOnSelectedTable( ...args ), { priority: 'low' } );

tests/converters/table-cell-content-post-fixer.js → tests/converters/table-cell-paragraph-post-fixer.js

@@ -11,7 +11,7 @@ import TableEditing from '../../src/tableediting';
 import { formatTable } from './../_utils/utils';
 import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
 
-describe( 'Table cell content post-fixer', () => {
+describe( 'Table cell paragraph post-fixer', () => {
 	let editor, model, root;
 
 	beforeEach( () => {

tests/converters/tablecell-post-fixer.js → tests/converters/table-cell-refresh-post-fixer.js

@@ -8,15 +8,15 @@
 import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
 
 import { defaultConversion, defaultSchema, formatTable, formattedViewTable, viewTable } from '../_utils/utils';
-import injectTableCellPostFixer from '../../src/converters/tablecell-post-fixer';
+import injectTableCellRefreshPostFixer from '../../src/converters/table-cell-refresh-post-fixer';
 
 import env from '@ckeditor/ckeditor5-utils/src/env';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
 
 import Delete from '@ckeditor/ckeditor5-typing/src/delete';
 
-describe( 'TableCell post-fixer', () => {
+describe( 'Table cell refresh post-fixer', () => {
 	let editor, model, doc, root, view;
 
 	testUtils.createSinonSandbox();
@@ -47,7 +47,7 @@ describe( 'TableCell post-fixer', () => {
 				model.schema.extend( '$block', { allowAttributes: 'foo' } );
 				editor.conversion.attributeToAttribute( { model: 'foo', view: 'foo' } );
 
-				injectTableCellPostFixer( model, editor.editing );
+				injectTableCellRefreshPostFixer( model );
 			} );
 	} );
 
@@ -215,33 +215,6 @@ describe( 'TableCell post-fixer', () => {
 		], { asWidget: true } ) );
 	} );
 
-	it( 'should not crash when view.change() block was called in model.change()', () => {
-		editor.setData( viewTable( [ [ '<p>foobar</p>' ] ] ) );
-
-		const table = root.getChild( 0 );
-
-		expect( formatTable( getViewData( view, { withoutSelection: true } ) ) )
-			.to.equal( formattedViewTable( [ [ 'foobar' ] ], { asWidget: true } ) );
-
-		expect( () => {
-			model.change( writer => {
-				const tableCell = table.getNodeByPath( [ 0, 0 ] );
-
-				writer.insertElement( 'paragraph', null, writer.createPositionAt( tableCell, 'end' ) );
-				writer.setSelection( writer.createRangeIn( tableCell ) );
-
-				// Do some change in the view while inside model change.
-				editor.editing.view.change( writer => {
-					writer.addClass( 'foo', editor.editing.mapper.toViewElement( tableCell ) );
-				} );
-			} );
-		} ).to.not.throw();
-
-		expect( formatTable( getViewData( view ) ) ).to.equal( formattedViewTable( [
-			[ { class: 'foo', contents: '<p>{foobar</p><p>]</p>' } ]
-		], { asWidget: true } ) );
-	} );
-
 	it( 'should keep <p> in the view when <paragraph> attribute value is changed (table cell with multiple blocks)', () => {
 		editor.setData( viewTable( [ [ '<p>00</p><p>00</p>' ] ] ) );