Merge pull request #8411 from ckeditor/cf/3602

Fix (undo): Fixed restoring selection on undo for some scenarios when some selection ranges are in the graveyard after restoring them.
ckeditor · Nov 5, 2020 · 772f45d · 772f45d
2 parents 2d8207a + c209fbe
commit 772f45d
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 11 deletions.
diff --git a/packages/ckeditor5-undo/src/basecommand.js b/packages/ckeditor5-undo/src/basecommand.js
@@ -100,24 +100,23 @@ export default class BaseCommand extends Command {
 
 		for ( const rangeGroup of transformedRangeGroups ) {
 			// While transforming there could appear ranges that are contained by other ranges, we shall ignore them.
-			const transformed = rangeGroup.filter( range => !isRangeContainedByAnyOtherRange( range, allRanges ) );
+			const transformed = rangeGroup
+				.filter( range => range.root != document.graveyard )
+				.filter( range => !isRangeContainedByAnyOtherRange( range, allRanges ) );
+
+			// All the transformed ranges ended up in graveyard.
+			if ( !transformed.length ) {
+				continue;
+			}
 
 			// After the range got transformed, we have an array of ranges. Some of those
 			// ranges may be "touching" -- they can be next to each other and could be merged.
 			normalizeRanges( transformed );
 
 			// For each `range` from `ranges`, we take only one transformed range.
 			// This is because we want to prevent situation where single-range selection
-			// got transformed to multi-range selection. We will take the first range that
-			// is not in the graveyard.
-			const newRange = transformed.find(
-				range => range.root != document.graveyard
-			);
-
-			// `transformedRange` might be `undefined` if transformed range ended up in graveyard.
-			if ( newRange ) {
-				selectionRanges.push( newRange );
-			}
+			// got transformed to multi-range selection.
+			selectionRanges.push( transformed[ 0 ] );
 		}
 
 		// @if CK_DEBUG_ENGINE // console.log( `Restored selection by undo: ${ selectionRanges.join( ', ' ) }` );

diff --git a/packages/ckeditor5-undo/tests/undoediting-integration.js b/packages/ckeditor5-undo/tests/undoediting-integration.js
@@ -1100,6 +1100,87 @@ describe( 'UndoEditing integration', () => {
 				'</table>'
 			);
 		} );
+
+		it( 'undo table cells content wrapping', () => {
+			model.schema.register( 'tableCellContent', {
+				allowIn: 'tableCell',
+				allowContentOf: 'tableCell',
+				isLimit: true
+			} );
+
+			editor.conversion.elementToElement( { model: 'tableCellContent', view: 'td-content' } );
+
+			input(
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>00</paragraph></tableCell>' +
+						'[<tableCell><paragraph>01</paragraph></tableCell>]' +
+						'[<tableCell><paragraph>02</paragraph></tableCell>]' +
+						'<tableCell><paragraph>03</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>10</paragraph></tableCell>' +
+						'[<tableCell><paragraph>11</paragraph></tableCell>]' +
+						'[<tableCell><paragraph>12</paragraph></tableCell>]' +
+						'<tableCell><paragraph>13</paragraph></tableCell>' +
+					'</tableRow>' +
+				'</table>'
+			);
+
+			model.change( writer => {
+				const targetCell = root.getNodeByPath( [ 0, 0, 1 ] );
+				const insertionWrapper = writer.createElement( 'tableCellContent' );
+				const deletionWrapper = writer.createElement( 'tableCellContent' );
+				const paragraph = writer.createElement( 'paragraph' );
+
+				writer.wrap( writer.createRangeIn( targetCell ), deletionWrapper );
+				writer.insert( insertionWrapper, targetCell, 0 );
+
+				writer.insert( writer.createText( 'foobar' ), paragraph, 0 );
+				writer.insert( paragraph, insertionWrapper, 'end' );
+
+				writer.setSelection( writer.createRangeOn( targetCell ) );
+			} );
+
+			output(
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>00</paragraph></tableCell>' +
+						'[<tableCell>' +
+							'<tableCellContent><paragraph>foobar</paragraph></tableCellContent>' +
+							'<tableCellContent><paragraph>01</paragraph></tableCellContent>' +
+						'</tableCell>]' +
+						'<tableCell><paragraph>02</paragraph></tableCell>' +
+						'<tableCell><paragraph>03</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>10</paragraph></tableCell>' +
+						'<tableCell><paragraph>11</paragraph></tableCell>' +
+						'<tableCell><paragraph>12</paragraph></tableCell>' +
+						'<tableCell><paragraph>13</paragraph></tableCell>' +
+					'</tableRow>' +
+				'</table>'
+			);
+
+			editor.execute( 'undo' );
+
+			output(
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>00</paragraph></tableCell>' +
+						'[<tableCell><paragraph>01</paragraph></tableCell>]' +
+						'[<tableCell><paragraph>02</paragraph></tableCell>]' +
+						'<tableCell><paragraph>03</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>10</paragraph></tableCell>' +
+						'[<tableCell><paragraph>11</paragraph></tableCell>]' +
+						'[<tableCell><paragraph>12</paragraph></tableCell>]' +
+						'<tableCell><paragraph>13</paragraph></tableCell>' +
+					'</tableRow>' +
+				'</table>'
+			);
+		} );
 	} );
 
 	it( 'postfixers should not add another undo step when fixing undo changes', () => {