Forráskód Böngészése

Merge branch 't/1463' into t/ot-followups

Szymon Cofalik 7 éve
szülő
commit
e27ade5b4a

+ 2 - 2
packages/ckeditor5-engine/src/model/documentselection.js

@@ -750,14 +750,14 @@ class LiveSelection extends Selection {
 
 		const liveRange = LiveRange.createFromRange( range );
 
-		liveRange.on( 'change:range', ( evt, oldRange, operation ) => {
+		liveRange.on( 'change:range', ( evt, oldRange, deletionPosition ) => {
 			this._hasChangedRange = true;
 
 			// If `LiveRange` is in whole moved to the graveyard, save necessary data. It will be fixed on `Model#applyOperation` event.
 			if ( liveRange.root == this._document.graveyard ) {
 				this._fixGraveyardRangesData.push( {
 					liveRange,
-					sourcePosition: operation.sourcePosition
+					sourcePosition: deletionPosition
 				} );
 			}
 		} );

+ 14 - 5
packages/ckeditor5-engine/src/model/liverange.js

@@ -86,9 +86,7 @@ export default class LiveRange extends Range {
 	 * @param {Object} data Object with additional information about the change. Those parameters are passed from
 	 * {@link module:engine/model/document~Document#event:change document change event}.
 	 * @param {String} data.type Change type.
-	 * @param {module:engine/model/batch~Batch} data.batch Batch which changed the live range.
-	 * @param {module:engine/model/range~Range} data.range Range containing the result of applied change.
-	 * @param {module:engine/model/position~Position} data.sourcePosition Source position for move, remove and reinsert change types.
+	 * @param {module:engine/model/position~Position|null} deletionPosition Source position for move, remove and reinsert change types.
 	 */
 
 	/**
@@ -147,16 +145,27 @@ function transform( operation ) {
 	const contentChanged = doesOperationChangeRangeContent( this, operation );
 
 	if ( boundariesChanged ) {
+		let deletionPosition = null;
+
+		if ( result.root.rootName == '$graveyard' ) {
+			if ( operation.type == 'remove' ) {
+				deletionPosition = operation.sourcePosition;
+			} else {
+				// Merge operation.
+				deletionPosition = operation.deletionPosition;
+			}
+		}
+
 		// If range boundaries have changed, fire `change:range` event.
 		const oldRange = Range.createFromRange( this );
 
 		this.start = result.start;
 		this.end = result.end;
 
-		this.fire( 'change:range', oldRange, operation );
+		this.fire( 'change:range', oldRange, deletionPosition );
 	} else if ( contentChanged ) {
 		// If range boundaries have not changed, but there was change inside the range, fire `change:content` event.
-		this.fire( 'change:content', Range.createFromRange( this ), operation );
+		this.fire( 'change:content', Range.createFromRange( this ), null );
 	}
 }
 

+ 40 - 17
packages/ckeditor5-engine/tests/model/liverange.js

@@ -3,7 +3,6 @@
  * For licensing, see LICENSE.md.
  */
 
-import Batch from '../../src/model/batch';
 import Model from '../../src/model/model';
 import Element from '../../src/model/element';
 import Position from '../../src/model/position';
@@ -90,7 +89,7 @@ describe( 'LiveRange', () => {
 		range.detach();
 	} );
 
-	it( 'should fire change:range event with proper data when its boundaries are changed', () => {
+	it( 'should fire change:range event with when its boundaries are changed', () => {
 		const live = new LiveRange( new Position( root, [ 0, 1, 4 ] ), new Position( root, [ 0, 2, 2 ] ) );
 		const copy = Range.createFromRange( live );
 
@@ -99,15 +98,11 @@ describe( 'LiveRange', () => {
 
 		const sourcePosition = new Position( root, [ 2 ] );
 		const targetPosition = new Position( root, [ 0 ] );
-		const batch = new Batch();
-		let op = null;
 
-		model.enqueueChange( batch, writer => {
+		model.change( writer => {
 			const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 );
 
 			writer.move( sourceRange, targetPosition );
-
-			op = batch.operations[ 0 ];
 		} );
 
 		expect( spy.calledOnce ).to.be.true;
@@ -115,11 +110,11 @@ describe( 'LiveRange', () => {
 		// First parameter available in event should be a range that is equal to the live range before the live range changed.
 		expect( spy.args[ 0 ][ 1 ].isEqual( copy ) ).to.be.true;
 
-		// Second parameter is the operation that changed the range.
-		expect( spy.args[ 0 ][ 2 ] ).to.equal( op );
+		// Second parameter is null for operations that did not move the range into graveyard.
+		expect( spy.args[ 0 ][ 2 ] ).to.be.null;
 	} );
 
-	it( 'should fire change:content event with proper data when content inside the range has changed', () => {
+	it( 'should fire change:content event when content inside the range has changed', () => {
 		const live = new LiveRange( new Position( root, [ 0, 1 ] ), new Position( root, [ 0, 3 ] ) );
 
 		const spy = sinon.spy();
@@ -127,15 +122,11 @@ describe( 'LiveRange', () => {
 
 		const sourcePosition = new Position( root, [ 0, 2, 0 ] );
 		const targetPosition = new Position( root, [ 0, 4, 0 ] );
-		const batch = new Batch();
-		let op = null;
 
-		model.enqueueChange( batch, writer => {
+		model.change( writer => {
 			const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 );
 
 			writer.move( sourceRange, targetPosition );
-
-			op = batch.operations[ 0 ];
 		} );
 
 		expect( spy.calledOnce ).to.be.true;
@@ -143,8 +134,40 @@ describe( 'LiveRange', () => {
 		// First parameter available in event should be a range that is equal to the live range before the live range changed.
 		expect( spy.args[ 0 ][ 1 ].isEqual( live ) ).to.be.true;
 
-		// Second parameter is the operation that changed the range.
-		expect( spy.args[ 0 ][ 2 ] ).to.equal( op );
+		// Second parameter is null for operations that did not move the range into graveyard.
+		expect( spy.args[ 0 ][ 2 ] ).to.be.null;
+	} );
+
+	it( 'should pass deletion position if range was removed (remove)', () => {
+		const live = new LiveRange( new Position( root, [ 0, 2 ] ), new Position( root, [ 0, 4 ] ) );
+
+		const spy = sinon.spy();
+		live.on( 'change:range', spy );
+
+		const sourcePosition = new Position( root, [ 0, 0 ] );
+
+		model.change( writer => {
+			writer.remove( Range.createFromPositionAndShift( sourcePosition, 6 ) );
+		} );
+
+		// Second parameter is deletion position.
+		expect( spy.args[ 0 ][ 2 ].isEqual( sourcePosition ) ).to.be.true;
+	} );
+
+	it( 'should pass deletion position if range was removed (merge)', () => {
+		const live = new LiveRange( new Position( root, [ 1 ] ), new Position( root, [ 2 ] ) );
+
+		const spy = sinon.spy();
+		live.on( 'change:range', spy );
+
+		const mergePosition = new Position( root, [ 1 ] );
+
+		model.change( writer => {
+			writer.merge( mergePosition );
+		} );
+
+		// Second parameter is deletion position.
+		expect( spy.args[ 0 ][ 2 ].isEqual( mergePosition ) ).to.be.true;
 	} );
 
 	describe( 'should get transformed and fire change:range if', () => {