ソースを参照

Changed: refactored undo feature, fixing #12.

Szymon Cofalik 9 年 前
コミット
2b0c0bb454

+ 111 - 0
packages/ckeditor5-undo/src/basecommand.js

@@ -69,6 +69,43 @@ export default class BaseCommand extends Command {
 	_checkEnabled() {
 		return this._stack.length > 0;
 	}
+
+	/**
+	 * Restores {@link engine.model.Document#selection document selection} state after a batch has been undone.
+	 *
+	 * @protected
+	 * @param {Array.<engine.model.Range>} ranges Ranges to be restored.
+	 * @param {Boolean} isBackward Flag describing if restored range was selected forward or backward.
+	 */
+	_restoreSelection( ranges, isBackward, deltas ) {
+		const document = this.editor.document;
+
+		// This will keep the transformed selection ranges.
+		const selectionRanges = [];
+
+		// Transform all ranges from the restored selection.
+		for ( let range of ranges ) {
+			const transformedRanges = transformSelectionRange( range, deltas );
+
+			// 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 transformedRange = transformedRanges.find(
+				( range ) => range.start.root != document.graveyard
+			);
+
+			// `transformedRange` might be `undefined` if transformed range ended up in graveyard.
+			if ( transformedRange ) {
+				selectionRanges.push( transformedRange );
+			}
+		}
+
+		// `selectionRanges` may be empty if all ranges ended up in graveyard. If that is the case, do not restore selection.
+		if ( selectionRanges.length ) {
+			document.selection.setRanges( selectionRanges, isBackward );
+		}
+	}
 }
 
 // Performs a transformation of delta set `setToTransform` by given delta set `setToTransformBy`.
@@ -92,3 +129,77 @@ export function transformDelta( setToTransform, setToTransformBy, isStrong = fal
 
 	return results;
 }
+
+// Transforms given range `range` by deltas from `document` history, starting from a delta with given `baseVersion`.
+// Returns an array containing one or more ranges, which are result of the transformation.
+function transformSelectionRange( range, deltas ) {
+	// The range will be transformed by history deltas that happened after the selection got stored.
+	// Note, that at this point, the document history is already updated by undo command execution. We will
+	// not transform the range by deltas that got undone or their reversing counterparts.
+	let transformed = transformRangesByDeltas( [ range ], deltas );
+
+	// After `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.
+	// First, we have to sort those ranges because they don't have to be in an order.
+	transformed.sort( ( a, b ) => a.start.isBefore( b.start ) ? -1 : 1 );
+
+	// Then, we check if two consecutive ranges are touching.
+	for ( let i = 1 ; i < transformed.length; i++ ) {
+		let a = transformed[ i - 1 ];
+		let b = transformed[ i ];
+
+		if ( a.end.isTouching( b.start ) ) {
+			a.end = b.end;
+			transformed.splice( i, 1 );
+			i--;
+		}
+	}
+
+	return transformed;
+}
+
+// Transforms given set of `ranges` by given set of `deltas`. Returns transformed `ranges`.
+export function transformRangesByDeltas( ranges, deltas ) {
+	for ( let delta of deltas ) {
+		for ( let operation of delta.operations ) {
+			// We look through all operations from all deltas.
+
+			for ( let i = 0; i < ranges.length; i++ ) {
+				// We transform every range by every operation.
+				let result;
+
+				switch ( operation.type ) {
+					case 'insert':
+						result = ranges[ i ]._getTransformedByInsertion(
+							operation.position,
+							operation.nodes.maxOffset,
+							true
+						);
+						break;
+
+					case 'move':
+					case 'remove':
+					case 'reinsert':
+						result = ranges[ i ]._getTransformedByMove(
+							operation.sourcePosition,
+							operation.targetPosition,
+							operation.howMany,
+							true
+						);
+						break;
+				}
+
+				// If we have a transformation result, we substitute transformed range with it in `transformed` array.
+				// Keep in mind that the result is an array and may contain multiple ranges.
+				if ( result ) {
+					ranges.splice( i, 1, ...result );
+
+					// Fix iterator.
+					i = i + result.length - 1;
+				}
+			}
+		}
+	}
+
+	return ranges;
+}

