diff --git a/src/model/differ.js b/src/model/differ.js index b521fd78d..8e3398655 100644 --- a/src/model/differ.js +++ b/src/model/differ.js @@ -107,30 +107,54 @@ export default class Differ { */ bufferOperation( operation ) { switch ( operation.type ) { - case 'insert': + case 'insert': { + if ( this._isInInsertedElement( operation.position.parent ) ) { + return; + } + this._markInsert( operation.position.parent, operation.position.offset, operation.nodes.maxOffset ); break; + } case 'addAttribute': case 'removeAttribute': - case 'changeAttribute': + case 'changeAttribute': { for ( const item of operation.range.getItems() ) { + if ( this._isInInsertedElement( item.parent ) ) { + continue; + } + this._markAttribute( item ); } break; + } case 'remove': case 'move': - case 'reinsert': - this._markRemove( operation.sourcePosition.parent, operation.sourcePosition.offset, operation.howMany ); - this._markInsert( operation.targetPosition.parent, operation.getMovedRangeStart().offset, operation.howMany ); + case 'reinsert': { + const sourceParentInserted = this._isInInsertedElement( operation.sourcePosition.parent ); + const targetParentInserted = this._isInInsertedElement( operation.targetPosition.parent ); + + if ( !sourceParentInserted ) { + this._markRemove( operation.sourcePosition.parent, operation.sourcePosition.offset, operation.howMany ); + } + + if ( !targetParentInserted ) { + this._markInsert( operation.targetPosition.parent, operation.getMovedRangeStart().offset, operation.howMany ); + } break; - case 'rename': + } + case 'rename': { + if ( this._isInInsertedElement( operation.position.parent ) ) { + return; + } + this._markRemove( operation.position.parent, operation.position.offset, 1 ); this._markInsert( operation.position.parent, operation.position.offset, 1 ); break; + } } // Clear cache after each buffered operation as it is no longer valid. @@ -396,10 +420,6 @@ export default class Differ { * @param {Number} howMany */ _markInsert( parent, offset, howMany ) { - if ( this._isInInsertedElement( parent ) ) { - return; - } - const changeItem = { type: 'insert', offset, howMany, count: this._changeCount++ }; this._markChange( parent, changeItem ); @@ -414,10 +434,6 @@ export default class Differ { * @param {Number} howMany */ _markRemove( parent, offset, howMany ) { - if ( this._isInInsertedElement( parent ) ) { - return; - } - const changeItem = { type: 'remove', offset, howMany, count: this._changeCount++ }; this._markChange( parent, changeItem ); @@ -432,10 +448,6 @@ export default class Differ { * @param {module:engine/model/item~Item} item */ _markAttribute( item ) { - if ( this._isInInsertedElement( item.parent ) ) { - return; - } - const changeItem = { type: 'attribute', offset: item.startOffset, howMany: item.offsetSize, count: this._changeCount++ }; this._markChange( item.parent, changeItem ); diff --git a/tests/model/differ.js b/tests/model/differ.js index 69c81ffec..8a977f4a4 100644 --- a/tests/model/differ.js +++ b/tests/model/differ.js @@ -641,6 +641,18 @@ describe( 'Differ', () => { ] ); } ); } ); + + it( 'inside a new element', () => { + // Since the rename is inside a new element, it should not be listed on changes list. + model.change( () => { + insert( new Element( 'blockQuote', null, new Element( 'paragraph' ) ), new Position( root, [ 2 ] ) ); + rename( root.getChild( 2 ).getChild( 0 ), 'listItem' ); + + expectChanges( [ + { type: 'insert', name: 'blockQuote', length: 1, position: new Position( root, [ 2 ] ) } + ] ); + } ); + } ); } ); describe( 'attribute', () => { @@ -1230,7 +1242,10 @@ describe( 'Differ', () => { } ); } ); - it( 'proper filtering of changes in inserted elements', () => { + // In this scenario we create a new element, then remove something from before it to mess up with offsets, + // finally we insert some content into a new element. Since we are inserting into a new element, the + // inserted children should not be shown on changes list. + it( 'proper filtering of changes in inserted elements #1', () => { root.removeChildren( 0, root.childCount ); root.appendChildren( new Element( 'image' ) ); @@ -1250,6 +1265,26 @@ describe( 'Differ', () => { ] ); } ); } ); + + // In this scenario we create a new element, then move another element that was before the new element into + // the new element. This way we mess up with offsets and insert content into a new element in one operation. + // Since we are inserting into a new element, the insertion of moved element should not be shown on changes list. + it( 'proper filtering of changes in inserted elements #2', () => { + root.removeChildren( 0, root.childCount ); + root.appendChildren( new Element( 'image' ) ); + + model.change( () => { + // Insert `div` after `image`. + insert( new Element( 'div' ), new Position( root, [ 1 ] ) ); + // Move `image` to the new `div`. + move( new Position( root, [ 0 ] ), 1, new Position( root, [ 1, 0 ] ) ); + + expectChanges( [ + { type: 'remove', name: 'image', length: 1, position: new Position( root, [ 0 ] ) }, + { type: 'insert', name: 'div', length: 1, position: new Position( root, [ 0 ] ) } + ] ); + } ); + } ); } ); describe( 'getChanges()', () => { @@ -1410,6 +1445,8 @@ describe( 'Differ', () => { function expectChanges( expected, includeChangesInGraveyard = false ) { const changes = differ.getChanges( { includeChangesInGraveyard } ); + expect( changes.length ).to.equal( expected.length ); + for ( let i = 0; i < expected.length; i++ ) { for ( const key in expected[ i ] ) { if ( expected[ i ].hasOwnProperty( key ) ) {