Merge pull request #176 from ckeditor/t/ckeditor5/1554

Fix: Table cell view post-fixer should not fix valid view selection. Closes ckeditor/ckeditor5#1554.

src/converters/tablecell-post-fixer.js

@@ -91,7 +91,7 @@ function tableCellPostFixer( writer, model, mapper, view ) {
 	// Selection in the view might not be updated to renamed elements. Happens mostly when other feature inserts paragraph to the table cell
 	// (ie. when deleting table cell contents) and sets selection to it while table-post fixer changes view <p> to <span> element.
 	// The view.selection would have outdated nodes.
-	if ( wasFixed ) {
+	if ( wasFixed && selectionNeedsFix( view.document.selection, mapper ) ) {
 		updateRangesInViewSelection( model.document.selection, mapper, writer );
 	}
 
@@ -186,3 +186,13 @@ function updateRangesInViewSelection( selection, mapper, writer ) {
 
 	writer.setSelection( fixedRanges, { backward: selection.isBackward } );
 }
+
+// Checks if selection needs to be fixed by ensuring that current view selection position's parents are present in the editable view.
+//
+// @param {module:engine/view/selection~Selection} viewSelection
+function selectionNeedsFix( viewSelection ) {
+	const anchor = viewSelection.anchor;
+	const focus = viewSelection.focus;
+
+	return viewSelection.rangeCount && ( !anchor.root.is( 'rootElement' ) || !focus.root.is( 'rootElement' ) );
+}

tests/converters/tablecell-post-fixer.js

@@ -279,6 +279,39 @@ describe( 'TableCell post-fixer', () => {
 		expect( () => view.domConverter.viewRangeToDom( viewRange ) ).to.not.throw();
 	} );
 
+	it( 'should not update view selection after other feature set selection', () => {
+		editor.model.schema.register( 'widget', {
+			isObject: true,
+			isBlock: true,
+			allowWhere: '$block'
+		} );
+		editor.conversion.elementToElement( {
+			model: 'widget',
+			view: 'widget'
+		} );
+
+		editor.setData( viewTable( [ [ '<p>foo[]</p>' ] ] ) );
+
+		const spy = sinon.spy();
+
+		view.document.selection.on( 'change', spy );
+
+		// Insert a widget in table cell and select it.
+		model.change( writer => {
+			const widgetElement = writer.createElement( 'widget' );
+			const tableCell = root.getNodeByPath( [ 0, 0, 0, 0 ] );
+
+			writer.insert( widgetElement, writer.createPositionAfter( tableCell ) );
+
+			// Update the selection so it will be set on the widget and not in renamed paragraph.
+			writer.setSelection( widgetElement, 'on' );
+		} );
+
+		// View selection should be updated only twice - will be set to null and then to widget.
+		// If called thrice the selection post fixer for table cell was also called.
+		sinon.assert.calledTwice( spy );
+	} );
+
 	// https://github.com/ckeditor/ckeditor5-table/issues/191.
 	it( 'should not fire (and crash) for removed view elements', () => {
 		editor.setData( viewTable( [ [ '<p>foo</p>' ] ] ) );