浏览代码

Merge pull request #1375 from ckeditor/t/1348

Fix: Upcasting element with multiple attributes should not consume the element name by default. Closes #1348.
Szymon Cofalik 7 年之前
父节点
当前提交
f82d2deebc

+ 18 - 6
packages/ckeditor5-engine/src/conversion/upcast-converters.js

@@ -138,7 +138,7 @@ export function upcastElementToAttribute( config ) {
 
 	_normalizeModelAttributeConfig( config );
 
-	const converter = _prepareToAttributeConverter( config, true );
+	const converter = _prepareToAttributeConverter( config );
 
 	const elementName = _getViewElementNameFromConfig( config );
 	const eventName = elementName ? 'element:' + elementName : 'element';
@@ -223,7 +223,7 @@ export function upcastAttributeToAttribute( config ) {
 
 	_normalizeModelAttributeConfig( config, viewKey );
 
-	const converter = _prepareToAttributeConverter( config, false );
+	const converter = _prepareToAttributeConverter( config );
 
 	return dispatcher => {
 		dispatcher.on( 'element', converter, { priority: config.priority || 'low' } );
@@ -442,7 +442,7 @@ function _normalizeModelAttributeConfig( config, viewAttributeKeyToCopy = null )
 // @param {Object|Array.<Object>} config Conversion configuration. It is possible to provide multiple configurations in an array.
 // @param {Boolean} consumeName If set to `true` converter will try to consume name. If set to `false` converter will not try to
 // consume name. This flag overwrites parameter returned by `Matcher#match`.
-function _prepareToAttributeConverter( config, consumeName ) {
+function _prepareToAttributeConverter( config ) {
 	const matcher = new Matcher( config.view );
 
 	return ( evt, data, conversionApi ) => {
@@ -461,11 +461,11 @@ function _prepareToAttributeConverter( config, consumeName ) {
 			return;
 		}
 
-		if ( !consumeName ) {
+		if ( _onlyViewNameIsDefined( config ) ) {
+			match.match.name = true;
+		} else {
 			// Do not test or consume `name` consumable.
 			delete match.match.name;
-		} else {
-			match.match.name = true;
 		}
 
 		// Try to consume appropriate values from consumable values list.
@@ -489,6 +489,18 @@ function _prepareToAttributeConverter( config, consumeName ) {
 	};
 }
 
+// Helper function that checks if element name should be consumed in attribute converters.
+//
+// @param {Object} config Conversion config.
+// @returns {Boolean}
+function _onlyViewNameIsDefined( config ) {
+	if ( typeof config.view == 'object' && !_getViewElementNameFromConfig( config ) ) {
+		return false;
+	}
+
+	return !config.view.class && !config.view.attribute && !config.view.style;
+}
+
 // Helper function for to-model-attribute converter. Sets model attribute on given range. Checks {@link module:engine/model/schema~Schema}
 // to ensure proper model structure.
 //

+ 44 - 1
packages/ckeditor5-engine/tests/conversion/upcast-converters.js

@@ -36,7 +36,7 @@ describe( 'upcast-helpers', () => {
 
 		schema.extend( '$text', {
 			allowIn: '$root',
-			allowAttributes: [ 'bold' ]
+			allowAttributes: [ 'bold', 'attribA', 'attribB' ]
 		} );
 
 		schema.register( 'paragraph', {
@@ -300,6 +300,49 @@ describe( 'upcast-helpers', () => {
 				'<$text bold="true">foo</$text>'
 			);
 		} );
+
+		it( 'should allow two converters to convert attributes on the same element', () => {
+			const helperA = upcastElementToAttribute( {
+				model: 'attribA',
+				view: { name: 'span', class: 'attrib-a' }
+			} );
+
+			const helperB = upcastElementToAttribute( {
+				model: 'attribB',
+				view: { name: 'span', style: { color: 'attrib-b' } }
+			} );
+
+			conversion.for( 'upcast' ).add( helperA ).add( helperB );
+
+			expectResult(
+				new ViewAttributeElement( 'span', { class: 'attrib-a', style: 'color:attrib-b;' }, new ViewText( 'foo' ) ),
+				'<$text attribA="true" attribB="true">foo</$text>'
+			);
+		} );
+
+		it( 'should consume element only when only is name specified', () => {
+			const helperBold = upcastElementToAttribute( {
+				model: 'bold',
+				view: { name: 'strong' }
+			} );
+
+			const helperA = upcastElementToAttribute( {
+				model: 'attribA',
+				view: { name: 'strong' }
+			} );
+
+			const helperB = upcastElementToAttribute( {
+				model: 'attribB',
+				view: { name: 'strong', class: 'foo' }
+			} );
+
+			conversion.for( 'upcast' ).add( helperBold ).add( helperA ).add( helperB );
+
+			expectResult(
+				new ViewAttributeElement( 'strong', { class: 'foo' }, new ViewText( 'foo' ) ),
+				'<$text attribB="true" bold="true">foo</$text>'
+			);
+		} );
 	} );
 
 	describe( 'upcastAttributeToAttribute', () => {