8
0
Просмотр исходного кода

Merge pull request #1047 from ckeditor/t/1046

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.
Piotrek Koszuliński 8 лет назад
Родитель
Сommit
e24f2a857f

+ 9 - 10
packages/ckeditor5-engine/src/model/documentselection.js

@@ -649,12 +649,7 @@ export default class DocumentSelection extends Selection {
 		const positionCandidate = Position.createFromPosition( removedRange.start );
 		const positionCandidate = Position.createFromPosition( removedRange.start );
 
 
 		// Find a range that is a correct selection range and is closest to the start of removed range.
 		// 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,
 		// 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).
 		// 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 );
 		this._ranges.splice( index, 1 );
 		liveRange.detach();
 		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.
 		// Fire an event informing about selection change.
 		this.fire( 'change:range', { directChange: false } );
 		this.fire( 'change:range', { directChange: false } );

+ 24 - 0
packages/ckeditor5-engine/tests/model/documentselection.js

@@ -680,6 +680,30 @@ describe( 'DocumentSelection', () => {
 
 
 				expect( selection.getFirstPosition().path ).to.deep.equal( [ 0, 6 ] );
 				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 ] );
+			} );
 		} );
 		} );
 	} );
 	} );