Sfoglia il codice sorgente

Add warning message to point out converters misconfiguration.

Krzysztof Krztoń 7 anni fa
parent
commit
6bbcb5b4fd

+ 38 - 2
packages/ckeditor5-engine/src/conversion/downcast-converters.js

@@ -10,6 +10,7 @@ import ModelElement from '../model/element';
 import ViewAttributeElement from '../view/attributeelement';
 import DocumentSelection from '../model/documentselection';
 
+import log from '@ckeditor/ckeditor5-utils/src/log';
 import { cloneDeep } from 'lodash-es';
 
 /**
@@ -685,9 +686,44 @@ export function changeAttribute( attributeCreator ) {
 		const viewElement = conversionApi.mapper.toViewElement( data.item );
 		const viewWriter = conversionApi.writer;
 
-		// If model item cannot be mapped to view element, it means item is not an `Element` instance (it is a `TextProxy`).
-		// Only elements can have attributes in a view so do not proceed for anything else (see #1587).
+		// If model item cannot be mapped to a view element, it means item is not an `Element` instance but a `TextProxy` node.
+		// Only elements can have attributes in a view so do not proceed for anything else (#1587).
 		if ( !viewElement ) {
+			/**
+			 * This error occurs when a {@link module:engine/model/textproxy~TextProxy text node} is to be downcasted
+			 * by {@link module:engine/conversion/conversion~Conversion#attributeToAttribute `Attribute to Attribute converter`}.
+			 * In most cases it is caused by converters misconfiguration when only "generic" converter is defined:
+			 *
+			 *		editor.conversion.for( 'downcast' ).add( downcastAttributeToAttribute( {
+			 *			model: 'attribute-name',
+			 *			view: 'attribute-name'
+			 *		} ) );
+			 *
+			 * and given attribute is used on text node, for example:
+			 *
+			 *		model.change( writer => {
+			 *			writer.insertText( 'Foo', { 'attribute-name': 'bar' }, parent, 0 );
+			 *		} );
+			 *
+			 * In such cases, to convert the same attribute for both {@link module:engine/model/element~Element}
+			 * and {@link module:engine/model/textproxy~TextProxy `Text`} nodes, text specific
+			 * {@link module:engine/conversion/conversion~Conversion#attributeToElement `Attribute to Element converter`}
+			 * with higher {@link module:utils/priorities~PriorityString priority} must also be defined:
+			 *
+			 *		conversion.for( 'downcast' ).add( downcastAttributeToElement( {
+			 *			model: {
+			 *				key: 'attribute-name',
+			 *				name: '$text'
+			 *			},
+			 *			view: ( value, writer ) => {
+			 *				return writer.createAttributeElement( 'span', { 'attribute-name': value } );
+			 *			}
+			 *		} ), { priority: 'high' } );
+			 *
+			 * @error conversion-attribute-to-attribute-on-text
+			 */
+			log.warn( 'conversion-attribute-to-attribute-on-text: Trying to convert text node with attribute to attribute converter.' );
+
 			return;
 		}
 

+ 12 - 0
packages/ckeditor5-engine/tests/conversion/downcast-converters.js

@@ -17,6 +17,9 @@ import ViewContainerElement from '../../src/view/containerelement';
 import ViewUIElement from '../../src/view/uielement';
 import ViewText from '../../src/view/text';
 
+import log from '@ckeditor/ckeditor5-utils/src/log';
+import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
+
 import {
 	downcastElementToElement, downcastAttributeToElement, downcastAttributeToAttribute, downcastMarkerToElement, downcastMarkerToHighlight,
 	insertElement, insertUIElement, changeAttribute, wrap, removeUIElement,
@@ -264,6 +267,8 @@ describe( 'downcast-helpers', () => {
 	} );
 
 	describe( 'downcastAttributeToAttribute', () => {
+		testUtils.createSinonSandbox();
+
 		beforeEach( () => {
 			conversion.for( 'downcast' ).add( downcastElementToElement( { model: 'image', view: 'img' } ) );
 		} );
@@ -463,6 +468,8 @@ describe( 'downcast-helpers', () => {
 
 		// #1587
 		it( 'config.view and config.model as strings in generic conversion (elements only)', () => {
+			const logSpy = testUtils.sinon.spy( log, 'warn' );
+
 			conversion.for( 'downcast' ).add( downcastElementToElement( { model: 'paragraph', view: 'p' } ) );
 
 			conversion.for( 'downcast' ).add( downcastAttributeToAttribute( { model: 'test', view: 'test' } ) );
@@ -473,6 +480,7 @@ describe( 'downcast-helpers', () => {
 			} );
 
 			expectResult( '<p test="1"></p><p test="2"></p>' );
+			expect( logSpy.callCount ).to.equal( 0 );
 
 			model.change( writer => {
 				writer.removeAttribute( 'test', modelRoot.getChild( 1 ) );
@@ -483,6 +491,8 @@ describe( 'downcast-helpers', () => {
 
 		// #1587
 		it( 'config.view and config.model as strings in generic conversion (elements + text)', () => {
+			const logSpy = testUtils.sinon.spy( log, 'warn' );
+
 			conversion.for( 'downcast' ).add( downcastElementToElement( { model: 'paragraph', view: 'p' } ) );
 
 			conversion.for( 'downcast' ).add( downcastAttributeToAttribute( { model: 'test', view: 'test' } ) );
@@ -496,6 +506,8 @@ describe( 'downcast-helpers', () => {
 			} );
 
 			expectResult( '<p>Foo</p><p test="1">Bar</p>' );
+			expect( logSpy.callCount ).to.equal( 2 );
+			expect( logSpy.alwaysCalledWithMatch( 'conversion-attribute-to-attribute-on-text' ) ).to.true;
 
 			model.change( writer => {
 				writer.removeAttribute( 'test', modelRoot.getChild( 1 ) );