+ 12 - 15
packages/ckeditor5-undo/src/redocommand.js

@@ -31,8 +31,19 @@ export default class RedoCommand extends BaseCommand {
 		// All changes have to be done in one `enqueueChanges` callback so other listeners will not
 		// step between consecutive deltas, or won't do changes to the document before selection is properly restored.
 		this.editor.document.enqueueChanges( () => {
+			const lastDelta = item.batch.deltas[ item.batch.deltas.length - 1 ];
+			const nextBaseVersion = lastDelta.baseVersion + lastDelta.operations.length;
+
+			// Selection state is from the moment after undo happened. It needs to be transformed by all the deltas
+			// that happened after the selection state got saved. Unfortunately it is tricky, because those deltas
+			// are already compressed in the history (they are removed).
+			// Because of that we will transform the selection only by non-redo deltas
+			const deltas = Array.from( this.editor.document.history.getDeltas( nextBaseVersion ) ).filter( ( delta ) => {
+				return !this._createdBatches.has( delta.batch );
+			} );
+
+			this._restoreSelection( item.selection.ranges, item.selection.isBackward, deltas );
 			this._redo( item.batch );
-			this._restoreSelection( item.selection.ranges, item.selection.isBackward );
 		} );
 
 		this.refreshState();
@@ -44,8 +55,6 @@ export default class RedoCommand extends BaseCommand {
 	 *
 	 * @private
 	 * @param {engine.model.Batch} storedBatch Batch, which deltas will be reversed, transformed and applied.
-	 * @param {engine.model.Batch} redoingBatch Batch that will contain transformed and applied deltas from `storedBatch`.
-	 * @param {engine.model.Document} document Document that is operated on by the command.
 	 */
 	_redo( storedBatch ) {
 		const document = this.editor.document;
@@ -93,16 +102,4 @@ export default class RedoCommand extends BaseCommand {
 			}
 		}
 	}
-
-	/**
-	 * Restores {@link engine.model.Document#selection document selection} state after a batch has been re-done. This
-	 * is a helper method for {@link undo.RedoCommand#_doExecute}.
-	 *
-	 * @private
-	 * @param {Array.<engine.model.Range>} ranges Ranges to be restored.
-	 * @param {Boolean} isBackward Flag describing if restored range was selected forward or backward.
-	 */
-	_restoreSelection( ranges, isBackward ) {
-		this.editor.document.selection.setRanges( ranges, isBackward );
-	}
 }

+ 8 - 127
packages/ckeditor5-undo/src/undocommand.js

@@ -4,7 +4,7 @@
  */
 
 import BaseCommand from './basecommand.js';
-import { transformDelta as transformDelta } from './basecommand.js';
+import { transformDelta, transformRangesByDeltas } from './basecommand.js';
 
 /**
  * Undo command stores {@link engine.model.Batch batches} applied to the {@link engine.model.Document document}
@@ -36,11 +36,14 @@ export default class UndoCommand extends BaseCommand {
 		// All changes has to be done in one `enqueueChanges` callback so other listeners will not
 		// step between consecutive deltas, or won't do changes to the document before selection is properly restored.
 		this.editor.document.enqueueChanges( () => {
-			this._undo( item.batch );
-			this._restoreSelection( item.selection.ranges, item.selection.isBackward, item.batch.baseVersion );
+			const undoingBatch = this._undo( item.batch );
+
+			const deltas = this.editor.document.history.getDeltas( item.batch.baseVersion );
+			this._restoreSelection( item.selection.ranges, item.selection.isBackward, deltas );
+
+			this.fire( 'revert', item.batch, undoingBatch );
 		} );
 
-		this.fire( 'revert', item.batch );
 		this.refreshState();
 	}
 
@@ -68,8 +71,6 @@ export default class UndoCommand extends BaseCommand {
 	 *
 	 * @private
 	 * @param {engine.model.Batch} batchToUndo Batch, which deltas will be reversed, transformed and applied.
-	 * @param {engine.model.Batch} undoingBatch Batch that will contain transformed and applied deltas from `batchToUndo`.
-	 * @param {engine.model.Document} document Document that is operated on by the command.
 	 */
 	_undo( batchToUndo ) {
 		const document = this.editor.document;
@@ -172,127 +173,7 @@ export default class UndoCommand extends BaseCommand {
 				history.updateDelta( Number( historyBaseVersion ), updatedHistoryDeltas[ historyBaseVersion ] );
 			}
 		}
