Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions src/model/liveselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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 );
}
Expand Down Expand Up @@ -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 );
}
}
Expand Down
33 changes: 30 additions & 3 deletions tests/model/liveselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
} );
Expand Down Expand Up @@ -226,6 +231,7 @@ describe( 'LiveSelection', () => {
sinon.spy( ranges[ 0 ], 'detach' );
sinon.spy( ranges[ 1 ], 'detach' );

sinon.spy( selection, 'stopListening' );
selection.removeAllRanges();
} );

Expand All @@ -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;
} );
Expand All @@ -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 );

Expand All @@ -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;
Expand Down Expand Up @@ -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;
} );
} );
} );

Expand Down