Merge pull request #1241 from ckeditor/t/1214

Feature: Consumable type name is now normalized inside `conversion.ModelConsumable` methods. Closes #1214.

Author: Piotr Jasiun

src/conversion/model-selection-to-view-converters.js

@@ -181,19 +181,18 @@ export function convertSelectionMarker( highlightDescriptor ) {
 		}
 
 		const viewElement = createViewElementFromHighlightDescriptor( descriptor );
-		const consumableName = 'selectionMarker:' + data.markerName;
 
-		wrapCollapsedSelectionPosition( data.selection, conversionApi.viewSelection, viewElement, consumable, consumableName );
+		wrapCollapsedSelectionPosition( data.selection, conversionApi.viewSelection, viewElement, consumable, evt.name );
 	};
 }
 
 // Helper function for `convertSelectionAttribute` and `convertSelectionMarker`, which perform similar task.
-function wrapCollapsedSelectionPosition( modelSelection, viewSelection, viewElement, consumable, consumableName ) {
+function wrapCollapsedSelectionPosition( modelSelection, viewSelection, viewElement, consumable, eventName ) {
 	if ( !modelSelection.isCollapsed ) {
 		return;
 	}
 
-	if ( !consumable.consume( modelSelection, consumableName ) ) {
+	if ( !consumable.consume( modelSelection, eventName ) ) {
 		return;
 	}
 

src/conversion/model-to-view-converters.js

@@ -153,18 +153,17 @@ 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 ) ) {
+		if ( markerRange.isCollapsed && !consumable.consume( markerRange, evt.name ) ) {
 			return;
 		}
 
 		// If marker's range is not collapsed - consume all items inside.
 		for ( const value of markerRange ) {
-			if ( !consumable.consume( value.item, eventName ) ) {
+			if ( !consumable.consume( value.item, evt.name ) ) {
 				return;
 			}
 		}
@@ -263,7 +262,7 @@ export function changeAttribute( attributeCreator ) {
 	attributeCreator = attributeCreator || ( ( value, key ) => ( { value, key } ) );
 
 	return ( evt, data, consumable, conversionApi ) => {
-		if ( !consumable.consume( data.item, eventNameToConsumableType( evt.name ) ) ) {
+		if ( !consumable.consume( data.item, evt.name ) ) {
 			return;
 		}
 
@@ -322,7 +321,7 @@ export function wrap( elementCreator ) {
 			return;
 		}
 
-		if ( !consumable.consume( data.item, eventNameToConsumableType( evt.name ) ) ) {
+		if ( !consumable.consume( data.item, evt.name ) ) {
 			return;
 		}
 
@@ -524,18 +523,6 @@ function _prepareDescriptor( highlightDescriptor, data, conversionApi ) {
 	return descriptor;
 }
 
-/**
- * Returns the consumable type that is to be consumed in an event, basing on that event name.
- *
- * @param {String} evtName Event name.
- * @returns {String} Consumable type.
- */
-export function eventNameToConsumableType( evtName ) {
-	const parts = evtName.split( ':' );
-
-	return parts[ 0 ] + ':' + parts[ 1 ];
-}
-
 /**
  * Creates `span` {@link module:engine/view/attributeelement~AttributeElement view attribute element} from information
  * provided by {@link module:engine/conversion/model-to-view-converters~HighlightDescriptor} object. If priority

src/conversion/modelconsumable.js

@@ -125,9 +125,12 @@ export default class ModelConsumable {
 	 *
 	 * @param {module:engine/model/item~Item|module:engine/model/selection~Selection|module:engine/model/range~Range} item
 	 * Model item, range or selection that has the consumable.
-	 * @param {String} type Consumable type.
+	 * @param {String} type Consumable type. Will be normalized to a proper form, that is either `<word>` or `<part>:<part>`.
+	 * Second colon and everything after will be cut. Passing event name is a safe and good practice.
 	 */
 	add( item, type ) {
+		type = _normalizeConsumableType( type );
+
 		if ( item instanceof TextProxy ) {
 			item = this._getSymbolForTextProxy( item );
 		}
@@ -151,10 +154,13 @@ export default class ModelConsumable {
 	 *
 	 * @param {module:engine/model/item~Item|module:engine/model/selection~Selection|module:engine/model/range~Range} item
 	 * Model item, range or selection from which consumable will be consumed.
-	 * @param {String} type Consumable type.
+	 * @param {String} type Consumable type. Will be normalized to a proper form, that is either `<word>` or `<part>:<part>`.
+	 * Second colon and everything after will be cut. Passing event name is a safe and good practice.
 	 * @returns {Boolean} `true` if consumable value was available and was consumed, `false` otherwise.
 	 */
 	consume( item, type ) {
+		type = _normalizeConsumableType( type );
+
 		if ( item instanceof TextProxy ) {
 			item = this._getSymbolForTextProxy( item );
 		}
@@ -180,11 +186,14 @@ export default class ModelConsumable {
 	 *
 	 * @param {module:engine/model/item~Item|module:engine/model/selection~Selection|module:engine/model/range~Range} item
 	 * Model item, range or selection to be tested.
-	 * @param {String} type Consumable type.
+	 * @param {String} type Consumable type. Will be normalized to a proper form, that is either `<word>` or `<part>:<part>`.
+	 * Second colon and everything after will be cut. Passing event name is a safe and good practice.
 	 * @returns {null|Boolean} `null` if such consumable was never added, `false` if the consumable values was
 	 * already consumed or `true` if it was added and not consumed yet.
 	 */
 	test( item, type ) {
+		type = _normalizeConsumableType( type );
+
 		if ( item instanceof TextProxy ) {
 			item = this._getSymbolForTextProxy( item );
 		}
@@ -221,6 +230,8 @@ export default class ModelConsumable {
 	 * never been added.
 	 */
 	revert( item, type ) {
+		type = _normalizeConsumableType( type );
+
 		if ( item instanceof TextProxy ) {
 			item = this._getSymbolForTextProxy( item );
 		}
@@ -302,3 +313,15 @@ export default class ModelConsumable {
 		return symbol;
 	}
 }
+
+// Returns a normalized consumable type name from given string. A normalized consumable type name is a string that has
+// at most one colon, for example: `insert` or `addMarker:highlight`. If string to normalize has more "parts" (more colons),
+// the other parts are dropped, for example: `addAttribute:bold:$text` -> `addAttribute:bold`.
+//
+// @param {String} type Consumable type.
+// @returns {String} Normalized consumable type.
+function _normalizeConsumableType( type ) {
+	const parts = type.split( ':' );
+
+	return parts.length > 1 ? parts[ 0 ] + ':' + parts[ 1 ] : parts[ 0 ];
+}

tests/conversion/modelconsumable.js

@@ -40,6 +40,17 @@ describe( 'ModelConsumable', () => {
 
 			expect( modelConsumable.test( modelTextProxy, 'type' ) ).to.be.true;
 		} );
+
+		it( 'should normalize type name', () => {
+			modelConsumable.add( modelElement, 'foo:bar:baz:abc' );
+
+			expect( modelConsumable.test( modelElement, 'foo:bar:baz:abc' ) ).to.be.true;
+			expect( modelConsumable.test( modelElement, 'foo:bar:baz' ) ).to.be.true;
+			expect( modelConsumable.test( modelElement, 'foo:bar' ) ).to.be.true;
+			expect( modelConsumable.test( modelElement, 'foo:bar:xxx' ) ).to.be.true;
+
+			expect( modelConsumable.test( modelElement, 'foo:xxx' ) ).to.be.null;
+		} );
 	} );
 
 	describe( 'consume', () => {
@@ -74,6 +85,17 @@ describe( 'ModelConsumable', () => {
 			expect( result ).to.be.true;
 			expect( modelConsumable.test( proxy1To4, 'type' ) ).to.be.false;
 		} );
+
+		it( 'should normalize type name', () => {
+			modelConsumable.add( modelElement, 'foo:bar:baz:abc' );
+			const result = modelConsumable.consume( modelElement, 'foo:bar:baz' );
+
+			expect( result ).to.be.true;
+
+			expect( modelConsumable.test( modelElement, 'foo:bar:baz:abc' ) ).to.be.false;
+			expect( modelConsumable.test( modelElement, 'foo:bar:baz' ) ).to.be.false;
+			expect( modelConsumable.test( modelElement, 'foo:bar' ) ).to.be.false;
+		} );
 	} );
 
 	describe( 'revert', () => {
@@ -112,6 +134,19 @@ describe( 'ModelConsumable', () => {
 			expect( result ).to.be.true;
 			expect( modelConsumable.test( modelTextProxy, 'type' ) ).to.be.true;
 		} );
+
+		it( 'should normalize type name', () => {
+			modelConsumable.add( modelElement, 'foo:bar:baz:abc' );
+			modelConsumable.consume( modelElement, 'foo:bar:baz' );
+
+			const result = modelConsumable.revert( modelElement, 'foo:bar:baz' );
+
+			expect( result ).to.be.true;
+
+			expect( modelConsumable.test( modelElement, 'foo:bar:baz:abc' ) ).to.be.true;
+			expect( modelConsumable.test( modelElement, 'foo:bar:baz' ) ).to.be.true;
+			expect( modelConsumable.test( modelElement, 'foo:bar' ) ).to.be.true;
+		} );
 	} );
 
 	describe( 'test', () => {