Procházet zdrojové kódy

Merge branch 'master' into t/1556

Piotr Jasiun před 7 roky
rodič
revize
b412a31d34

+ 1 - 1
packages/ckeditor5-engine/docs/api/engine.md

@@ -12,7 +12,7 @@ Together with the {@link api/core core editor architecture} and the {@link api/u
 
 ## Documentation
 
-See the introduction to the {@link framework/guides/architecture/intro#editing-engine editing engine's architecture}.
+See the introduction to the {@link framework/guides/architecture/editing-engine editing engine's architecture}.
 
 You can also browse the API documentation of this package by using the module tree on the left.
 

+ 14 - 8
packages/ckeditor5-engine/src/conversion/downcastdispatcher.js

@@ -321,22 +321,28 @@ export default class DowncastDispatcher {
 		// In markers' case, event name == consumable name.
 		const eventName = 'addMarker:' + markerName;
 
-		// When range is collapsed - fire single event with collapsed range in consumable.
-		if ( markerRange.isCollapsed ) {
-			const consumable = new Consumable();
-			consumable.add( markerRange, eventName );
+		//
+		// First, fire an event for the whole marker.
+		//
+		const consumable = new Consumable();
+		consumable.add( markerRange, eventName );
 
-			this.conversionApi.consumable = consumable;
+		this.conversionApi.consumable = consumable;
 
-			this.fire( eventName, { markerName, markerRange }, this.conversionApi );
+		this.fire( eventName, { markerName, markerRange }, this.conversionApi );
 
+		//
+		// Do not fire events for each item inside the range if the range got consumed.
+		//
+		if ( !consumable.test( markerRange, eventName ) ) {
 			return;
 		}
 
-		// Create consumable for each item in range.
+		//
+		// Then, fire an event for each item inside the marker range.
+		//
 		this.conversionApi.consumable = this._createConsumableForRange( markerRange, eventName );
 
-		// Create separate event for each node in the range.
 		for ( const item of markerRange.getItems() ) {
 			// Do not fire event for already consumed items.
 			if ( !this.conversionApi.consumable.test( item, eventName ) ) {

+ 2 - 2
packages/ckeditor5-engine/src/conversion/downcasthelpers.js

@@ -763,7 +763,7 @@ function changeAttribute( attributeCreator ) {
 // @returns {Function}
 function highlightText( highlightDescriptor ) {
 	return ( evt, data, conversionApi ) => {
-		if ( data.markerRange.isCollapsed ) {
+		if ( !data.item ) {
 			return;
 		}
 
@@ -824,7 +824,7 @@ function highlightText( highlightDescriptor ) {
 // @returns {Function}
 function highlightElement( highlightDescriptor ) {
 	return ( evt, data, conversionApi ) => {
-		if ( data.markerRange.isCollapsed ) {
+		if ( !data.item ) {
 			return;
 		}
 

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

@@ -696,7 +696,8 @@ class LiveSelection extends Selection {
 			 * UID obtained from the {@link module:engine/model/writer~Writer#overrideSelectionGravity} to restore.
 			 *
 			 * @error document-selection-gravity-wrong-restore
-			 * @param {String} uid The unique identifier returned by {@link #overrideGravity}.
+			 * @param {String} uid The unique identifier returned by
+			 * {@link module:engine/model/documentselection~DocumentSelection#_overrideGravity}.
 			 */
 			throw new CKEditorError(
 				'document-selection-gravity-wrong-restore: Attempting to restore the selection gravity for an unknown UID.',

+ 1 - 1
packages/ckeditor5-engine/src/model/markercollection.js

@@ -230,7 +230,7 @@ export default class MarkerCollection {
 	 * Fired whenever marker is added, updated or removed from `MarkerCollection`.
 	 *
 	 * @event update
-	 * @param {module:engine/model/markercollection~Marker} Updated Marker.
+	 * @param {module:engine/model/markercollection~Marker} marker Updated Marker.
 	 * @param {module:engine/model/range~Range|null} oldRange Marker range before the update. When is not defined it
 	 * means that marker is just added.
 	 * @param {module:engine/model/range~Range|null} newRange Marker range after update. When is not defined it

+ 70 - 2
packages/ckeditor5-engine/src/model/operation/transform.js

@@ -515,6 +515,48 @@ class ContextFactory {
 
 				break;
 			}
+
+			case MarkerOperation: {
+				const markerRange = opA.newRange;
+
+				if ( !markerRange ) {
+					return;
+				}
+
+				switch ( opB.constructor ) {
+					case MoveOperation: {
+						const movedRange = Range._createFromPositionAndShift( opB.sourcePosition, opB.howMany );
+
+						const affectedLeft = movedRange.containsPosition( markerRange.start ) ||
+							movedRange.start.isEqual( markerRange.start );
+
+						const affectedRight = movedRange.containsPosition( markerRange.end ) ||
+							movedRange.end.isEqual( markerRange.end );
+
+						if ( ( affectedLeft || affectedRight ) && !movedRange.containsRange( markerRange ) ) {
+							this._setRelation( opA, opB, {
+								side: affectedLeft ? 'left' : 'right',
+								offset: affectedLeft ? markerRange.start.offset : markerRange.end.offset
+							} );
+						}
+
+						break;
+					}
+
+					case MergeOperation: {
+						const wasInLeftElement = markerRange.start.isEqual( opB.targetPosition );
+						const wasInRightElement = markerRange.end.isEqual( opB.sourcePosition );
+
+						if ( wasInLeftElement || wasInRightElement ) {
+							this._setRelation( opA, opB, { wasInLeftElement, wasInRightElement } );
+						}
+
+						break;
+					}
+				}
+
+				break;
+			}
 		}
 	}
 
@@ -1068,24 +1110,49 @@ setTransformation( MarkerOperation, MergeOperation, ( a, b ) => {
 	return [ a ];
 } );
 
-setTransformation( MarkerOperation, MoveOperation, ( a, b ) => {
+setTransformation( MarkerOperation, MoveOperation, ( a, b, context ) => {
 	if ( a.oldRange ) {
 		a.oldRange = Range._createFromRanges( a.oldRange._getTransformedByMoveOperation( b ) );
 	}
 
 	if ( a.newRange ) {
+		if ( context.abRelation ) {
+			if ( context.abRelation.side == 'left' && b.targetPosition.isEqual( a.newRange.start ) ) {
+				a.newRange.start.offset = context.abRelation.offset;
+				a.newRange.end.offset += b.howMany;
+
+				return [ a ];
+			} else if ( context.abRelation.side == 'right' && b.targetPosition.isEqual( a.newRange.end ) ) {
+				a.newRange.end.offset = context.abRelation.offset;
+
+				return [ a ];
+			}
+		}
+
 		a.newRange = Range._createFromRanges( a.newRange._getTransformedByMoveOperation( b ) );
 	}
 
 	return [ a ];
 } );
 
-setTransformation( MarkerOperation, SplitOperation, ( a, b ) => {
+setTransformation( MarkerOperation, SplitOperation, ( a, b, context ) => {
 	if ( a.oldRange ) {
 		a.oldRange = a.oldRange._getTransformedBySplitOperation( b );
 	}
 
 	if ( a.newRange ) {
+		if ( context.abRelation ) {
+			if ( a.newRange.start.isEqual( b.splitPosition ) && !context.abRelation.wasInLeftElement ) {
+				a.newRange.start = Position._createAt( b.moveTargetPosition );
+			}
+
+			if ( a.newRange.end.isEqual( b.splitPosition ) && context.abRelation.wasInRightElement ) {
+				a.newRange.end = Position._createAt( b.moveTargetPosition );
+			}
+
+			return [ a ];
+		}
+
 		a.newRange = a.newRange._getTransformedBySplitOperation( b );
 	}
 
@@ -1719,6 +1786,7 @@ setTransformation( MoveOperation, MergeOperation, ( a, b, context ) => {
 				targetPositionPath.push( 0 );
 
 				const splitNodesMoveTarget = new Position( gyMove.targetPosition.root, targetPositionPath );
+				splitNodesMoveSource = splitNodesMoveSource._getTransformedByMove( gyMoveSource, gyMoveTarget, 1 );
 				const splitNodesMove = new MoveOperation( splitNodesMoveSource, b.howMany, splitNodesMoveTarget, 0 );
 
 				results.push( gyMove );

+ 15 - 0
packages/ckeditor5-engine/src/model/range.js

@@ -562,6 +562,21 @@ export default class Range {
 	 * @returns {module:engine/model/range~Range}
 	 */
 	_getTransformedByMergeOperation( operation ) {
+		// Special case when the marker is set on "the closing tag" of an element. Marker can be set like that during
+		// transformations, especially when a content of a few block elements were removed. For example:
+		//
+		// {} is the transformed range, [] is the removed range.
+		// <p>F[o{o</p><p>B}ar</p><p>Xy]z</p>
+		//
+		// <p>Fo{o</p><p>B}ar</p><p>z</p>
+		// <p>F{</p><p>B}ar</p><p>z</p>
+		// <p>F{</p>}<p>z</p>
+		// <p>F{}z</p>
+		//
+		if ( this.start.isEqual( operation.targetPosition ) && this.end.isEqual( operation.deletionPosition ) ) {
+			return new Range( this.start );
+		}
+
 		let start = this.start._getTransformedByMergeOperation( operation );
 		let end = this.end._getTransformedByMergeOperation( operation );
 

+ 65 - 12
packages/ckeditor5-engine/src/model/writer.js

@@ -466,6 +466,14 @@ export default class Writer {
 
 		const position = Position._createAt( itemOrPosition, offset );
 
+		// Do not move anything if the move target is same as moved range start.
+		if ( position.isEqual( range.start ) ) {
+			return;
+		}
+
+		// If part of the marker is removed, create additional marker operation for undo purposes.
+		this._addOperationForAffectedMarkers( 'move', range );
+
 		if ( !isSameTree( range.root, position.root ) ) {
 			/**
 			 * Range is going to be moved within not the same document. Please use
@@ -491,17 +499,15 @@ export default class Writer {
 	remove( itemOrRange ) {
 		this._assertWriterUsedCorrectly();
 
-		if ( itemOrRange instanceof Range ) {
-			// The array is reversed, so the ranges to remove are in correct order and do not have to be updated.
-			const ranges = itemOrRange.getMinimalFlatRanges().reverse();
+		const rangeToRemove = itemOrRange instanceof Range ? itemOrRange : Range._createOn( itemOrRange );
 
-			for ( const flat of ranges ) {
-				applyRemoveOperation( flat.start, flat.end.offset - flat.start.offset, this.batch, this.model );
-			}
-		} else {
-			const howMany = itemOrRange.is( 'text' ) ? itemOrRange.offsetSize : 1;
+		// If part of the marker is removed, create additional marker operation for undo purposes.
+		this._addOperationForAffectedMarkers( 'move', rangeToRemove );
 
-			applyRemoveOperation( Position._createBefore( itemOrRange ), howMany, this.batch, this.model );
+		const ranges = rangeToRemove.getMinimalFlatRanges().reverse();
+
+		for ( const flat of ranges ) {
+			applyRemoveOperation( flat.start, flat.end.offset - flat.start.offset, this.batch, this.model );
 		}
 	}
 
@@ -519,6 +525,9 @@ export default class Writer {
 		const nodeBefore = position.nodeBefore;
 		const nodeAfter = position.nodeAfter;
 
+		// If part of the marker is removed, create additional marker operation for undo purposes.
+		this._addOperationForAffectedMarkers( 'merge', position );
+
 		if ( !( nodeBefore instanceof Element ) ) {
 			/**
 			 * Node before merge position must be an element.
@@ -888,12 +897,12 @@ export default class Writer {
 
 		if ( !options || typeof options.usingOperation != 'boolean' ) {
 			/**
-			 * The `options.usingOperations` parameter is required when adding a new marker.
+			 * The `options.usingOperation` parameter is required when adding a new marker.
 			 *
-			 * @error writer-addMarker-no-usingOperations
+			 * @error writer-addMarker-no-usingOperation
 			 */
 			throw new CKEditorError(
-				'writer-addMarker-no-usingOperations: The options.usingOperations parameter is required when adding a new marker.'
+				'writer-addMarker-no-usingOperation: The options.usingOperation parameter is required when adding a new marker.'
 			);
 		}
 
@@ -1294,6 +1303,50 @@ export default class Writer {
 			throw new CKEditorError( 'writer-incorrect-use: Trying to use a writer outside the change() block.' );
 		}
 	}
+
+	/**
+	 * For given action `type` and `positionOrRange` where the action happens, this function finds all affected markers
+	 * and applies a marker operation with the new marker range equal to the current range. Thanks to this, the marker range
+	 * can be later correctly processed during undo.
+	 *
+	 * @private
+	 * @param {'move'|'merge'} type Writer action type.
+	 * @param {module:engine/model/position~Position|module:engine/model/range~Range} positionOrRange Position or range
+	 * where the writer action happens.
+	 */
+	_addOperationForAffectedMarkers( type, positionOrRange ) {
+		for ( const marker of this.model.markers ) {
+			if ( !marker.managedUsingOperations ) {
+				continue;
+			}
+
+			const markerRange = marker.getRange();
+			let isAffected = false;
+
+			if ( type == 'move' ) {
+				const intersecting =
+					positionOrRange.containsPosition( markerRange.start ) ||
+					positionOrRange.start.isEqual( markerRange.start ) ||
+					positionOrRange.containsPosition( markerRange.end ) ||
+					positionOrRange.end.isEqual( markerRange.end );
+
+				isAffected = intersecting && !positionOrRange.containsRange( markerRange );
+			} else {
+				// if type == 'merge'.
+				const elementBefore = positionOrRange.nodeBefore;
+				const elementAfter = positionOrRange.nodeAfter;
+
+				const affectedOnLeft = markerRange.start.parent == elementBefore && markerRange.start.isAtEnd;
+				const affectedOnRight = markerRange.end.parent == elementAfter && markerRange.end.offset == 0;
+
+				isAffected = affectedOnLeft || affectedOnRight;
+			}
+
+			if ( isAffected ) {
+				this.updateMarker( marker.name, { range: markerRange } );
+			}
+		}
+	}
 }
 
 // Sets given attribute to each node in given range. When attribute value is null then attribute will be removed.

+ 98 - 18
packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js

@@ -446,22 +446,29 @@ describe( 'DowncastDispatcher', () => {
 	} );
 
 	describe( 'convertMarkerAdd', () => {
-		let range, element, text;
+		let element, text;
 
 		beforeEach( () => {
 			text = new ModelText( 'foo bar baz' );
 			element = new ModelElement( 'paragraph', null, [ text ] );
 			root._appendChild( [ element ] );
-
-			range = model.createRange( model.createPositionAt( element, 0 ), model.createPositionAt( element, 4 ) );
 		} );
 
-		it( 'should fire addMarker event', () => {
-			sinon.spy( dispatcher, 'fire' );
+		it( 'should fire addMarker event for whole collapsed marker', () => {
+			const range = model.createRange( model.createPositionAt( element, 2 ), model.createPositionAt( element, 2 ) );
+
+			const spy = sinon.spy();
+
+			dispatcher.on( 'addMarker:name', ( evt, data ) => {
+				spy();
+
+				expect( data.markerName ).to.equal( 'name' );
+				expect( data.markerRange.isEqual( range ) ).to.be.true;
+			} );
 
 			dispatcher.convertMarkerAdd( 'name', range );
 
-			expect( dispatcher.fire.calledWith( 'addMarker:name' ) ).to.be.true;
+			expect( spy.calledOnce ).to.be.true;
 		} );
 
 		it( 'should not convert marker if it is in graveyard', () => {
@@ -483,37 +490,110 @@ describe( 'DowncastDispatcher', () => {
 			expect( dispatcher.fire.called ).to.be.false;
 		} );
 
-		it( 'should fire conversion for each item in the range', () => {
-			range = model.createRangeIn( root );
+		it( 'should fire addMarker event for whole non-collapsed marker and for each item in the range', () => {
+			const range = model.createRangeIn( root );
+
+			const spyWholeRange = sinon.spy();
+
+			dispatcher.on( 'addMarker:name', ( evt, data ) => {
+				if ( !data.item ) {
+					spyWholeRange();
+
+					expect( data.markerName ).to.equal( 'name' );
+					expect( data.markerRange.isEqual( range ) ).to.be.true;
+				}
+			} );
 
+			const spyItems = sinon.spy();
 			const items = [];
 
 			dispatcher.on( 'addMarker:name', ( evt, data, conversionApi ) => {
-				expect( data.markerName ).to.equal( 'name' );
-				expect( data.markerRange.isEqual( range ) ).to.be.true;
-				expect( conversionApi.consumable.test( data.item, 'addMarker:name' ) );
+				if ( data.item ) {
+					spyItems();
 
-				items.push( data.item );
+					expect( data.markerName ).to.equal( 'name' );
+					expect( data.markerRange.isEqual( range ) ).to.be.true;
+					expect( conversionApi.consumable.test( data.item, 'addMarker:name' ) );
+
+					items.push( data.item );
+				}
 			} );
 
 			dispatcher.convertMarkerAdd( 'name', range );
 
+			expect( spyWholeRange.calledOnce ).to.be.true;
+			expect( spyItems.calledTwice ).to.be.true;
+
 			expect( items[ 0 ] ).to.equal( element );
 			expect( items[ 1 ].data ).to.equal( text.data );
 		} );
 
-		it( 'should be possible to override', () => {
-			range = model.createRangeIn( root );
+		it( 'should not fire conversion for non-collapsed marker items if marker was consumed in earlier event', () => {
+			const range = model.createRangeIn( root );
+
+			dispatcher.on( 'addMarker:name', ( evt, data, conversionApi ) => {
+				if ( !data.item ) {
+					conversionApi.consumable.consume( data.markerRange, evt.name );
+				}
+			}, { priority: 'high' } );
+
+			const spyItems = sinon.spy();
+
+			dispatcher.on( 'addMarker:name', ( evt, data ) => {
+				if ( data.item ) {
+					spyItems();
+				}
+			} );
+
+			dispatcher.convertMarkerAdd( 'name', range );
+
+			expect( spyItems.called ).to.be.false;
+		} );
+
+		it( 'should be possible to override #1', () => {
+			const range = model.createRangeIn( root );
 
 			const addMarkerSpy = sinon.spy();
 			const highAddMarkerSpy = sinon.spy();
 
-			dispatcher.on( 'addMarker:marker', addMarkerSpy );
+			dispatcher.on( 'addMarker:marker', ( evt, data ) => {
+				if ( !data.item ) {
+					addMarkerSpy();
+				}
+			} );
 
-			dispatcher.on( 'addMarker:marker', evt => {
-				highAddMarkerSpy();
+			dispatcher.on( 'addMarker:marker', ( evt, data ) => {
+				if ( !data.item ) {
+					highAddMarkerSpy();
 
-				evt.stop();
+					evt.stop();
+				}
+			}, { priority: 'high' } );
+
+			dispatcher.convertMarkerAdd( 'marker', range );
+
+			expect( addMarkerSpy.called ).to.be.false;
+			expect( highAddMarkerSpy.calledOnce ).to.be.true;
+		} );
+
+		it( 'should be possible to override #2', () => {
+			const range = model.createRangeIn( root );
+
+			const addMarkerSpy = sinon.spy();
+			const highAddMarkerSpy = sinon.spy();
+
+			dispatcher.on( 'addMarker:marker', ( evt, data ) => {
+				if ( data.item ) {
+					addMarkerSpy();
+				}
+			} );
+
+			dispatcher.on( 'addMarker:marker', ( evt, data ) => {
+				if ( data.item ) {
+					highAddMarkerSpy();
+
+					evt.stop();
+				}
 			}, { priority: 'high' } );
 
 			dispatcher.convertMarkerAdd( 'marker', range );

+ 46 - 0
packages/ckeditor5-engine/tests/model/operation/transform/marker.js

@@ -421,6 +421,52 @@ describe( 'transform', () => {
 					'<paragraph><m1:start></m1:start>Foo Bar<m1:end></m1:end></paragraph>'
 				);
 			} );
+
+			it( 'left side of marker moved, insertion at the moved range start, move undo', () => {
+				john.setData( '<paragraph>Foo[bar]</paragraph><paragraph></paragraph>' );
+				kate.setData( '<paragraph>Foo[bar]</paragraph><paragraph></paragraph>' );
+
+				john.setMarker( 'm1' );
+				john.setSelection( [ 0, 2 ], [ 0, 4 ] );
+				john.move( [ 1, 0 ] );
+
+				syncClients();
+
+				kate.setSelection( [ 0, 2 ] );
+				kate.type( 'xx' );
+
+				syncClients();
+
+				expectClients( '<paragraph>Foxx<m1:start></m1:start>ar<m1:end></m1:end></paragraph><paragraph>ob</paragraph>' );
+
+				john.undo();
+				syncClients();
+
+				expectClients( '<paragraph>Foobxx<m1:start></m1:start>ar<m1:end></m1:end></paragraph><paragraph></paragraph>' );
+			} );
+
+			it( 'right side of marker moved, insertion at the moved range start, move undo', () => {
+				john.setData( '<paragraph>[Foo]bar</paragraph><paragraph></paragraph>' );
+				kate.setData( '<paragraph>[Foo]bar</paragraph><paragraph></paragraph>' );
+
+				john.setMarker( 'm1' );
+				john.setSelection( [ 0, 2 ], [ 0, 4 ] );
+				john.move( [ 1, 0 ] );
+
+				syncClients();
+
+				kate.setSelection( [ 0, 2 ] );
+				kate.type( 'xx' );
+
+				syncClients();
+
+				expectClients( '<paragraph><m1:start></m1:start>Fo<m1:end></m1:end>xxar</paragraph><paragraph>ob</paragraph>' );
+
+				john.undo();
+				syncClients();
+
+				expectClients( '<paragraph><m1:start></m1:start>Foo<m1:end></m1:end>bxxar</paragraph><paragraph></paragraph>' );
+			} );
 		} );
 
 		describe( 'by remove', () => {

+ 70 - 0
packages/ckeditor5-engine/tests/model/operation/transform/undo.js

@@ -388,4 +388,74 @@ describe( 'transform', () => {
 		john.undo();
 		expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
 	} );
+
+	it( 'collapsed marker at the beginning of merged element then undo', () => {
+		john.setData( '<paragraph>Foo</paragraph><paragraph>[]Bar</paragraph>' );
+
+		john.setMarker( 'm1' );
+		john.setSelection( [ 1 ] );
+		john.merge();
+
+		expectClients( '<paragraph>Foo<m1:start></m1:start>Bar</paragraph>' );
+
+		john.undo();
+
+		expectClients( '<paragraph>Foo</paragraph><paragraph><m1:start></m1:start>Bar</paragraph>' );
+	} );
+
+	it( 'collapsed marker at the end of merge-target element then undo', () => {
+		john.setData( '<paragraph>Foo[]</paragraph><paragraph>Bar</paragraph>' );
+
+		john.setMarker( 'm1' );
+		john.setSelection( [ 1 ] );
+		john.merge();
+
+		expectClients( '<paragraph>Foo<m1:start></m1:start>Bar</paragraph>' );
+
+		john.undo();
+
+		expectClients( '<paragraph>Foo<m1:start></m1:start></paragraph><paragraph>Bar</paragraph>' );
+	} );
+
+	it( 'empty marker between merged elements then undo', () => {
+		john.setData( '<paragraph>Foo[</paragraph><paragraph>]Bar</paragraph>' );
+
+		john.setMarker( 'm1' );
+		john.setSelection( [ 1 ] );
+		john.merge();
+
+		expectClients( '<paragraph>Foo<m1:start></m1:start>Bar</paragraph>' );
+
+		john.undo();
+
+		expectClients( '<paragraph>Foo<m1:start></m1:start></paragraph><paragraph><m1:end></m1:end>Bar</paragraph>' );
+	} );
+
+	it( 'left side of marker moved then undo', () => {
+		john.setData( '<paragraph>Foo[bar]</paragraph><paragraph></paragraph>' );
+
+		john.setMarker( 'm1' );
+		john.setSelection( [ 0, 2 ], [ 0, 4 ] );
+		john.move( [ 1, 0 ] );
+
+		expectClients( '<paragraph>Fo<m1:start></m1:start>ar<m1:end></m1:end></paragraph><paragraph>ob</paragraph>' );
+
+		john.undo();
+
+		expectClients( '<paragraph>Foo<m1:start></m1:start>bar<m1:end></m1:end></paragraph><paragraph></paragraph>' );
+	} );
+
+	it( 'right side of marker moved then undo', () => {
+		john.setData( '<paragraph>[Foo]bar</paragraph><paragraph></paragraph>' );
+
+		john.setMarker( 'm1' );
+		john.setSelection( [ 0, 2 ], [ 0, 4 ] );
+		john.move( [ 1, 0 ] );
+
+		expectClients( '<paragraph><m1:start></m1:start>Fo<m1:end></m1:end>ar</paragraph><paragraph>ob</paragraph>' );
+
+		john.undo();
+
+		expectClients( '<paragraph><m1:start></m1:start>Foo<m1:end></m1:end>bar</paragraph><paragraph></paragraph>' );
+	} );
 } );

+ 21 - 0
packages/ckeditor5-engine/tests/model/range.js

@@ -1179,6 +1179,27 @@ describe( 'Range', () => {
 				expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 2 ] );
 				expect( transformed[ 0 ].end.path ).to.deep.equal( [ 2 ] );
 			} );
+
+			it( 'range is set on closing tag of merge target element', () => {
+				// <p>aa{</p>}<p>bb</p>
+				const range = new Range( new Position( root, [ 0, 2 ] ), new Position( root, [ 1 ] ) );
+
+				const op = new MergeOperation(
+					new Position( root, [ 1, 0 ] ),
+					2,
+					new Position( root, [ 0, 2 ] ),
+					gyPos,
+					1
+				);
+
+				const transformed = range.getTransformedByOperation( op );
+
+				expect( transformed.length ).to.equal( 1 );
+
+				// <p>aa{}bb</p>
+				expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 2 ] );
+				expect( transformed[ 0 ].end.path ).to.deep.equal( [ 0, 2 ] );
+			} );
 		} );
 	} );
 

+ 140 - 4
packages/ckeditor5-engine/tests/model/writer.js

@@ -1413,6 +1413,51 @@ describe( 'Writer', () => {
 			expect( docFrag.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foobar' );
 		} );
 
+		it( 'should create a marker operation if a marker was affected', () => {
+			const markerRange = new Range( Position._createAt( p2, 0 ), Position._createAt( p2, 0 ) );
+
+			addMarker( 'name', {
+				range: markerRange,
+				usingOperation: true
+			} );
+
+			const documentVersion = model.document.version;
+
+			merge( Position._createAfter( p1 ) );
+
+			const history = model.document.history;
+
+			const lastOperation = history._operations[ history._operations.length - 1 ];
+			const secondLastOperation = history._operations[ history._operations.length - 2 ];
+
+			expect( secondLastOperation.type ).to.equal( 'marker' );
+			expect( secondLastOperation.oldRange.isEqual( markerRange ) );
+			expect( secondLastOperation.newRange.isEqual( markerRange ) );
+
+			expect( lastOperation.type ).to.equal( 'merge' );
+			expect( model.document.version ).to.equal( documentVersion + 2 );
+		} );
+
+		it( 'should not create a marker operation if affected marker was not using operations', () => {
+			const markerRange = new Range( Position._createAt( p2, 0 ), Position._createAt( p2, 2 ) );
+
+			addMarker( 'name', {
+				range: markerRange,
+				usingOperation: false
+			} );
+
+			const documentVersion = model.document.version;
+
+			merge( Position._createAfter( p1 ) );
+
+			const history = model.document.history;
+
+			const lastOperation = history._operations[ history._operations.length - 1 ];
+
+			expect( lastOperation.type ).to.equal( 'merge' );
+			expect( model.document.version ).to.equal( documentVersion + 1 );
+		} );
+
 		it( 'should throw if there is no element after', () => {
 			expect( () => {
 				merge( new Position( root, [ 2 ] ) );
@@ -1458,6 +1503,51 @@ describe( 'Writer', () => {
 			expect( getNodesAndText( Range._createIn( root.getChild( 1 ) ) ) ).to.equal( 'abcobarxyz' );
 		} );
 
+		it( 'should create a marker operation if a marker was affected', () => {
+			const markerRange = new Range( Position._createAt( p, 1 ), Position._createAt( p, 4 ) );
+
+			addMarker( 'name', {
+				range: markerRange,
+				usingOperation: true
+			} );
+
+			const documentVersion = model.document.version;
+
+			move( new Range( Position._createAt( p, 0 ), Position._createAt( p, 2 ) ), Position._createAt( div, 0 ) );
+
+			const history = model.document.history;
+
+			const lastOperation = history._operations[ history._operations.length - 1 ];
+			const secondLastOperation = history._operations[ history._operations.length - 2 ];
+
+			expect( secondLastOperation.type ).to.equal( 'marker' );
+			expect( secondLastOperation.oldRange.isEqual( markerRange ) );
+			expect( secondLastOperation.newRange.isEqual( markerRange ) );
+
+			expect( lastOperation.type ).to.equal( 'move' );
+			expect( model.document.version ).to.equal( documentVersion + 2 );
+		} );
+
+		it( 'should not create a marker operation if affected marker was not using operations', () => {
+			const markerRange = new Range( Position._createAt( p, 1 ), Position._createAt( p, 4 ) );
+
+			addMarker( 'name', {
+				range: markerRange,
+				usingOperation: false
+			} );
+
+			const documentVersion = model.document.version;
+
+			move( new Range( Position._createAt( p, 0 ), Position._createAt( p, 2 ) ), Position._createAt( div, 0 ) );
+
+			const history = model.document.history;
+
+			const lastOperation = history._operations[ history._operations.length - 1 ];
+
+			expect( lastOperation.type ).to.equal( 'move' );
+			expect( model.document.version ).to.equal( documentVersion + 1 );
+		} );
+
 		it( 'should throw if object to move is not a range', () => {
 			expect( () => {
 				move( root.getChild( 0 ), new Position( root, [ 1, 3 ] ) );
@@ -1555,6 +1645,52 @@ describe( 'Writer', () => {
 				expect( batch.operations[ 0 ].type ).to.equal( 'remove' );
 			} );
 
+			it( 'should create a marker operation if a marker was affected', () => {
+				const markerRange = new Range( Position._createAt( p, 1 ), Position._createAt( p, 4 ) );
+
+				addMarker( 'name', {
+					range: markerRange,
+					usingOperation: true
+				} );
+
+				const documentVersion = model.document.version;
+
+				remove( new Range( Position._createAt( p, 0 ), Position._createAt( p, 2 ) ) );
+
+				const history = model.document.history;
+
+				const lastOperation = history._operations[ history._operations.length - 1 ];
+				const secondLastOperation = history._operations[ history._operations.length - 2 ];
+
+				expect( secondLastOperation.type ).to.equal( 'marker' );
+				expect( secondLastOperation.oldRange.isEqual( markerRange ) );
+				expect( secondLastOperation.newRange.isEqual( markerRange ) );
+
+				expect( lastOperation.type ).to.equal( 'remove' );
+
+				expect( model.document.version ).to.equal( documentVersion + 2 );
+			} );
+
+			it( 'should not create a marker operation if affected marker was not using operations', () => {
+				const markerRange = new Range( Position._createAt( p, 1 ), Position._createAt( p, 4 ) );
+
+				addMarker( 'name', {
+					range: markerRange,
+					usingOperation: false
+				} );
+
+				const documentVersion = model.document.version;
+
+				remove( new Range( Position._createAt( p, 0 ), Position._createAt( p, 2 ) ) );
+
+				const history = model.document.history;
+
+				const lastOperation = history._operations[ history._operations.length - 1 ];
+
+				expect( lastOperation.type ).to.equal( 'remove' );
+				expect( model.document.version ).to.equal( documentVersion + 1 );
+			} );
+
 			it( 'should throw when trying to use detached writer', () => {
 				const writer = new Writer( model, batch );
 
@@ -1922,16 +2058,16 @@ describe( 'Writer', () => {
 			range = Range._createIn( root );
 		} );
 
-		it( 'should throw if options.usingOperations is not defined', () => {
+		it( 'should throw if options.usingOperation is not defined', () => {
 			expect( () => {
 				addMarker( 'name' );
-			} ).to.throw( CKEditorError, /^writer-addMarker-no-usingOperations/ );
+			} ).to.throw( CKEditorError, /^writer-addMarker-no-usingOperation/ );
 		} );
 
-		it( 'should throw if name and range is defined but options.usingOperations is not defined', () => {
+		it( 'should throw if name and range is defined but options.usingOperation is not defined', () => {
 			expect( () => {
 				addMarker( 'name', { range } );
-			} ).to.throw( CKEditorError, /^writer-addMarker-no-usingOperations/ );
+			} ).to.throw( CKEditorError, /^writer-addMarker-no-usingOperation/ );
 		} );
 
 		it( 'should add marker to the document marker collection', () => {