浏览代码

Merge branch 't/1462' into t/ot-followups

Szymon Cofalik 7 年之前
父节点
当前提交
279b81fd1c

+ 6 - 0
packages/ckeditor5-engine/src/model/operation/transform.js

@@ -1132,6 +1132,12 @@ setTransformation( RenameOperation, InsertOperation, ( a, b ) => {
 } );
 
 setTransformation( RenameOperation, MergeOperation, ( a, b ) => {
+	if ( a.position.isEqual( b.deletionPosition ) ) {
+		a.position = Position.createFromPosition( b.graveyardPosition );
+
+		return [ a ];
+	}
+
 	a.position = a.position._getTransformedByMergeOperation( b );
 
 	return [ a ];

+ 2 - 0
packages/ckeditor5-engine/src/model/position.js

@@ -563,6 +563,8 @@ export default class Position {
 				// Above happens during OT when the merged element is moved before the merged-to element.
 				pos = pos._getTransformedByDeletion( operation.deletionPosition, 1 );
 			}
+		} else if ( this.isEqual( operation.deletionPosition ) ) {
+			pos = Position.createFromPosition( operation.deletionPosition );
 		} else {
 			pos = this._getTransformedByMove( operation.deletionPosition, operation.graveyardPosition, 1 );
 		}

+ 27 - 6
packages/ckeditor5-engine/src/model/range.js

@@ -493,16 +493,37 @@ export default class Range {
 	}
 
 	_getTransformedByMergeOperation( operation ) {
-		const start = this.start._getTransformedByMergeOperation( operation );
-		const end = this.end._getTransformedByMergeOperation( operation );
+		let start = this.start._getTransformedByMergeOperation( operation );
+		let end = this.end._getTransformedByMergeOperation( operation );
 
 		if ( start.isAfter( end ) ) {
-			// This happens in the following case:
+			// This happens in the following two, similar cases:
 			//
-			// <p>abc</p>{<p>x}yz</p> -> <p>abcx}yz</p>{
-			//                           <p>abcx{yz</p>}
+			// Case 1: Range start is directly before merged node.
+			//         Resulting range should include only nodes from the merged element:
 			//
-			return new Range( end, start );
+			// Before: <p>aa</p>{<p>b}b</p><p>cc</p>
+			// Merge:  <p>aab}b</p>{<p>cc</p>
+			// Fix:    <p>aa{b}b</p><p>cc</p>
+			//
+			// Case 2: Range start is not directly before merged node.
+			//         Result should include all nodes that were in the original range.
+			//
+			// Before: <p>aa</p>{<p>cc</p><p>b}b</p>
+			// Merge:  <p>aab}b</p>{<p>cc</p>
+			// Fix:    <p>aa{bb</p><p>cc</p>}
+			//
+			//         The range is expanded by an additional `b` letter but it is better than dropping the whole `cc` paragraph.
+			//
+			if ( !operation.deletionPosition.isEqual( start ) ) {
+				// Case 2.
+				end = operation.deletionPosition;
+			}
+
+			// In both cases start is at the end of the merge-to element.
+			start = operation.targetPosition;
+
+			return new Range( start, end );
 		}
 
 		return new Range( start, end );

+ 35 - 0
packages/ckeditor5-engine/tests/model/operation/transform/marker.js

@@ -789,6 +789,41 @@ describe( 'transform', () => {
 					'</blockQuote>'
 				);
 			} );
+
+			it( 'only marker end is inside merged element #1', () => {
+				john.setData( '<paragraph>Foo</paragraph>[<paragraph>B]ar</paragraph>' );
+				kate.setData( '<paragraph>Foo</paragraph>[]<paragraph>Bar</paragraph>' );
+
+				john.setMarker( 'm1' );
+				kate.merge();
+
+				syncClients();
+
+				expectClients( '<paragraph>Foo<m1:start></m1:start>B<m1:end></m1:end>ar</paragraph>' );
+			} );
+
+			it( 'only marker end is inside merged element #2', () => {
+				john.setData( '<paragraph>Foo[]Bar</paragraph>' );
+				kate.setData( '<paragraph>Foo[]Bar</paragraph>' );
+
+				kate.split();
+
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
+
+				john.setSelection( [ 1 ] );
+				john.insert( '<paragraph>Xyz</paragraph>' );
+
+				syncClients();
+				expectClients( '<paragraph>Foo</paragraph><paragraph>Xyz</paragraph><paragraph>Bar</paragraph>' );
+
+				john.setSelection( [ 1 ], [ 2, 1 ] );
+				john.setMarker( 'm1' );
+				kate.undo();
+
+				syncClients();
+				expectClients( '<paragraph>Foo<m1:start></m1:start>Bar</paragraph><paragraph>Xyz</paragraph><m1:end></m1:end>' );
+			} );
 		} );
 
 		describe( 'by rename', () => {