Răsfoiți Sursa

Changed: Minor changes in code and many clarification comments.

Szymon Cofalik 8 ani în urmă
părinte
comite
a8bc23ab58

+ 7 - 5
packages/ckeditor5-engine/src/conversion/mapper.js

@@ -222,15 +222,16 @@ export default class Mapper {
 	 *
 	 * @fires modelToViewPosition
 	 * @param {module:engine/model/position~Position} modelPosition Model position.
-	 * @param {Boolean} [isPhantom=false] Should be set to `true` if the model position to map is pointing to a place
+	 * @param {Object} [options] Additional options for position mapping process.
+	 * @param {Boolean} [options.isPhantom=false] Should be set to `true` if the model position to map is pointing to a place
 	 * in model tree which no longer exists. For example, it could be an end of a removed model range.
 	 * @returns {module:engine/view/position~Position} Corresponding view position.
 	 */
-	toViewPosition( modelPosition, isPhantom = false ) {
+	toViewPosition( modelPosition, options = { isPhantom: false } ) {
 		const data = {
 			modelPosition,
 			mapper: this,
-			isPhantom
+			isPhantom: options.isPhantom
 		};
 
 		this.fire( 'modelToViewPosition', data );
@@ -379,7 +380,7 @@ export default class Mapper {
 	 *		We are in the text node so we can simple find the offset.
 	 *		<p>fo<b>ba|r</b>bom</p> -> expected offset: 2, actual offset: 2 -> position found
 	 *
-	 * @protected
+	 * @private
 	 * @param {module:engine/view/element~Element} viewParent Tree view element in which we are looking for the position.
 	 * @param {Number} expectedOffset Expected offset.
 	 * @returns {module:engine/view/position~Position} Found position.
@@ -484,7 +485,8 @@ export default class Mapper {
 	 *				return;
 	 *			}
 	 *
-	 *			const sibling = modelPosition.nodeBefore; // Might crash for phantom position that does not exist in model.
+	 *			// Below line might crash for phantom position that does not exist in model.
+	 *			const sibling = data.modelPosition.nodeBefore;
 	 *
 	 *			// Check if this is the element we are interested in.
 	 *			if ( !sibling.is( 'customElement' ) ) {

+ 35 - 15
packages/ckeditor5-engine/src/conversion/model-to-view-converters.js

@@ -103,7 +103,7 @@ export function remove() {
 		const viewStart = conversionApi.mapper.toViewPosition( data.position );
 
 		const modelEnd = data.position.getShiftedBy( data.length );
-		const viewEnd = conversionApi.mapper.toViewPosition( modelEnd, true );
+		const viewEnd = conversionApi.mapper.toViewPosition( modelEnd, { isPhantom: true } );
 
 		const viewRange = new ViewRange( viewStart, viewEnd );
 
@@ -153,6 +153,22 @@ export function insertUIElement( elementCreator ) {
 		}
 
 		const markerRange = data.markerRange;
+		const eventName = evt.name;
+
+		// Marker that is collapsed has consumable build differently that non-collapsed one.
+		// For more information see `addMarker` event description.
+		// If marker's range is collapsed - check if it can be consumed.
+		if ( markerRange.isCollapsed && !consumable.consume( markerRange, eventName ) ) {
+			return;
+		}
+
+		// If marker's range is not collapsed - consume all items inside.
+		for ( const value of markerRange ) {
+			if ( !consumable.consume( value.item, eventName ) ) {
+				return;
+			}
+		}
+
 		const mapper = conversionApi.mapper;
 
 		// Add "opening" element.
@@ -329,6 +345,10 @@ export function wrap( elementCreator ) {
  * {@link module:engine/view/attributeelement~AttributeElement} created from provided descriptor.
  * See {link module:engine/conversion/model-to-view-converters~createViewElementFromHighlightDescriptor}.
  *
+ * If the highlight descriptor will not provide `priority` property, `10` will be used.
+ *
+ * If the highlight descriptor will not provide `id` property, name of the marker will be used.
+ *
  * @param {module:engine/conversion/model-to-view-converters~HighlightDescriptor|Function} highlightDescriptor
  * @return {Function}
  */
@@ -366,15 +386,14 @@ export function highlightText( highlightDescriptor ) {
  *
  * The converter checks if an element has `addHighlight` function stored as
  * {@link module:engine/view/element~Element#setCustomProperty custom property} and, if so, uses it to apply the highlight.
- *
  * In such case converter will consume all element's children, assuming that they were handled by element itself.
- * If the highlight descriptor will not provide priority, priority `10` will be used as default, to be compliant with
- * {@link module:engine/conversion/model-to-view-converters~highlightText} method which uses default priority of
- * {@link module:engine/view/attributeelement~AttributeElement}.
+ *
+ * When `addHighlight` custom property is not present, element is not converted in any special way.
+ * This means that converters will proceed to convert element's child nodes.
+ *
+ * If the highlight descriptor will not provide `priority` property, `10` will be used.
  *
  * If the highlight descriptor will not provide `id` property, name of the marker will be used.
- * When `addHighlight` and `removeHighlight` custom properties are not present, element is not converted
- * in any special way. This means that converters will proceed to convert element's child nodes.
  *
  * @param {module:engine/conversion/model-to-view-converters~HighlightDescriptor|Function} highlightDescriptor
  * @return {Function}
@@ -418,18 +437,19 @@ export function highlightElement( highlightDescriptor ) {
 }
 
 /**
- * Function factory, creates a converter that converts model marker add/change/remove to the view.
+ * Function factory, creates a converter that converts model marker remove to the view.
  *
- * The result of this conversion is different for text nodes and elements.
+ * Both text nodes and elements are handled by this converter by they are handled a bit differently.
  *
- * Text nodes are wrapped with {@link module:engine/view/attributeelement~AttributeElement} created from provided
+ * Text nodes are unwrapped using {@link module:engine/view/attributeelement~AttributeElement} created from provided
  * highlight descriptor. See {link module:engine/conversion/model-to-view-converters~highlightDescriptorToAttributeElement}.
  *
- * For elements, the converter checks if an element has `addHighlight` and `removeHighlight` functions stored as
- * {@link module:engine/view/element~Element#setCustomProperty custom properties}. If so, it uses them to apply the highlight.
- * In such case, children of that element will not be converted. When `addHighlight` and `removeHighlight` are not present,
- * element is not converted in any special way, instead converter will proceed to convert element's child nodes. Most
- * common case is that the element will be given a special class, style or attribute basing on the descriptor.
+ * For elements, the converter checks if an element has `removeHighlight` function stored as
+ * {@link module:engine/view/element~Element#setCustomProperty custom property}. If so, it uses it to remove the highlight.
+ * In such case, children of that element will not be converted.
+ *
+ * When `removeHighlight` is not present, element is not converted in any special way.
+ * Instead converter will proceed to convert element's child nodes.
  *
  * If the highlight descriptor will not provide `priority` property, `10` will be used.
  *

+ 15 - 36
packages/ckeditor5-engine/src/conversion/modelconversiondispatcher.js

@@ -86,40 +86,6 @@ import extend from '@ckeditor/ckeditor5-utils/src/lib/lodash/extend';
  *			// Remember to stop the event propagation.
  *			evt.stop();
  *		} );
- *
- * Callback that overrides other callback:
- *
- *		// Special converter for `linkHref` attribute added on custom `quote` element. Note, that this
- *		// attribute may be the same as the attribute added by other features (link feature in this case).
- *		// It might be even added by that feature! It makes sense that a part of content that is a quote is linked
- *		// to an external source so it makes sense that link feature works on the custom quote element.
- *		// However, we have to make sure that the attributes added by link feature are correctly converted.
- *		// To block default `linkHref` conversion we have to:
- *		// 1) add this callback with higher priority than link feature callback,
- *		// 2) consume `linkHref` attribute add change.
- *		modelConversionDispatcher.on( 'attribute:linkHref:quote', ( evt, data, consumable, conversionApi ) => {
- *			if ( consumable.consume( data.item, 'attribute:linkHref' ) ) {
- *				return;
- *			}
- *
- *			// Create a button that will represent the `linkHref` attribute.
- *			let viewSourceBtn = new ViewElement( 'a', {
- *				href: data.item.getAttribute( 'linkHref' ),
- *				title: 'source',
- *				class: 'source'
- *			} );
- *
- *			// Insert the button using writer API.
- *			// Note that attribute conversion is fired after insert conversion.
- *			// This means that we are safe to assume that the model `quote` element has already been converter to view.
- *			// `data.item` is model element on which attribute changed.
- *			const viewQuote = conversionApi.mapper.toViewElement( data.item );
- *			// Put `viewSourceBtn` at the end of quote.
- *			const position = ViewPosition.createAt( viewQuote, 'end' );
- *			viewWriter.insert( position, viewSourceBtn );
- *
- *			evt.stop();
- *		}, { priority: 'high' } );
  */
 export default class ModelConversionDispatcher {
 	/**
@@ -159,7 +125,7 @@ export default class ModelConversionDispatcher {
 		// Convert changes that happened on model tree.
 		for ( const entry of differ.getChanges() ) {
 			// Skip all the changes that happens in graveyard. These are not converted.
-			if ( entry.position.root.rootName == '$graveyard' ) {
+			if ( _isInGraveyard( entry ) ) {
 				continue;
 			}
 
@@ -168,6 +134,7 @@ export default class ModelConversionDispatcher {
 			} else if ( entry.type == 'remove' ) {
 				this.convertRemove( entry.position, entry.length, entry.name );
 			} else {
+				// entry.type == 'attribute'.
 				this.convertAttribute( entry.range, entry.attributeKey, entry.attributeOldValue, entry.attributeNewValue );
 			}
 		}
@@ -543,7 +510,13 @@ export default class ModelConversionDispatcher {
 	 * if markers are named `foo:abc`, `foo:bar`, then it is possible to listen to `addMarker:foo` or `addMarker:foo:abc` and
 	 * `addMarker:foo:bar` events.
 	 *
-	 * The event is fired for each item in the marker range, one by one.
+	 * If the marker range is not collapsed:
+	 * * the event is fired for each item in the marker range one by one,
+	 * * consumables object includes each item of the marker range and the consumable value is same as event name.
+	 *
+	 * If the marker range is collapsed:
+	 * * there is only one event,
+	 * * consumables object includes marker range with event name.
 	 *
 	 * @event addMarker
 	 * @param {Object} data Additional information about the change.
@@ -596,3 +569,9 @@ function shouldMarkerChangeBeConverted( modelPosition, marker, mapper ) {
 
 	return !hasCustomHandling;
 }
+
+// Checks whether entry change describes changes that happen in graveyard.
+function _isInGraveyard( entry ) {
+	return ( entry.position && entry.position.root.rootName == '$graveyard' ) ||
+		( entry.range && entry.range.root.rootName == '$graveyard' );
+}

+ 199 - 106
packages/ckeditor5-engine/src/model/differ.js

@@ -161,8 +161,8 @@ export default class Differ {
 	}
 
 	/**
-	 * Calculates diff between old model tree state (before all the buffered operations) and the new model tree state
-	 * (actual one). Should be called after all buffered operations are executed.
+	 * Calculates diff between old model tree state (state before the first buffered operations since the last {@link #reset} call)
+	 * and the new model tree state (actual one). Should be called after all buffered operations are executed.
 	 *
 	 * The diff set is returned as an array of diff items, each describing a change done on model. The items are sorted by
 	 * the position on which the change happened. If a position {@link module:engine/model/position~Position#isBefore is before}
@@ -176,7 +176,9 @@ export default class Differ {
 
 		// Check all changed elements.
 		for ( const element of this._changesInElement.keys() ) {
-			// If the element is inside other changed element, skip changes in this element.
+			// Each item in `this._changesInElement` describes changes of the _children_ of that element.
+			// If the element itself has been inserted we should skip all the changes in it because the element will be reconverted.
+			// If the element itself has been removed we should skip all the changes in it because they would be incorrect.
 			if ( this._isInsertedOrRemoved( element ) ) {
 				continue;
 			}
@@ -185,6 +187,9 @@ export default class Differ {
 			const changes = this._changesInElement.get( element ).sort( ( a, b ) => {
 				if ( a.offset === b.offset ) {
 					if ( a.type != b.type ) {
+						// If there are multiple changes at the same position, "remove" change should be first.
+						// If the order is different, for example, we would first add some nodes and then removed them
+						// (instead of the nodes that we should remove).
 						return a.type == 'remove' ? -1 : 1;
 					}
 
@@ -306,6 +311,11 @@ export default class Differ {
 		// Remove `changeCount` property from diff items. It is used only for sorting and is internal thing.
 		for ( const item of diffSet ) {
 			delete item.changeCount;
+
+			if ( item.type == 'attribute' ) {
+				delete item.position;
+				delete item.length;
+			}
 		}
 
 		this._changeCount = 0;
@@ -417,9 +427,7 @@ export default class Differ {
 		const changes = this._getChangesForElement( parent );
 
 		// Then, look through all the changes, and transform them or the new change.
-		for ( const oldChange of changes ) {
-			this._handleChange( changeItem, oldChange, changes );
-		}
+		this._handleChange( changeItem, changes );
 
 		// Add the new change.
 		changes.push( changeItem );
@@ -474,137 +482,174 @@ export default class Differ {
 	 *
 	 * @private
 	 * @param {Object} inc Incoming (new) change.
-	 * @param {Object} old Old change (already done on the element).
 	 * @param {Array.<Object>} changes Array containing all the changes done on that element.
 	 */
-	_handleChange( inc, old, changes ) {
-		const incEnd = inc.offset + inc.howMany;
-		const oldEnd = old.offset + old.howMany;
-
-		if ( inc.type == 'insert' ) {
-			if ( old.type == 'insert' ) {
-				if ( inc.offset <= old.offset ) {
-					old.offset += inc.howMany;
-				} else if ( inc.offset < oldEnd ) {
-					old.howMany += inc.howMany;
-					inc.howMany = 0;
-				}
-			}
+	_handleChange( inc, changes ) {
+		for ( const old of changes ) {
+			const incEnd = inc.offset + inc.howMany;
+			const oldEnd = old.offset + old.howMany;
 
-			if ( old.type == 'remove' ) {
-				if ( inc.offset < old.offset ) {
-					old.offset += inc.howMany;
+			if ( inc.type == 'insert' ) {
+				if ( old.type == 'insert' ) {
+					if ( inc.offset <= old.offset ) {
+						old.offset += inc.howMany;
+					} else if ( inc.offset < oldEnd ) {
+						old.howMany += inc.howMany;
+						inc.howMany = 0;
+					}
 				}
-			}
-
-			if ( old.type == 'attribute' ) {
-				if ( inc.offset <= old.offset ) {
-					old.offset += inc.howMany;
-				} else if ( inc.offset < oldEnd ) {
-					const howMany = old.howMany;
-
-					old.howMany = inc.offset - old.offset;
 
-					// Unshift to prevent further processing of this change.
-					changes.unshift( { type: 'attribute', offset: incEnd, howMany: howMany - old.howMany, count: this._changeCount++ } );
-				}
-			}
-		}
-
-		if ( inc.type == 'remove' ) {
-			if ( old.type == 'insert' ) {
-				if ( incEnd <= old.offset ) {
-					old.offset -= inc.howMany;
-				} else if ( incEnd <= oldEnd ) {
+				if ( old.type == 'remove' ) {
 					if ( inc.offset < old.offset ) {
-						const intersectionLength = incEnd - old.offset;
-
-						old.offset = inc.offset;
-
-						old.howMany -= intersectionLength;
-						inc.howMany -= intersectionLength;
-					} else {
-						old.howMany -= inc.howMany;
-						inc.howMany = 0;
+						old.offset += inc.howMany;
 					}
-				} else {
+				}
+
+				if ( old.type == 'attribute' ) {
 					if ( inc.offset <= old.offset ) {
-						inc.howMany = inc.howMany - old.howMany;
-						old.howMany = 0;
+						old.offset += inc.howMany;
 					} else if ( inc.offset < oldEnd ) {
-						const intersectionLength = oldEnd - inc.offset;
+						// This case is more complicated, because attribute change has to be split into two.
+						// Example (assume that uppercase and lowercase letters mean different attributes):
+						//
+						// initial state:		abcxyz
+						// attribute change:	aBCXYz
+						// incoming insert:		aBCfooXYz
+						//
+						// Change ranges cannot intersect because each item has to be described exactly (it was either
+						// not changed, inserted, removed, or its attribute was changed). That's why old attribute
+						// change has to be split and both parts has to be handled separately from now on.
+						const howMany = old.howMany;
 
-						old.howMany -= intersectionLength;
-						inc.howMany -= intersectionLength;
+						old.howMany = inc.offset - old.offset;
+
+						// Add the second part of attribute change to the beginning of processed array so it won't
+						// be processed again in this loop.
+						changes.unshift( {
+							type: 'attribute',
+							offset: incEnd,
+							howMany: howMany - old.howMany,
+							count: this._changeCount++
+						} );
 					}
 				}
 			}
 
-			if ( old.type == 'remove' ) {
-				if ( inc.offset + inc.howMany <= old.offset ) {
-					old.offset -= inc.howMany;
-				} else if ( inc.offset < old.offset ) {
-					old.offset = inc.offset;
-					old.howMany += inc.howMany;
-
-					inc.howMany = 0;
+			if ( inc.type == 'remove' ) {
+				if ( old.type == 'insert' ) {
+					if ( incEnd <= old.offset ) {
+						old.offset -= inc.howMany;
+					} else if ( incEnd <= oldEnd ) {
+						if ( inc.offset < old.offset ) {
+							const intersectionLength = incEnd - old.offset;
+
+							old.offset = inc.offset;
+
+							old.howMany -= intersectionLength;
+							inc.howMany -= intersectionLength;
+						} else {
+							old.howMany -= inc.howMany;
+							inc.howMany = 0;
+						}
+					} else {
+						if ( inc.offset <= old.offset ) {
+							inc.howMany = inc.howMany - old.howMany;
+							old.howMany = 0;
+						} else if ( inc.offset < oldEnd ) {
+							const intersectionLength = oldEnd - inc.offset;
+
+							old.howMany -= intersectionLength;
+							inc.howMany -= intersectionLength;
+						}
+					}
 				}
-			}
 
-			if ( old.type == 'attribute' ) {
-				if ( incEnd <= old.offset ) {
-					old.offset -= inc.howMany;
-				} else if ( inc.offset < old.offset ) {
-					const intersectionLength = incEnd - old.offset;
-
-					old.offset = inc.offset;
-					old.howMany -= intersectionLength;
-				} else if ( inc.offset < oldEnd ) {
-					if ( incEnd <= oldEnd ) {
-						const howMany = old.howMany;
+				if ( old.type == 'remove' ) {
+					if ( inc.offset + inc.howMany <= old.offset ) {
+						old.offset -= inc.howMany;
+					} else if ( inc.offset < old.offset ) {
+						old.offset = inc.offset;
+						old.howMany += inc.howMany;
 
-						old.howMany = inc.offset - old.offset;
+						inc.howMany = 0;
+					}
+				}
 
-						const howManyAfter = howMany - old.howMany - inc.howMany;
+				if ( old.type == 'attribute' ) {
+					if ( incEnd <= old.offset ) {
+						old.offset -= inc.howMany;
+					} else if ( inc.offset < old.offset ) {
+						const intersectionLength = incEnd - old.offset;
 
-						// Unshift to prevent further processing of this change.
-						changes.unshift( { type: 'attribute', offset: inc.offset, howMany: howManyAfter, count: this._changeCount++ } );
-					} else {
-						old.howMany -= oldEnd - inc.offset;
+						old.offset = inc.offset;
+						old.howMany -= intersectionLength;
+					} else if ( inc.offset < oldEnd ) {
+						if ( incEnd <= oldEnd ) {
+							// On first sight in this case we don't need to split attribute operation into two.
+							// However the changes set is later converted to actions (see `_generateActionsFromChanges`).
+							// For that reason, no two changes may intersect.
+							// So we cannot have an attribute change that "contains" remove change.
+							// Attribute change needs to be split.
+							const howMany = old.howMany;
+
+							old.howMany = inc.offset - old.offset;
+
+							const howManyAfter = howMany - old.howMany - inc.howMany;
+
+							// Add the second part of attribute change to the beginning of processed array so it won't
+							// be processed again in this loop.
+							changes.unshift( {
+								type: 'attribute',
+								offset: inc.offset,
+								howMany: howManyAfter,
+								count: this._changeCount++
+							} );
+						} else {
+							old.howMany -= oldEnd - inc.offset;
+						}
 					}
 				}
 			}
-		}
 
-		if ( inc.type == 'attribute' ) {
-			if ( old.type == 'insert' ) {
-				if ( inc.offset < old.offset && incEnd > old.offset ) {
-					if ( incEnd > oldEnd ) {
-						const changeItem = { type: 'attribute', offset: oldEnd, howMany: incEnd - oldEnd, count: this._changeCount++ };
-
-						for ( const oldChange of changes ) {
-							this._handleChange( changeItem, oldChange, changes );
+			if ( inc.type == 'attribute' ) {
+				if ( old.type == 'insert' ) {
+					if ( inc.offset < old.offset && incEnd > old.offset ) {
+						if ( incEnd > oldEnd ) {
+							// This case is similar to a case described when incoming change was insert and old change was attribute.
+							// See comment above.
+							//
+							// This time incoming change is attribute. We need to split incoming change in this case too.
+							// However this time, the second part of the attribute change needs to be processed further
+							// because there might be other changes that it collides with.
+							const attributePart = {
+								type: 'attribute',
+								offset: oldEnd,
+								howMany: incEnd - oldEnd,
+								count: this._changeCount++
+							};
+
+							this._handleChange( attributePart, changes );
+
+							changes.push( attributePart );
 						}
 
-						changes.push( changeItem );
+						inc.howMany = old.offset - inc.offset;
+					} else if ( inc.offset >= old.offset && inc.offset < oldEnd ) {
+						if ( incEnd > oldEnd ) {
+							inc.howMany = incEnd - oldEnd;
+							inc.offset = oldEnd;
+						} else {
+							inc.howMany = 0;
+						}
 					}
+				}
 
-					inc.howMany = old.offset - inc.offset;
-				} else if ( inc.offset >= old.offset && inc.offset < oldEnd ) {
-					if ( incEnd > oldEnd ) {
-						inc.howMany = incEnd - oldEnd;
-						inc.offset = oldEnd;
-					} else {
+				if ( old.type == 'attribute' ) {
+					if ( inc.offset >= old.offset && incEnd <= oldEnd ) {
 						inc.howMany = 0;
 					}
 				}
 			}
-
-			if ( old.type == 'attribute' ) {
-				if ( inc.offset >= old.offset && incEnd <= oldEnd ) {
-					inc.howMany = 0;
-				}
-			}
 		}
 	}
 
@@ -736,36 +781,84 @@ function _getChildrenSnapshot( children ) {
 // - 'i' for 'insert' - when item at that position was inserted,
 // - 'r' for 'remove' - when item at that position was removed,
 // - 'a' for 'attribute' - when item at that position has it attributes changed.
+//
+// Example (assume that uppercase letters have bold attribute, compare with function code):
+//
+// children before:	fooBAR
+// children after:	foxybAR
+//
+// changes: type: remove, offset: 1, howMany: 1
+//			type: insert, offset: 2, howMany: 2
+//			type: attribute, offset: 4, howMany: 1
+//
+// expected actions: equal (f), remove (o), equal (o), insert (x), insert (y), attribute (b), equal (A), equal (R)
+//
+// steps taken by th script:
+//
+// 1. change = "type: remove, offset: 1, howMany: 1"; offset = 0; oldChildrenHandled = 0
+//    1.1 between this change and the beginning is one not-changed node, fill with one equal action, one old child has been handled
+//    1.2 this change removes one node, add one remove action
+//    1.3 change last visited `offset` to 1
+//    1.4 since an old child has been removed, one more old child has been handled
+//    1.5 actions at this point are: equal, remove
+//
+// 2. change = "type: insert, offset: 2, howMany: 2"; offset = 1; oldChildrenHandled = 2
+//    2.1 between this change and previous change is one not-changed node, add equal action, another one old children has been handled
+//    2.2 this change inserts two nodes, add two insert actions
+//    2.3 change last visited offset to the end of the inserted range, that is 4
+//    2.4 actions at this point are: equal, remove, equal, insert, insert
+//
+// 3. change = "type: attribute, offset: 4, howMany: 1"; offset = 4, oldChildrenHandled = 3
+//    3.1 between this change and previous change are no not-changed nodes
+//    3.2 this change changes one node, add one attribute action
+//    3.3 change last visited `offset` to the end of change range, that is 5
+//    3.4 since an old child has been changed, one more old child has been handled
+//    3.5 actions at this point are: equal, remove, equal, insert, insert, attribute
+//
+// 4. after loop oldChildrenHandled = 4, oldChildrenLength = 6 (fooBAR is 6 characters)
+//    4.1 fill up with two equal actions
+//
+// The result actions are: equal, remove, equal, insert, insert, attribute, equal, equal.
 function _generateActionsFromChanges( oldChildrenLength, changes ) {
 	const actions = [];
 
 	let offset = 0;
 	let oldChildrenHandled = 0;
 
+	// Go through all buffered changes.
 	for ( const change of changes ) {
+		// First, fill "holes" between changes with "equal" actions.
 		if ( change.offset > offset ) {
 			actions.push( ...'e'.repeat( change.offset - offset ).split( '' ) );
 
 			oldChildrenHandled += change.offset - offset;
 		}
 
+		// Then, fill up actions accordingly to change type.
 		if ( change.type == 'insert' ) {
 			actions.push( ...'i'.repeat( change.howMany ).split( '' ) );
 
+			// The last handled offset is after inserted range.
 			offset = change.offset + change.howMany;
 		} else if ( change.type == 'remove' ) {
 			actions.push( ...'r'.repeat( change.howMany ).split( '' ) );
 
+			// The last handled offset is at the position where the nodes were removed.
 			offset = change.offset;
+			// We removed `howMany` old nodes, update `oldChildrenHandled`.
 			oldChildrenHandled += change.howMany;
 		} else {
 			actions.push( ...'a'.repeat( change.howMany ).split( '' ) );
 
+			// The last handled offset isa at the position after the changed range.
 			offset = change.offset + change.howMany;
+			// We changed `howMany` old nodes, update `oldChildrenHandled`.
 			oldChildrenHandled += change.howMany;
 		}
 	}
 
+	// Fill "equal" actions at the end of actions set. Use `oldChildrenHandled` to see how many children
+	// has not been changed / removed at the end of their parent.
 	if ( oldChildrenHandled < oldChildrenLength ) {
 		actions.push( ...'e'.repeat( oldChildrenLength - oldChildrenHandled ).split( '' ) );
 	}

+ 1 - 1
packages/ckeditor5-engine/tests/conversion/mapper.js

@@ -619,7 +619,7 @@ describe( 'Mapper', () => {
 			evt.stop();
 		} );
 
-		mapper.toViewPosition( {}, true );
+		mapper.toViewPosition( {}, { isPhantom: true } );
 	} );
 
 	describe( 'getModelLength', () => {

+ 12 - 36
packages/ckeditor5-engine/tests/conversion/model-to-view-converters.js

@@ -399,31 +399,20 @@ describe( 'model-to-view-converters', () => {
 				expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p></div>' );
 			} );
 
-			it( 'should be possible to overwrite', () => {
+			it( 'should not convert if consumable was consumed', () => {
 				const viewUi = new ViewUIElement( 'span', { 'class': 'marker' } );
-				const higherPriorityViewUi = new ViewUIElement( 'span', { 'class': 'high' } );
 
 				sinon.spy( dispatcher, 'fire' );
 
 				dispatcher.on( 'addMarker:marker', insertUIElement( viewUi ) );
-				dispatcher.on( 'removeMarker:marker', removeUIElement( viewUi ) );
-
-				dispatcher.on( 'addMarker:marker', insertUIElement( higherPriorityViewUi ), { priority: 'high' } );
-				dispatcher.on( 'removeMarker:marker', removeUIElement( higherPriorityViewUi ), { priority: 'high' } );
-
-				model.change( () => {
-					model.markers.set( 'marker', range );
-				} );
+				dispatcher.on( 'addMarker:marker', ( evt, data, consumable ) => {
+					consumable.consume( data.markerRange, 'addMarker:marker' );
+				}, { priority: 'high' } );
 
-				expect( viewToString( viewRoot ) ).to.equal( '<div><p>foo<span class="high"></span>bar</p></div>' );
-				expect( dispatcher.fire.calledWith( 'addMarker:marker' ) );
-
-				model.change( () => {
-					model.markers.remove( 'marker' );
-				} );
+				dispatcher.convertMarkerAdd( 'marker', range );
 
 				expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p></div>' );
-				expect( dispatcher.fire.calledWith( 'removeMarker:marker' ) );
+				expect( dispatcher.fire.calledWith( 'addMarker:marker' ) );
 			} );
 
 			it( 'should not convert if creator returned null', () => {
@@ -516,33 +505,20 @@ describe( 'model-to-view-converters', () => {
 				expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p></div>' );
 			} );
 
-			it( 'should be possible to overwrite', () => {
+			it( 'should not convert if consumable was consumed', () => {
 				const viewUi = new ViewUIElement( 'span', { 'class': 'marker' } );
-				const higherPriorityViewUi = new ViewUIElement( 'span', { 'class': 'high' } );
 
 				sinon.spy( dispatcher, 'fire' );
 
 				dispatcher.on( 'addMarker:marker', insertUIElement( viewUi ) );
-				dispatcher.on( 'removeMarker:marker', removeUIElement( viewUi ) );
-
-				dispatcher.on( 'addMarker:marker', insertUIElement( higherPriorityViewUi ), { priority: 'high' } );
-				dispatcher.on( 'removeMarker:marker', removeUIElement( higherPriorityViewUi ), { priority: 'high' } );
-
-				model.change( () => {
-					model.markers.set( 'marker', range );
-				} );
-
-				expect( viewToString( viewRoot ) ).to.equal(
-					'<div><p>fo<span class="high"></span>oba<span class="high"></span>r</p></div>'
-				);
-				expect( dispatcher.fire.calledWith( 'addMarker:marker' ) );
+				dispatcher.on( 'addMarker:marker', ( evt, data, consumable ) => {
+					consumable.consume( data.item, 'addMarker:marker' );
+				}, { priority: 'high' } );
 
-				model.change( () => {
-					model.markers.remove( 'marker' );
-				} );
+				dispatcher.convertMarkerAdd( 'marker', range );
 
 				expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p></div>' );
-				expect( dispatcher.fire.calledWith( 'removeMarker:marker' ) );
+				expect( dispatcher.fire.calledWith( 'addMarker:marker' ) );
 			} );
 		} );
 	} );