Преглед изворни кода

Other: Make view Range immutable.

Maciej Gołaszewski пре 8 година
родитељ
комит
0f55cd254f

+ 28 - 13
packages/ckeditor5-engine/src/view/range.js

@@ -10,6 +10,9 @@
 import Position from './position';
 import TreeWalker from './treewalker';
 
+const _start = Symbol( 'start' );
+const _end = Symbol( 'end' );
+
 /**
  * Tree view range.
  */
@@ -23,19 +26,8 @@ export default class Range {
 	 * @param {module:engine/view/position~Position} [end] End position. If not set, range will be collapsed at `start` position.
 	 */
 	constructor( start, end = null ) {
-		/**
-		 * Start position.
-		 *
-		 * @member {module:engine/view/position~Position}
-		 */
-		this.start = start;
-
-		/**
-		 * End position.
-		 *
-		 * @member {module:engine/view/position~Position}
-		 */
-		this.end = end ? end : start;
+		this[ _start ] = start;
+		this[ _end ] = end ? end : start;
 	}
 
 	/**
@@ -53,9 +45,30 @@ export default class Range {
 		yield* new TreeWalker( { boundaries: this, ignoreElementEnd: true } );
 	}
 
+	/**
+	 * Start position.
+	 *
+	 * @readonly
+	 * @type {module:engine/view/position~Position}
+	 */
+	get start() {
+		return this[ _start ];
+	}
+
+	/**
+	 * End position.
+	 *
+	 * @readonly
+	 * @type {module:engine/view/position~Position}
+	 */
+	get end() {
+		return this[ _end ];
+	}
+
 	/**
 	 * Returns whether the range is collapsed, that is it start and end positions are equal.
 	 *
+	 * @readonly
 	 * @type {Boolean}
 	 */
 	get isCollapsed() {
@@ -66,6 +79,7 @@ export default class Range {
 	 * Returns whether this range is flat, that is if {@link module:engine/view/range~Range#start start} position and
 	 * {@link module:engine/view/range~Range#end end} position are in the same {@link module:engine/view/position~Position#parent parent}.
 	 *
+	 * @readonly
 	 * @type {Boolean}
 	 */
 	get isFlat() {
@@ -75,6 +89,7 @@ export default class Range {
 	/**
 	 * Range root element.
 	 *
+	 * @readonly
 	 * @type {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment}
 	 */
 	get root() {

+ 16 - 10
packages/ckeditor5-engine/src/view/writer.js

@@ -353,9 +353,7 @@ export function remove( range ) {
 	const removed = parentContainer.removeChildren( breakStart.offset, count );
 
 	// Merge after removing.
-	const mergePosition = mergeAttributes( breakStart );
-	range.start = mergePosition;
-	range.end = mergePosition;
+	mergeAttributes( breakStart );
 
 	// Return removed nodes.
 	return new DocumentFragment( removed );
@@ -406,17 +404,20 @@ export function clear( range, element ) {
 
 		// If we have found element to remove.
 		if ( rangeToRemove ) {
+			let rangeEnd = rangeToRemove.end;
+			let rangeStart = rangeToRemove.start;
+
 			// We need to check if element range stick out of the given range and truncate if it is.
 			if ( rangeToRemove.end.isAfter( range.end ) ) {
-				rangeToRemove.end = range.end;
+				rangeEnd = range.end;
 			}
 
 			if ( rangeToRemove.start.isBefore( range.start ) ) {
-				rangeToRemove.start = range.start;
+				rangeStart = range.start;
 			}
 
 			// At the end we remove range with found element.
-			remove( rangeToRemove );
+			remove( new Range( rangeStart, rangeEnd ) );
 		}
 	}
 }
@@ -511,10 +512,13 @@ export function wrap( range, attribute ) {
 	const start = mergeAttributes( newRange.start );
 
 	// If start position was merged - move end position back.
+	let rangeEnd = newRange.end;
+
 	if ( !start.isEqual( newRange.start ) ) {
-		newRange.end = newRange.end.getShiftedBy( -1 );
+		rangeEnd = rangeEnd.getShiftedBy( -1 );
 	}
-	const end = mergeAttributes( newRange.end );
+
+	const end = mergeAttributes( rangeEnd );
 
 	return new Range( start, end );
 }
@@ -625,10 +629,12 @@ export function unwrap( range, attribute ) {
 	const start = mergeAttributes( newRange.start );
 
 	// If start position was merged - move end position back.
+	let rangeEnd = newRange.end;
+
 	if ( !start.isEqual( newRange.start ) ) {
-		newRange.end = newRange.end.getShiftedBy( -1 );
+		rangeEnd = rangeEnd.getShiftedBy( -1 );
 	}
-	const end = mergeAttributes( newRange.end );
+	const end = mergeAttributes( rangeEnd );
 
 	return new Range( start, end );
 }

+ 12 - 13
packages/ckeditor5-engine/tests/view/writer/remove.js

@@ -15,8 +15,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 describe( 'writer', () => {
 	/**
-	 * Executes test using `parse` and `stringify` utils functions. Uses range delimiters `[]{}` to create and
-	 * test ranges.
+	 * Executes test using `parse` and `stringify` utils functions. Uses range delimiters `[]{}` to create ranges.
 	 *
 	 * @param {String} input
 	 * @param {String} expectedResult
@@ -27,7 +26,7 @@ describe( 'writer', () => {
 
 		const range = selection.getFirstRange();
 		const removed = remove( range );
-		expect( stringify( view, range, { showType: true, showPriority: true } ) ).to.equal( expectedResult );
+		expect( stringify( view, null, { showType: true, showPriority: true } ) ).to.equal( expectedResult );
 		expect( stringify( removed, null, { showType: true, showPriority: true } ) ).to.equal( expectedRemoved );
 	}
 
@@ -60,21 +59,21 @@ describe( 'writer', () => {
 		} );
 
 		it( 'should remove single text node', () => {
-			test( '<container:p>[foobar]</container:p>', '<container:p>[]</container:p>', 'foobar' );
+			test( '<container:p>[foobar]</container:p>', '<container:p></container:p>', 'foobar' );
 		} );
 
 		it( 'should not leave empty text nodes', () => {
-			test( '<container:p>{foobar}</container:p>', '<container:p>[]</container:p>', 'foobar' );
+			test( '<container:p>{foobar}</container:p>', '<container:p></container:p>', 'foobar' );
 		} );
 
 		it( 'should remove part of the text node', () => {
-			test( '<container:p>f{oob}ar</container:p>', '<container:p>f{}ar</container:p>', 'oob' );
+			test( '<container:p>f{oob}ar</container:p>', '<container:p>far</container:p>', 'oob' );
 		} );
 
 		it( 'should remove parts of nodes #1', () => {
 			test(
 				'<container:p>f{oo<attribute:b view-priority="10">ba}r</attribute:b></container:p>',
-				'<container:p>f[]<attribute:b view-priority="10">r</attribute:b></container:p>',
+				'<container:p>f<attribute:b view-priority="10">r</attribute:b></container:p>',
 				'oo<attribute:b view-priority="10">ba</attribute:b>'
 			);
 		} );
@@ -82,7 +81,7 @@ describe( 'writer', () => {
 		it( 'should support unicode', () => {
 			test(
 				'<container:p>நி{லை<attribute:b view-priority="10">க்}கு</attribute:b></container:p>',
-				'<container:p>நி[]<attribute:b view-priority="10">கு</attribute:b></container:p>',
+				'<container:p>நி<attribute:b view-priority="10">கு</attribute:b></container:p>',
 				'லை<attribute:b view-priority="10">க்</attribute:b>'
 			);
 		} );
@@ -92,7 +91,7 @@ describe( 'writer', () => {
 				'<container:p>' +
 					'<attribute:b view-priority="1">foo</attribute:b>[bar]<attribute:b view-priority="1">bazqux</attribute:b>' +
 				'</container:p>',
-				'<container:p><attribute:b view-priority="1">foo{}bazqux</attribute:b></container:p>',
+				'<container:p><attribute:b view-priority="1">foobazqux</attribute:b></container:p>',
 				'bar'
 			);
 		} );
@@ -102,19 +101,19 @@ describe( 'writer', () => {
 				'<container:p>' +
 					'<attribute:b view-priority="1">fo{o</attribute:b>bar<attribute:b view-priority="1">ba}zqux</attribute:b>' +
 				'</container:p>',
-				'<container:p><attribute:b view-priority="1">fo{}zqux</attribute:b></container:p>',
+				'<container:p><attribute:b view-priority="1">fozqux</attribute:b></container:p>',
 				'<attribute:b view-priority="1">o</attribute:b>bar<attribute:b view-priority="1">ba</attribute:b>'
 			);
 		} );
 
 		it( 'should remove part of the text node in document fragment', () => {
-			test( 'fo{ob}ar', 'fo{}ar', 'ob' );
+			test( 'fo{ob}ar', 'foar', 'ob' );
 		} );
 
 		it( 'should remove EmptyElement', () => {
 			test(
 				'<container:p>foo[<empty:img></empty:img>]bar</container:p>',
-				'<container:p>foo{}bar</container:p>',
+				'<container:p>foobar</container:p>',
 				'<empty:img></empty:img>'
 			);
 		} );
@@ -133,7 +132,7 @@ describe( 'writer', () => {
 		it( 'should remove UIElement', () => {
 			test(
 				'<container:p>foo[<ui:span></ui:span>]bar</container:p>',
-				'<container:p>foo{}bar</container:p>',
+				'<container:p>foobar</container:p>',
 				'<ui:span></ui:span>'
 			);
 		} );