Merge pull request #1059 from ckeditor/t/1058

Fix: `view.Range#getTrimmed()` was returning incorrect ranges in some cases. Fixes #1058.

Author: Reinmar

src/view/range.js

@@ -98,8 +98,8 @@ export default class Range {
 	 * @returns {module:engine/view/range~Range} Enlarged range.
 	 */
 	getEnlarged() {
-		let start = this.start.getLastMatchingPosition( enlargeShrinkSkip, { direction: 'backward' } );
-		let end = this.end.getLastMatchingPosition( enlargeShrinkSkip );
+		let start = this.start.getLastMatchingPosition( enlargeTrimSkip, { direction: 'backward' } );
+		let end = this.end.getLastMatchingPosition( enlargeTrimSkip );
 
 		// Fix positions, in case if they are in Text node.
 		if ( start.parent.is( 'text' ) && start.isAtStart ) {
@@ -130,8 +130,8 @@ export default class Range {
 	 * @returns {module:engine/view/range~Range} Shrink range.
 	 */
 	getTrimmed() {
-		let start = this.start.getLastMatchingPosition( enlargeShrinkSkip );
-		let end = this.end.getLastMatchingPosition( enlargeShrinkSkip, { direction: 'backward' } );
+		let start = this.start.getLastMatchingPosition( enlargeTrimSkip, { boundaries: new Range( this.start, this.end ) } );
+		let end = this.end.getLastMatchingPosition( enlargeTrimSkip, { boundaries: new Range( start, this.end ), direction: 'backward' } );
 		const nodeAfterStart = start.nodeAfter;
 		const nodeBeforeEnd = end.nodeBefore;
 
@@ -438,7 +438,7 @@ export default class Range {
 }
 
 // Function used by getEnlagred and getShrinked methods.
-function enlargeShrinkSkip( value ) {
+function enlargeTrimSkip( value ) {
 	if ( value.item.is( 'attributeElement' ) || value.item.is( 'uiElement' ) ) {
 		return true;
 	}

tests/view/range.js

@@ -162,16 +162,36 @@ describe( 'Range', () => {
 				.to.equal( '<p><b>f</b>{oo}<b><span></span>bar</b></p>' );
 		} );
 
-		it( 'case6', () => {
+		it( 'case 6', () => {
 			expect( trim( '<p>foo[</p><p>bar</p><p>]bom</p>' ) )
 				.to.equal( '<p>foo[</p><p>bar</p><p>]bom</p>' );
 		} );
 
-		it( 'case7', () => {
+		it( 'case 7', () => {
 			expect( trim( '<p>foo[<b><img></img></b>]bom</p>' ) )
 				.to.equal( '<p>foo<b>[<img></img>]</b>bom</p>' );
 		} );
 
+		// Other results may theoretically be correct too. It is not decided whether the trimmed range should
+		// be collapsed in attribute element, at its start or its end. This is one of possible correct results
+		// and we won't know for sure unless we have more cases. See #1058.
+		it( 'case 8', () => {
+			expect( trim( '<p>[<b></b>]</p>' ) )
+				.to.equal( '<p><b></b>[]</p>' );
+		} );
+
+		// As above.
+		it( 'case 9', () => {
+			expect( trim( '<p><b></b>[<b></b>]<b></b></p>' ) )
+				.to.equal( '<p><b></b><b></b>[]<b></b></p>' );
+		} );
+
+		// As above.
+		it( 'case 10', () => {
+			expect( trim( '<p>[<b></b><b></b>]</p>' ) )
+				.to.equal( '<p><b></b><b></b>[]</p>' );
+		} );
+
 		function trim( data ) {
 			data = data
 				.replace( /<p>/g, '<container:p>' )