Преглед на файлове

Changed: Range#_getTransformedByDocumentChange will now be "inclusive" if range boundary is moved out of the range.

Szymon Cofalik преди 8 години
родител
ревизия
dcd1ba9df7

+ 2 - 0
packages/ckeditor5-engine/src/model/delta/basic-transformations.js

@@ -111,6 +111,8 @@ addTransformationCase( MarkerDelta, SplitDelta, transformMarkerDelta );
 addTransformationCase( MarkerDelta, MergeDelta, transformMarkerDelta );
 addTransformationCase( MarkerDelta, WrapDelta, transformMarkerDelta );
 addTransformationCase( MarkerDelta, UnwrapDelta, transformMarkerDelta );
+addTransformationCase( MarkerDelta, MoveDelta, transformMarkerDelta );
+addTransformationCase( MarkerDelta, RenameDelta, transformMarkerDelta );
 
 // Add special case for MoveDelta x MergeDelta transformation.
 addTransformationCase( MoveDelta, MergeDelta, ( a, b, isStrong ) => {

+ 31 - 49
packages/ckeditor5-engine/src/model/range.js

@@ -447,61 +447,43 @@ export default class Range {
 	 * @returns {Array.<module:engine/model/range~Range>}
 	 */
 	_getTransformedByDocumentChange( type, deltaType, targetPosition, howMany, sourcePosition ) {
-		// IMPORTANT! Every special case added here has to be reflected in MarkerDelta transformations!
-		// Check /src/model/delta/basic-transformations.js.
 		if ( type == 'insert' ) {
 			return this._getTransformedByInsertion( targetPosition, howMany, false, false );
 		} else {
-			const ranges = this._getTransformedByMove( sourcePosition, targetPosition, howMany );
-
-			// Don't ask. Just debug.
-			// Like this: https://github.com/ckeditor/ckeditor5-engine/issues/841#issuecomment-282706488.
-			//
-			// In following cases, in examples, the last step is the fix step.
-			// When there are multiple ranges in an example, ranges[] array indices are represented as follows:
-			// * [] is ranges[ 0 ],
-			// * {} is ranges[ 1 ],
-			// * () is ranges[ 2 ].
-			if ( type == 'move' ) {
-				const sourceRange = Range.createFromPositionAndShift( sourcePosition, howMany );
-
-				if ( deltaType == 'split' && this.containsPosition( sourcePosition ) ) {
-					// Range contains a position where an element is split.
-					// <p>f[ooba]r</p> -> <p>f[ooba]r</p><p></p> -> <p>f[oo]</p><p>{ba}r</p> -> <p>f[oo</p><p>ba]r</p>
-					return [ new Range( ranges[ 0 ].start, ranges[ 1 ].end ) ];
-				} else if ( deltaType == 'merge' && this.isCollapsed && ranges[ 0 ].start.isEqual( sourcePosition ) ) {
-					// Collapsed range is in merged element.
-					// 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 ( deltaType == 'wrap' ) {
-					// Range intersects (at the start) with wrapped element (<p>ab</p>).
-					// <p>a[b</p><p>c]d</p> -> <p>a[b</p><w></w><p>c]d</p> -> [<w>]<p>a(b</p>){</w><p>c}d</p> -> <w><p>a[b</p></w><p>c]d</p>
-					if ( sourceRange.containsPosition( this.start ) && this.containsPosition( sourceRange.end ) ) {
-						return [ new Range( ranges[ 2 ].start, ranges[ 1 ].end ) ];
-					}
-					// Range intersects (at the end) with wrapped element (<p>cd</p>).
-					// <p>a[b</p><p>c]d</p> -> <p>a[b</p><p>c]d</p><w></w> -> <p>a[b</p>]<w>{<p>c}d</p></w> -> <p>a[b</p><w><p>c]d</p></w>
-					else if ( sourceRange.containsPosition( this.end ) && this.containsPosition( sourceRange.start ) ) {
-						return [ new Range( ranges[ 0 ].start, ranges[ 1 ].end ) ];
-					}
-				} else if ( deltaType == 'unwrap' ) {
-					// Range intersects (at the beginning) with unwrapped element (<w></w>).
-					// <w><p>a[b</p></w><p>c]d</p> -> <p>a{b</p>}<w>[</w><p>c]d</p> -> <p>a[b</p><w></w><p>c]d</p>
-					// <w></w> is removed in next operation, but the remove does not mess up ranges.
-					if ( sourceRange.containsPosition( this.start ) && this.containsPosition( sourceRange.end ) ) {
-						return [ new Range( ranges[ 1 ].start, ranges[ 0 ].end ) ];
-					}
-					// Range intersects (at the end) with unwrapped element (<w></w>).
-					// <p>a[b</p><w><p>c]d</p></w> -> <p>a[b</p>](<p>c)d</p>{<w>}</w> -> <p>a[b</p><p>c]d</p><w></w>
-					// <w></w> is removed in next operation, but the remove does not mess up ranges.
-					else if ( sourceRange.containsPosition( this.end ) && this.containsPosition( sourceRange.start ) ) {
-						return [ new Range( ranges[ 0 ].start, ranges[ 2 ].end ) ];
-					}
+			const sourceRange = Range.createFromPositionAndShift( sourcePosition, howMany );
+
+			if ( deltaType == 'merge' && this.isCollapsed && ( this.start.isEqual( sourceRange.start ) || this.start.isEqual( sourceRange.end ) ) ) {
+				// Collapsed range is in merged element.
+				// 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 ) ) {
+					let start = this.start._getCombined( sourcePosition, targetPosition._getTransformedByDeletion( sourcePosition, howMany ) );
+					const end = this.end._getTransformedByMove( sourcePosition, targetPosition, howMany, false, false );
+
+					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 );
+					let end = this.end._getCombined( sourcePosition, targetPosition._getTransformedByDeletion( sourcePosition, howMany ) );
+
+					return [ new Range( start, end ) ];
 				}
 			}
 
-			return ranges;
+			return this._getTransformedByMove( sourcePosition, targetPosition, howMany );
 		}
 	}
 

+ 3 - 3
packages/ckeditor5-engine/tests/model/liverange.js

@@ -292,7 +292,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] );
-				expect( live.end.path ).to.deep.equal( [ 0, 2, 1 ] );
+				expect( live.end.path ).to.deep.equal( [ 2, 1 ] ); // Included some nodes.
 				expect( spy.calledOnce ).to.be.true;
 			} );
 
