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

Commit

Permalink
Merge pull request #1047 from ckeditor/t/1046
Browse files Browse the repository at this point in the history
Fixed: If a new position of `DocumentSelection` cannot be calculated after the content in which the selection was located was removed from the document, the position of the selection should use the "default selection" so it does not end up in disallowed places. Closes #1046.
  • Loading branch information
Reinmar authored Jul 25, 2017
2 parents 552198e + 0996409 commit 9f7e0a2
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 10 deletions.
19 changes: 9 additions & 10 deletions src/model/documentselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -649,24 +649,23 @@ export default class DocumentSelection extends Selection {
const positionCandidate = Position.createFromPosition( removedRange.start );

// Find a range that is a correct selection range and is closest to the start of removed range.
let selectionRange = this._document.getNearestSelectionRange( positionCandidate );

// If nearest valid selection range cannot be found - use one created at the beginning of the root.
if ( !selectionRange ) {
selectionRange = new Range( new Position( positionCandidate.root, [ 0 ] ) );
}
const selectionRange = this._document.getNearestSelectionRange( positionCandidate );

// Remove the old selection range before preparing and adding new selection range. This order is important,
// because new range, in some cases, may intersect with old range (it depends on `getNearestSelectionRange()` result).
const index = this._ranges.indexOf( liveRange );
this._ranges.splice( index, 1 );
liveRange.detach();

// Check the range, convert it to live range, bind events, etc.
const newRange = this._prepareRange( selectionRange );
// If nearest valid selection range has been found - add it in the place of old range.
if ( selectionRange ) {
// Check the range, convert it to live range, bind events, etc.
const newRange = this._prepareRange( selectionRange );

// Add new range in the place of old range.
this._ranges.splice( index, 0, newRange );
// Add new range in the place of old range.
this._ranges.splice( index, 0, newRange );
}
// If nearest valid selection range cannot be found - just removing the old range is fine.

// Fire an event informing about selection change.
this.fire( 'change:range', { directChange: false } );
Expand Down
24 changes: 24 additions & 0 deletions tests/model/documentselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,30 @@ describe( 'DocumentSelection', () => {

expect( selection.getFirstPosition().path ).to.deep.equal( [ 0, 6 ] );
} );

it( 'fix selection range if it ends up in graveyard #4 - whole content removed', () => {
doc.applyOperation( wrapInDelta(
new RemoveOperation(
new Position( root, [ 0 ] ),
3,
new Position( doc.graveyard, [ 0 ] ),
doc.version
)
) );

expect( selection.getFirstPosition().path ).to.deep.equal( [ 0 ] );

doc.applyOperation( wrapInDelta(
new InsertOperation(
new Position( root, [ 0 ] ),
new Element( 'p' ),
doc.version
)
) );

// Now it's clear that it's the default range.
expect( selection.getFirstPosition().path ).to.deep.equal( [ 0, 0 ] );
} );
} );
} );

Expand Down

0 comments on commit 9f7e0a2

Please sign in to comment.