Merge pull request #5242 from ckeditor/t/4284

Fix merging cells with rowspan

CHANGES.md

@@ -10,7 +10,7 @@ Fixed Issues:
 * [#5125](/~https://github.com/ckeditor/ckeditor4/issues/5125): Fixed: Deleting a widget with disabled [autoParagraph](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_config.html#cfg-autoParagraph) by the keyboard `backspace` key removes editor editable area and crash it.
 * [#5135](/~https://github.com/ckeditor/ckeditor4/issues/5135): Fixed: [`checkbox.setValue`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_ui_dialog_checkbox.html#method-setValue) and [`radio.setValue`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_ui_dialog_radio.html#method-setValue) methods are not chainable as stated in documentation. Thanks to [Jordan Bradford](/~https://github.com/LordPachelbel)!
 * [#5085](/~https://github.com/ckeditor/ckeditor4/issues/5085): Fixed: [Language](https://ckeditor.com/cke4/addon/language) plugin removes the element marking the text in foreign language if this element does not have an information about text direction.
-
+* [#4284](/~https://github.com/ckeditor/ckeditor4/issues/4284): Fixed: [Tableselection](https://ckeditor.com/cke4/addon/tableselection) Merging cells with a rowspan was throwing error and did not create undo step.
 
 ## CKEditor 4.19.0
 

plugins/tableselection/plugin.js

@@ -921,8 +921,11 @@
 		editor.on( 'afterCommandExec', function( evt ) {
 			if ( CKEDITOR.tools.array.indexOf( cmds, evt.data.name ) !== -1 ) {
 				callback( editor, evt.data );
+
 			}
-		} );
+		// This listener is connected with undo plugin listener and require a higher priority
+		// than the listener in undo plugin to create a correct undo step (#4284).
+		}, null, null, 9 );
 	}
 
 	/**

tests/plugins/tabletools/manual/mergecells.html

@@ -0,0 +1,34 @@
+<div id="editor">
+	<table border="1" cellspacing="1" cellpadding="1" style="width:500px">
+		<tbody>
+			<tr>
+				<td>&nbsp;</td>
+				<td>&nbsp;</td>
+				<td>&nbsp;</td>
+			</tr>
+			<tr>
+				<td>&nbsp;</td>
+				<td rowspan="3">&nbsp;</td>
+				<td>1</td>
+			</tr>
+			<tr>
+				<td>&nbsp;</td>
+				<td>1</td>
+			</tr>
+			<tr>
+				<td>&nbsp;</td>
+				<td>1</td>
+			</tr>
+		</tbody>
+	</table>
+</div>
+
+<script>
+	if ( bender.tools.env.mobile ) {
+		bender.ignore();
+	}
+
+	bender.tools.ignoreUnsupportedEnvironment( 'tableselection' );
+
+	CKEDITOR.replace( 'editor' );
+</script>

tests/plugins/tabletools/manual/mergecells.md

@@ -0,0 +1,11 @@
+@bender-tags: 4.19.1, bug, 4284
+@bender-ui: collapsed
+@bender-ckeditor-plugins: wysiwygarea, toolbar, undo, tableselection
+
+1. Open console.
+2. Select column with a rowspanned cell and cells containing `1` character.
+3. Open context menu and choose "Cell" -> "Merge cells" option.
+
+**Expected:** Cells are merged, undo step is created and there is error in the console.
+
+**Unexpected:** Cells are merged, an error is thrown and no undo step is created.

tests/plugins/tabletools/tabletools.html

@@ -1585,4 +1585,49 @@
 	</table>
 </textarea>
 
+<textarea id="merge-rowspanned-cells">
+	<table>
+		<tbody>
+			<tr>
+				<td>&nbsp;</td>
+				<td>&nbsp;</td>
+				<td>&nbsp;</td>
+			</tr>
+			<tr>
+				<td>&nbsp;</td>
+				<td rowspan="3">[&nbsp;]</td>
+				<td>[1]</td>
+			</tr>
+			<tr>
+				<td>&nbsp;</td>
+				<td>[1]</td>
+			</tr>
+			<tr>
+				<td>&nbsp;</td>
+				<td>[1]</td>
+			</tr>
+		</tbody>
+	</table>
+	=>
+	<table>
+		<tbody>
+			<tr>
+				<td>&nbsp;</td>
+				<td>&nbsp;</td>
+				<td>&nbsp;</td>
+			</tr>
+			<tr>
+				<td>&nbsp;</td>
+				<td colspan="2" rowspan="3">&nbsp;1<br />1<br />1</td>
+			</tr>
+			<tr>
+				<td>&nbsp;</td>
+			</tr>
+			<tr>
+				<td>&nbsp;</td>
+			</tr>
+		</tbody>
+	</table>
+</textarea>
+
 <div id="playground"></div>

tests/plugins/tabletools/tabletools.js

@@ -262,6 +262,30 @@
 				bot.setHtmlWithSelection( source );
 				assert.beautified.html( expected, bot.getData( true ) );
 			} );
+		},
+
+		'test merge cells with a rowspan should create undo step': function() {
+			bender.editorBot.create( {
+				name: 'editor_merge_rowspanned_cells',
+				config: {
+					plugins: 'undo,table,tableselection'
+				}
+			}, function( bot ) {
+				bender.tools.ignoreUnsupportedEnvironment( 'tableselection' );
+
+				var editor = bot.editor;
+
+				bender.tools.testInputOut( 'merge-rowspanned-cells', function( source, expected ) {
+					bot.setHtmlWithSelection( source );
+					bot.execCommand( 'cellMerge' );
+
+					var output = bot.getData( true ),
+						undo = editor.getCommand( 'undo' );
+
+					assert.areSame( bender.tools.compatHtml( expected ), output );
+					assert.isTrue( undo.state === CKEDITOR.TRISTATE_OFF );
+				} );
+			} );
 		}
 	} );
 } )();