diff --git a/src/conversion/upcasthelpers.js b/src/conversion/upcasthelpers.js index a01022325..719bd7053 100644 --- a/src/conversion/upcasthelpers.js +++ b/src/conversion/upcasthelpers.js @@ -405,7 +405,7 @@ function upcastElementToElement( config ) { const converter = prepareToElementConverter( config ); - const elementName = getViewElementNameFromConfig( config ); + const elementName = getViewElementNameFromConfig( config.view ); const eventName = elementName ? 'element:' + elementName : 'element'; return dispatcher => { @@ -431,7 +431,7 @@ function upcastElementToAttribute( config ) { const converter = prepareToAttributeConverter( config, false ); - const elementName = getViewElementNameFromConfig( config ); + const elementName = getViewElementNameFromConfig( config.view ); const eventName = elementName ? 'element:' + elementName : 'element'; return dispatcher => { @@ -493,15 +493,15 @@ function upcastElementToMarker( config ) { // Helper function for from-view-element conversion. Checks if `config.view` directly specifies converted view element's name // and if so, returns it. // -// @param {Object} config Conversion config. +// @param {Object} config Conversion view config. // @returns {String|null} View element name or `null` if name is not directly set. -function getViewElementNameFromConfig( config ) { - if ( typeof config.view == 'string' ) { - return config.view; +function getViewElementNameFromConfig( viewConfig ) { + if ( typeof viewConfig == 'string' ) { + return viewConfig; } - if ( typeof config.view == 'object' && typeof config.view.name == 'string' ) { - return config.view.name; + if ( typeof viewConfig == 'object' && typeof viewConfig.name == 'string' ) { + return viewConfig.name; } return null; @@ -684,7 +684,7 @@ function prepareToAttributeConverter( config, shallow ) { return; } - if ( onlyViewNameIsDefined( config ) ) { + if ( onlyViewNameIsDefined( config.view, data.viewItem ) ) { match.match.name = true; } else { // Do not test or consume `name` consumable. @@ -714,14 +714,17 @@ function prepareToAttributeConverter( config, shallow ) { // Helper function that checks if element name should be consumed in attribute converters. // -// @param {Object} config Conversion config. +// @param {Object} config Conversion view config. // @returns {Boolean} -function onlyViewNameIsDefined( config ) { - if ( typeof config.view == 'object' && !getViewElementNameFromConfig( config ) ) { +function onlyViewNameIsDefined( viewConfig, viewItem ) { + // https://github.com/ckeditor/ckeditor5-engine/issues/1786 + const configToTest = typeof viewConfig == 'function' ? viewConfig( viewItem ) : viewConfig; + + if ( typeof configToTest == 'object' && !getViewElementNameFromConfig( configToTest ) ) { return false; } - return !config.view.classes && !config.view.attributes && !config.view.styles; + return !configToTest.classes && !configToTest.attributes && !configToTest.styles; } // Helper function for to-model-attribute converter. Sets model attribute on given range. Checks {@link module:engine/model/schema~Schema} diff --git a/tests/conversion/conversion.js b/tests/conversion/conversion.js index e17ce29b2..659e0ae94 100644 --- a/tests/conversion/conversion.js +++ b/tests/conversion/conversion.js @@ -301,6 +301,14 @@ describe( 'Conversion', () => { } ); it( 'config.view is an object with upcastAlso defined', () => { + schema.extend( '$text', { + allowAttributes: [ 'bold', 'xBold' ] + } ); + conversion.attributeToElement( { + model: 'xBold', + view: 'x-bold' + } ); + conversion.attributeToElement( { model: 'bold', view: 'strong', @@ -310,22 +318,18 @@ describe( 'Conversion', () => { name: 'span', classes: 'bold' }, - { - name: 'span', - styles: { - 'font-weight': 'bold' - } - }, viewElement => { const fontWeight = viewElement.getStyle( 'font-weight' ); - if ( viewElement.is( 'span' ) && fontWeight && /\d+/.test( fontWeight ) && Number( fontWeight ) > 500 ) { + if ( fontWeight == 'bold' || Number( fontWeight ) > 500 ) { return { - name: true, styles: [ 'font-weight' ] }; } - } + }, + // Duplicates the `x-bold` from above to test if only one attribute would be converted. + // It should not convert to both bold & x-bold. + viewElement => viewElement.is( 'x-bold' ) ? { name: 'x-bold' } : null ] } ); @@ -363,6 +367,18 @@ describe( 'Conversion', () => { '<$text bold="true">Foo', '

Foo

' ); + + test( + '

Foo

', + '<$text bold="true">Foo', + '

Foo

' + ); + + test( + '

Foo

', + '<$text xBold="true">Foo', + '

Foo

' + ); } ); it( 'model attribute value is enumerable', () => {