Fixed: Remove and reinsert change types needs fixes during range tran…

…sformations since `RemoveOperation` no longer creates `graveyardHolder`.
ckeditor · Jun 24, 2017 · f4ba06f · f4ba06f
1 parent 634b9f6
commit f4ba06f
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 44 deletions.
diff --git a/src/model/liverange.js b/src/model/liverange.js
@@ -129,7 +129,7 @@ function transform( changeType, deltaType, targetRange, sourcePosition ) {
 	const howMany = targetRange.end.offset - targetRange.start.offset;
 	let targetPosition = targetRange.start;
 
-	if ( changeType == 'move' ) {
+	if ( changeType == 'move' || changeType == 'remove' || changeType == 'reinsert' ) {
 		// Range._getTransformedByDocumentChange is expecting `targetPosition` to be "before" move
 		// (before transformation). `targetRange.start` is already after the move happened.
 		// We have to revert `targetPosition` to the state before the move.
@@ -142,7 +142,7 @@ function transform( changeType, deltaType, targetRange, sourcePosition ) {
 	//
 	// First, this concerns only `move` change, because insert change includes inserted part always (changeType == 'move').
 	// Second, this is a case only if moved range was intersecting with this range and was inserted into this range (result.length == 3).
-	if ( changeType == 'move' && result.length == 3 ) {
+	if ( ( changeType == 'move' || changeType == 'remove' || changeType == 'reinsert' ) && result.length == 3 ) {
 		// `result[ 2 ]` is a "common part" of this range and moved range. We substitute that common part with the whole
 		// `targetRange` because we want to include whole `targetRange` in this range.
 		result[ 2 ] = targetRange;

diff --git a/src/model/range.js b/src/model/range.js
@@ -462,6 +462,7 @@ export default class Range {
 		} else {
 			const sourceRange = Range.createFromPositionAndShift( sourcePosition, howMany );
 
+			// Edge case for merge detla.
 			if (
 				deltaType == 'merge' &&
 				this.isCollapsed &&
@@ -471,50 +472,52 @@ export default class Range {
 				// Without fix, the range would end up in the graveyard, together with removed element.
 				// <p>foo</p><p>[]bar</p> -> <p>foobar</p><p>[]</p> -> <p>foobar</p> -> <p>foo[]bar</p>
 				return [ new Range( targetPosition.getShiftedBy( this.start.offset ) ) ];
-			} else if ( type == 'move' ) {
-				// In all examples `[]` is `this` and `{}` is `sourceRange`, while `^` is move target position.
-				//
-				// Example:
-				// <p>xx</p>^<w>{<p>a[b</p>}</w><p>c]d</p>   -->   <p>xx</p><p>a[b</p><w></w><p>c]d</p>
-				// ^<p>xx</p><w>{<p>a[b</p>}</w><p>c]d</p>   -->   <p>a[b</p><p>xx</p><w></w><p>c]d</p>  // Note <p>xx</p> inclusion.
-				// <w>{<p>a[b</p>}</w>^<p>c]d</p>            -->   <w></w><p>a[b</p><p>c]d</p>
-				if (
-					sourceRange.containsPosition( this.start ) &&
-					this.containsPosition( sourceRange.end ) &&
-					this.end.isAfter( targetPosition )
-				) {
-					const start = this.start._getCombined(
-						sourcePosition,
-						targetPosition._getTransformedByDeletion( sourcePosition, howMany )
-					);
-					const end = this.end._getTransformedByMove( sourcePosition, targetPosition, howMany, false, false );
-
-					return [ new Range( start, end ) ];
-				}
+			}
+			//
+			// Other edge cases:
+			//
+			// In all examples `[]` is `this` and `{}` is `sourceRange`, while `^` is move target position.
+			//
+			// Example:
+			// <p>xx</p>^<w>{<p>a[b</p>}</w><p>c]d</p>   -->   <p>xx</p><p>a[b</p><w></w><p>c]d</p>
+			// ^<p>xx</p><w>{<p>a[b</p>}</w><p>c]d</p>   -->   <p>a[b</p><p>xx</p><w></w><p>c]d</p>  // Note <p>xx</p> inclusion.
+			// <w>{<p>a[b</p>}</w>^<p>c]d</p>            -->   <w></w><p>a[b</p><p>c]d</p>
+			if (
+				sourceRange.containsPosition( this.start ) &&
+				this.containsPosition( sourceRange.end ) &&
+				this.end.isAfter( targetPosition )
+			) {
+				const start = this.start._getCombined(
+					sourcePosition,
+					targetPosition._getTransformedByDeletion( sourcePosition, howMany )
+				);
+				const end = this.end._getTransformedByMove( sourcePosition, targetPosition, howMany, false, false );
 
-				// Example:
-				// <p>c[d</p><w>{<p>a]b</p>}</w>^<p>xx</p>   -->   <p>c[d</p><w></w><p>a]b</p><p>xx</p>
-				// <p>c[d</p><w>{<p>a]b</p>}</w><p>xx</p>^   -->   <p>c[d</p><w></w><p>xx</p><p>a]b</p>  // Note <p>xx</p> inclusion.
-				// <p>c[d</p>^<w>{<p>a]b</p>}</w>            -->   <p>c[d</p><p>a]b</p><w></w>
-				if (
-					sourceRange.containsPosition( this.end ) &&
-					this.containsPosition( sourceRange.start ) &&
-					this.start.isBefore( targetPosition )
-				) {
-					const start = this.start._getTransformedByMove(
-						sourcePosition,
-						targetPosition,
-						howMany,
-						true,
-						false
-					);
-					const end = this.end._getCombined(
-						sourcePosition,
-						targetPosition._getTransformedByDeletion( sourcePosition, howMany )
-					);
+				return [ new Range( start, end ) ];
+			}
 
-					return [ new Range( start, end ) ];
-				}
+			// Example:
+			// <p>c[d</p><w>{<p>a]b</p>}</w>^<p>xx</p>   -->   <p>c[d</p><w></w><p>a]b</p><p>xx</p>
+			// <p>c[d</p><w>{<p>a]b</p>}</w><p>xx</p>^   -->   <p>c[d</p><w></w><p>xx</p><p>a]b</p>  // Note <p>xx</p> inclusion.
+			// <p>c[d</p>^<w>{<p>a]b</p>}</w>            -->   <p>c[d</p><p>a]b</p><w></w>
+			if (
+				sourceRange.containsPosition( this.end ) &&
+				this.containsPosition( sourceRange.start ) &&
+				this.start.isBefore( targetPosition )
+			) {
+				const start = this.start._getTransformedByMove(
+					sourcePosition,
+					targetPosition,
+					howMany,
+					true,
+					false
+				);
+				const end = this.end._getCombined(
+					sourcePosition,
+					targetPosition._getTransformedByDeletion( sourcePosition, howMany )
+				);
+
+				return [ new Range( start, end ) ];
 			}
 
 			return this._getTransformedByMove( sourcePosition, targetPosition, howMany );