-	}
-
-	/**
-	 * Restores {@link engine.model.Document#selection document selection} state after a batch has been undone. This
-	 * is a helper method for {@link undo.UndoCommand#_doExecute}.
-	 *
-	 * @private
-	 * @param {Array.<engine.model.Range>} ranges Ranges to be restored.
-	 * @param {Boolean} isBackward Flag describing if restored range was selected forward or backward.
-	 * @param {Number} baseVersion
-	 * @param {engine.model.Document} document Document that is operated on by the command.
-	 */
-	_restoreSelection( ranges, isBackward, baseVersion ) {
-		const document = this.editor.document;
-
-		// This will keep the transformed selection ranges.
-		const selectionRanges = [];
-
-		// Transform all ranges from the restored selection.
-		for ( let range of ranges ) {
-			const transformedRanges = transformSelectionRange( range, baseVersion, document );
-
-			// 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 transformedRange = transformedRanges.find(
-				( range ) => range.start.root != document.graveyard
-			);
-
-			// `transformedRange` might be `undefined` if transformed range ended up in graveyard.
-			if ( transformedRange ) {
-				selectionRanges.push( transformedRange );
-			}
-		}
 
-		// `selectionRanges` may be empty if all ranges ended up in graveyard. If that is the case, do not restore selection.
-		if ( selectionRanges.length ) {
-			document.selection.setRanges( selectionRanges, isBackward );
-		}
+		return undoingBatch;
 	}
 }
-
-// Transforms given range `range` by deltas from `document` history, starting from a delta with given `baseVersion`.
-// Returns an array containing one or more ranges, which are result of the transformation.
-function transformSelectionRange( range, baseVersion, document ) {
-	const history = document.history;
-
-	// We create `transformed` array. At the beginning it will have only the original range.
-	// During transformation the original range will change or even break into smaller ranges.
-	// After the range is broken into two ranges, we have to transform both of those ranges separately.
-	// For that reason, we keep all transformed ranges in one array and operate on it.
-	let transformed = [ range ];
-
-	// The ranges will be transformed by history deltas that happened after the selection got stored.
-	// Note, that at this point, the document history is already updated by undo command execution. We will
-	// not transform the range by deltas that got undone or their reversing counterparts.
-	transformed = transformRangesByDeltas( transformed, history.getDeltas( baseVersion ) );
-
-	// After `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.
-	// First, we have to sort those ranges because they don't have to be in an order.
-	transformed.sort( ( a, b ) => a.start.isBefore( b.start ) ? -1 : 1 );
-
-	// Then, we check if two consecutive ranges are touching.
-	for ( let i = 1 ; i < transformed.length; i++ ) {
-		let a = transformed[ i - 1 ];
-		let b = transformed[ i ];
-
-		if ( a.end.isTouching( b.start ) ) {
-			a.end = b.end;
-			transformed.splice( i, 1 );
-			i--;
-		}
-	}
-
-	return transformed;
-}
-
-// Transforms given set of `ranges` by given set of `deltas`. Returns transformed `ranges`.
-function transformRangesByDeltas( ranges, deltas ) {
-	for ( let delta of deltas ) {
-		for ( let operation of delta.operations ) {
-			// We look through all operations from all deltas.
-
-			for ( let i = 0; i < ranges.length; i++ ) {
-				// We transform every range by every operation.
-				let result;
-
-				switch ( operation.type ) {
-					case 'insert':
-						result = ranges[ i ]._getTransformedByInsertion(
-							operation.position,
-							operation.nodes.maxOffset,
-							true
-						);
-						break;
-
-					case 'move':
-					case 'remove':
-					case 'reinsert':
-						result = ranges[ i ]._getTransformedByMove(
-							operation.sourcePosition,
-							operation.targetPosition,
-							operation.howMany,
-							true
-						);
-						break;
-				}
-
-				// If we have a transformation result, we substitute transformed range with it in `transformed` array.
-				// Keep in mind that the result is an array and may contain multiple ranges.
-				if ( result ) {
-					ranges.splice( i, 1, ...result );
-
-					// Fix iterator.
-					i = i + result.length - 1;
-				}
-			}
-		}
-	}
-
-	return ranges;
-}

