Merge pull request #1745 from ckeditor/t/1721

Fix: `Mode.writer#insert` will no longer crash when the data to set contains markers that are already in the editor content. Closes #1721.

src/model/writer.js

@@ -216,7 +216,13 @@ export default class Writer {
 					markerRange.end._getCombined( rangeRootPosition, position )
 				);
 
-				this.addMarker( markerName, { range, usingOperation: true, affectsData: true } );
+				const options = { range, usingOperation: true, affectsData: true };
+
+				if ( this.model.markers.has( markerName ) ) {
+					this.updateMarker( markerName, options );
+				} else {
+					this.addMarker( markerName, options );
+				}
 			}
 		}
 	}

tests/controller/datacontroller.js

@@ -336,6 +336,24 @@ describe( 'DataController', () => {
 
 			expect( getData( model, { withoutSelection: true } ) ).to.equal( 'foo' );
 		} );
+
+		// https://github.com/ckeditor/ckeditor5-engine/issues/1721.
+		it( 'should not throw when setting the data with markers that already exist in the editor', () => {
+			schema.extend( '$text', { allowIn: '$root' } );
+
+			data.set( 'foo' );
+
+			downcastHelpers.markerToElement( { model: 'marker', view: 'marker' } );
+			upcastHelpers.elementToMarker( { view: 'marker', model: 'marker' } );
+
+			model.change( writer => {
+				writer.addMarker( 'marker', { range: writer.createRangeIn( modelDocument.getRoot() ), usingOperation: true } );
+			} );
+
+			expect( () => {
+				data.set( data.get() );
+			} ).not.to.throw();
+		} );
 	} );
 
 	describe( 'get()', () => {

tests/model/writer.js

@@ -307,10 +307,10 @@ describe( 'Writer', () => {
 
 		it( 'should transfer markers from given DocumentFragment', () => {
 			const root = doc.createRoot();
-
 			const docFrag = createDocumentFragment();
 
 			appendText( 'abcd', root );
+
 			appendElement( 'p', docFrag );
 			insertText( 'foo bar', new Position( docFrag, [ 0, 0 ] ) );
 
@@ -331,6 +331,39 @@ describe( 'Writer', () => {
 			expect( modelMarker.affectsData ).to.be.true;
 		} );
 
+		// https://github.com/ckeditor/ckeditor5-engine/issues/1721.
+		it( 'should update a marker if DocumentFragment has a marker that is already in the model (markers have the same name)', () => {
+			const root = doc.createRoot();
+			const docFrag = createDocumentFragment();
+
+			// <root><p></p><p>[ab]cd</p></root>.
+			appendText( 'abcd', root );
+
+			// <docFrag><p>f[oo b]ar</p></docFrag>.
+			appendElement( 'p', docFrag );
+			insertText( 'foo bar', new Position( docFrag, [ 0, 0 ] ) );
+
+			model.change( writer => {
+				const range = new Range( new Position( root, [ 1, 0 ] ), new Position( root, [ 1, 2 ] ) );
+
+				writer.addMarker( 'marker', { range, usingOperation: true } );
+			} );
+
+			docFrag.markers.set( 'marker', new Range( new Position( docFrag, [ 0, 1 ] ), new Position( docFrag, [ 0, 5 ] ) ) );
+
+			insert( docFrag, new Position( root, [ 2 ] ) );
+
+			expect( Array.from( model.markers ).length ).to.equal( 1 );
+
+			const modelMarker = model.markers.get( 'marker' );
+			const range = modelMarker.getRange();
+			expect( range.root ).to.equal( root );
+			expect( range.start.path ).to.deep.equal( [ 2, 1 ] );
+			expect( range.end.path ).to.deep.equal( [ 2, 5 ] );
+			expect( modelMarker.managedUsingOperations ).to.be.true;
+			expect( modelMarker.affectsData ).to.be.true;
+		} );
+
 		it( 'should throw when trying to use detached writer', () => {
 			const writer = new Writer( model, batch );
 			const root = doc.createRoot();