Changed: Fixed detaching model.LiveRange in model.LiveSelection.

Author: scofalik

src/model/liveselection.js

@@ -116,8 +116,8 @@ export default class LiveSelection extends Selection {
 	 * Unbinds all events previously bound by document selection.
 	 */
 	destroy() {
-		for ( let i = 0; i < this._ranges.length; i++ ) {
-			this._ranges[ i ].detach();
+		while ( this._ranges.length ) {
+			this._popRange();
 		}
 
 		this.stopListening();
@@ -255,7 +255,19 @@ export default class LiveSelection extends Selection {
 	 * @inheritDoc
 	 */
 	_popRange() {
-		this._ranges.pop().detach();
+		this._removeRangeAtIndex( this._ranges.length - 1 );
+	}
+
+	/**
+	 * Removes a range from `LiveSelection` and detaches it.
+	 *
+	 * @private
+	 * @params {Number} index Index from which a range should be removed.
+	 */
+	_removeRangeAtIndex( index ) {
+		const range = this._ranges.splice( index, 1 )[ 0 ];
+
+		range.detach();
 	}
 
 	/**
@@ -302,7 +314,8 @@ export default class LiveSelection extends Selection {
 		this._checkRange( range );
 
 		const liveRange = LiveRange.createFromRange( range );
-		this.listenTo( liveRange, 'change', ( evt, oldRange ) => {
+
+		liveRange.on( 'change', ( evt, oldRange ) => {
 			if ( liveRange.root == this._document.graveyard ) {
 				this._fixGraveyardSelection( liveRange, oldRange );
 			}
@@ -638,8 +651,11 @@ export default class LiveSelection extends Selection {
 
 		const newRange = this._prepareRange( selectionRange );
 		const index = this._ranges.indexOf( gyRange );
-		gyRange.detach();
 
+		// Remove incorrect range.
+		this._removeRangeAtIndex( index );
+
+		// Splice in correct range.
 		this._ranges.splice( index, 1, newRange );
 	}
 }

tests/model/liveselection.js

@@ -171,16 +171,21 @@ describe( 'LiveSelection', () => {
 			selection.addRange( liveRange );
 			selection.addRange( range );
 
-			const ranges = selection._ranges;
+			const ranges = Array.from( selection._ranges );
 
 			sinon.spy( ranges[ 0 ], 'detach' );
 			sinon.spy( ranges[ 1 ], 'detach' );
 
+			sinon.spy( selection, 'stopListening' );
+
 			selection.destroy();
 
 			expect( ranges[ 0 ].detach.called ).to.be.true;
 			expect( ranges[ 1 ].detach.called ).to.be.true;
 
+			expect( selection.stopListening.calledWith( ranges[ 0 ] ) ).to.be.true;
+			expect( selection.stopListening.calledWith( ranges[ 1 ] ) ).to.be.true;
+
 			ranges[ 0 ].detach.restore();
 			ranges[ 1 ].detach.restore();
 		} );
@@ -226,6 +231,7 @@ describe( 'LiveSelection', () => {
 			sinon.spy( ranges[ 0 ], 'detach' );
 			sinon.spy( ranges[ 1 ], 'detach' );
 
+			sinon.spy( selection, 'stopListening' );
 			selection.removeAllRanges();
 		} );
 
@@ -240,7 +246,7 @@ describe( 'LiveSelection', () => {
 			expect( selection.focus.isEqual( new Position( root, [ 0, 0 ] ) ) ).to.be.true;
 		} );
 
-		it( 'should detach ranges', () => {
+		it( 'should detach ranges and stop listening to removed ranges', () => {
 			expect( ranges[ 0 ].detach.called ).to.be.true;
 			expect( ranges[ 1 ].detach.called ).to.be.true;
 		} );
@@ -261,7 +267,7 @@ describe( 'LiveSelection', () => {
 			} ).to.throw( CKEditorError, /model-selection-added-not-range/ );
 		} );
 
-		it( 'should detach removed ranges', () => {
+		it( 'should detach and stop listening to removed ranges', () => {
 			selection.addRange( liveRange );
 			selection.addRange( range );
 
@@ -270,6 +276,8 @@ describe( 'LiveSelection', () => {
 			sinon.spy( oldRanges[ 0 ], 'detach' );
 			sinon.spy( oldRanges[ 1 ], 'detach' );
 
+			sinon.spy( selection, 'stopListening' );
+
 			selection.setRanges( [] );
 
 			expect( oldRanges[ 0 ].detach.called ).to.be.true;
@@ -599,6 +607,25 @@ describe( 'LiveSelection', () => {
 
 				expect( selection.getFirstPosition().path ).to.deep.equal( [ 0, 6 ] );
 			} );
+
+			it( 'detach and stop listening to a range that ended up in in graveyard', () => {
+				selection.collapse( new Position( root, [ 1, 3 ] ) );
+
+				const range = selection._ranges[ 0 ];
+
+				sinon.spy( range, 'detach' );
+				sinon.spy( selection, 'stopListening' );
+
+				doc.applyOperation( wrapInDelta(
+					new RemoveOperation(
+						new Position( root, [ 1, 2 ] ),
+						2,
+						doc.version
+					)
+				) );
+
+				expect( range.detach.called ).to.be.true;
+			} );
 		} );
 	} );