+ 6 - 5
packages/ckeditor5-undo/src/undoengine.js

@@ -65,13 +65,10 @@ export default class UndoEngine extends Feature {
 			if ( this._batchRegistry.has( batch ) || batch.type == 'transparent' ) {
 				return;
 			} else {
-				if ( this._undoCommand._createdBatches.has( batch ) ) {
-					// If this batch comes from `undoCommand`, add it to `redoCommand` stack.
-					this._redoCommand.addBatch( batch );
-				} else if ( this._redoCommand._createdBatches.has( batch ) ) {
+				if ( this._redoCommand._createdBatches.has( batch ) ) {
 					// If this batch comes from `redoCommand`, add it to `undoCommand` stack.
 					this._undoCommand.addBatch( batch );
-				} else {
+				} else if ( !this._undoCommand._createdBatches.has( batch ) ) {
 					// A default batch - these are new changes in the document, not introduced by undo feature.
 					// Add them to `undoCommand` stack and clear `redoCommand` stack.
 					this._undoCommand.addBatch( batch );
@@ -82,5 +79,9 @@ export default class UndoEngine extends Feature {
 			// Add the batch to the registry so it will not be processed again.
 			this._batchRegistry.add( batch );
 		} );
+
+		this.listenTo( this._undoCommand, 'revert', ( evt, undoneBatch, undoingBatch ) => {
+			this._redoCommand.addBatch( undoingBatch );
+		} );
 	}
 }

+ 67 - 33
packages/ckeditor5-undo/tests/redocommand.js

@@ -37,10 +37,10 @@ describe( 'RedoCommand', () => {
 			undo = new UndoCommand( editor );
 
 			// Simple integration with undo.
-			doc.on( 'change', ( evt, type, data, batch ) => {
-				if ( undo._createdBatches.has( batch ) && !batches.has( batch ) ) {
-					redo.addBatch( batch );
-					batches.add( batch );
+			undo.on( 'revert', ( evt, undoneBatch, undoingBatch ) => {
+				if ( !batches.has( undoingBatch ) ) {
+					redo.addBatch( undoingBatch );
+					batches.add( undoingBatch );
 				}
 			} );
 
@@ -113,9 +113,9 @@ describe( 'RedoCommand', () => {
 		} );
 
 		it( 'should redo series of batches undone by undo command', () => {
-			undo._execute( batch2 );
-			undo._execute( batch1 );
-			undo._execute( batch0 );
+			undo._execute();
+			undo._execute();
+			undo._execute();
 
 			redo._execute();
 			// Should be like after applying `batch0`:
@@ -123,25 +123,25 @@ describe( 'RedoCommand', () => {
 			 [root]
 			 - f
 			 - o
-			 - {o
-			 - b}
+			 - o
+			 - b
 			 - a
-			 - r
+			 - r{}
 			 */
 			expect( getText( root ) ).to.equal( 'foobar' );
 			expect( Array.from( root.getChildren() ).find( node => node.hasAttribute( 'key' ) ) ).to.be.undefined;
 
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( 2, 4 ) ) ).to.be.true;
-			expect( editor.document.selection.isBackward ).to.be.true;
+			expect( editor.document.selection.getRanges().next().value.isEqual( r( 6, 6 ) ) ).to.be.true;
+			expect( editor.document.selection.isBackward ).to.be.false;
 
 			redo._execute();
 			// Should be like after applying `batch1`:
 			/*
 			 [root]
 			 - f
-			 - {o
-			 - o} (key: value)
-			 - b (key: value)
+			 - o
+			 - {o (key: value)
+			 - b} (key: value)
 			 - a
 			 - r
 			 */
@@ -149,8 +149,8 @@ describe( 'RedoCommand', () => {
 			expect( itemAt( root, 2 ).getAttribute( 'key' ) ).to.equal( 'value' );
 			expect( itemAt( root, 3 ).getAttribute( 'key' ) ).to.equal( 'value' );
 
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( 1, 3 ) ) ).to.be.true;
-			expect( editor.document.selection.isBackward ).to.be.false;
+			expect( editor.document.selection.getRanges().next().value.isEqual( r( 2, 4 ) ) ).to.be.true;
+			expect( editor.document.selection.isBackward ).to.be.true;
 
 			redo._execute();
 			// Should be like after applying `batch2`:
@@ -182,38 +182,72 @@ describe( 'RedoCommand', () => {
 			 - b (key: value)
 			 - a
 			 - r
-			 - {o
-			 - o} (key: value)
+			 - o
+			 - o{} (key: value)
 			 */
 			expect( getText( root ) ).to.equal( 'fbaroo' );
 			expect( itemAt( root, 1 ).getAttribute( 'key' ) ).to.equal( 'value' );
 			expect( itemAt( root, 5 ).getAttribute( 'key' ) ).to.equal( 'value' );
 
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( 4, 6 ) ) ).to.be.true;
+			expect( editor.document.selection.getRanges().next().value.isEqual( r( 6, 6 ) ) ).to.be.true;
 			expect( editor.document.selection.isBackward ).to.be.false;
 		} );
 
-		/*
-		it( 'should transform redo batch by changes written in history that happened after undo but before redo', () => {
-			// Undo moving "oo" to the end of string. Now it is "foobar".
+		it( 'should redo batch selectively undone by undo command #2', () => {
+			undo._execute( batch1 );
 			undo._execute( batch2 );
+			redo._execute();
+			redo._execute();
+
+			// Should be back to original state:
+			/*
+			 [root]
+			 - f
+			 - {b} (key: value)
+			 - a
+			 - r
+			 - o
+			 - o (key: value)
+			 */
+			expect( getText( root ) ).to.equal( 'fbaroo' );
+			expect( itemAt( root, 1 ).getAttribute( 'key' ) ).to.equal( 'value' );
+			expect( itemAt( root, 5 ).getAttribute( 'key' ) ).to.equal( 'value' );
+
+			expect( editor.document.selection.getRanges().next().value.isEqual( r( 1, 2 ) ) ).to.be.true;
+			expect( editor.document.selection.isBackward ).to.be.true;
+		} );
+
+		it( 'should transform redo batch by changes written in history that happened after undo but before redo #2', () => {
+			// Now it is "fBaroO".
+			// Undo moving "oo" to the end of string. Now it is "foOBar". Capitals mean set attribute.
+			undo._execute();
 
 			// Remove "ar".
-			editor.document.selection.setRanges( [ r( 4, 6 ) ] );
 			doc.batch().remove( r( 4, 6 ) );
-			editor.document.selection.setRanges( [ r( 4, 4 ) ] );
 
-			// Redo moving "oo" to the end of string. It should be "fboo".
+			// Undo setting attribute on "ob". Now it is "foob".
+			undo._execute();
+
+			// Append "xx" at the beginning. Now it is "xxfoob".
+			doc.batch().insert( p( 0 ), 'xx' );
+
+			// Redo setting attribute on "ob". Now it is "xxfoOB".
 			redo._execute();
 
-			expect( getText( root ) ).to.equal( 'fboo' );
-			expect( itemAt( root, 1 ).getAttribute( 'key' ) ).to.equal( 'value' );
-			expect( itemAt( root, 3 ).getAttribute( 'key' ) ).to.equal( 'value' );
+			expect( getText( root ) ).to.equal( 'xxfoob' );
+			expect( itemAt( root, 4 ).getAttribute( 'key' ) ).to.equal( 'value' );
+			expect( itemAt( root, 5 ).getAttribute( 'key' ) ).to.equal( 'value' );
+			expect( editor.document.selection.getFirstRange().isEqual( r( 4, 6 ) ) ).to.be.true;
+			expect( editor.document.selection.isBackward ).to.be.true;
+
+			// Redo moving "oo". Now it is "xxfBoO". Selection is expected to be on just moved "oO".
+			redo._execute();
 
-			// Selection after redo is not working properly if there was another batch in-between.
-			// Thankfully this will be very rare situation outside of OT, because normally an applied batch
-			// would reset the redo stack so you won't be able to redo. #12
+			expect( getText( root ) ).to.equal( 'xxfboo' );
+			expect( itemAt( root, 3 ).getAttribute( 'key' ) ).to.equal( 'value' );
+			expect( itemAt( root, 5 ).getAttribute( 'key' ) ).to.equal( 'value' );
+			expect( editor.document.selection.getFirstRange().isEqual( r( 4, 6 ) ) ).to.be.true;
+			expect( editor.document.selection.isBackward ).to.be.false;
 		} );
-		*/
 	} );
 } );

