Merge pull request #231 from ckeditor/i/6062

Fix: Fixed a bug with spellchecking or pasting via the context menu into a table cell crashing the editor. Closes ckeditor/ckeditor5#6062
ckeditor · Feb 4, 2020 · be0d759 · be0d759
2 parents ec22240 + 9bcddfe
commit be0d759
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 9 deletions.
diff --git a/src/converters/downcast.js b/src/converters/downcast.js
@@ -397,7 +397,9 @@ function createViewTableCellElement( tableWalkerValue, tableAttributes, insertPo
 		conversionApi.consumable.consume( innerParagraph, 'insert' );
 
 		if ( options.asWidget ) {
-			const fakeParagraph = conversionApi.writer.createContainerElement( 'span' );
+			// Use display:inline-block to force Chrome/Safari to limit text mutations to this element.
+			// See #6062.
+			const fakeParagraph = conversionApi.writer.createContainerElement( 'span', { style: 'display:inline-block' } );
 
 			conversionApi.mapper.bindElements( innerParagraph, fakeParagraph );
 			conversionApi.writer.insert( paragraphInsertPosition, fakeParagraph );

diff --git a/tests/_utils/utils.js b/tests/_utils/utils.js
@@ -368,7 +368,10 @@ function makeRows( tableData, options ) {
 				}
 
 				if ( !( contents.replace( '[', '' ).replace( ']', '' ).startsWith( '<' ) ) && enforceWrapping ) {
-					contents = `<${ wrappingElement }>${ contents }</${ wrappingElement }>`;
+					contents =
+						`<${ wrappingElement == 'span' ? 'span style="display:inline-block"' : wrappingElement }>` +
+							contents +
+						`</${ wrappingElement }>`;
 				}
 
 				const formattedAttributes = formatAttributes( attributes );

diff --git a/tests/converters/downcast.js b/tests/converters/downcast.js
@@ -26,7 +26,10 @@ function paragraphInTableCell() {
 
 				if ( viewElement && viewElement.name === 'span' ) {
 					conversionApi.mapper.unbindModelElement( tableCell );
+
+					conversionApi.writer.removeStyle( 'display', viewElement );
 					conversionApi.writer.rename( 'p', viewElement );
+
 					conversionApi.mapper.bindElements( child, viewElement );
 				}
 			}
@@ -339,7 +342,9 @@ describe( 'downcast converters', () => {
 						'<table>' +
 							'<tbody>' +
 								'<tr>' +
-									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true"><span></span></td>' +
+									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
+										'<span style="display:inline-block"></span>' +
+									'</td>' +
 								'</tr>' +
 							'</tbody>' +
 						'</table>' +
@@ -572,7 +577,7 @@ describe( 'downcast converters', () => {
 							'<tbody>' +
 								'<tr>' +
 									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
-										'<span>00</span>' +
+										'<span style="display:inline-block">00</span>' +
 									'</td>' +
 								'</tr>' +
 								'<tr>' +
@@ -718,7 +723,7 @@ describe( 'downcast converters', () => {
 							'<tbody>' +
 								'<tr>' +
 									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
-										'<span>00</span>' +
+										'<span style="display:inline-block">00</span>' +
 									'</td>' +
 									'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true"></td>' +
 								'</tr>' +
@@ -895,7 +900,7 @@ describe( 'downcast converters', () => {
 							'<thead>' +
 								'<tr>' +
 									'<th class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
-										'<span>00</span>' +
+										'<span style="display:inline-block">00</span>' +
 									'</th>' +
 								'</tr>' +
 							'</thead>' +
@@ -1109,7 +1114,7 @@ describe( 'downcast converters', () => {
 							'<tbody>' +
 								'<tr>' +
 									'<th class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">' +
-										'<span>00</span>' +
+										'<span style="display:inline-block">00</span>' +
 									'</th>' +
 								'</tr>' +
 							'</tbody>' +

diff --git a/tests/converters/table-cell-refresh-post-fixer.js b/tests/converters/table-cell-refresh-post-fixer.js
@@ -169,7 +169,7 @@ describe( 'Table cell refresh post-fixer', () => {
 		} );
 
 		assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-			[ '<span>00</span>' ]
+			[ '<span style="display:inline-block">00</span>' ]
 		], { asWidget: true } ) );
 		sinon.assert.calledOnce( refreshItemSpy );
 	} );
@@ -279,7 +279,7 @@ describe( 'Table cell refresh post-fixer', () => {
 		} );
 
 		assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [
-			[ '<span>00</span>' ]
+			[ '<span style="display:inline-block">00</span>' ]
 		], { asWidget: true } ) );
 		sinon.assert.calledOnce( refreshItemSpy );
 	} );

diff --git a/tests/manual/tickets/6062.html b/tests/manual/tickets/6062.html
@@ -0,0 +1,20 @@
+<div id="editor">
+	<figure class="table">
+		<table>
+			<thead>
+				<tr>
+					<th>at the end:</th>
+					<th>at the beginning:</th>
+					<th>entire cell:</th>
+				</tr>
+			</thead>
+			<tbody>
+				<tr>
+					<td>Hello, commter</td>
+					<td>Commter, hello!</td>
+					<td>Commter</td>
+				</tr>
+			</tbody>
+		</table>
+	</figure>
+</div>
diff --git a/tests/manual/tickets/6062.js b/tests/manual/tickets/6062.js
@@ -0,0 +1,27 @@
+/**
+ * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+/* globals console, window, document */
+
+import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
+import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
+
+ClassicEditor
+	.create( document.querySelector( '#editor' ), {
+		plugins: [ ArticlePluginSet ],
+		toolbar: [
+			'heading', '|', 'insertTable', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo'
+		],
+		table: {
+			contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells' ],
+			tableToolbar: [ 'bold', 'italic' ]
+		}
+	} )
+	.then( editor => {
+		window.editor = editor;
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );
diff --git a/tests/manual/tickets/6062.md b/tests/manual/tickets/6062.md
@@ -0,0 +1,9 @@
+# Ticket test: [#6062](https://github.com/ckeditor/ckeditor5/issues/6062)
+
+1. Right-click the misspelled word.
+2. Choose one of the corrections.
+
+Expected:
+
+* No errors.
+* The misspelled word should be correctly replaced. E.g., you shouldn't see a new paragraph being created.