diff --git a/src/model/differ.js b/src/model/differ.js
index ed1e8c0ca..69ab36b51 100644
--- a/src/model/differ.js
+++ b/src/model/differ.js
@@ -423,7 +423,7 @@ export default class Differ {
// If change happens at the same position...
if ( a.position.isEqual( b.position ) ) {
// Keep chronological order of operations.
- return a.changeCount < b.changeCount ? -1 : 1;
+ return a.changeCount - b.changeCount;
}
// If positions differ, position "on the left" should be earlier in the result.
diff --git a/src/model/operation/transform.js b/src/model/operation/transform.js
index 51852430b..f2dd78341 100644
--- a/src/model/operation/transform.js
+++ b/src/model/operation/transform.js
@@ -1669,12 +1669,21 @@ setTransformation( MoveOperation, SplitOperation, ( a, b, context ) => {
// The default case.
//
const transformed = moveRange._getTransformedBySplitOperation( b );
+ const ranges = [ transformed ];
- a.sourcePosition = transformed.start;
- a.howMany = transformed.end.offset - transformed.start.offset;
- a.targetPosition = newTargetPosition;
+ // Case 5:
+ //
+ // Moved range contains graveyard element used by split operation. Add extra move operation to the result.
+ //
+ if ( b.graveyardPosition ) {
+ const movesGraveyardElement = moveRange.start.isEqual( b.graveyardPosition ) || moveRange.containsPosition( b.graveyardPosition );
- return [ a ];
+ if ( a.howMany > 1 && movesGraveyardElement ) {
+ ranges.push( Range.createFromPositionAndShift( b.insertionPosition, 1 ) );
+ }
+ }
+
+ return _makeMoveOperationsFromRanges( ranges, newTargetPosition );
} );
setTransformation( MoveOperation, MergeOperation, ( a, b, context ) => {
@@ -1692,16 +1701,30 @@ setTransformation( MoveOperation, MergeOperation, ( a, b, context ) => {
// removed nodes might be unexpected. This means that in this scenario we will reverse merging and remove the element.
//
if ( !context.aWasUndone ) {
- const gyMoveTarget = Position.createFromPosition( b.graveyardPosition );
- const gyMove = new MoveOperation( b.graveyardPosition, 1, gyMoveTarget, 0 );
+ const results = [];
- const targetPositionPath = b.graveyardPosition.path.slice();
+ let gyMoveSource = Position.createFromPosition( b.graveyardPosition );
+ let splitNodesMoveSource = Position.createFromPosition( b.targetPosition );
+
+ if ( a.howMany > 1 ) {
+ results.push( new MoveOperation( a.sourcePosition, a.howMany - 1, a.targetPosition, 0 ) );
+ gyMoveSource = gyMoveSource._getTransformedByInsertion( a.targetPosition, a.howMany - 1 );
+ splitNodesMoveSource = splitNodesMoveSource._getTransformedByMove( a.sourcePosition, a.targetPosition, a.howMany - 1 );
+ }
+
+ const gyMoveTarget = b.deletionPosition._getCombined( a.sourcePosition, a.targetPosition );
+ const gyMove = new MoveOperation( gyMoveSource, 1, gyMoveTarget, 0 );
+
+ const targetPositionPath = gyMove.getMovedRangeStart().path.slice();
targetPositionPath.push( 0 );
- return [
- gyMove,
- new MoveOperation( b.targetPosition, b.howMany, new Position( a.targetPosition.root, targetPositionPath ), 0 )
- ];
+ const splitNodesMoveTarget = new Position( gyMove.targetPosition.root, targetPositionPath );
+ const splitNodesMove = new MoveOperation( splitNodesMoveSource, b.howMany, splitNodesMoveTarget, 0 );
+
+ results.push( gyMove );
+ results.push( splitNodesMove );
+
+ return results;
}
} else {
// Case 2:
@@ -1934,14 +1957,21 @@ setTransformation( SplitOperation, MoveOperation, ( a, b, context ) => {
if ( a.graveyardPosition ) {
// Case 1:
//
- // Split operation graveyard node was moved. In this case move operation is stronger and the split insertion position
- // should be corrected.
+ // Split operation graveyard node was moved. In this case move operation is stronger. Since graveyard element
+ // is already moved to the correct position, we need to only move the nodes after the split position.
+ // This will be done by `MoveOperation` instead of `SplitOperation`.
//
- if ( rangeToMove.containsPosition( a.graveyardPosition ) || rangeToMove.start.isEqual( a.graveyardPosition ) ) {
- a.insertionPosition = Position.createFromPosition( b.targetPosition );
- a.splitPosition = a.splitPosition._getTransformedByMoveOperation( b );
+ if ( rangeToMove.start.isEqual( a.graveyardPosition ) || rangeToMove.containsPosition( a.graveyardPosition ) ) {
+ const sourcePosition = a.splitPosition._getTransformedByMoveOperation( b );
- return [ a ];
+ const newParentPosition = a.graveyardPosition._getTransformedByMoveOperation( b );
+ const newTargetPath = newParentPosition.path.slice();
+ newTargetPath.push( 0 );
+
+ const newTargetPosition = new Position( newParentPosition.root, newTargetPath );
+ const moveOp = new MoveOperation( sourcePosition, a.howMany, newTargetPosition, 0 );
+
+ return [ moveOp ];
}
a.graveyardPosition = a.graveyardPosition._getTransformedByMoveOperation( b );
@@ -2172,7 +2202,14 @@ function _makeMoveOperationsFromRanges( ranges, targetPosition ) {
for ( let i = 0; i < ranges.length; i++ ) {
// Create new operation out of a range and target position.
const range = ranges[ i ];
- const op = new MoveOperation( range.start, range.end.offset - range.start.offset, targetPosition, 0 );
+ const op = new MoveOperation(
+ range.start,
+ range.end.offset - range.start.offset,
+ // If the target is the end of the move range this operation doesn't really move anything.
+ // In this case, it is better for OT to use range start instead of range end.
+ targetPosition.isEqual( range.end ) ? range.start : targetPosition,
+ 0
+ );
operations.push( op );
diff --git a/src/model/range.js b/src/model/range.js
index 143c83a8a..61b3d340e 100644
--- a/src/model/range.js
+++ b/src/model/range.js
@@ -517,13 +517,17 @@ export default class Range {
*/
_getTransformedBySplitOperation( operation ) {
const start = this.start._getTransformedBySplitOperation( operation );
-
- let end;
+ let end = this.end._getTransformedBySplitOperation( operation );
if ( this.end.isEqual( operation.insertionPosition ) ) {
end = this.end.getShiftedBy( 1 );
- } else {
- end = this.end._getTransformedBySplitOperation( operation );
+ }
+
+ // Below may happen when range contains graveyard element used by split operation.
+ if ( start.root != end.root ) {
+ // End position was next to the moved graveyard element and was moved with it.
+ // Fix it by using old `end` which has proper `root`.
+ end = this.end.getShiftedBy( -1 );
}
return new Range( start, end );
diff --git a/tests/model/operation/transform/merge.js b/tests/model/operation/transform/merge.js
index 4e3a54dff..306299561 100644
--- a/tests/model/operation/transform/merge.js
+++ b/tests/model/operation/transform/merge.js
@@ -174,9 +174,7 @@ describe( 'transform', () => {
kate.undo();
syncClients();
- expectClients(
- 'Foo'
- );
+ expectClients( 'Foo' );
} );
} );
@@ -222,6 +220,40 @@ describe( 'transform', () => {
expectClients( 'FooBar' );
} );
+
+ it( 'remove merged element then undo #3', () => {
+ john.setData( '[AB]C' );
+ kate.setData( 'A[]BC' );
+
+ kate.merge();
+ john.remove();
+
+ syncClients();
+ expectClients( 'C' );
+
+ john.undo();
+ kate.undo();
+
+ syncClients();
+ expectClients( 'ABC' );
+ } );
+
+ it( 'remove merged element then undo #4', () => {
+ john.setData( 'A[BC]' );
+ kate.setData( 'A[]BC' );
+
+ kate.merge();
+ john.remove();
+
+ syncClients();
+ expectClients( 'A' );
+
+ john.undo();
+ kate.undo();
+
+ syncClients();
+ expectClients( 'ABC' );
+ } );
} );
describe( 'by delete', () => {
@@ -409,5 +441,23 @@ describe( 'transform', () => {
);
} );
} );
+
+ describe( 'by split', () => {
+ it( 'merge element which got split (the element is in blockquote) and undo', () => {
+ john.setData( 'Foo[]Bar
' );
+ kate.setData( 'FooB[]ar
' );
+
+ john._processExecute( 'delete' );
+ kate.split();
+
+ syncClients();
+ expectClients( 'FooBar' );
+
+ john.undo();
+
+ syncClients();
+ expectClients( 'FooBar
' );
+ } );
+ } );
} );
} );