diff --git a/packages/ckeditor5-engine/src/model/batch.ts b/packages/ckeditor5-engine/src/model/batch.ts index af8e52888e4..46e60e6ced3 100644 --- a/packages/ckeditor5-engine/src/model/batch.ts +++ b/packages/ckeditor5-engine/src/model/batch.ts @@ -126,8 +126,12 @@ export default class Batch implements BatchType { * @returns The added operation. */ public addOperation( operation: Operation ): Operation { - operation.batch = this; - this.operations.push( operation ); + if ( operation.isDocumentOperation ) { + // Store only document operations in the batch. + // Non-document operations are temporary and should be discarded after they are applied. + operation.batch = this; + this.operations.push( operation ); + } return operation; } diff --git a/packages/ckeditor5-engine/src/model/operation/operation.ts b/packages/ckeditor5-engine/src/model/operation/operation.ts index 50cb1635b01..1d35595b3f4 100644 --- a/packages/ckeditor5-engine/src/model/operation/operation.ts +++ b/packages/ckeditor5-engine/src/model/operation/operation.ts @@ -33,6 +33,9 @@ export default abstract class Operation { /** * {@link module:engine/model/batch~Batch Batch} to which the operation is added or `null` if the operation is not * added to any batch yet. + * + * Note, that a {@link ~isDocumentOperation non-document operation} has this property always set to `null`, and is never added + * to any batch. */ public batch: Batch | null; @@ -111,7 +114,7 @@ export default abstract class Operation { // Remove reference to the parent `Batch` to avoid circular dependencies. delete json.batch; - // Only document operations are shared with other clients so it is not necessary to keep this information. + // This can be derived from `baseVersion` so we can remove it. delete json.isDocumentOperation; return json; diff --git a/packages/ckeditor5-engine/tests/model/model.js b/packages/ckeditor5-engine/tests/model/model.js index f7c416c63db..9454b8480b2 100644 --- a/packages/ckeditor5-engine/tests/model/model.js +++ b/packages/ckeditor5-engine/tests/model/model.js @@ -1091,10 +1091,10 @@ describe( 'Model', () => { setData( model, 'fo[ob]ar' ); - model.change( writer => { - model.getSelectedContent( model.document.selection ); - expect( writer.batch.operations ).to.length( 1 ); - } ); + const version = model.document.version; + model.getSelectedContent( model.document.selection ); + + expect( model.document.version ).to.equal( version ); } ); } ); diff --git a/packages/ckeditor5-engine/tests/model/utils/getselectedcontent.js b/packages/ckeditor5-engine/tests/model/utils/getselectedcontent.js index 2ff104fc2ab..42fd952e86b 100644 --- a/packages/ckeditor5-engine/tests/model/utils/getselectedcontent.js +++ b/packages/ckeditor5-engine/tests/model/utils/getselectedcontent.js @@ -12,7 +12,7 @@ describe( 'DataController utils', () => { let model, doc; describe( 'getSelectedContent', () => { - it( 'should use parent batch', () => { + it( 'should not generate any document operations', () => { model = new Model(); doc = model.document; doc.createRoot(); @@ -20,10 +20,10 @@ describe( 'DataController utils', () => { model.schema.extend( '$text', { allowIn: '$root' } ); setData( model, 'x[abc]x' ); - model.change( writer => { - getSelectedContent( model, doc.selection ); - expect( writer.batch.operations ).to.length( 1 ); - } ); + const version = model.document.version; + getSelectedContent( model, doc.selection ); + + expect( model.document.version ).to.equal( version ); } ); describe( 'in simple scenarios', () => { diff --git a/packages/ckeditor5-engine/tests/model/writer.js b/packages/ckeditor5-engine/tests/model/writer.js index e85c18209ab..33d739f4518 100644 --- a/packages/ckeditor5-engine/tests/model/writer.js +++ b/packages/ckeditor5-engine/tests/model/writer.js @@ -206,8 +206,8 @@ describe( 'Writer', () => { expect( parent.childCount ).to.equal( 0 ); } ); - it( 'should create proper operation for inserting element', () => { - const parent = createDocumentFragment(); + it( 'should create proper operation for inserting element #1 (document operation)', () => { + const parent = doc.createRoot(); const element = createElement( 'child' ); const spy = sinon.spy( model, 'applyOperation' ); @@ -222,8 +222,24 @@ describe( 'Writer', () => { expect( spy.lastCall.args[ 0 ].batch ).to.equal( batch ); } ); - it( 'should create proper operation for inserting text', () => { + it( 'should create proper operation for inserting element #2 (non-document operation)', () => { const parent = createDocumentFragment(); + const element = createElement( 'child' ); + + const spy = sinon.spy( model, 'applyOperation' ); + + insert( element, parent ); + + sinon.assert.calledOnce( spy ); + + expect( spy.lastCall.args[ 0 ].type ).to.equal( 'insert' ); + expect( spy.lastCall.args[ 0 ] ).to.instanceof( InsertOperation ); + expect( spy.lastCall.args[ 0 ].shouldReceiveAttributes ).to.be.false; + expect( spy.lastCall.args[ 0 ].batch ).to.be.null; + } ); + + it( 'should create proper operation for inserting text', () => { + const parent = doc.createRoot(); const text = createText( 'child' ); const spy = sinon.spy( model, 'applyOperation' ); @@ -303,7 +319,7 @@ describe( 'Writer', () => { // Verify operations. sinon.assert.calledOnce( spy ); expect( spy.firstCall.args[ 0 ].type ).to.equal( 'move' ); - expect( spy.firstCall.args[ 0 ].batch ).to.equal( batch ); + expect( spy.firstCall.args[ 0 ].batch ).to.be.null; } ); it( 'should move element from one parent to the other within different document (docFragA -> docFragB)', () => { @@ -324,9 +340,9 @@ describe( 'Writer', () => { // Verify operations. sinon.assert.calledTwice( spy ); expect( spy.firstCall.args[ 0 ].type ).to.equal( 'detach' ); - expect( spy.firstCall.args[ 0 ].batch ).to.equal( batch ); + expect( spy.firstCall.args[ 0 ].batch ).to.be.null; expect( spy.secondCall.args[ 0 ].type ).to.equal( 'insert' ); - expect( spy.secondCall.args[ 0 ].batch ).to.equal( batch ); + expect( spy.secondCall.args[ 0 ].batch ).to.be.null; } ); it( 'should throw when moving element from document to document fragment', () => { @@ -515,7 +531,7 @@ describe( 'Writer', () => { } ); it( 'should create proper operation', () => { - const parent = createDocumentFragment(); + const parent = doc.createRoot(); const spy = sinon.spy( model, 'applyOperation' ); insertText( 'foo', parent ); @@ -637,7 +653,7 @@ describe( 'Writer', () => { } ); it( 'should create proper operation', () => { - const parent = createDocumentFragment(); + const parent = doc.createRoot(); const spy = sinon.spy( model, 'applyOperation' ); insertElement( 'foo', parent ); @@ -672,7 +688,7 @@ describe( 'Writer', () => { } ); it( 'should create proper operation', () => { - const parent = createDocumentFragment(); + const parent = doc.createRoot(); const text = createText( 'foo' ); const spy = sinon.spy( model, 'applyOperation' ); @@ -750,7 +766,7 @@ describe( 'Writer', () => { // Verify operations. sinon.assert.calledOnce( spy ); expect( spy.firstCall.args[ 0 ].type ).to.equal( 'move' ); - expect( spy.firstCall.args[ 0 ].batch ).to.equal( batch ); + expect( spy.firstCall.args[ 0 ].batch ).to.be.null; } ); it( 'should move element from one parent to the other within different document fragments (docFragA -> docFragB)', () => { @@ -771,9 +787,9 @@ describe( 'Writer', () => { // Verify operations. sinon.assert.calledTwice( spy ); expect( spy.firstCall.args[ 0 ].type ).to.equal( 'detach' ); - expect( spy.firstCall.args[ 0 ].batch ).to.equal( batch ); + expect( spy.firstCall.args[ 0 ].batch ).to.be.null; expect( spy.secondCall.args[ 0 ].type ).to.equal( 'insert' ); - expect( spy.secondCall.args[ 0 ].batch ).to.equal( batch ); + expect( spy.secondCall.args[ 0 ].batch ).to.be.null; } ); it( 'should throw when moving element from document to document fragment', () => { @@ -823,7 +839,7 @@ describe( 'Writer', () => { } ); it( 'should create proper operations', () => { - const parent = createDocumentFragment(); + const parent = doc.createRoot(); const spy = sinon.spy( model, 'applyOperation' ); appendText( 'foo', parent ); @@ -877,7 +893,7 @@ describe( 'Writer', () => { } ); it( 'should create proper operation', () => { - const parent = createDocumentFragment(); + const parent = doc.createRoot(); const spy = sinon.spy( model, 'applyOperation' ); appendElement( 'foo', parent ); @@ -1840,14 +1856,14 @@ describe( 'Writer', () => { batch = new Batch(); remove( range ); - expect( batch.operations.length ).to.equal( 2 ); + expect( batch.operations.length ).to.equal( 0 ); } ); it( 'should use DetachOperation', () => { - batch = new Batch(); + sinon.spy( model, 'applyOperation' ); remove( div ); - expect( batch.operations[ 0 ].type ).to.equal( 'detach' ); + expect( model.applyOperation.firstCall.args[ 0 ].type ).to.equal( 'detach' ); } ); it( 'should throw when trying to use detached writer', () => {