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,10 @@ export default class LiveSelection extends Selection {
 	 * @inheritDoc
 	 */
 	_popRange() {
-		this._ranges.pop().detach();
+		const range = this._ranges.pop();
+
+		range.detach();
+		this.stopListening( range );
 	}
 
 	/**
@@ -302,6 +305,7 @@ export default class LiveSelection extends Selection {
 		this._checkRange( range );
 
 		const liveRange = LiveRange.createFromRange( range );
+
 		this.listenTo( liveRange, 'change', ( evt, oldRange ) => {
 			if ( liveRange.root == this._document.graveyard ) {
 				this._fixGraveyardSelection( liveRange, oldRange );
@@ -638,7 +642,9 @@ export default class LiveSelection extends Selection {
 
 		const newRange = this._prepareRange( selectionRange );
 		const index = this._ranges.indexOf( gyRange );
+
 		gyRange.detach();
+		this.stopListening( gyRange );
 
 		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,8 @@ describe( 'LiveSelection', () => {
 			sinon.spy( ranges[ 0 ], 'detach' );
 			sinon.spy( ranges[ 1 ], 'detach' );
 
+			sinon.spy( selection, 'stopListening' );
+
 			selection.removeAllRanges();
 		} );
 
@@ -240,9 +247,12 @@ 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;
+
+			expect( selection.stopListening.calledWith( ranges[ 0 ] ) ).to.be.true;
+			expect( selection.stopListening.calledWith( ranges[ 1 ] ) ).to.be.true;
 		} );
 
 		it( 'should refresh attributes', () => {
@@ -261,7 +271,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,10 +280,15 @@ 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;
 			expect( oldRanges[ 1 ].detach.called ).to.be.true;
+
+			expect( selection.stopListening.calledWith( oldRanges[ 0 ] ) ).to.be.true;
+			expect( selection.stopListening.calledWith( oldRanges[ 1 ] ) ).to.be.true;
 		} );
 
 		it( 'should refresh attributes', () => {
@@ -599,6 +614,26 @@ 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;
+				expect( selection.stopListening.calledWith( range ) ).to.be.true;
+			} );
 		} );
 	} );