From 22e611810d46d4fe5d6bb3f901829f1e7bf7d96c Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Tue, 25 Jul 2017 12:45:57 +0200 Subject: [PATCH 1/2] Fixed: `DocumentSelection#_fixGraveyardSelection` now does not add incorrect range when all content is removed from document, instead it is empty. --- src/model/documentselection.js | 19 +++++++++---------- tests/model/documentselection.js | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/model/documentselection.js b/src/model/documentselection.js index 19fcb0b2a..cb6f9ad48 100644 --- a/src/model/documentselection.js +++ b/src/model/documentselection.js @@ -649,12 +649,7 @@ 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). @@ -662,11 +657,15 @@ export default class DocumentSelection extends Selection { 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 } ); diff --git a/tests/model/documentselection.js b/tests/model/documentselection.js index c80075a46..9807c2544 100644 --- a/tests/model/documentselection.js +++ b/tests/model/documentselection.js @@ -680,6 +680,29 @@ 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 + ) + ) ); + + expect( selection.getFirstPosition().path ).to.deep.equal( [ 0, 0 ] ); + } ); } ); } ); From 09964094bd84fa48afb003d85d3804e38b2d83bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 25 Jul 2017 16:25:41 +0200 Subject: [PATCH 2/2] Made suere that the test will be understandable. --- tests/model/documentselection.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/model/documentselection.js b/tests/model/documentselection.js index 9807c2544..33b602605 100644 --- a/tests/model/documentselection.js +++ b/tests/model/documentselection.js @@ -701,6 +701,7 @@ describe( 'DocumentSelection', () => { ) ) ); + // Now it's clear that it's the default range. expect( selection.getFirstPosition().path ).to.deep.equal( [ 0, 0 ] ); } ); } );