Merge pull request #8229 from ckeditor/i/8098

Fix (table): Pasting nested tables with unsupported by the editor elements will not blow up. Closes #8098. MINOR BREAKING CHANGES (table): Removed the `ensureParagraphInTableCell()` converter that ensures that the model state after the conversion process will be correct. Now the model will be fixed (if needed) by the post-fixer (`injectTableCellParagraphPostFixer()`).
ckeditor · Oct 9, 2020 · c8e3a94 · c8e3a94
2 parents 606a44b + 347580e
commit c8e3a94
Show file tree

Hide file tree

Showing 7 changed files with 79 additions and 65 deletions.
diff --git a/...es/ckeditor5-paste-from-office/tests/_data/image/adjacent-groups/model.edge.word2016.html b/...es/ckeditor5-paste-from-office/tests/_data/image/adjacent-groups/model.edge.word2016.html
@@ -2,30 +2,30 @@
 
 <table>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
+		<tableCell></tableCell>
+		<tableCell></tableCell>
+		<tableCell></tableCell>
+		<tableCell></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell colspan="4"><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
+		<tableCell colspan="4"></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell colspan="2"><paragraph></paragraph></tableCell>
-		<tableCell rowspan="2"><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
+		<tableCell colspan="2"></tableCell>
+		<tableCell rowspan="2"></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell rowspan="2"><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
+		<tableCell rowspan="2"></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
 	</tableRow>
 </table>
 

diff --git a/.../ckeditor5-paste-from-office/tests/_data/image/adjacent-groups/model.safari.word2016.html b/.../ckeditor5-paste-from-office/tests/_data/image/adjacent-groups/model.safari.word2016.html
@@ -1,20 +1,20 @@
 <paragraph></paragraph>
 <table>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
+		<tableCell></tableCell>
+		<tableCell></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell colspan="2"><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
+		<tableCell colspan="2"></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
+		<tableCell></tableCell>
 	</tableRow>
 </table>
 

diff --git a/packages/ckeditor5-paste-from-office/tests/_data/image/adjacent-groups/model.word2016.html b/packages/ckeditor5-paste-from-office/tests/_data/image/adjacent-groups/model.word2016.html
@@ -1,29 +1,29 @@
 <table>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
+		<tableCell></tableCell>
+		<tableCell></tableCell>
+		<tableCell></tableCell>
+		<tableCell></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell colspan="4"><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
+		<tableCell colspan="4"></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell colspan="2"><paragraph></paragraph></tableCell>
-		<tableCell rowspan="2"><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
+		<tableCell colspan="2"></tableCell>
+		<tableCell rowspan="2"></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
-		<tableCell rowspan="2"><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
+		<tableCell rowspan="2"></tableCell>
 	</tableRow>
 	<tableRow>
-		<tableCell><paragraph></paragraph></tableCell>
+		<tableCell></tableCell>
 	</tableRow>
 </table>
 

diff --git a/packages/ckeditor5-table/src/converters/upcasttable.js b/packages/ckeditor5-table/src/converters/upcasttable.js
@@ -84,31 +84,6 @@ export function skipEmptyTableRow() {
 	};
 }
 
-/**
- * A converter that ensures an empty paragraph is inserted in a table cell if no other content was converted.
- *
- * @returns {Function} Conversion helper.
- */
-export function ensureParagraphInTableCell( elementName ) {
-	return dispatcher => {
-		dispatcher.on( `element:${ elementName }`, ( evt, data, conversionApi ) => {
-			// The default converter will create a model range on converted table cell.
-			if ( !data.modelRange ) {
-				return;
-			}
-
-			const tableCell = data.modelRange.start.nodeAfter;
-
-			// Ensure a paragraph in the model for empty table cells for converted table cells.
-			if ( !tableCell.childCount ) {
-				const modelCursor = conversionApi.writer.createPositionAt( tableCell, 0 );
-
-				conversionApi.writer.insertElement( 'paragraph', modelCursor );
-			}
-		}, { priority: 'low' } );
-	};
-}
-
 // Scans table rows and extracts required metadata from the table:
 //
 // headingRows    - The number of rows that go as table headers.

