Changed: Minor changes in code and many clarification comments.

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' ) ) {

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.

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' );
+}