@@ -307,7 +307,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] );
-				expect( live.end.path ).to.deep.equal( [ 0, 2, 6 ] );
+				expect( live.end.path ).to.deep.equal( [ 0, 2, 1 ] );
 				expect( spy.calledOnce ).to.be.true;
 			} );
 
@@ -357,7 +357,7 @@ describe( 'LiveRange', () => {
 				};
 				doc.fire( 'change', 'move', changes, null );
 
-				expect( live.start.path ).to.deep.equal( [ 0, 1, 2 ] );
+				expect( live.start.path ).to.deep.equal( [ 0, 1, 9 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 1, 12 ] );
 				expect( spy.calledOnce ).to.be.true;
 			} );

+ 1 - 1
packages/ckeditor5-engine/tests/model/liveselection.js

@@ -453,7 +453,7 @@ describe( 'LiveSelection', () => {
 				let range = selection.getFirstRange();
 
 				expect( range.start.path ).to.deep.equal( [ 0, 2 ] );
-				expect( range.end.path ).to.deep.equal( [ 1, 3 ] );
+				expect( range.end.path ).to.deep.equal( [ 5 ] );
 				expect( spyRange.calledOnce ).to.be.true;
 			} );
 

+ 88 - 6
packages/ckeditor5-engine/tests/model/range.js