+ 9 - 9
packages/ckeditor5-undo/tests/undocommand.js

@@ -125,7 +125,7 @@ describe( 'UndoCommand', () => {
 			expect( itemAt( root, 0 ).getAttribute( 'key' ) ).to.equal( 'value' );
 			expect( itemAt( root, 5 ).getAttribute( 'key' ) ).to.equal( 'value' );
 
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( 0, 3 ) ) ).to.be.true;
+			expect( editor.document.selection.getFirstRange().isEqual( r( 0, 3 ) ) ).to.be.true;
 			expect( editor.document.selection.isBackward ).to.be.false;
 
 			undo._execute();
@@ -145,7 +145,7 @@ describe( 'UndoCommand', () => {
 			expect( itemAt( root, 2 ).getAttribute( 'key' ) ).to.equal( 'value' );
 			expect( itemAt( root, 3 ).getAttribute( 'key' ) ).to.equal( 'value' );
 
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( 1, 3 ) ) ).to.be.true;
+			expect( editor.document.selection.getFirstRange().isEqual( r( 1, 3 ) ) ).to.be.true;
 			expect( editor.document.selection.isBackward ).to.be.false;
 
 			undo._execute();
@@ -165,7 +165,7 @@ describe( 'UndoCommand', () => {
 			expect( itemAt( root, 2 ).hasAttribute( 'key' ) ).to.be.false;
 			expect( itemAt( root, 3 ).hasAttribute( 'key' ) ).to.be.false;
 
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( 2, 4 ) ) ).to.be.true;
+			expect( editor.document.selection.getFirstRange().isEqual( r( 2, 4 ) ) ).to.be.true;
 			expect( editor.document.selection.isBackward ).to.be.true;
 
 			undo._execute();
