Kaynağa Gözat

Fix: DocumentSelection will validate only the changed range instead of the whole selection during updating one of them.

Kamil Piechaczek 8 yıl önce
ebeveyn
işleme
a8f5d5f037

+ 7 - 3
packages/ckeditor5-engine/src/model/documentselection.js

@@ -456,8 +456,12 @@ class LiveSelection extends Selection {
 		this._attributePriority = new Map();
 
 		// Add events that will ensure selection correctness.
-		this.on( 'change:range', () => {
-			for ( const range of this.getRanges() ) {
+		this.on( 'change:range', ( evt, options ) => {
+			// If the `options.range` is specified, only the passed Range will be checked.
+			// It prevents to checking selection's ranges before they are updated. Read more #1281.
+			const ranges = options.range ? [ options.range ] : Array.from( this.getRanges() );
+
+			for ( const range of ranges ) {
 				if ( !this._document._validateSelectionRange( range ) ) {
 					/**
 					 * Range from {@link module:engine/model/documentselection~DocumentSelection document selection}
@@ -624,7 +628,7 @@ class LiveSelection extends Selection {
 			}
 
 			// Whenever a live range from selection changes, fire an event informing about that change.
-			this.fire( 'change:range', { directChange: false } );
+			this.fire( 'change:range', { directChange: false, range: liveRange } );
 		} );
 
 		return liveRange;

+ 216 - 0
packages/ckeditor5-engine/tests/tickets/1281.js

@@ -0,0 +1,216 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* globals document */
+
+import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+import Position from '../../src/model/position';
+
+import { setData as setModelData, getData as getModelData } from '../../src/dev-utils/model';
+
+describe( 'Bug ckeditor5-engine#1281', () => {
+	let element, editor, model;
+
+	beforeEach( () => {
+		element = document.createElement( 'div' );
+		document.body.appendChild( element );
+
+		return ClassicTestEditor
+			.create( element, { plugins: [ Paragraph ] } )
+			.then( newEditor => {
+				editor = newEditor;
+				model = editor.model;
+			} );
+	} );
+
+	afterEach( () => {
+		element.remove();
+
+		return editor.destroy();
+	} );
+
+	it( 'loads content that contains multi-range selection', () => {
+		setModelData( model,
+			'<paragraph>Paragraph 1.</paragraph>' +
+			'<paragraph>Paragraph 2.</paragraph>' +
+			'<paragraph>[Paragraph 3.]</paragraph>' +
+			'<paragraph>[Paragraph 4.]</paragraph>'
+		);
+
+		const root = model.document.getRoot();
+		const thirdParagraph = root.getNodeByPath( [ 2 ] );
+		const fourthParagraph = root.getNodeByPath( [ 3 ] );
+		const selRanges = Array.from( model.document.selection.getRanges() );
+
+		expect( selRanges.length ).to.equal( 2 );
+
+		assertPositions( Position.createAt( thirdParagraph ), selRanges[ 0 ].start );
+		assertPositions( Position.createAt( thirdParagraph, 'end' ), selRanges[ 0 ].end );
+
+		assertPositions( Position.createAt( fourthParagraph ), selRanges[ 1 ].start );
+		assertPositions( Position.createAt( fourthParagraph, 'end' ), selRanges[ 1 ].end );
+	} );
+
+	it( 'does not throw an error when content before the selection is being removed (last element is selected)', () => {
+		setModelData( model,
+			'<paragraph>Paragraph 1.</paragraph>' +
+			'<paragraph>Paragraph 2.</paragraph>' +
+			'<paragraph>[Paragraph 3.]</paragraph>' +
+			'<paragraph>[Paragraph 4.]</paragraph>'
+		);
+
+		model.change( writer => {
+			const root = model.document.getRoot();
+			const firstParagraph = root.getNodeByPath( [ 0 ] );
+
+			expect( () => {
+				writer.remove( firstParagraph );
+			} ).to.not.throw();
+
+			assertOutput(
+				'<paragraph>Paragraph 2.</paragraph>' +
+				'<paragraph>[Paragraph 3.]</paragraph>' +
+				'<paragraph>[Paragraph 4.]</paragraph>'
+			);
+		} );
+	} );
+
+	it( 'does not throw an error when content before the selection is being removed (last element is not selected)', () => {
+		setModelData( model,
+			'<paragraph>Paragraph 1.</paragraph>' +
+			'<paragraph>Paragraph 2.</paragraph>' +
+			'<paragraph>[Paragraph 3.]</paragraph>' +
+			'<paragraph>[Paragraph 4.]</paragraph>' +
+			'<paragraph>Paragraph 5.</paragraph>'
+		);
+
+		model.change( writer => {
+			const root = model.document.getRoot();
+			const firstParagraph = root.getNodeByPath( [ 0 ] );
+
+			expect( () => {
+				writer.remove( firstParagraph );
+			} ).to.not.throw();
+
+			assertOutput(
+				'<paragraph>Paragraph 2.</paragraph>' +
+				'<paragraph>[Paragraph 3.]</paragraph>' +
+				'<paragraph>[Paragraph 4.]</paragraph>' +
+				'<paragraph>Paragraph 5.</paragraph>'
+			);
+		} );
+	} );
+
+	it( 'does not throw an error when content after the selection is being removed (first element is selected)', () => {
+		setModelData( model,
+			'<paragraph>[Paragraph 1.]</paragraph>' +
+			'<paragraph>Paragraph 2.</paragraph>' +
+			'<paragraph>Paragraph 3.</paragraph>' +
+			'<paragraph>[Paragraph 4.]</paragraph>' +
+			'<paragraph>Paragraph 5.</paragraph>'
+		);
+
+		model.change( writer => {
+			const root = model.document.getRoot();
+			const lastParagraph = root.getNodeByPath( [ 4 ] );
+
+			expect( () => {
+				writer.remove( lastParagraph );
+			} ).to.not.throw();
+
+			assertOutput(
+				'<paragraph>[Paragraph 1.]</paragraph>' +
+				'<paragraph>Paragraph 2.</paragraph>' +
+				'<paragraph>Paragraph 3.</paragraph>' +
+				'<paragraph>[Paragraph 4.]</paragraph>'
+			);
+		} );
+	} );
+
+	it( 'does not throw an error when content after the selection is being removed (first element is not selected)', () => {
+		setModelData( model,
+			'<paragraph>Paragraph 1.</paragraph>' +
+			'<paragraph>Paragraph 2.</paragraph>' +
+			'<paragraph>[Paragraph 3.]</paragraph>' +
+			'<paragraph>[Paragraph 4.]</paragraph>' +
+			'<paragraph>Paragraph 5.</paragraph>'
+		);
+
+		model.change( writer => {
+			const root = model.document.getRoot();
+			const lastParagraph = root.getNodeByPath( [ 4 ] );
+
+			expect( () => {
+				writer.remove( lastParagraph );
+			} ).to.not.throw();
+
+			assertOutput(
+				'<paragraph>Paragraph 1.</paragraph>' +
+				'<paragraph>Paragraph 2.</paragraph>' +
+				'<paragraph>[Paragraph 3.]</paragraph>' +
+				'<paragraph>[Paragraph 4.]</paragraph>'
+			);
+		} );
+	} );
+
+	it( 'does not throw an error when content between the selection\'s ranges is being removed (last element is selected)', () => {
+		setModelData( model,
+			'<paragraph>Paragraph 1.</paragraph>' +
+			'<paragraph>[Paragraph 2.]</paragraph>' +
+			'<paragraph>Paragraph 3.</paragraph>' +
+			'<paragraph>[Paragraph 4.]</paragraph>'
+		);
+
+		model.change( writer => {
+			const root = model.document.getRoot();
+			const thirdParagraph = root.getNodeByPath( [ 2 ] );
+
+			expect( () => {
+				writer.remove( thirdParagraph );
+			} ).to.not.throw();
+
+			assertOutput(
+				'<paragraph>Paragraph 1.</paragraph>' +
+				'<paragraph>[Paragraph 2.]</paragraph>' +
+				'<paragraph>[Paragraph 4.]</paragraph>'
+			);
+		} );
+	} );
+
+	it( 'does not throw an error when content between the selection\'s ranges is being removed (last element is not selected)', () => {
+		setModelData( model,
+			'<paragraph>Paragraph 1.</paragraph>' +
+			'<paragraph>[Paragraph 2.]</paragraph>' +
+			'<paragraph>Paragraph 3.</paragraph>' +
+			'<paragraph>[Paragraph 4.]</paragraph>' +
+			'<paragraph>Paragraph 5.</paragraph>'
+		);
+
+		model.change( writer => {
+			const root = model.document.getRoot();
+			const thirdParagraph = root.getNodeByPath( [ 2 ] );
+
+			expect( () => {
+				writer.remove( thirdParagraph );
+			} ).to.not.throw();
+
+			assertOutput(
+				'<paragraph>Paragraph 1.</paragraph>' +
+				'<paragraph>[Paragraph 2.]</paragraph>' +
+				'<paragraph>[Paragraph 4.]</paragraph>' +
+				'<paragraph>Paragraph 5.</paragraph>'
+			);
+		} );
+	} );
+
+	function assertPositions( firstPosition, secondPosition ) {
+		expect( firstPosition.isEqual( secondPosition ) ).to.be.true;
+	}
+
+	function assertOutput( output ) {
+		expect( getModelData( model ) ).to.equal( output );
+	}
+} );