Przeglądaj źródła

Merge pull request #848 from ckeditor/t/842

T/842
Piotr Jasiun 8 lat temu
rodzic
commit
dd075225fe

+ 11 - 2
packages/ckeditor5-engine/src/view/range.js

@@ -98,8 +98,17 @@ export default class Range {
 	 * @returns {module:engine/view/range~Range} Enlarged range.
 	 */
 	getEnlarged() {
-		const start = this.start.getLastMatchingPosition( enlargeShrinkSkip, { direction: 'backward' } );
-		const end = this.end.getLastMatchingPosition( enlargeShrinkSkip );
+		let start = this.start.getLastMatchingPosition( enlargeShrinkSkip, { direction: 'backward' } );
+		let end = this.end.getLastMatchingPosition( enlargeShrinkSkip );
+
+		// Fix positions, in case if they are in Text node.
+		if ( start.parent.is( 'text' ) && start.isAtStart ) {
+			start = Position.createBefore( start.parent );
+		}
+
+		if ( end.parent.is( 'text' ) && end.isAtEnd ) {
+			end = Position.createAfter( end.parent );
+		}
 
 		return new Range( start, end );
 	}

+ 14 - 20
packages/ckeditor5-engine/src/view/treewalker.js

@@ -129,8 +129,6 @@ export default class TreeWalker {
 		 * @member {module:engine/view/node~Node} module:engine/view/treewalker~TreeWalker#_boundaryEndParent
 		 */
 		this._boundaryEndParent = this.boundaries ? this.boundaries.end.parent : null;
-
-		this._fixStartPositionInText();
 	}
 
 	/**
@@ -208,6 +206,13 @@ export default class TreeWalker {
 
 		// Text is a specific parent because it contains string instead of child nodes.
 		if ( parent instanceof Text ) {
+			if ( position.isAtEnd ) {
+				// Prevent returning "elementEnd" for Text node. Skip that value and return the next walker step.
+				this.position = Position.createAfter( parent );
+
+				return this._next();
+			}
+
 			node = parent.data[ position.offset ];
 		} else {
 			node = parent.getChild( position.offset );
@@ -306,6 +311,13 @@ export default class TreeWalker {
 
 		// Text {@link module:engine/view/text~Text} element is a specific parent because contains string instead of child nodes.
 		if ( parent instanceof Text ) {
+			if ( position.isAtStart ) {
+				// Prevent returning "elementStart" for Text node. Skip that value and return the next walker step.
+				this.position = Position.createBefore( parent );
+
+				return this._previous();
+			}
+
 			node = parent.data[ position.offset - 1 ];
 		} else {
 			node = parent.getChild( position.offset - 1 );
@@ -432,24 +444,6 @@ export default class TreeWalker {
 			}
 		};
 	}
-
-	/**
-	 * Fixes tree walker start position if it is at the beginning or at the end of a {@link module:engine/view/text~Text text node}.
-	 *
-	 * Without the fix, the first returned value by tree walker would have type `elementEnd` or `elementStart` but the
-	 * item would be a {@link module:engine/view/text~Text text node}.
-	 *
-	 * @private
-	 */
-	_fixStartPositionInText() {
-		const parent = this.position.parent;
-
-		if ( parent instanceof Text && this.position.isAtStart ) {
-			this.position = Position.createBefore( parent );
-		} else if ( parent instanceof Text && this.position.isAtEnd ) {
-			this.position = Position.createAfter( parent );
-		}
-	}
 }
 
 /**

+ 13 - 1
packages/ckeditor5-engine/tests/view/treewalker.js

@@ -976,7 +976,7 @@ describe( 'TreeWalker', () => {
 		} );
 	} );
 
-	it( 'should not return elementEnd for a text node when iteration begins at the start or the end of that text node', () => {
+	it( 'should not return elementEnd for a text node when iteration begins at the end of that text node', () => {
 		let iterator = new TreeWalker( {
 			startPosition: Position.createAt( textAbcd, 'end' )
 		} );
@@ -987,6 +987,18 @@ describe( 'TreeWalker', () => {
 		expect( step.value.item ).to.equal( bold );
 	} );
 
+	it( 'should not return elementStart for a text node when iteration begins at the start of that text node', () => {
+		let iterator = new TreeWalker( {
+			startPosition: Position.createAt( textAbcd, 0 ),
+			direction: 'backward'
+		} );
+
+		const step = iterator.next();
+
+		expect( step.value.type ).to.equal( 'elementStart' );
+		expect( step.value.item ).to.equal( bold );
+	} );
+
 	it( 'should iterate over document fragment', () => {
 		const foo = new Text( 'foo' );
 		const bar = new Text( 'bar' );