瀏覽代碼

Merge pull request #1372 from ckeditor/t/1369

Fixed: conversion.downcast-converters.changeAttribute should not consume if element creators returned null. Closes #1369.
Maciej 7 年之前
父節點
當前提交
a19b290bcf

+ 8 - 5
packages/ckeditor5-engine/src/conversion/downcast-converters.js

@@ -654,6 +654,13 @@ export function changeAttribute( attributeCreator ) {
 	attributeCreator = attributeCreator || ( ( value, data ) => ( { value, key: data.attributeKey } ) );
 
 	return ( evt, data, conversionApi ) => {
+		const oldAttribute = attributeCreator( data.attributeOldValue, data );
+		const newAttribute = attributeCreator( data.attributeNewValue, data );
+
+		if ( !oldAttribute && !newAttribute ) {
+			return;
+		}
+
 		if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
 			return;
 		}
@@ -662,8 +669,6 @@ export function changeAttribute( attributeCreator ) {
 		const viewWriter = conversionApi.writer;
 
 		// First remove the old attribute if there was one.
-		const oldAttribute = attributeCreator( data.attributeOldValue, data );
-
 		if ( data.attributeOldValue !== null && oldAttribute ) {
 			if ( oldAttribute.key == 'class' ) {
 				const classes = Array.isArray( oldAttribute.value ) ? oldAttribute.value : [ oldAttribute.value ];
@@ -682,9 +687,7 @@ export function changeAttribute( attributeCreator ) {
 			}
 		}
 
-		// Then, if conversion was successful, set the new attribute.
-		const newAttribute = attributeCreator( data.attributeNewValue, data );
-
+		// Then set the new attribute.
 		if ( data.attributeNewValue !== null && newAttribute ) {
 			if ( newAttribute.key == 'class' ) {
 				const classes = Array.isArray( newAttribute.value ) ? newAttribute.value : [ newAttribute.value ];

+ 17 - 1
packages/ckeditor5-engine/tests/conversion/downcast-converters.js

@@ -646,7 +646,7 @@ describe( 'downcast-converters', () => {
 		} );
 	} );
 
-	describe( 'setAttribute', () => {
+	describe( 'changeAttribute', () => {
 		it( 'should convert attribute insert/change/remove on a model node', () => {
 			const modelElement = new ModelElement( 'paragraph', { class: 'foo' }, new ModelText( 'foobar' ) );
 
@@ -717,6 +717,22 @@ describe( 'downcast-converters', () => {
 			// No attribute set.
 			expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p></div>' );
 		} );
+
+		it( 'should not convert or consume if element creator returned null', () => {
+			const callback = sinon.stub().returns( null );
+
+			dispatcher.on( 'attribute:class', changeAttribute( callback ) );
+
+			const modelElement = new ModelElement( 'paragraph', { class: 'foo' }, new ModelText( 'foobar' ) );
+
+			model.change( writer => {
+				writer.insert( modelElement, modelRootStart );
+			} );
+
+			expect( viewToString( viewRoot ) ).to.equal( '<div><p class="foo">foobar</p></div>' );
+
+			sinon.assert.called( callback );
+		} );
 	} );
 
 	describe( 'wrap', () => {