8
0
Эх сурвалжийг харах

Changed Range.constructor passed positions are now always cloned. Doc LivePosition typo.

Szymon Cofalik 10 жил өмнө
parent
commit
bb20608e5a

+ 1 - 1
packages/ckeditor5-engine/src/treemodel/liveposition.js

@@ -23,7 +23,7 @@ CKEDITOR.define( [
 		/**
 		 * Creates a smart position.
 		 *
-		 * @see treeModel.Position
+		 * @see {@link treeModel.Position}
 		 * @param root
 		 * @param path
 		 * @param {Boolean} [stickToLeft] Flag representing what side the smart position is "sticking to". LivePosition

+ 4 - 9
packages/ckeditor5-engine/src/treemodel/liverange.js

@@ -28,6 +28,9 @@ CKEDITOR.define( [
 		constructor( start, end ) {
 			super( start, end );
 
+			this.start = new LivePosition( this.start.root, this.start.path.slice(), false );
+			this.end = new LivePosition( this.end.root, this.end.path.slice(), true );
+
 			bindWithDocument.call( this );
 		}
 
@@ -40,7 +43,7 @@ CKEDITOR.define( [
 		 */
 		clone( makeLiveRange ) {
 			if ( makeLiveRange ) {
-				return new LiveRange( this.start.clone(), this.end.clone() );
+				return new LiveRange( this.start, this.end );
 			} else {
 				return super.clone();
 			}
@@ -67,14 +70,6 @@ CKEDITOR.define( [
 	function bindWithDocument() {
 		/*jshint validthis: true */
 
-		if ( !( this.start instanceof LivePosition ) ) {
-			this.start = new LivePosition( this.start.root, this.start.path.slice(), false );
-		}
-
-		if ( !( this.end instanceof LivePosition ) ) {
-			this.end = new LivePosition( this.end.root, this.end.path.slice(), true );
-		}
-
 		this.listenTo(
 			this.root.document,
 			'change',

+ 7 - 20
packages/ckeditor5-engine/src/treemodel/range.js

@@ -25,14 +25,14 @@ CKEDITOR.define( [ 'treemodel/position', 'treemodel/positioniterator', 'utils' ]
 			 *
 			 * @property {treeModel.Position}
 			 */
-			this.start = start;
+			this.start = start.clone();
 
 			/**
 			 * End position.
 			 *
 			 * @property {treeModel.Position}
 			 */
-			this.end = end;
+			this.end = end.clone();
 		}
 
 		/**
@@ -60,7 +60,7 @@ CKEDITOR.define( [ 'treemodel/position', 'treemodel/positioniterator', 'utils' ]
 		 * @returns {treeModel.Position} Cloned {@link treeModel.Range range}.
 		 */
 		clone() {
-			return new Range( this.start.clone(), this.end.clone() );
+			return new Range( this.start, this.end );
 		}
 
 		/**
@@ -114,23 +114,13 @@ CKEDITOR.define( [ 'treemodel/position', 'treemodel/positioniterator', 'utils' ]
 				if ( this.containsPosition( otherRange.start ) ) {
 					// Given range start is inside this range. This means that we have to
 					// add shrunken range - from the start to the middle of this range.
-					ranges.push(
-						new Range(
-							this.start.clone(),
-							otherRange.start.clone()
-						)
-					);
+					ranges.push( new Range( this.start, otherRange.start ) );
 				}
 
 				if ( this.containsPosition( otherRange.end ) ) {
 					// Given range end is inside this range. This means that we have to
 					// add shrunken range - from the middle of this range to the end.
-					ranges.push(
-						new Range(
-							otherRange.end.clone(),
-							this.end.clone()
-						)
-					);
+					ranges.push( new Range( otherRange.end, this.end ) );
 				}
 			} else {
 				// Ranges do not intersect, return the original range.
@@ -175,7 +165,7 @@ CKEDITOR.define( [ 'treemodel/position', 'treemodel/positioniterator', 'utils' ]
 					commonRangeEnd = otherRange.end;
 				}
 
-				return new Range( commonRangeStart.clone(), commonRangeEnd.clone() );
+				return new Range( commonRangeStart, commonRangeEnd );
 			}
 
 			// Ranges do not intersect, so they do not have common part.
@@ -238,10 +228,7 @@ CKEDITOR.define( [ 'treemodel/position', 'treemodel/positioniterator', 'utils' ]
 				// insertion to reflect insertion changes.
 
 				return [
-					new Range(
-						this.start.clone(),
-						insertPosition.clone()
-					),
+					new Range( this.start, insertPosition ),
 					new Range(
 						insertPosition.getTransformedByInsertion( insertPosition, howMany, true ),
 						this.end.getTransformedByInsertion( insertPosition, howMany, true )

+ 4 - 8
packages/ckeditor5-engine/tests/treemodel/operation/transform.js

@@ -86,7 +86,7 @@ describe( 'transform', () => {
 
 			expected = {
 				type: InsertOperation,
-				position: position.clone(),
+				position: position,
 				baseVersion: baseVersion + 1
 			};
 		} );
@@ -189,12 +189,8 @@ describe( 'transform', () => {
 
 		describe( 'by AttributeOperation', () => {
 			it( 'no position update', () => {
-				let rangeStart = position.clone();
-				let rangeEnd = position.clone();
-				rangeEnd.offset += 2;
-
 				let transformBy = new AttributeOperation(
-					new Range( rangeStart, rangeEnd ),
+					Range.createFromPositionAndShift( position, 2 ),
 					null,
 					new Attribute( 'foo', 'bar' ),
 					baseVersion
@@ -437,7 +433,7 @@ describe( 'transform', () => {
 
 				op = new AttributeOperation( range, oldAttr, newAttr, baseVersion );
 
-				expected.range = new Range( start.clone(), end.clone() );
+				expected.range = new Range( start, end );
 			} );
 
 			describe( 'by InsertOperation', () => {
@@ -1076,7 +1072,7 @@ describe( 'transform', () => {
 
 				op = new AttributeOperation( range, oldAttr, newAttr, baseVersion );
 
-				expected.range = new Range( start.clone(), end.clone() );
+				expected.range = new Range( start, end );
 			} );
 
 			describe( 'by InsertOperation', () => {

+ 2 - 2
packages/ckeditor5-engine/tests/treemodel/range.js

@@ -40,8 +40,8 @@ describe( 'Range', () => {
 
 	describe( 'constructor', () => {
 		it( 'should create a range with given positions', () => {
-			expect( range ).to.have.property( 'start' ).that.equal( start );
-			expect( range ).to.have.property( 'end' ).that.equal( end );
+			expect( range.start.isEqual( start ) ).to.be.true;
+			expect( range.end.isEqual( end ) ).to.be.true;
 		} );
 	} );