Maciej Gołaszewski преди 5 години
родител
ревизия
8081dcecc7

+ 13 - 137
packages/ckeditor5-engine/src/model/differ.js

@@ -111,14 +111,12 @@ export default class Differ {
 	}
 
 	/**
-	 * Marks given `item` in differ to be re-inserted. It means that the item will be marked as removed and inserted in the differ changes
-	 * set. The `item` and all its children will be effectively re-converted when differ changes will be handled by a dispatcher.
-	 *
-	 * *Note*: To reconvert only `item` without reconverting children use {@link #refreshItem `differ.refreshItem()`}.
+	 * Marks given `item` in differ to be "refreshed". It means that the item will be marked as removed and inserted in the differ changes
+	 * set, so it will be effectively re-converted when differ changes will be handled by a dispatcher.
 	 *
 	 * @param {module:engine/model/item~Item} item Item to refresh.
 	 */
-	reInsertItem( item ) {
+	refreshItem( item ) {
 		if ( this._isInInsertedElement( item.parent ) ) {
 			return;
 		}
@@ -138,33 +136,6 @@ export default class Differ {
 		this._cachedChanges = null;
 	}
 
-	/**
-	 * Marks given `item` in differ to be "refreshed".
-	 *
-	 * It means that the item will be marked as a "refreshed" inserted in the differ changes. It will be then re-converted
-	 * when differ changes will be handled by a dispatcher.
-	 *
-	 * @param {module:engine/model/item~Item} item Item to refresh.
-	 */
-	refreshItem( item ) {
-		if ( this._isInInsertedElement( item.parent ) ) {
-			return;
-		}
-
-		this._markRefresh( item.parent, item.startOffset, item.offsetSize );
-
-		const range = Range._createOn( item );
-
-		for ( const marker of this._markerCollection.getMarkersIntersectingRange( range ) ) {
-			const markerRange = marker.getRange();
-
-			this.bufferMarkerChange( marker.name, markerRange, markerRange, marker.affectsData );
-		}
-
-		// Clear cache after each buffered operation as it is no longer valid.
-		this._cachedChanges = null;
-	}
-
 	/**
 	 * Buffers the given operation. An operation has to be buffered before it is executed.
 	 *
@@ -180,7 +151,7 @@ export default class Differ {
 		//
 		switch ( operation.type ) {
 			case 'insert': {
-				if ( this._isInInsertedElement( operation.position.parent ) || this._isInRefreshedElement( operation.position.parent ) ) {
+				if ( this._isInInsertedElement( operation.position.parent ) ) {
 					return;
 				}
 
@@ -192,7 +163,7 @@ export default class Differ {
 			case 'removeAttribute':
 			case 'changeAttribute': {
 				for ( const item of operation.range.getItems( { shallow: true } ) ) {
-					if ( this._isInInsertedElement( item.parent ) || this._isInRefreshedElement( item ) ) {
+					if ( this._isInInsertedElement( item.parent ) ) {
 						continue;
 					}
 
@@ -215,14 +186,12 @@ export default class Differ {
 
 				const sourceParentInserted = this._isInInsertedElement( operation.sourcePosition.parent );
 				const targetParentInserted = this._isInInsertedElement( operation.targetPosition.parent );
-				const sourceParentRefreshed = this._isInRefreshedElement( operation.sourcePosition.parent );
-				const targetParentRefreshed = this._isInRefreshedElement( operation.sourcePosition.parent );
 
-				if ( !sourceParentInserted && !sourceParentRefreshed ) {
+				if ( !sourceParentInserted ) {
 					this._markRemove( operation.sourcePosition.parent, operation.sourcePosition.offset, operation.howMany );
 				}
 
-				if ( !targetParentInserted && !targetParentRefreshed ) {
+				if ( !targetParentInserted ) {
 					this._markInsert( operation.targetPosition.parent, operation.getMovedRangeStart().offset, operation.howMany );
 				}
 
@@ -484,12 +453,6 @@ export default class Differ {
 					// there is a single diff for each of them) and insert them into the diff set.
 					diffSet.push( ...this._getAttributesDiff( range, snapshotAttributes, elementAttributes ) );
 
-					i++;
-					j++;
-				} else if ( action === 'x' ) {
-					// Swap action - similar to 'equal'.
-					diffSet.push( this._getRefreshDiff( element, i, elementChildren[ i ].name ) );
-
 					i++;
 					j++;
 				} else {
@@ -572,13 +535,13 @@ export default class Differ {
 		this._changeCount = 0;
 
 		// Cache changes.
-		this._cachedChangesWithGraveyard = diffSet;
-		this._cachedChanges = diffSet.filter( _changesInGraveyardFilter );
+		this._cachedChangesWithGraveyard = diffSet.slice();
+		this._cachedChanges = diffSet.slice().filter( _changesInGraveyardFilter );
 
 		if ( options.includeChangesInGraveyard ) {
-			return this._cachedChangesWithGraveyard.slice();
+			return this._cachedChangesWithGraveyard;
 		} else {
-			return this._cachedChanges.slice();
+			return this._cachedChanges;
 		}
 	}
 
@@ -622,20 +585,6 @@ export default class Differ {
 		this._removeAllNestedChanges( parent, offset, howMany );
 	}
 
-	/**
-	 * Saves and handles a refresh change.
-	 *
-	 * @private
-	 * @param {module:engine/model/element~Element} parent
-	 * @param {Number} offset
-	 * @param {Number} howMany
-	 */
-	_markRefresh( parent, offset, howMany ) {
-		const changeItem = { type: 'refresh', offset, howMany, count: this._changeCount++ };
-
-		this._markChange( parent, changeItem );
-	}
-
 	/**
 	 * Saves and handles an attribute change.
 	 *
@@ -931,26 +880,6 @@ export default class Differ {
 					}
 				}
 			}
-
-			if ( inc.type === 'refresh' ) {
-				if ( old.type === 'insert' ) {
-					if ( inc.offset >= old.offset && inc.offset < oldEnd ) {
-						inc.nodesToHandle = 0;
-					}
-				}
-
-				if ( old.type === 'attribute' ) {
-					if ( inc.offset === old.offset && inc.howMany === old.howMany ) {
-						old.howMany = 0;
-					}
-				}
-
-				if ( old.type === 'refresh' ) {
-					if ( inc.offset === old.offset && inc.howMany === old.howMany ) {
-						inc.nodesToHandle = 0;
-					}
-				}
-			}
 		}
 
 		inc.howMany = inc.nodesToHandle;
@@ -963,7 +892,7 @@ export default class Differ {
 	 * @private
 	 * @param {module:engine/model/element~Element} parent The element in which the change happened.
 	 * @param {Number} offset The offset at which change happened.
-	 * @param {String} name The name of the inserted element or `'$text'` for a character.
+	 * @param {String} name The name of the removed element or `'$text'` for a character.
 	 * @returns {Object} The diff item.
 	 */
 	_getInsertDiff( parent, offset, name ) {
@@ -1053,25 +982,6 @@ export default class Differ {
 		return diffs;
 	}
 
-	/**
-	 * Returns an object with a single refresh change description.
-	 *
-	 * @private
-	 * @param {module:engine/model/element~Element} parent The element in which the change happened.
-	 * @param {Number} offset The offset at which change happened.
-	 * @param {String} name The name of the refreshed element.
-	 * @returns {Object} The diff item.
-	 */
-	_getRefreshDiff( parent, offset, name ) {
-		return {
-			type: 'refresh',
-			position: Position._createAt( parent, offset ),
-			name,
-			length: 1,
-			changeCount: this._changeCount++
-		};
-	}
-
 	/**
 	 * Checks whether given element or any of its parents is an element that is buffered as an inserted element.
 	 *
@@ -1100,34 +1010,6 @@ export default class Differ {
 		return this._isInInsertedElement( parent );
 	}
 
-	/**
-	 * Checks whether given element or any of its parents is an element that is buffered as a refreshed element.
-	 *
-	 * @private
-	 * @param {module:engine/model/element~Element} element Element to check.
-	 * @returns {Boolean}
-	 */
-	_isInRefreshedElement( element ) {
-		const parent = element.parent;
-
-		if ( !parent ) {
-			return false;
-		}
-
-		const changes = this._changesInElement.get( parent );
-		const offset = element.startOffset;
-
-		if ( changes ) {
-			for ( const change of changes ) {
-				if ( change.type === 'refresh' && offset >= change.offset && offset < change.offset + change.howMany ) {
-					return true;
-				}
-			}
-		}
-
-		return this._isInRefreshedElement( parent );
-	}
-
 	/**
 	 * Removes deeply all buffered changes that are registered in elements from range specified by `parent`, `offset`
 	 * and `howMany`.
@@ -1254,19 +1136,13 @@ function _generateActionsFromChanges( oldChildrenLength, changes ) {
 			offset = change.offset;
 			// We removed `howMany` old nodes, update `oldChildrenHandled`.
 			oldChildrenHandled += change.howMany;
-		} else if ( change.type == 'attribute' ) {
+		} else {
 			actions.push( ...'a'.repeat( change.howMany ).split( '' ) );
 
 			// The last handled offset is at the position after the changed range.
 			offset = change.offset + change.howMany;
 			// We changed `howMany` old nodes, update `oldChildrenHandled`.
 			oldChildrenHandled += change.howMany;
-		} else {
-			// In order to properly handle item refreshing we do not merge "x" action nor do we allow range refreshing.
-			actions.push( 'x' );
-
-			// The last handled offset is after inserted item (singular see above comment).
-			offset = change.offset + 1;
 		}
 	}
 

+ 5 - 325
packages/ckeditor5-engine/tests/model/differ.js

@@ -1789,11 +1789,11 @@ describe( 'Differ', () => {
 		} );
 	} );
 
-	describe( 'reInsertItem()', () => {
+	describe( 'refreshItem()', () => {
 		it( 'should mark given element to be removed and added again', () => {
 			const p = root.getChild( 0 );
 
-			differ.reInsertItem( p );
+			differ.refreshItem( p );
 
 			expectChanges( [
 				{ type: 'remove', name: 'paragraph', length: 1, position: model.createPositionBefore( p ) },
@@ -1806,7 +1806,7 @@ describe( 'Differ', () => {
 			const range = model.createRangeIn( p );
 			const textProxy = [ ...range.getItems() ][ 0 ];
 
-			differ.reInsertItem( textProxy );
+			differ.refreshItem( textProxy );
 
 			expectChanges( [
 				{ type: 'remove', name: '$text', length: 3, position: model.createPositionAt( p, 0 ) },
@@ -1814,327 +1814,6 @@ describe( 'Differ', () => {
 			], true );
 		} );
 
-		it( 'inside a new element', () => {
-			// Since the refreshed element 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 ] ) );
-
-				differ.reInsertItem( root.getChild( 2 ).getChild( 0 ) );
-
-				expectChanges( [
-					{ type: 'insert', name: 'blockQuote', length: 1, position: new Position( root, [ 2 ] ) }
-				] );
-			} );
-		} );
-
-		it( 'markers refreshing', () => {
-			model.change( () => {
-				// Refreshed element contains marker.
-				model.markers._set( 'markerA', new Range( new Position( root, [ 1, 1 ] ), new Position( root, [ 1, 2 ] ) ) );
-
-				// Marker contains refreshed element.
-				model.markers._set( 'markerB', new Range( new Position( root, [ 0 ] ), new Position( root, [ 2 ] ) ) );
-
-				// Intersecting.
-				model.markers._set( 'markerC', new Range( new Position( root, [ 0, 2 ] ), new Position( root, [ 1, 2 ] ) ) );
-
-				// Not intersecting.
-				model.markers._set( 'markerD', new Range( new Position( root, [ 0, 0 ] ), new Position( root, [ 1 ] ) ) );
-			} );
-
-			const markersToRefresh = [ 'markerA', 'markerB', 'markerC' ];
-
-			differ.reInsertItem( root.getChild( 1 ) );
-
-			expectChanges( [
-				{ type: 'remove', name: 'paragraph', length: 1, position: new Position( root, [ 1 ] ) },
-				{ type: 'insert', name: 'paragraph', length: 1, position: new Position( root, [ 1 ] ) }
-			] );
-
-			const markersToRemove = differ.getMarkersToRemove().map( entry => entry.name );
-			const markersToAdd = differ.getMarkersToAdd().map( entry => entry.name );
-
-			expect( markersToRefresh ).to.deep.equal( markersToRemove );
-			expect( markersToRefresh ).to.deep.equal( markersToAdd );
-		} );
-	} );
-
-	describe( 'refreshItem()', () => {
-		beforeEach( () => {
-			root._appendChild( [
-				new Element( 'complex', null, [
-					new Element( 'slot', null, [
-						new Element( 'paragraph', null, new Text( '1' ) )
-					] ),
-					new Element( 'slot', null, [
-						new Element( 'paragraph', null, new Text( '2' ) )
-					] )
-				] )
-			] );
-		} );
-
-		it( 'a refreshed element (block)', () => {
-			const p = root.getChild( 0 );
-
-			differ.refreshItem( p );
-
-			expectChanges( [
-				{ type: 'refresh', name: 'paragraph', length: 1, position: model.createPositionBefore( p ) }
-			], true );
-		} );
-
-		it( 'an element refreshed multiple times', () => {
-			const p = root.getChild( 0 );
-
-			differ.refreshItem( p );
-			differ.refreshItem( p );
-			differ.refreshItem( p );
-
-			expectChanges( [
-				{ type: 'refresh', name: 'paragraph', length: 1, position: model.createPositionBefore( p ) }
-			], true );
-		} );
-
-		it( 'a refreshed element (complex)', () => {
-			const complex = root.getChild( 2 );
-
-			differ.refreshItem( complex );
-
-			expectChanges( [
-				{ type: 'refresh', name: 'complex', length: 1, position: model.createPositionBefore( complex ) }
-			], true );
-		} );
-
-		it( 'a multiple elements refreshed', () => {
-			const complex = root.getChild( 2 );
-
-			differ.refreshItem( complex.getChild( 0 ) );
-			differ.refreshItem( complex.getChild( 1 ) );
-
-			expectChanges( [
-				{ type: 'refresh', name: 'slot', length: 1, position: model.createPositionAt( complex, 0 ) },
-				{ type: 'refresh', name: 'slot', length: 1, position: model.createPositionAt( complex, 1 ) }
-			], true );
-		} );
-
-		it( 'a refreshed element with a child removed (refresh + remove)', () => {
-			const complex = root.getChild( 2 );
-
-			model.change( () => {
-				differ.refreshItem( complex );
-				remove( model.createPositionAt( complex, 1 ), 1 );
-
-				expectChanges( [
-					{ type: 'refresh', name: 'complex', length: 1, position: model.createPositionBefore( complex ) }
-				], true );
-			} );
-		} );
-
-		it( 'a refreshed element with a child removed (remove + refresh)', () => {
-			const complex = root.getChild( 2 );
-
-			model.change( () => {
-				remove( model.createPositionAt( complex, 1 ), 1 );
-				differ.refreshItem( complex );
-
-				expectChanges( [
-					{ type: 'refresh', name: 'complex', length: 1, position: model.createPositionBefore( complex ) },
-					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAfter( complex.getChild( 0 ) ) }
-				], false );
-			} );
-		} );
-
-		it( 'a refreshed element with a child added (refresh + insert)', () => {
-			const complex = root.getChild( 2 );
-
-			model.change( () => {
-				differ.refreshItem( complex );
-				insert( new Element( 'slot' ), model.createPositionAt( complex, 2 ) );
-
-				expectChanges( [
-					{ type: 'refresh', name: 'complex', length: 1, position: model.createPositionBefore( complex ) }
-				], true );
-			} );
-		} );
-
-		it( 'a refreshed element with a child added (insert + refresh)', () => {
-			const complex = root.getChild( 2 );
-
-			model.change( () => {
-				const slot = new Element( 'slot' );
-				insert( slot, model.createPositionAt( complex, 2 ) );
-				differ.refreshItem( complex );
-
-				expectChanges( [
-					{ type: 'refresh', name: 'complex', length: 1, position: model.createPositionBefore( complex ) },
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionBefore( slot ) }
-				], true );
-			} );
-		} );
-
-		it( 'a refreshed element with attribute set (refresh + attribute)', () => {
-			const complex = root.getChild( 2 );
-
-			model.change( () => {
-				differ.refreshItem( complex );
-				attribute( model.createRangeOn( complex ), 'foo', undefined, true );
-
-				expectChanges( [
-					{ type: 'refresh', name: 'complex', length: 1, position: model.createPositionBefore( complex ) }
-				], true );
-			} );
-		} );
-
-		it( 'a refreshed element with attribute set (attribute + refresh)', () => {
-			const complex = root.getChild( 2 );
-
-			model.change( () => {
-				attribute( model.createRangeOn( complex ), 'foo', undefined, true );
-				differ.refreshItem( complex );
-
-				expectChanges( [
-					{ type: 'refresh', name: 'complex', length: 1, position: model.createPositionBefore( complex ) }
-				], true );
-			} );
-		} );
-
-		it( 'an element added and refreshed', () => {
-			const complex = root.getChild( 2 );
-
-			model.change( () => {
-				const slot = new Element( 'slot' );
-				insert( slot, model.createPositionAt( complex, 2 ) );
-				differ.refreshItem( slot );
-
-				expectChanges( [
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionBefore( slot ) }
-				], true );
-			} );
-		} );
-
-		it( 'multiple elements added and one of them refreshed (first)', () => {
-			const complexSource = root.getChild( 2 );
-			complexSource._appendChild( new Element( 'slot' ) );
-			root._appendChild( [ new Element( 'complex' ) ] );
-
-			const complexTarget = root.getChild( 3 );
-
-			model.change( () => {
-				move( model.createPositionAt( complexSource, 0 ), 3, model.createPositionAt( complexTarget, 0 ) );
-				const slot = complexTarget.getChild( 0 );
-				differ.refreshItem( slot );
-
-				expectChanges( [
-					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAt( complexSource, 0 ) },
-					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAt( complexSource, 0 ) },
-					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAt( complexSource, 0 ) },
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionAt( complexTarget, 0 ) },
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionAt( complexTarget, 1 ) },
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionAt( complexTarget, 2 ) }
-				], false );
-			} );
-		} );
-
-		it( 'multiple elements added and one of them refreshed (last)', () => {
-			const complexSource = root.getChild( 2 );
-			complexSource._appendChild( new Element( 'slot' ) );
-			root._appendChild( [ new Element( 'complex' ) ] );
-
-			const complexTarget = root.getChild( 3 );
-
-			model.change( () => {
-				move( model.createPositionAt( complexSource, 0 ), 3, model.createPositionAt( complexTarget, 0 ) );
-				const slot = complexTarget.getChild( 2 );
-				differ.refreshItem( slot );
-
-				expectChanges( [
-					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAt( complexSource, 0 ) },
-					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAt( complexSource, 0 ) },
-					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAt( complexSource, 0 ) },
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionAt( complexTarget, 0 ) },
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionAt( complexTarget, 1 ) },
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionAt( complexTarget, 2 ) }
-				], false );
-			} );
-		} );
-
-		it( 'multiple elements added and one of them refreshed (inner)', () => {
-			const complexSource = root.getChild( 2 );
-			complexSource._appendChild( new Element( 'slot' ) );
-			root._appendChild( [ new Element( 'complex' ) ] );
-
-			const complexTarget = root.getChild( 3 );
-
-			model.change( () => {
-				move( model.createPositionAt( complexSource, 0 ), 3, model.createPositionAt( complexTarget, 0 ) );
-				const slot = complexTarget.getChild( 1 );
-				differ.refreshItem( slot );
-
-				expectChanges( [
-					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAt( complexSource, 0 ) },
-					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAt( complexSource, 0 ) },
-					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAt( complexSource, 0 ) },
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionAt( complexTarget, 0 ) },
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionAt( complexTarget, 1 ) },
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionAt( complexTarget, 2 ) }
-				], false );
-			} );
-		} );
-
-		it( 'an element added and other refreshed', () => {
-			const complex = root.getChild( 2 );
-
-			model.change( () => {
-				const slot = new Element( 'slot' );
-				insert( slot, model.createPositionAt( complex, 2 ) );
-				differ.refreshItem( complex.getChild( 0 ) );
-
-				expectChanges( [
-					{ type: 'refresh', name: 'slot', length: 1, position: model.createPositionAt( complex, 0 ) },
-					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionAt( complex, 2 ) }
-				], true );
-			} );
-		} );
-
-		it( 'an element added attribute and other refreshed', () => {
-			const complex = root.getChild( 2 );
-
-			model.change( () => {
-				const attributeChild = complex.getChild( 1 );
-				const refreshChild = complex.getChild( 0 );
-				attribute( model.createRangeOn( attributeChild ), 'foo', undefined, true );
-				differ.refreshItem( refreshChild );
-
-				expectChanges( [
-					{ type: 'refresh', name: 'slot', length: 1, position: model.createPositionBefore( refreshChild ) },
-					{
-						type: 'attribute',
-						range: model.createRange(
-							model.createPositionBefore( attributeChild ),
-							model.createPositionAt( attributeChild, 0 )
-						),
-						attributeKey: 'foo',
-						attributeOldValue: null,
-						attributeNewValue: true
-					}
-				], true );
-			} );
-		} );
-
-		it( 'an element refreshed and removed', () => {
-			const complex = root.getChild( 2 );
-
-			model.change( () => {
-				const slot = complex.getChild( 1 );
-				remove( model.createPositionAt( complex, 1 ), 1 );
-				differ.refreshItem( slot );
-
-				expectChanges( [
-					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAt( complex, 1 ) }
-				] );
-			} );
-		} );
-
 		it( 'inside a new element', () => {
 			// Since the refreshed element is inside a new element, it should not be listed on changes list.
 			model.change( () => {
@@ -2168,7 +1847,8 @@ describe( 'Differ', () => {
 			differ.refreshItem( root.getChild( 1 ) );
 
 			expectChanges( [
-				{ type: 'refresh', name: 'paragraph', length: 1, position: new Position( root, [ 1 ] ) }
+				{ type: 'remove', name: 'paragraph', length: 1, position: new Position( root, [ 1 ] ) },
+				{ type: 'insert', name: 'paragraph', length: 1, position: new Position( root, [ 1 ] ) }
 			] );
 
 			const markersToRemove = differ.getMarkersToRemove().map( entry => entry.name );

+ 1 - 1
packages/ckeditor5-table/src/converters/table-heading-rows-refresh-post-fixer.js

@@ -45,7 +45,7 @@ function tableHeadingRowsRefreshPostFixer( model ) {
 
 		for ( const table of tablesToRefresh.values() ) {
 			// Should be handled by a `triggerBy` configuration. See: https://github.com/ckeditor/ckeditor5/issues/8138.
-			differ.reInsertItem( table );
+			differ.refreshItem( table );
 		}
 
 		return true;