diff --git a/packages/ckeditor5-table/src/tableediting.js b/packages/ckeditor5-table/src/tableediting.js
@@ -9,7 +9,7 @@
 
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 
-import upcastTable, { ensureParagraphInTableCell, skipEmptyTableRow } from './converters/upcasttable';
+import upcastTable, { skipEmptyTableRow } from './converters/upcasttable';
 import {
 	downcastInsertCell,
 	downcastInsertRow,
@@ -106,8 +106,6 @@ export default class TableEditing extends Plugin {
 		// Table cell conversion.
 		conversion.for( 'upcast' ).elementToElement( { model: 'tableCell', view: 'td' } );
 		conversion.for( 'upcast' ).elementToElement( { model: 'tableCell', view: 'th' } );
-		conversion.for( 'upcast' ).add( ensureParagraphInTableCell( 'td' ) );
-		conversion.for( 'upcast' ).add( ensureParagraphInTableCell( 'th' ) );
 
 		conversion.for( 'editingDowncast' ).add( downcastInsertCell() );
 

diff --git a/packages/ckeditor5-table/tests/converters/upcasttable.js b/packages/ckeditor5-table/tests/converters/upcasttable.js
@@ -285,7 +285,7 @@ describe( 'upcastTable()', () => {
 		);
 
 		expectModel(
-			'<fooTable><fooRow><fooCell><paragraph></paragraph></fooCell></fooRow></fooTable>'
+			'<fooTable><fooRow><fooCell></fooCell></fooRow></fooTable>'
 		);
 	} );
 

diff --git a/packages/ckeditor5-table/tests/tableclipboard-paste.js b/packages/ckeditor5-table/tests/tableclipboard-paste.js
@@ -3917,6 +3917,36 @@ describe( 'table clipboard', () => {
 				[ '02', '21', '22' ]
 			] ) );
 		} );
+
+		it( 'should not blow up when pasting unsupported element in table', async () => {
+			await createEditor( [ TableCellPropertiesEditing ] );
+
+			pasteHtml( editor,
+				'<table>' +
+					'<tbody>' +
+						'<tr>' +
+							'<td>' +
+								'<div>' +
+									'<table>' +
+										'<tbody>' +
+											'<tr>' +
+												'<td style="border: 2px solid rgb(242, 242, 242);">' +
+													'<p>Test</p>' +
+												'</td>' +
+											'</tr>' +
+										'</tbody>' +
+									'</table>' +
+								'</div>' +
+							'</td>' +
+						'</tr>' +
+					'</tbody>' +
+				'</table>'
+			);
+
+			assertEqualMarkup( getModelData( model, { withoutSelection: true } ), modelTable( [
+				[ '<paragraph>Test</paragraph><paragraph></paragraph>' ]
+			] ) );
+		} );
 	} );
 
 	async function createEditor( extraPlugins = [] ) {
@@ -3930,6 +3960,17 @@ describe( 'table clipboard', () => {
 		tableSelection = editor.plugins.get( 'TableSelection' );
 	}
 
+	function pasteHtml( editor, html ) {
+		const data = {
+			dataTransfer: createDataTransfer(),
+			stopPropagation() {},
+			preventDefault() {}
+		};
+
+		data.dataTransfer.setData( 'text/html', html );
+		editor.editing.view.document.fire( 'paste', data );
+	}
+
 	function pasteTable( tableData, attributes = {} ) {
 		const data = {
 			dataTransfer: createDataTransfer(),
-Original file line number
+Diff line change
@@ Expand Up / @@ -285,7 +285,7 @@ describe( 'upcastTable()', () => { @@
     		);
     		expectModel(
-    			'<fooTable><fooRow><fooCell><paragraph></paragraph></fooCell></fooRow></fooTable>'
+    			'<fooTable><fooRow><fooCell></fooCell></fooRow></fooTable>'
     		);
     	} );
@@ Expand Down @@