@@ -750,8 +750,8 @@ describe( 'Range', () => {
 		describe( 'by MoveDelta', () => {
 			it( 'move before range', () => {
 				const start = new Position( root, [ 0 ] );
-				const end = new Position( otherRoot, [ 0 ] );
-				const delta = getMoveDelta( start, 2, end, 1 );
+				const target = new Position( otherRoot, [ 0 ] );
+				const delta = getMoveDelta( start, 2, target, 1 );
 
 				const transformed = range.getTransformedByDelta( delta );
 
@@ -760,8 +760,8 @@ describe( 'Range', () => {
 
 			it( 'move intersecting with range (and targeting before range)', () => {
 				const start = new Position( root, [ 4 ] );
-				const end = new Position( root, [ 0 ] );
-				const delta = getMoveDelta( start, 2, end, 1 );
+				const target = new Position( root, [ 0 ] );
+				const delta = getMoveDelta( start, 2, target, 1 );
 
 				const transformed = range.getTransformedByDelta( delta );
 
@@ -772,8 +772,8 @@ describe( 'Range', () => {
 			it( 'move inside the range', () => {
 				range.end.offset = 6;
 				const start = new Position( root, [ 3 ] );
-				const end = new Position( root, [ 5 ] );
-				const delta = getMoveDelta( start, 1, end, 1 );
+				const target = new Position( root, [ 5 ] );
+				const delta = getMoveDelta( start, 1, target, 1 );
 
 				const transformed = range.getTransformedByDelta( delta );
 
@@ -781,6 +781,71 @@ describe( 'Range', () => {
 				expectRange( transformed[ 1 ], 5, 6 );
 				expectRange( transformed[ 2 ], 4, 5 );
 			} );
+
+			// #877.
+			it( 'moved element contains range start and is moved towards inside of range', () => {
+				// Initial state:
+				// <w><p>abc</p><p>x[x</p></w><p>d]ef</p>
+				// Expected state after moving `<p>` out of `<w>`:
+				// <w><p>abc</p></w><p>x[x</p><p>d]ef</p>
+
+				const range = new Range( new Position( root, [ 0, 1, 1 ] ), new Position( root, [ 1, 1 ] ) );
+				const delta = getMoveDelta( new Position( root, [ 0, 1 ] ), 1, new Position( root, [ 1 ] ), 1 );
+
+				const transformed = range.getTransformedByDelta( delta );
+
+				expect( transformed.length ).to.equal( 1 );
+				expect( transformed[ 0 ].start.path ).to.deep.equal( [ 1, 1 ] );
+				expect( transformed[ 0 ].end.path ).to.deep.equal( [ 2, 1 ] );
+			} );
+
+			it( 'moved element contains range start and is moved out of range', () => {
+				// Initial state:
+				// <p>abc</p><p>x[x</p><p>d]ef</p>
+				// Expected state after moving:
+				// <p>x[x</p><p>abc</p><p>d]ef</p>
+
+				const range = new Range( new Position( root, [ 1, 1 ] ), new Position( root, [ 2, 1 ] ) );
+				const delta = getMoveDelta( new Position( root, [ 1 ] ), 1, new Position( root, [ 0 ] ), 1 );
+
+				const transformed = range.getTransformedByDelta( delta );
+
+				expect( transformed.length ).to.equal( 1 );
+				expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 1 ] );
+				expect( transformed[ 0 ].end.path ).to.deep.equal( [ 2, 1 ] );
+			} );
+
+			it( 'moved element contains range end and is moved towards range', () => {
+				// Initial state:
+				// <p>a[bc</p><p>def</p><p>x]x</p>
+				// Expected state after moving:
+				// <p>a[bc</p><p>x]x</p><p>def</p>
+
+				const range = new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 2, 1 ] ) );
+				const delta = getMoveDelta( new Position( root, [ 2 ] ), 1, new Position( root, [ 1 ] ), 1 );
+
+				const transformed = range.getTransformedByDelta( delta );
+
+				expect( transformed.length ).to.equal( 1 );
+				expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 1 ] );
+				expect( transformed[ 0 ].end.path ).to.deep.equal( [ 1, 1 ] );
+			} );
+
+			it( 'moved element contains range end and is moved out of range', () => {
+				// Initial state:
+				// <p>a[bc</p><p>x]x</p><p>def</p>
+				// Expected state after moving:
+				// <p>a[bc</p><p>def</p><p>x]x</p>
+
+				const range = new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 1, 1 ] ) );
+				const delta = getMoveDelta( new Position( root, [ 1 ] ), 1, new Position( root, [ 3 ] ), 1 );
+
+				const transformed = range.getTransformedByDelta( delta );
+
+				expect( transformed.length ).to.equal( 1 );
+				expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 1 ] );
+				expect( transformed[ 0 ].end.path ).to.deep.equal( [ 2, 1 ] );
+			} );
 		} );
 
 		describe( 'by RemoveDelta', () => {
@@ -858,6 +923,23 @@ describe( 'Range', () => {
 					expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 3 ] );
 					expect( transformed[ 0 ].end.path ).to.deep.equal( [ 0, 3 ] );
 				} );
+
+				// #877.
+				it( 'merge elements that contain elements with range boundaries', () => {
+					// Initial state:
+					// <w><p>x[x</p></w><w><p>y]y</p></w>
+					// Expected state after merge:
+					// <w><p>x[x</p><p>y]y</p></w>
+
+					const range = new Range( new Position( root, [ 0, 0, 1 ] ), new Position( root, [ 1, 0, 1 ] ) );
+					const delta = getMergeDelta( new Position( root, [ 1 ] ), 1, 1, 1 );
+
+					const transformed = range.getTransformedByDelta( delta );
+
+					expect( transformed.length ).to.equal( 1 );
+					expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 0, 1 ] );
+					expect( transformed[ 0 ].end.path ).to.deep.equal( [ 0, 1, 1 ] );
+				} );
 			} );
 
 			describe( 'by WrapDelta', () => {