@@ -176,7 +176,7 @@ describe( 'UndoCommand', () => {
 			 */
 
 			expect( root.childCount ).to.equal( 0 );
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( 0, 0 ) ) ).to.be.true;
+			expect( editor.document.selection.getFirstRange().isEqual( r( 0, 0 ) ) ).to.be.true;
 		} );
 
 		it( 'should revert changes done by deltas from given batch, if parameter was passed (test: revert set attribute)', () => {
@@ -203,7 +203,7 @@ describe( 'UndoCommand', () => {
 
 			// Selection is only partially restored because the range got broken.
 			// The selection would have to best on letter "b" and letter "o", but it is set only on letter "b".
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( [ 0, 0 ], [ 0, 1 ] ) ) ).to.be.true;
+			expect( editor.document.selection.getFirstRange().isEqual( r( [ 0, 0 ], [ 0, 1 ] ) ) ).to.be.true;
 			expect( editor.document.selection.isBackward ).to.be.true;
 		} );
 
@@ -221,7 +221,7 @@ describe( 'UndoCommand', () => {
 			expect( root.childCount ).to.equal( 1 );
 			expect( itemAt( root, 0 ).name ).to.equal( 'p' );
 
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( 0, 0 ) ) ).to.be.true;
+			expect( editor.document.selection.getFirstRange().isEqual( r( 0, 0 ) ) ).to.be.true;
 			expect( editor.document.selection.isBackward ).to.be.false;
 
 			undo._execute( batch1 );
