diff --git a/src/undocommand.js b/src/undocommand.js index 3e3847c..bdc739f 100644 --- a/src/undocommand.js +++ b/src/undocommand.js @@ -14,8 +14,8 @@ import Command from '../command/command.js'; */ export default class UndoCommand extends Command { /** - * @see core.command.Command - * @param {core.Editor} editor + * @see engine.command.Command + * @param {engine.Editor} editor */ constructor( editor ) { super( editor ); @@ -24,18 +24,29 @@ export default class UndoCommand extends Command { * Batches which are saved by the command. They can be reversed. * * @private - * @member {Array.} core.command.UndoCommand#_batchStack + * @member {Array.} undo.UndoCommand#_batchStack */ this._batchStack = []; + + /** + * Stores the selection ranges for each batch and use them to recreate selection after undo. + * + * @private + * @member {Map.} undo.UndoCommand#_batchSelection + */ + this._batchSelection = new Map(); } /** * Stores a batch in the command. Stored batches can be then reverted. * - * @param {core.teeModel.Batch} batch Batch to add. + * @param {engine.treeModel.Batch} batch Batch to add. + * @param {Array.} selectionRanges All ranges that were in the selection when batch got + * created. */ addBatch( batch ) { this._batchStack.push( batch ); + this._batchSelection.set( batch, Array.from( this.editor.document.selection.getRanges() ) ); } /** @@ -43,6 +54,7 @@ export default class UndoCommand extends Command { */ clearStack() { this._batchStack = []; + this._batchSelection.clear(); } /** @@ -56,7 +68,7 @@ export default class UndoCommand extends Command { } /** - * Executes the command: reverts a {@link core.treeModel.Batch batch} added to the command's stack, + * Executes the command: reverts a {@link engine.treeModel.Batch batch} added to the command's stack, * applies it on the document and removes the batch from the stack. * * Fires `undo` event with reverted batch as a parameter. @@ -68,11 +80,13 @@ export default class UndoCommand extends Command { _doExecute( batchIndex ) { batchIndex = this._batchStack[ batchIndex ] ? batchIndex : this._batchStack.length - 1; + // Get the batch to undo. const undoBatch = this._batchStack.splice( batchIndex, 1 )[ 0 ]; const undoDeltas = undoBatch.deltas.slice(); - + // Deltas have to be applied in reverse order, so if batch did A B C, it has to do reversed C, reversed B, reversed A. undoDeltas.reverse(); + // Reverse the deltas from the batch, transform them, apply them. for ( let undoDelta of undoDeltas ) { const undoDeltaReversed = undoDelta.getReversed(); const updatedDeltas = this.editor.document.history.getTransformedDelta( undoDeltaReversed ); @@ -84,6 +98,95 @@ export default class UndoCommand extends Command { } } + // Take all selection ranges that were stored with undone batch. + const ranges = this._batchSelection.get( undoBatch ); + + // The ranges will be transformed by deltas from history that took place + // after the selection got stored. + const deltas = this.editor.document.history.getDeltas( undoBatch.deltas[ 0 ].baseVersion ); + + // This will keep the transformed ranges. + const transformedRanges = []; + + for ( let originalRange of ranges ) { + // 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 = [ originalRange ]; + + for ( let delta of deltas ) { + for ( let operation of delta.operations ) { + // We look through all operations from all deltas. + + for ( let t = 0; t < transformed.length; t++ ) { + // We transform every range by every operation. + // We keep current state of transformation in `transformed` array and update it. + let result; + + switch ( operation.type ) { + case 'insert': + result = transformed[ t ].getTransformedByInsertion( operation.position, operation.nodeList.length, true ); + break; + + case 'move': + case 'remove': + case 'reinsert': + result = transformed[ t ].getTransformedByMove( operation.sourcePosition, operation.movedRangeStart, operation.howMany, true ); + break; + } + + // If we have a transformation result, we substitute it in `transformed` array with + // the range that got transformed. Keep in mind that the result is an array + // and may contain multiple ranges. + if ( result ) { + transformed.splice( t, 1, ...result ); + + // Fix iterator. + t = t + result.length - 1; + } + } + } + } + + // After `originalRange` 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. + // Let's do this. 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. We can do it pair by pair + // in one dimensional loop because ranges are sorted. + 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--; + } + } + + // For each `originalRange` from `ranges`, we take only one transformed range. + // This is because we want to prevent situation where single-range selection + // got transformed to mulit-range selection. We will take the first range that + // is not in the graveyard. + const transformedRange = transformed.find( + ( range ) => range.start.root != this.editor.document.graveyard + ); + + if ( transformedRange ) { + transformedRanges.push( transformedRange ); + } + } + + // `transformedRanges` may be empty if all ranges ended up in graveyard. + // If that is the case, do not restore selection. + if ( transformedRanges.length ) { + this.editor.document.selection.setRanges( transformedRanges ); + } + this.fire( 'undo', undoBatch ); } } diff --git a/tests/undocommand.js b/tests/undocommand.js index 71ad5d7..ae83214 100644 --- a/tests/undocommand.js +++ b/tests/undocommand.js @@ -6,8 +6,9 @@ 'use strict'; import Editor from '/ckeditor5/editor.js'; -import Position from '/ckeditor5/core/treemodel/position.js'; -import Range from '/ckeditor5/core/treemodel/range.js'; +import ModelDocument from '/ckeditor5/engine/treemodel/document.js'; +import Range from '/ckeditor5/engine/treemodel/range.js'; +import Position from '/ckeditor5/engine/treemodel/position.js'; import UndoCommand from '/ckeditor5/undo/undocommand.js'; let element, editor, doc, root, undo; @@ -19,7 +20,9 @@ beforeEach( () => { editor = new Editor( element ); undo = new UndoCommand( editor ); - doc = editor.document; + doc = new ModelDocument(); + editor.document = doc; + root = doc.createRoot( 'root' ); } ); @@ -35,12 +38,18 @@ describe( 'UndoCommand', () => { } ); describe( 'addBatch', () => { - it( 'should add a batch to command stack', () => { + it( 'should add a batch and selection to command stack', () => { + root.appendChildren( 'foobar' ); + + editor.document.selection.setRanges( Range.createFromElement( root ) ); + const batch = doc.batch(); undo.addBatch( batch ); expect( undo._batchStack.length ).to.equal( 1 ); expect( undo._batchStack[ 0 ] ).to.equal( batch ); + + expect( undo._batchSelection.get( batch ) ).to.deep.equal( Array.from( editor.document.selection.getRanges() ) ); } ); } ); @@ -50,6 +59,7 @@ describe( 'UndoCommand', () => { undo.clearStack(); expect( undo._batchStack.length ).to.equal( 0 ); + expect( undo._batchSelection.size ).to.equal( 0 ); } ); } ); @@ -66,7 +76,7 @@ describe( 'UndoCommand', () => { } ); describe( '_doExecute', () => { - const p = pos => new Position( root, [ pos ] ); + const p = pos => new Position( root, [].concat( pos ) ); const r = ( a, b ) => new Range( p( a ), p( b ) ); let batch0, batch1, batch2, batch3; @@ -74,8 +84,12 @@ describe( 'UndoCommand', () => { beforeEach( () => { /* [root] + - {} */ - batch0 = doc.batch().insert( p( 0 ), 'foobar' ); + editor.document.selection.setRanges( [ r( 0, 0 ) ] ); + batch0 = doc.batch(); + undo.addBatch( batch0 ); + batch0.insert( p( 0 ), 'foobar' ); /* [root] - f @@ -83,84 +97,92 @@ describe( 'UndoCommand', () => { - o - b - a - - r + - r{} */ - batch1 = doc.batch().setAttr( 'key', 'value', r( 2, 4 ) ); + editor.document.selection.setRanges( [ r( 2, 4 ) ] ); + batch1 = doc.batch(); + undo.addBatch( batch1 ); + batch1.setAttr( 'key', 'value', r( 2, 4 ) ); /* [root] - f - o - - o {key: value} - - b {key: value} + - {o (key: value) + - b} (key: value) - a - r */ - batch2 = doc.batch().move( r( 1, 3 ), p( 6 ) ); + editor.document.selection.setRanges( [ r( 1, 3 ) ] ); + batch2 = doc.batch(); + undo.addBatch( batch2 ); + batch2.move( r( 1, 3 ), p( 6 ) ); /* [root] - f - - b {key: value} + - b (key: value) - a - r - - o - - o {key: value} + - {o + - o} (key: value) */ - batch3 = doc.batch().wrap( r( 1, 4 ), 'p' ); + editor.document.selection.setRanges( [ r( 1, 4 ) ] ); + batch3 = doc.batch(); + undo.addBatch( batch3 ); + batch3.wrap( r( 1, 4 ), 'p' ); /* [root] - f - [p] - - b {key: value} + - {b (key: value) - a - - r + - r} - o - - o {key: value} + - o (key: value) */ + editor.document.selection.setRanges( [ r( 0, 1 ) ] ); batch2.move( r( 0, 1 ), p( 3 ) ); /* [root] - [p] - - b {key: value} + - b (key: value) - a - r - o - f - - o {key: value} + - o{} (key: value) */ - - undo.addBatch( batch0 ); - undo.addBatch( batch1 ); - undo.addBatch( batch2 ); - undo.addBatch( batch3 ); + editor.document.selection.setRanges( [ r( 4, 4 ) ] ); } ); it( 'should revert changes done by deltas from the batch that was most recently added to the command stack', () => { undo._doExecute(); - // Wrap is removed: + // Selection is restored. Wrap is removed: /* [root] - - b {key: value} + - {b (key: value) - a - - r + - r} - o - f - - o {key: value} + - o (key: value) */ expect( Array.from( root._children._nodes.map( node => node.text ) ).join( '' ) ).to.equal( 'barofo' ); expect( root.getChild( 0 ).getAttribute( 'key' ) ).to.equal( 'value' ); expect( root.getChild( 5 ).getAttribute( 'key' ) ).to.equal( 'value' ); + expect( editor.document.selection.getRanges().next().value.isEqual( r( 0, 3 ) ) ).to.be.true; + undo._doExecute(); // Two moves are removed: /* [root] - f - - o - - o {key: value} - - b {key: value} + - {o + - o} (key: value) + - b (key: value) - a - r */ @@ -169,6 +191,8 @@ describe( 'UndoCommand', () => { expect( root.getChild( 2 ).getAttribute( 'key' ) ).to.equal( 'value' ); expect( root.getChild( 3 ).getAttribute( 'key' ) ).to.equal( 'value' ); + expect( editor.document.selection.getRanges().next().value.isEqual( r( 1, 3 ) ) ).to.be.true; + undo._doExecute(); // Set attribute is undone: @@ -176,8 +200,8 @@ describe( 'UndoCommand', () => { [root] - f - o - - o - - b + - {o + - b} - a - r */ @@ -186,6 +210,8 @@ describe( 'UndoCommand', () => { expect( root.getChild( 2 ).hasAttribute( 'key' ) ).to.be.false; expect( root.getChild( 3 ).hasAttribute( 'key' ) ).to.be.false; + expect( editor.document.selection.getRanges().next().value.isEqual( r( 2, 4 ) ) ).to.be.true; + undo._doExecute(); // Insert is undone: @@ -194,9 +220,12 @@ describe( 'UndoCommand', () => { */ expect( root.getChildCount() ).to.equal( 0 ); + expect( editor.document.selection.getRanges().next().value.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)', () => { + // editor.document.selection.setRanges( [ r( [ 0, 1 ], [ 0, 2 ] ) ] ); + undo._doExecute( 1 ); // Remove attribute: /* @@ -215,6 +244,10 @@ describe( 'UndoCommand', () => { expect( root.getChild( 0 ).getChild( 0 ).hasAttribute( 'key' ) ).to.be.false; expect( root.getChild( 2 ).hasAttribute( 'key' ) ).to.be.false; expect( root.getChild( 3 ).hasAttribute( 'key' ) ).to.be.false; + + // 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; } ); it( 'should revert changes done by deltas from given batch, if parameter was passed (test: revert insert foobar)', () => { @@ -231,6 +264,10 @@ describe( 'UndoCommand', () => { expect( root.getChildCount() ).to.equal( 1 ); expect( root.getChild( 0 ).name ).to.equal( 'p' ); + // Because P element was inserted in the middle of removed text and it was not removed, + // the selection is set after it. + expect( editor.document.selection.getRanges().next().value.isEqual( r( 1, 1 ) ) ).to.be.true; + undo._doExecute( 0 ); // Remove attributes. // This does nothing in the `root` because attributes were set on nodes that already got removed. @@ -239,10 +276,11 @@ describe( 'UndoCommand', () => { expect( root.getChildCount() ).to.equal( 1 ); expect( root.getChild( 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( 1, 1 ) ) ).to.be.true; + expect( doc.graveyard.getChildCount() ).to.equal( 6 ); - // TODO: This one does not work because nodes are moved inside graveyard... - // TODO: Perfect situation would be if the nodes are moved to graveyard in the order they are in original tree. - // expect( Array.from( doc.graveyard._children._nodes.map( node => node.text ) ).join( '' ) ).to.equal( 'barofo' ); + for ( let char of doc.graveyard._children ) { expect( char.hasAttribute( 'key' ) ).to.be.false; } @@ -250,6 +288,11 @@ describe( 'UndoCommand', () => { // Let's undo wrapping. This should leave us with empty root. undo._doExecute( 1 ); expect( root.getChildCount() ).to.equal( 0 ); + + // Once again transformed range ends up in the graveyard. + // So we do not restore it. But since Selection is a LiveRange itself it will update + // because the node before it (P element) got removed. + expect( editor.document.selection.getRanges().next().value.isEqual( r( 0, 0 ) ) ).to.be.true; } ); it( 'should fire undo event with the undone batch', () => {