diff --git a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js index d759b8a95cf..afab06b5e07 100644 --- a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js @@ -21,7 +21,6 @@ import ConversionHelpers from './conversionhelpers'; import { cloneDeep } from 'lodash-es'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import toArray from '@ckeditor/ckeditor5-utils/src/toarray'; -import { logWarning } from '@ckeditor/ckeditor5-utils'; /** * Downcast conversion helper functions. @@ -1042,9 +1041,6 @@ export function insertElement( elementCreator, consumer = defaultConsumer ) { return; } - // Check if only one element has been created. - validateChildren( viewElement ); - // Consume an element insertion and all present attributes that are specified as a reconversion triggers. consumer( data.item, conversionApi.consumable ); @@ -1703,6 +1699,48 @@ function downcastElementToStructure( config ) { config.model.children = true; return dispatcher => { + if ( dispatcher._conversionApi.schema.checkChild( config.model.name, '$text' ) ) { + /** + * This error occurs when a {@link module:engine/model/element~Element model element} is downcasted + * via {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToStructure} helper but the element was + * allowed to host `$text` by the {@link module:engine/model/schema~Schema model schema}. + * + * For instance, this may be the result of `myElement` allowing the content of + * {@glink framework/guides/deep-dive/schema#generic-items `$block`} in its schema definition: + * + * // Element definition in schema. + * schema.register( 'myElement', { + * allowContentOf: '$block', + * + * // ... + * } ); + * + * // ... + * + * // Conversion of myElement with the use of elementToStructure(). + * editor.conversion.for( 'downcast' ).elementToStructure( { + * model: 'myElement', + * view: ( modelElement, { writer } ) => { + * // ... + * } + * } ); + * + * In such case, {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement `elementToElement()`} helper + * can be used instead to get around this problem: + * + * editor.conversion.for( 'downcast' ).elementToElement( { + * model: 'myElement', + * view: ( modelElement, { writer } ) => { + * // ... + * } + * } ); + * + * @error conversion-element-to-structure-disallowed-text + * @param {String} elementName The name of the element the structure is to be created for. + */ + throw new CKEditorError( 'conversion-element-to-structure-disallowed-text', dispatcher, { elementName: config.model.name } ); + } + dispatcher.on( 'insert:' + config.model.name, insertStructure( config.view, createConsumer( config.model ) ), @@ -2121,31 +2159,6 @@ function createConsumer( model ) { }; } -// Check if given element children list contains only UI elements and warns otherwise. -// -// @param {module:engine/view/element~Element} viewElement. -function validateChildren( viewElement ) { - const children = Array.from( viewElement.getChildren() ); - const hasNonUiChildren = children.some( element => !element.is( 'uiElement' ) ); - - if ( hasNonUiChildren ) { - /** - * Only one container element without any children elements other than - * {@link module:engine/view/uielement~UIElement `UIElement`}s should be created in - * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement} function. - * - * Please make sure you don't create more than one element in - * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement} and if you need - * to create multiple elements use {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToStructure} - * instead. - * - * @error conversion-element-to-element-created-multiple-elements - * @param {module:engine/model/element~Element} viewElement - */ - logWarning( 'conversion-element-to-element-created-multiple-elements', { viewElement } ); - } -} - // Creates a function that create view slots. // // @param {module:engine/model/element~Element} element diff --git a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js index c4267061d3d..a0766219136 100644 --- a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js @@ -808,7 +808,7 @@ describe( 'DowncastHelpers', () => { } ); describe( 'with multiple child elements', () => { - it( 'warns if multiple child elements are created', () => { + it( 'does not warn if multiple child elements are created', () => { let viewElement; testUtils.sinon.stub( console, 'warn' ); @@ -828,12 +828,7 @@ describe( 'DowncastHelpers', () => { writer.insertElement( 'multiItemBox', null, modelRoot, 0 ); } ); - sinon.assert.calledOnce( console.warn ); - sinon.assert.calledWithExactly( console.warn, - sinon.match( /^conversion-element-to-element-created-multiple-elements/ ), - { viewElement }, - sinon.match.string // Link to the documentation - ); + sinon.assert.notCalled( console.warn ); } ); it( 'does not warn if multiple child UI elements are created', () => { @@ -2301,6 +2296,25 @@ describe( 'DowncastHelpers', () => { } ); }, /^conversion-slot-filter-incomplete/, controller.downcastDispatcher ); } ); + + // https://github.com/ckeditor/ckeditor5/issues/11163 + it( 'should throw an exception when invoked for a model element that allows $text', () => { + model.schema.register( 'myElement', { + allowIn: '$root', + + // This makes it accept $text. + allowContentOf: '$block' + } ); + + expectToThrowCKEditorError( () => { + downcastHelpers.elementToStructure( { + model: 'myElement', + view: ( modelElement, { writer } ) => { + return writer.createContainerElement( 'div' ); + } + } ); + }, /^conversion-element-to-structure-disallowed-text/, controller.downcastDispatcher, { elementName: 'myElement' } ); + } ); } ); describe( 'attributeToElement()', () => { diff --git a/packages/ckeditor5-widget/tests/widget-events.js b/packages/ckeditor5-widget/tests/widget-events.js index b854faf9518..e91c1fcd958 100644 --- a/packages/ckeditor5-widget/tests/widget-events.js +++ b/packages/ckeditor5-widget/tests/widget-events.js @@ -72,7 +72,7 @@ describe( 'Widget - Events', () => { function defineSchema( editor ) { editor.model.schema.register( 'simpleWidgetElement', { - inheritAllFrom: '$block', + allowIn: '$root', isObject: true } ); } diff --git a/packages/ckeditor5-widget/tests/widget.js b/packages/ckeditor5-widget/tests/widget.js index 32205462d13..19ea08dc4df 100644 --- a/packages/ckeditor5-widget/tests/widget.js +++ b/packages/ckeditor5-widget/tests/widget.js @@ -96,13 +96,12 @@ describe( 'Widget', () => { .elementToElement( { model: 'imageBlock', view: 'img' } ) .elementToElement( { model: 'blockQuote', view: 'blockquote' } ) .elementToElement( { model: 'div', view: 'div' } ) - .elementToStructure( { + .elementToElement( { model: 'widget', view: ( modelItem, { writer } ) => { const b = writer.createAttributeElement( 'b' ); const div = writer.createContainerElement( 'div' ); writer.insert( writer.createPositionAt( div, 0 ), b ); - writer.insert( writer.createPositionAt( div, 0 ), writer.createSlot() ); return toWidget( div, writer, { label: 'element label' } ); } diff --git a/packages/ckeditor5-widget/tests/widgetresize.js b/packages/ckeditor5-widget/tests/widgetresize.js index 3bae48bf5fd..86fec2a5e21 100644 --- a/packages/ckeditor5-widget/tests/widgetresize.js +++ b/packages/ckeditor5-widget/tests/widgetresize.js @@ -693,7 +693,7 @@ describe( 'WidgetResize', () => { } ); editor.conversion.for( 'downcast' ) - .elementToStructure( { + .elementToElement( { model: 'widget', view: ( modelItem, { writer } ) => { const parentDiv = writer.createContainerElement( 'div' ); diff --git a/packages/ckeditor5-widget/tests/widgettypearound/widgettypearound.js b/packages/ckeditor5-widget/tests/widgettypearound/widgettypearound.js index e728946c0ae..3df1bea09e3 100644 --- a/packages/ckeditor5-widget/tests/widgettypearound/widgettypearound.js +++ b/packages/ckeditor5-widget/tests/widgettypearound/widgettypearound.js @@ -1872,14 +1872,13 @@ describe( 'WidgetTypeAround', () => { } ); editor.conversion.for( 'downcast' ) - .elementToStructure( { + .elementToElement( { model: 'blockWidget', view: ( modelItem, { writer } ) => { const container = writer.createContainerElement( 'div' ); const viewText = writer.createText( 'block-widget' ); writer.insert( writer.createPositionAt( container, 0 ), viewText ); - writer.insert( writer.createPositionAt( container, 0 ), writer.createSlot() ); return toWidget( container, writer, { label: 'block widget'