@@ -233,7 +233,7 @@ describe( 'UndoCommand', () => {
 			expect( itemAt( root, 0 ).name ).to.equal( 'p' );
 
 			// Operations for undoing that batch were working on graveyard so document selection should not change.
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( 0, 0 ) ) ).to.be.true;
+			expect( editor.document.selection.getFirstRange().isEqual( r( 0, 0 ) ) ).to.be.true;
 			expect( editor.document.selection.isBackward ).to.be.false;
 
 			expect( doc.graveyard.getChild( 0 ).maxOffset ).to.equal( 6 );
@@ -247,7 +247,7 @@ describe( 'UndoCommand', () => {
 			expect( root.maxOffset ).to.equal( 0 );
 
 			// Once again transformed range ends up in the graveyard.
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( 0, 0 ) ) ).to.be.true;
+			expect( editor.document.selection.getFirstRange().isEqual( r( 0, 0 ) ) ).to.be.true;
 			expect( editor.document.selection.isBackward ).to.be.false;
 		} );
 	} );
@@ -339,7 +339,7 @@ describe( 'UndoCommand', () => {
 			// After undo-attr: acdbef <--- "cdb" should be selected, it would look weird if only "cd" or "b" is selected
 			// but the whole unbroken part "cdb" changed attribute.
 			expect( getCaseText( root ) ).to.equal( 'adbcef' );
-			expect( editor.document.selection.getRanges().next().value.isEqual( r( 1, 4 ) ) ).to.be.true;
+			expect( editor.document.selection.getFirstRange().isEqual( r( 1, 4 ) ) ).to.be.true;
 		} );
 
 		it( 'does nothing (and not crashes) if delta to undo is no longer in history', () => {

+ 2 - 4
packages/ckeditor5-undo/tests/undoengine.js

@@ -63,13 +63,11 @@ describe( 'UndoEngine', () => {
 		expect( undo._redoCommand.clearStack.called ).to.be.false;
 	} );
 
-	it( 'should add a batch to redo command, if it\'s type is undo', () => {
+	it( 'should add a batch to redo command on undo revert event', () => {
 		sinon.spy( undo._redoCommand, 'addBatch' );
 		sinon.spy( undo._redoCommand, 'clearStack' );
 
-		undo._undoCommand._createdBatches.add( batch );
-
-		batch.insert( new Position( root, [ 0 ] ), 'foobar' );
+		undo._undoCommand.fire( 'revert', null, batch );
 
 		expect( undo._redoCommand.addBatch.calledOnce ).to.be.true;
 		expect( undo._redoCommand.clearStack.called ).to.be.false;