From cd543b619b7397419d26437402e2576d6dc3858d Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Fri, 21 Sep 2018 17:40:38 +0200 Subject: [PATCH] Fix: Added merge x merge transformation case handling when merges source position is same but target different. --- src/model/operation/transform.js | 50 ++++++++++++++++++++++++ tests/model/operation/transform/merge.js | 12 ++++++ 2 files changed, 62 insertions(+) diff --git a/src/model/operation/transform.js b/src/model/operation/transform.js index cf4a2dd0a..f517e5a42 100644 --- a/src/model/operation/transform.js +++ b/src/model/operation/transform.js @@ -1247,6 +1247,56 @@ setTransformation( MergeOperation, MergeOperation, ( a, b, context ) => { } } + // Case 2: + // + // Same merge source position but different target position. + // + // This can happen during collaboration. For example, if one client merged a paragraph to the previous paragraph + // and the other person removed that paragraph and merged the same paragraph to something before: + // + // Client A: + //

Foo

Bar

[]Xyz

+ //

Foo

BarXyz

+ // + // Client B: + //

Foo

[

Bar

]

Xyz

+ //

Foo

[]Xyz

+ //

FooXyz

+ // + // In this case we need to decide where finally "Xyz" will land: + // + //

FooXyz

graveyard:

Bar

+ //

Foo

graveyard:

BarXyz

+ // + // Let's move it in a way so that a merge operation that does not target to graveyard is more important so that + // nodes does not end up in the graveyard. It makes sense. Both for Client A and for Client B "Xyz" finally did not + // end up in the graveyard (see above). + // + // If neither or both operations point to graveyard, then let `aIsStrong` decide. + // + if ( a.sourcePosition.isEqual( b.sourcePosition ) && !a.targetPosition.isEqual( b.targetPosition ) && !context.bWasUndone ) { + const aToGraveyard = a.targetPosition.root.rootName == '$graveyard'; + const bToGraveyard = b.targetPosition.root.rootName == '$graveyard'; + + // If `aIsWeak` it means that `a` points to graveyard while `b` doesn't. Don't move nodes then. + const aIsWeak = aToGraveyard && !bToGraveyard; + + // If `bIsWeak` it means that `b` points to graveyard while `a` doesn't. Force moving nodes then. + const bIsWeak = bToGraveyard && !aToGraveyard; + + // Force move if `b` is weak or neither operation is weak but `a` is stronger through `context.aIsStrong`. + const forceMove = bIsWeak || ( !aIsWeak && context.aIsStrong ); + + if ( forceMove ) { + const sourcePosition = b.targetPosition._getTransformedByMergeOperation( b ); + const targetPosition = a.targetPosition._getTransformedByMergeOperation( b ); + + return [ new MoveOperation( sourcePosition, a.howMany, targetPosition, 0 ) ]; + } else { + return [ new NoOperation( 0 ) ]; + } + } + // The default case. // if ( a.sourcePosition.hasSameParentAs( b.targetPosition ) ) { diff --git a/tests/model/operation/transform/merge.js b/tests/model/operation/transform/merge.js index 6c70e15c0..e47f16f07 100644 --- a/tests/model/operation/transform/merge.js +++ b/tests/model/operation/transform/merge.js @@ -116,6 +116,18 @@ describe( 'transform', () => { expectClients( 'For' ); } ); + + it( 'merged elements and some text', () => { + john.setData( 'Foo[]Bar' ); + kate.setData( 'F[ooBa]r' ); + + john.merge(); + kate.delete(); + + syncClients(); + + expectClients( 'Fr' ); + } ); } ); describe( 'by wrap', () => {