From d10933b2f3903240058b64aa068c9950a3fe4614 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 11 May 2022 16:39:44 +0200 Subject: [PATCH 01/13] PoC of handling custom elements by GHS. --- .../src/conversion/upcasthelpers.js | 5 + .../ckeditor5-engine/src/view/domconverter.js | 11 +- .../src/generalhtmlsupport.js | 4 +- .../src/integrations/customelement.js | 120 ++++++++++++++++++ .../src/schemadefinitions.js | 8 ++ 5 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 packages/ckeditor5-html-support/src/integrations/customelement.js diff --git a/packages/ckeditor5-engine/src/conversion/upcasthelpers.js b/packages/ckeditor5-engine/src/conversion/upcasthelpers.js index b67d64e7611..62996c0fb60 100644 --- a/packages/ckeditor5-engine/src/conversion/upcasthelpers.js +++ b/packages/ckeditor5-engine/src/conversion/upcasthelpers.js @@ -465,6 +465,11 @@ export function convertText() { return; } + // Do not auto-paragraph whitespaces. + if ( data.viewItem.data.trim().length == 0 ) { + return; + } + position = wrapInParagraph( position, writer ); } diff --git a/packages/ckeditor5-engine/src/view/domconverter.js b/packages/ckeditor5-engine/src/view/domconverter.js index 10afabedb02..25f181ea8a5 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.js +++ b/packages/ckeditor5-engine/src/view/domconverter.js @@ -35,7 +35,6 @@ const NBSP_FILLER_REF = NBSP_FILLER( document ); // eslint-disable-line new-cap const MARKED_NBSP_FILLER_REF = MARKED_NBSP_FILLER( document ); // eslint-disable-line new-cap const UNSAFE_ATTRIBUTE_NAME_PREFIX = 'data-ck-unsafe-attribute-'; const UNSAFE_ELEMENT_REPLACEMENT_ATTRIBUTE = 'data-ck-unsafe-element'; -const UNSAFE_ELEMENTS = [ 'script', 'style' ]; /** * `DomConverter` is a set of tools to do transformations between DOM nodes and view nodes. It also handles @@ -127,6 +126,14 @@ export default class DomConverter { 'object', 'iframe', 'input', 'button', 'textarea', 'select', 'option', 'video', 'embed', 'audio', 'img', 'canvas' ]; + /** + * TODO + * + * @readonly + * @member {Array.} module:engine/view/domconverter~DomConverter#unsafeElements + */ + this.unsafeElements = [ 'script', 'style' ]; + /** * The DOM-to-view mapping. * @@ -1564,7 +1571,7 @@ export default class DomConverter { _shouldRenameElement( elementName ) { const name = elementName.toLowerCase(); - return this.renderingMode === 'editing' && UNSAFE_ELEMENTS.includes( name ); + return this.renderingMode === 'editing' && this.unsafeElements.includes( name ); } /** diff --git a/packages/ckeditor5-html-support/src/generalhtmlsupport.js b/packages/ckeditor5-html-support/src/generalhtmlsupport.js index f7ee9ee0c31..7bbe4edc5a8 100644 --- a/packages/ckeditor5-html-support/src/generalhtmlsupport.js +++ b/packages/ckeditor5-html-support/src/generalhtmlsupport.js @@ -20,6 +20,7 @@ import ScriptElementSupport from './integrations/script'; import TableElementSupport from './integrations/table'; import StyleElementSupport from './integrations/style'; import DocumentListElementSupport from './integrations/documentlist'; +import CustomElementSupport from './integrations/customelement'; /** * The General HTML Support feature. @@ -51,7 +52,8 @@ export default class GeneralHtmlSupport extends Plugin { ScriptElementSupport, TableElementSupport, StyleElementSupport, - DocumentListElementSupport + DocumentListElementSupport, + CustomElementSupport ]; } diff --git a/packages/ckeditor5-html-support/src/integrations/customelement.js b/packages/ckeditor5-html-support/src/integrations/customelement.js new file mode 100644 index 00000000000..e39cf0750b3 --- /dev/null +++ b/packages/ckeditor5-html-support/src/integrations/customelement.js @@ -0,0 +1,120 @@ +/** + * @license Copyright (c) 2003-2022, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module html-support/integrations/script + */ + +import { Plugin } from 'ckeditor5/src/core'; +import { UpcastWriter } from 'ckeditor5/src/engine'; + +import DataSchema from '../dataschema'; +import DataFilter from '../datafilter'; +import { setViewAttributes } from '../conversionutils'; + +/** + * Provides the General HTML Support for custom elements (not registered in the DataSchema). + * + * @extends module:core/plugin~Plugin + */ +export default class CustomElementSupport extends Plugin { + /** + * @inheritDoc + */ + static get requires() { + return [ DataFilter, DataSchema ]; + } + + /** + * @inheritDoc + */ + init() { + const dataFilter = this.editor.plugins.get( DataFilter ); + const dataSchema = this.editor.plugins.get( DataSchema ); + + dataFilter.on( 'register:$customElement', ( evt, definition ) => { + evt.stop(); + + const editor = this.editor; + const schema = editor.model.schema; + const conversion = editor.conversion; + const unsafeElements = editor.editing.view.domConverter.unsafeElements; + + schema.register( definition.model, definition.modelSchema ); + schema.extend( definition.model, { + allowAttributes: [ 'htmlElementName', 'htmlAttributes', 'htmlContent' ], + isContent: true + } ); + + conversion.for( 'upcast' ).elementToElement( { + view: /.*/, + model: ( viewElement, conversionApi ) => { + // Allow for fallback only if this element is not defined in data schema to make sure + // that this will handle only custom elements not registered in the data schema. + if ( dataSchema.getDefinitionsForView( viewElement.name ).size ) { + return; + } + + const modelElement = conversionApi.writer.createElement( definition.model, { + htmlElementName: viewElement.name + } ); + + const htmlAttributes = dataFilter.processViewAttributes( viewElement, conversionApi ); + + if ( htmlAttributes ) { + conversionApi.writer.setAttribute( 'htmlAttributes', htmlAttributes, modelElement ); + } + + const viewWriter = new UpcastWriter( viewElement.document ); + const childNodes = []; + + // Replace filler offset so the block filler won't get injected. + for ( const node of Array.from( viewElement.getChildren() ) ) { + node.getFillerOffset = () => null; + childNodes.push( node ); + } + + const documentFragment = viewWriter.createDocumentFragment( childNodes ); + + if ( !documentFragment.isEmpty ) { + const htmlContent = editor.data.processor.toData( documentFragment ); + + conversionApi.writer.setAttribute( 'htmlContent', htmlContent, modelElement ); + } + + if ( !unsafeElements.includes( viewElement.name ) ) { + unsafeElements.push( viewElement.name ); + } + + return modelElement; + }, + converterPriority: 'low' + } ); + + conversion.for( 'downcast' ).elementToElement( { + model: { + name: definition.model, + attributes: [ 'htmlElementName', 'htmlAttributes', 'htmlContent' ] + }, + view: ( modelElement, { writer } ) => { + const viewName = modelElement.getAttribute( 'htmlElementName' ); + const htmlContent = modelElement.getAttribute( 'htmlContent' ); + + const viewElement = writer.createRawElement( viewName, null, ( domElement, domConverter ) => { + if ( htmlContent ) { + domConverter.setContentOf( domElement, htmlContent ); + } + } ); + + if ( modelElement.hasAttribute( 'htmlAttributes' ) ) { + setViewAttributes( writer, modelElement.getAttribute( 'htmlAttributes' ), viewElement ); + } + + return viewElement; + } + } ); + } ); + } +} diff --git a/packages/ckeditor5-html-support/src/schemadefinitions.js b/packages/ckeditor5-html-support/src/schemadefinitions.js index 2429c1170b4..9d466d94612 100644 --- a/packages/ckeditor5-html-support/src/schemadefinitions.js +++ b/packages/ckeditor5-html-support/src/schemadefinitions.js @@ -839,6 +839,14 @@ export default { allowWhere: [ '$text', '$block' ], isInline: true } + }, + { + model: 'htmlCustomElement', + view: '$customElement', + modelSchema: { + allowWhere: [ '$text', '$block' ], + isInline: true + } } ] }; From 41d207dea1ce12b7dfe4a12baece10ab1214416a Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 11 May 2022 16:42:10 +0200 Subject: [PATCH 02/13] Fix comment. --- .../ckeditor5-html-support/src/integrations/customelement.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-html-support/src/integrations/customelement.js b/packages/ckeditor5-html-support/src/integrations/customelement.js index e39cf0750b3..2910154d438 100644 --- a/packages/ckeditor5-html-support/src/integrations/customelement.js +++ b/packages/ckeditor5-html-support/src/integrations/customelement.js @@ -4,7 +4,7 @@ */ /** - * @module html-support/integrations/script + * @module html-support/integrations/customelement */ import { Plugin } from 'ckeditor5/src/core'; From cc5c341ec0a5b75767a6b0389fd1bc99749279a1 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Thu, 12 May 2022 18:17:26 +0200 Subject: [PATCH 03/13] Checking alternative approach. --- .../src/controller/datacontroller.js | 6 +++++- .../src/dataprocessor/htmldataprocessor.js | 2 +- .../ckeditor5-engine/src/view/domconverter.js | 9 +++++++++ .../src/integrations/customelement.js | 19 ++++++------------- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/packages/ckeditor5-engine/src/controller/datacontroller.js b/packages/ckeditor5-engine/src/controller/datacontroller.js index 23bfa5d6e04..5142cf74ba9 100644 --- a/packages/ckeditor5-engine/src/controller/datacontroller.js +++ b/packages/ckeditor5-engine/src/controller/datacontroller.js @@ -406,7 +406,11 @@ export default class DataController { const viewDocumentFragment = this.processor.toView( data ); // view -> model - return this.toModel( viewDocumentFragment, context ); + const modelDocumentFragment = this.toModel( viewDocumentFragment, context ); + + this.processor.domConverter.clearBindings(); + + return modelDocumentFragment; } /** diff --git a/packages/ckeditor5-engine/src/dataprocessor/htmldataprocessor.js b/packages/ckeditor5-engine/src/dataprocessor/htmldataprocessor.js index 4083be4cd9c..eee25b2a143 100644 --- a/packages/ckeditor5-engine/src/dataprocessor/htmldataprocessor.js +++ b/packages/ckeditor5-engine/src/dataprocessor/htmldataprocessor.js @@ -73,7 +73,7 @@ export default class HtmlDataProcessor { const domFragment = this._toDom( data ); // Convert DOM DocumentFragment to view DocumentFragment. - return this.domConverter.domToView( domFragment ); + return this.domConverter.domToView( domFragment, { bind: true } ); } /** diff --git a/packages/ckeditor5-engine/src/view/domconverter.js b/packages/ckeditor5-engine/src/view/domconverter.js index 25f181ea8a5..0bca9849f6e 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.js +++ b/packages/ckeditor5-engine/src/view/domconverter.js @@ -232,6 +232,15 @@ export default class DomConverter { } } + /** + * TODO + */ + clearBindings() { + this._domToViewMapping = new WeakMap(); + this._viewToDomMapping = new WeakMap(); + this._fakeSelectionMapping = new WeakMap(); + } + /** * Binds DOM and view document fragments, so it will be possible to get corresponding document fragments using * {@link module:engine/view/domconverter~DomConverter#mapDomToView} and diff --git a/packages/ckeditor5-html-support/src/integrations/customelement.js b/packages/ckeditor5-html-support/src/integrations/customelement.js index 2910154d438..36959a4907a 100644 --- a/packages/ckeditor5-html-support/src/integrations/customelement.js +++ b/packages/ckeditor5-html-support/src/integrations/customelement.js @@ -8,7 +8,6 @@ */ import { Plugin } from 'ckeditor5/src/core'; -import { UpcastWriter } from 'ckeditor5/src/engine'; import DataSchema from '../dataschema'; import DataFilter from '../datafilter'; @@ -67,21 +66,15 @@ export default class CustomElementSupport extends Plugin { conversionApi.writer.setAttribute( 'htmlAttributes', htmlAttributes, modelElement ); } - const viewWriter = new UpcastWriter( viewElement.document ); - const childNodes = []; + const domElement = editor.data.processor.domConverter.mapViewToDom( viewElement ); + const rawContent = domElement.innerHTML; - // Replace filler offset so the block filler won't get injected. - for ( const node of Array.from( viewElement.getChildren() ) ) { - node.getFillerOffset = () => null; - childNodes.push( node ); + if ( rawContent ) { + conversionApi.writer.setAttribute( 'htmlContent', rawContent, modelElement ); } - const documentFragment = viewWriter.createDocumentFragment( childNodes ); - - if ( !documentFragment.isEmpty ) { - const htmlContent = editor.data.processor.toData( documentFragment ); - - conversionApi.writer.setAttribute( 'htmlContent', htmlContent, modelElement ); + for ( const node of viewElement.getChildren() ) { + conversionApi.consumable.consume( node, { name: true } ); } if ( !unsafeElements.includes( viewElement.name ) ) { From 76d4fce81e5baad7e2b78f3158f4d994e4d16b68 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 13 May 2022 13:34:12 +0200 Subject: [PATCH 04/13] Revert "Checking alternative approach." This reverts commit cc5c341ec0a5b75767a6b0389fd1bc99749279a1. --- .../src/controller/datacontroller.js | 6 +----- .../src/dataprocessor/htmldataprocessor.js | 2 +- .../ckeditor5-engine/src/view/domconverter.js | 9 --------- .../src/integrations/customelement.js | 19 +++++++++++++------ 4 files changed, 15 insertions(+), 21 deletions(-) diff --git a/packages/ckeditor5-engine/src/controller/datacontroller.js b/packages/ckeditor5-engine/src/controller/datacontroller.js index 5142cf74ba9..23bfa5d6e04 100644 --- a/packages/ckeditor5-engine/src/controller/datacontroller.js +++ b/packages/ckeditor5-engine/src/controller/datacontroller.js @@ -406,11 +406,7 @@ export default class DataController { const viewDocumentFragment = this.processor.toView( data ); // view -> model - const modelDocumentFragment = this.toModel( viewDocumentFragment, context ); - - this.processor.domConverter.clearBindings(); - - return modelDocumentFragment; + return this.toModel( viewDocumentFragment, context ); } /** diff --git a/packages/ckeditor5-engine/src/dataprocessor/htmldataprocessor.js b/packages/ckeditor5-engine/src/dataprocessor/htmldataprocessor.js index eee25b2a143..4083be4cd9c 100644 --- a/packages/ckeditor5-engine/src/dataprocessor/htmldataprocessor.js +++ b/packages/ckeditor5-engine/src/dataprocessor/htmldataprocessor.js @@ -73,7 +73,7 @@ export default class HtmlDataProcessor { const domFragment = this._toDom( data ); // Convert DOM DocumentFragment to view DocumentFragment. - return this.domConverter.domToView( domFragment, { bind: true } ); + return this.domConverter.domToView( domFragment ); } /** diff --git a/packages/ckeditor5-engine/src/view/domconverter.js b/packages/ckeditor5-engine/src/view/domconverter.js index 0bca9849f6e..25f181ea8a5 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.js +++ b/packages/ckeditor5-engine/src/view/domconverter.js @@ -232,15 +232,6 @@ export default class DomConverter { } } - /** - * TODO - */ - clearBindings() { - this._domToViewMapping = new WeakMap(); - this._viewToDomMapping = new WeakMap(); - this._fakeSelectionMapping = new WeakMap(); - } - /** * Binds DOM and view document fragments, so it will be possible to get corresponding document fragments using * {@link module:engine/view/domconverter~DomConverter#mapDomToView} and diff --git a/packages/ckeditor5-html-support/src/integrations/customelement.js b/packages/ckeditor5-html-support/src/integrations/customelement.js index 36959a4907a..2910154d438 100644 --- a/packages/ckeditor5-html-support/src/integrations/customelement.js +++ b/packages/ckeditor5-html-support/src/integrations/customelement.js @@ -8,6 +8,7 @@ */ import { Plugin } from 'ckeditor5/src/core'; +import { UpcastWriter } from 'ckeditor5/src/engine'; import DataSchema from '../dataschema'; import DataFilter from '../datafilter'; @@ -66,15 +67,21 @@ export default class CustomElementSupport extends Plugin { conversionApi.writer.setAttribute( 'htmlAttributes', htmlAttributes, modelElement ); } - const domElement = editor.data.processor.domConverter.mapViewToDom( viewElement ); - const rawContent = domElement.innerHTML; + const viewWriter = new UpcastWriter( viewElement.document ); + const childNodes = []; - if ( rawContent ) { - conversionApi.writer.setAttribute( 'htmlContent', rawContent, modelElement ); + // Replace filler offset so the block filler won't get injected. + for ( const node of Array.from( viewElement.getChildren() ) ) { + node.getFillerOffset = () => null; + childNodes.push( node ); } - for ( const node of viewElement.getChildren() ) { - conversionApi.consumable.consume( node, { name: true } ); + const documentFragment = viewWriter.createDocumentFragment( childNodes ); + + if ( !documentFragment.isEmpty ) { + const htmlContent = editor.data.processor.toData( documentFragment ); + + conversionApi.writer.setAttribute( 'htmlContent', htmlContent, modelElement ); } if ( !unsafeElements.includes( viewElement.name ) ) { From 43463c74bd44ec7f0f7bb36518bb1a5d76738e4e Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 13 May 2022 16:27:09 +0200 Subject: [PATCH 05/13] Fixed a case with spaces replaced with nbsp entity inside a custom elements. --- .../src/integrations/customelement.js | 62 ++++++++++++++----- 1 file changed, 45 insertions(+), 17 deletions(-) diff --git a/packages/ckeditor5-html-support/src/integrations/customelement.js b/packages/ckeditor5-html-support/src/integrations/customelement.js index 2910154d438..c8071bd9b53 100644 --- a/packages/ckeditor5-html-support/src/integrations/customelement.js +++ b/packages/ckeditor5-html-support/src/integrations/customelement.js @@ -41,6 +41,7 @@ export default class CustomElementSupport extends Plugin { const schema = editor.model.schema; const conversion = editor.conversion; const unsafeElements = editor.editing.view.domConverter.unsafeElements; + const preLikeElements = editor.data.htmlProcessor.domConverter.preElements; schema.register( definition.model, definition.modelSchema ); schema.extend( definition.model, { @@ -57,6 +58,16 @@ export default class CustomElementSupport extends Plugin { return; } + // Make sure that this element will not render in the editing view. + if ( !unsafeElements.includes( viewElement.name ) ) { + unsafeElements.push( viewElement.name ); + } + + // Make sure that whitespaces will not be trimmed or replaced by nbsps while stringify content. + if ( !preLikeElements.includes( viewElement.name ) ) { + preLikeElements.push( viewElement.name ); + } + const modelElement = conversionApi.writer.createElement( definition.model, { htmlElementName: viewElement.name } ); @@ -67,25 +78,16 @@ export default class CustomElementSupport extends Plugin { conversionApi.writer.setAttribute( 'htmlAttributes', htmlAttributes, modelElement ); } + // Store the whole element in the attribute so that DomConverter will be able to use the pre like element context. const viewWriter = new UpcastWriter( viewElement.document ); - const childNodes = []; - - // Replace filler offset so the block filler won't get injected. - for ( const node of Array.from( viewElement.getChildren() ) ) { - node.getFillerOffset = () => null; - childNodes.push( node ); - } + const documentFragment = viewWriter.createDocumentFragment( viewElement ); + const htmlContent = editor.data.processor.toData( documentFragment ); - const documentFragment = viewWriter.createDocumentFragment( childNodes ); + conversionApi.writer.setAttribute( 'htmlContent', htmlContent, modelElement ); - if ( !documentFragment.isEmpty ) { - const htmlContent = editor.data.processor.toData( documentFragment ); - - conversionApi.writer.setAttribute( 'htmlContent', htmlContent, modelElement ); - } - - if ( !unsafeElements.includes( viewElement.name ) ) { - unsafeElements.push( viewElement.name ); + // Consume the content of the element. + for ( const { item } of editor.editing.view.createRangeIn( viewElement ) ) { + conversionApi.consumable.consume( item, { name: true } ); } return modelElement; @@ -93,7 +95,24 @@ export default class CustomElementSupport extends Plugin { converterPriority: 'low' } ); - conversion.for( 'downcast' ).elementToElement( { + conversion.for( 'editingDowncast' ).elementToElement( { + model: { + name: definition.model, + attributes: [ 'htmlElementName', 'htmlAttributes', 'htmlContent' ] + }, + view: ( modelElement, { writer } ) => { + const viewName = modelElement.getAttribute( 'htmlElementName' ); + const viewElement = writer.createRawElement( viewName ); + + if ( modelElement.hasAttribute( 'htmlAttributes' ) ) { + setViewAttributes( writer, modelElement.getAttribute( 'htmlAttributes' ), viewElement ); + } + + return viewElement; + } + } ); + + conversion.for( 'dataDowncast' ).elementToElement( { model: { name: definition.model, attributes: [ 'htmlElementName', 'htmlAttributes', 'htmlContent' ] @@ -105,6 +124,15 @@ export default class CustomElementSupport extends Plugin { const viewElement = writer.createRawElement( viewName, null, ( domElement, domConverter ) => { if ( htmlContent ) { domConverter.setContentOf( domElement, htmlContent ); + + // Unwrap the custom element content (it was stored in the attribute as the whole custom element). + const customElement = domElement.firstChild; + + customElement.remove(); + + while ( customElement.firstChild ) { + domElement.appendChild( customElement.firstChild ); + } } } ); From af95af9f35fcfc4e29f3c4197c7ba06cc0956424 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 23 May 2022 18:05:48 +0200 Subject: [PATCH 06/13] Adding tests. --- .../tests/integrations/customelement.js | 310 ++++++++++++++++++ 1 file changed, 310 insertions(+) create mode 100644 packages/ckeditor5-html-support/tests/integrations/customelement.js diff --git a/packages/ckeditor5-html-support/tests/integrations/customelement.js b/packages/ckeditor5-html-support/tests/integrations/customelement.js new file mode 100644 index 00000000000..952f6c027ed --- /dev/null +++ b/packages/ckeditor5-html-support/tests/integrations/customelement.js @@ -0,0 +1,310 @@ +/** + * @license Copyright (c) 2003-2022, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import CodeBlock from '@ckeditor/ckeditor5-code-block/src/codeblock'; +import GeneralHtmlSupport from '../../src/generalhtmlsupport'; +import { getModelDataWithAttributes } from '../_utils/utils'; +import { getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; + +/* global document */ + +describe( 'CustomElementSupport', () => { + let editor, model, editorElement, dataFilter; + + const excludeAttributes = [ 'htmlContent', 'htmlElementName' ]; + + beforeEach( () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor + .create( editorElement, { + plugins: [ CodeBlock, Paragraph, GeneralHtmlSupport ] + } ) + .then( newEditor => { + editor = newEditor; + model = editor.model; + + dataFilter = editor.plugins.get( 'DataFilter' ); + } ); + } ); + + afterEach( () => { + editorElement.remove(); + + return editor.destroy(); + } ); + + it( 'should allow unknown custom element', () => { + dataFilter.allowElement( /.*/ ); + editor.setData( 'bar' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: '', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( 'bar' ); + } ); + + it( 'should not allow unknown custom element if allow-all is not enabled', () => { + dataFilter.allowElement( /custom-foo-element/ ); + editor.setData( 'bar' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( '

bar

' ); + } ); + + describe( 'element position', () => { + const testCases = [ { + name: 'paragraph', + data: '

FooabcBar

', + model: + '' + + '' + + '' + + 'Foo' + + '' + + '' + + 'Bar' + + '' + + '' + + '' + }, { + name: 'section', + data: '

Foo

abc
', + model: + '' + + '' + + 'Foo' + + '' + + '' + + '' + + '' + }, { + name: 'article', + data: '

Foo

abc
', + model: + '' + + '' + + 'Foo' + + '' + + '' + + '' + + '' + }, { + name: 'root', + data: '

Foo

abc', + model: + '' + + '' + + 'Foo' + + '' + + '' + + '' + + '' + } ]; + + for ( const { name, data, model: modelData } of testCases ) { + it( `should allow element inside ${ name }`, () => { + dataFilter.allowElement( /.*/ ); + + editor.setData( data ); + + expect( getModelData( model, { withoutSelection: true, excludeAttributes } ) ).to.equal( modelData ); + + expect( editor.getData() ).to.equal( data ); + } ); + } + } ); + + it( 'should not inject nbsp in the element content', () => { + dataFilter.allowElement( /.*/ ); + editor.setData( ' c ' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: '', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( 'c' ); + } ); + + it( 'should allow attributes', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { attributes: { 'data-foo': /.*/ } } ); + + editor.setData( 'bar' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: { + 1: { + attributes: { + 'data-foo': 'foo' + } + } + } + } ); + + expect( editor.getData() ).to.equal( 'bar' ); + } ); + + it( 'should allow attributes (classes)', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { classes: 'foo' } ); + + editor.setData( 'bar' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: { + 1: { + classes: [ 'foo' ] + } + } + } ); + + expect( editor.getData() ).to.equal( 'bar' ); + } ); + + it( 'should allow attributes (styles)', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { styles: { background: true } } ); + + editor.setData( 'bar' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: { + 1: { + 'styles': { + 'background': 'red' + } + } + } + } ); + + expect( editor.getData() ).to.equal( 'bar' ); + } ); + + it( 'should disallow attributes', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { attributes: { 'data-foo': /.*/ } } ); + dataFilter.disallowAttributes( { attributes: { 'data-foo': /.*/ } } ); + + editor.setData( 'bar' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( 'bar' ); + } ); + + it( 'should disallow attributes (classes)', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { classes: 'foo' } ); + dataFilter.disallowAttributes( { classes: 'foo' } ); + + editor.setData( 'bar' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( 'bar' ); + } ); + + it( 'should disallow attributes (styles)', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { styles: { background: true } } ); + dataFilter.disallowAttributes( { styles: { background: true } } ); + + editor.setData( 'bar' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( 'bar' ); + } ); + + // it( 'should not consume attributes already consumed (downcast)', () => { + // dataFilter.allowAttributes( { name: 'script', attributes: true } ); + // + // editor.conversion.for( 'downcast' ).add( dispatcher => { + // dispatcher.on( 'attribute:htmlAttributes:htmlScript', ( evt, data, conversionApi ) => { + // conversionApi.consumable.consume( data.item, evt.name ); + // }, { priority: 'high' } ); + // } ); + // + // editor.setData( `

Foo

` ); + // + // expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + // data: `Foo`, + // attributes: { + // 1: { attributes: { nonce: 'qwerty' } }, + // 2: CODE + // } + // } ); + // + // expect( editor.getData() ).to.equal( `

Foo

` ); + // } ); + // + // it( 'should not consume attributes already consumed (upcast)', () => { + // dataFilter.allowAttributes( { name: 'script', attributes: true } ); + // + // editor.conversion.for( 'upcast' ).add( dispatcher => { + // dispatcher.on( 'element:script', ( evt, data, conversionApi ) => { + // conversionApi.consumable.consume( data.viewItem, { attributes: 'nonce' } ); + // }, { priority: 'high' } ); + // } ); + // + // editor.setData( `

Foo

` ); + // + // expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + // data: `Foo`, + // attributes: { + // 1: { attributes: { type: 'c++' } }, + // 2: CODE_CPP + // } + // } ); + // } ); +} ); From 7758d4d2af4d01d8518e2e4dfa820bf8380116b6 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 23 May 2022 18:06:11 +0200 Subject: [PATCH 07/13] Adding tests. --- .../ckeditor5-engine/src/view/domconverter.js | 3 +- .../tests/view/domconverter/view-to-dom.js | 27 ++++++++++++ .../whitespace-handling-integration.js | 42 ++++++++++++++++--- 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/domconverter.js b/packages/ckeditor5-engine/src/view/domconverter.js index 25f181ea8a5..f7df2243ecc 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.js +++ b/packages/ckeditor5-engine/src/view/domconverter.js @@ -127,7 +127,8 @@ export default class DomConverter { ]; /** - * TODO + * A list of elements which may affect the editing experience. To avoid this, those elements are replaced with + * `` while rendering in the editing mode. * * @readonly * @member {Array.} module:engine/view/domconverter~DomConverter#unsafeElements diff --git a/packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js b/packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js index 96328229c65..adee5515312 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js @@ -375,6 +375,33 @@ describe( 'DomConverter', () => { ); } ); + it( 'should replace registered custom element with span and add special data attribute', () => { + const viewScript = new ViewElement( viewDocument, 'custom-foo-element' ); + const viewText = new ViewText( viewDocument, 'foo' ); + const viewP = new ViewElement( viewDocument, 'p', { class: 'foo' } ); + + viewP._appendChild( viewScript ); + viewP._appendChild( viewText ); + + converter = new DomConverter( viewDocument, { + renderingMode: 'editing' + } ); + + converter.unsafeElements.push( 'custom-foo-element' ); + + const domP = converter.viewToDom( viewP, document ); + + expect( domP ).to.be.an.instanceof( HTMLElement ); + expect( domP.tagName ).to.equal( 'P' ); + expect( domP.getAttribute( 'class' ) ).to.equal( 'foo' ); + expect( domP.attributes.length ).to.equal( 1 ); + + expect( domP.childNodes.length ).to.equal( 2 ); + expect( domP.childNodes[ 0 ].tagName ).to.equal( 'SPAN' ); + expect( domP.childNodes[ 0 ].getAttribute( 'data-ck-unsafe-element' ) ).to.equal( 'custom-foo-element' ); + expect( domP.childNodes[ 1 ].data ).to.equal( 'foo' ); + } ); + describe( 'unsafe attribute names that were declaratively permitted', () => { let writer; diff --git a/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js b/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js index a2da8f8baf9..d2bc8bd99eb 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js @@ -107,16 +107,17 @@ describe( 'DomConverter – whitespace handling – integration', () => { } ); // Controversial result. See https://github.com/ckeditor/ckeditor5-engine/issues/987. - it( 'nbsp between blocks is not ignored (between paragraphs)', () => { + // https://github.com/ckeditor/ckeditor5/pull/11744/files#r871377976. + it( 'nbsp between blocks is ignored (between paragraphs)', () => { editor.setData( '

foo

 

bar

' ); expect( getData( editor.model, { withoutSelection: true } ) ) - .to.equal( 'foo bar' ); + .to.equal( 'foobar' ); - expect( editor.getData() ).to.equal( '

foo

 

bar

' ); + expect( editor.getData() ).to.equal( '

foo

bar

' ); } ); - it( 'nbsp between blocks is not ignored (different blocks)', () => { + it( 'nbsp between blocks is ignored (different blocks)', () => { editor.model.schema.register( 'block', { inheritAllFrom: '$block' } ); editor.conversion.elementToElement( { model: 'block', view: 'block' } ); editor.setData( 'foo 

bar

' ); @@ -124,14 +125,43 @@ describe( 'DomConverter – whitespace handling – integration', () => { expect( getData( editor.model, { withoutSelection: true } ) ) .to.equal( 'foo' + - ' ' + 'bar' ); expect( editor.getData() ) .to.equal( 'foo' + - '

 

' + + '

bar

' + ); + } ); + + it( 'whitespaces between custom elements at block level are ignored', () => { + editor.model.schema.register( 'custom-foo-element', { + allowWhere: [ '$text', '$block' ], + allowChildren: '$text', + isInline: true + } ); + editor.conversion.elementToElement( { model: 'custom-foo-element', view: 'custom-foo-element' } ); + editor.setData( + '

foo

' + + ' a' + + ' b' + + '

bar

' + ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( + 'foo' + + 'a' + + 'b' + + 'bar' + ); + + expect( editor.getData() ) + .to.equal( + '

foo

' + + 'a' + + 'b' + '

bar

' ); } ); From 1c888d4597279af7fde3d1813f89b961fe6f1e0a Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 25 May 2022 12:47:17 +0200 Subject: [PATCH 08/13] Added tests. --- .../src/integrations/customelement.js | 14 +- .../tests/integrations/customelement.js | 333 ++++++++++-------- 2 files changed, 201 insertions(+), 146 deletions(-) diff --git a/packages/ckeditor5-html-support/src/integrations/customelement.js b/packages/ckeditor5-html-support/src/integrations/customelement.js index c8071bd9b53..52eb1cc0bbb 100644 --- a/packages/ckeditor5-html-support/src/integrations/customelement.js +++ b/packages/ckeditor5-html-support/src/integrations/customelement.js @@ -122,17 +122,15 @@ export default class CustomElementSupport extends Plugin { const htmlContent = modelElement.getAttribute( 'htmlContent' ); const viewElement = writer.createRawElement( viewName, null, ( domElement, domConverter ) => { - if ( htmlContent ) { - domConverter.setContentOf( domElement, htmlContent ); + domConverter.setContentOf( domElement, htmlContent ); - // Unwrap the custom element content (it was stored in the attribute as the whole custom element). - const customElement = domElement.firstChild; + // Unwrap the custom element content (it was stored in the attribute as the whole custom element). + const customElement = domElement.firstChild; - customElement.remove(); + customElement.remove(); - while ( customElement.firstChild ) { - domElement.appendChild( customElement.firstChild ); - } + while ( customElement.firstChild ) { + domElement.appendChild( customElement.firstChild ); } } ); diff --git a/packages/ckeditor5-html-support/tests/integrations/customelement.js b/packages/ckeditor5-html-support/tests/integrations/customelement.js index 952f6c027ed..1a01bd0792b 100644 --- a/packages/ckeditor5-html-support/tests/integrations/customelement.js +++ b/packages/ckeditor5-html-support/tests/integrations/customelement.js @@ -6,9 +6,12 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import CodeBlock from '@ckeditor/ckeditor5-code-block/src/codeblock'; +import { getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; +import { INLINE_FILLER } from '@ckeditor/ckeditor5-engine/src/view/filler'; + import GeneralHtmlSupport from '../../src/generalhtmlsupport'; import { getModelDataWithAttributes } from '../_utils/utils'; -import { getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; /* global document */ @@ -65,6 +68,79 @@ describe( 'CustomElementSupport', () => { expect( editor.getData() ).to.equal( '

bar

' ); } ); + it( 'should convert only unknown elements (not defined in DataSchema)', () => { + dataFilter.allowElement( '$customElement' ); + + editor.setData( '
abc
' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'abc', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( '

abc

' ); + } ); + + it( 'should render in the editing view as an unsafe element', () => { + dataFilter.allowElement( /.*/ ); + editor.setData( 'barbaz' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: '' + + '', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( + 'bar' + + 'baz' + ); + expect( getViewData( editor.editing.view, { withoutSelection: true } ) ).to.equal( + '' + + '' + ); + expect( editor.editing.view.domConverter.unsafeElements ).include( 'custom-foo-element' ); + expect( editor.editing.view.domConverter.unsafeElements ) + .deep.equal( Array.from( new Set( editor.editing.view.domConverter.unsafeElements ).values() ) ); + + expect( editor.editing.view.getDomRoot().innerHTML ).equal( + INLINE_FILLER + + '' + + '' + ); + } ); + + it( 'should render in the editing view as a pre block (whitespace handling)', () => { + dataFilter.allowElement( /.*/ ); + editor.setData( + ' a ' + + ' b ' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: + '' + + '', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( + 'a ' + + 'b' + ); + expect( editor.data.htmlProcessor.domConverter.preElements ).include( 'custom-foo-element' ); + expect( editor.data.htmlProcessor.domConverter.preElements ) + .deep.equal( Array.from( new Set( editor.data.htmlProcessor.domConverter.preElements ).values() ) ); + } ); + describe( 'element position', () => { const testCases = [ { name: 'paragraph', @@ -136,175 +212,156 @@ describe( 'CustomElementSupport', () => { } } ); - it( 'should not inject nbsp in the element content', () => { - dataFilter.allowElement( /.*/ ); - editor.setData( ' c ' ); + describe( 'content', () => { + it( 'should preserve custom element content', () => { + dataFilter.allowElement( /.*/ ); + editor.setData( 'foo this is

some content

and more of it bar' ); - expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { - data: '', - attributes: {} + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: + 'foo ' + + '' + + 'bar', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( '

foo this is

some content

and more of it bar

' ); } ); - expect( editor.getData() ).to.equal( 'c' ); - } ); + it( 'should not inject nbsp in the element content', () => { + dataFilter.allowElement( /.*/ ); + editor.setData( ' c ' ); - it( 'should allow attributes', () => { - dataFilter.allowElement( /.*/ ); - dataFilter.allowAttributes( { attributes: { 'data-foo': /.*/ } } ); + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: '', + attributes: {} + } ); - editor.setData( 'bar' ); + expect( editor.getData() ).to.equal( 'c' ); + } ); + } ); - expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { - data: 'bar"' + - ' htmlElementName="custom-foo-element">', - attributes: { - 1: { - attributes: { - 'data-foo': 'foo' + describe( 'attributes', () => { + it( 'should allow attributes', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { attributes: { 'data-foo': /.*/ } } ); + + editor.setData( 'bar' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: { + 1: { + attributes: { + 'data-foo': 'foo' + } } } - } - } ); + } ); - expect( editor.getData() ).to.equal( 'bar' ); - } ); + expect( editor.getData() ).to.equal( 'bar' ); + } ); - it( 'should allow attributes (classes)', () => { - dataFilter.allowElement( /.*/ ); - dataFilter.allowAttributes( { classes: 'foo' } ); + it( 'should allow attributes (classes)', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { classes: 'foo' } ); - editor.setData( 'bar' ); + editor.setData( 'bar' ); - expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { - data: 'bar"' + - ' htmlElementName="custom-foo-element">', - attributes: { - 1: { - classes: [ 'foo' ] + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: { + 1: { + classes: [ 'foo' ] + } } - } - } ); - - expect( editor.getData() ).to.equal( 'bar' ); - } ); - - it( 'should allow attributes (styles)', () => { - dataFilter.allowElement( /.*/ ); - dataFilter.allowAttributes( { styles: { background: true } } ); + } ); - editor.setData( 'bar' ); + expect( editor.getData() ).to.equal( 'bar' ); + } ); - expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { - data: 'bar"' + - ' htmlElementName="custom-foo-element">', - attributes: { - 1: { - 'styles': { - 'background': 'red' + it( 'should allow attributes (styles)', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { styles: { background: true } } ); + + editor.setData( 'bar' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: { + 1: { + 'styles': { + 'background': 'red' + } } } - } + } ); + + expect( editor.getData() ).to.equal( 'bar' ); } ); - expect( editor.getData() ).to.equal( 'bar' ); - } ); + it( 'should disallow attributes', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { attributes: { 'data-foo': /.*/ } } ); + dataFilter.disallowAttributes( { attributes: { 'data-foo': /.*/ } } ); - it( 'should disallow attributes', () => { - dataFilter.allowElement( /.*/ ); - dataFilter.allowAttributes( { attributes: { 'data-foo': /.*/ } } ); - dataFilter.disallowAttributes( { attributes: { 'data-foo': /.*/ } } ); + editor.setData( 'bar' ); - editor.setData( 'bar' ); + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: {} + } ); - expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { - data: 'bar"' + - ' htmlElementName="custom-foo-element">', - attributes: {} + expect( editor.getData() ).to.equal( 'bar' ); } ); - expect( editor.getData() ).to.equal( 'bar' ); - } ); + it( 'should disallow attributes (classes)', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { classes: 'foo' } ); + dataFilter.disallowAttributes( { classes: 'foo' } ); - it( 'should disallow attributes (classes)', () => { - dataFilter.allowElement( /.*/ ); - dataFilter.allowAttributes( { classes: 'foo' } ); - dataFilter.disallowAttributes( { classes: 'foo' } ); + editor.setData( 'bar' ); - editor.setData( 'bar' ); + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: {} + } ); - expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { - data: 'bar"' + - ' htmlElementName="custom-foo-element">', - attributes: {} + expect( editor.getData() ).to.equal( 'bar' ); } ); - expect( editor.getData() ).to.equal( 'bar' ); - } ); + it( 'should disallow attributes (styles)', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { styles: { background: true } } ); + dataFilter.disallowAttributes( { styles: { background: true } } ); - it( 'should disallow attributes (styles)', () => { - dataFilter.allowElement( /.*/ ); - dataFilter.allowAttributes( { styles: { background: true } } ); - dataFilter.disallowAttributes( { styles: { background: true } } ); + editor.setData( 'bar' ); - editor.setData( 'bar' ); + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'bar"' + + ' htmlElementName="custom-foo-element">', + attributes: {} + } ); - expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { - data: 'bar"' + - ' htmlElementName="custom-foo-element">', - attributes: {} + expect( editor.getData() ).to.equal( 'bar' ); } ); - - expect( editor.getData() ).to.equal( 'bar' ); } ); - - // it( 'should not consume attributes already consumed (downcast)', () => { - // dataFilter.allowAttributes( { name: 'script', attributes: true } ); - // - // editor.conversion.for( 'downcast' ).add( dispatcher => { - // dispatcher.on( 'attribute:htmlAttributes:htmlScript', ( evt, data, conversionApi ) => { - // conversionApi.consumable.consume( data.item, evt.name ); - // }, { priority: 'high' } ); - // } ); - // - // editor.setData( `

Foo

` ); - // - // expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - // data: `Foo`, - // attributes: { - // 1: { attributes: { nonce: 'qwerty' } }, - // 2: CODE - // } - // } ); - // - // expect( editor.getData() ).to.equal( `

Foo

` ); - // } ); - // - // it( 'should not consume attributes already consumed (upcast)', () => { - // dataFilter.allowAttributes( { name: 'script', attributes: true } ); - // - // editor.conversion.for( 'upcast' ).add( dispatcher => { - // dispatcher.on( 'element:script', ( evt, data, conversionApi ) => { - // conversionApi.consumable.consume( data.viewItem, { attributes: 'nonce' } ); - // }, { priority: 'high' } ); - // } ); - // - // editor.setData( `

Foo

` ); - // - // expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - // data: `Foo`, - // attributes: { - // 1: { attributes: { type: 'c++' } }, - // 2: CODE_CPP - // } - // } ); - // } ); } ); From 1dccf1c33cfa661af405ab47f28db544990ffa5a Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 25 May 2022 13:06:36 +0200 Subject: [PATCH 09/13] Added integration with HTML comments. --- .../src/integrations/customelement.js | 5 +++++ .../tests/integrations/customelement.js | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/packages/ckeditor5-html-support/src/integrations/customelement.js b/packages/ckeditor5-html-support/src/integrations/customelement.js index 52eb1cc0bbb..b1357a1909b 100644 --- a/packages/ckeditor5-html-support/src/integrations/customelement.js +++ b/packages/ckeditor5-html-support/src/integrations/customelement.js @@ -52,6 +52,11 @@ export default class CustomElementSupport extends Plugin { conversion.for( 'upcast' ).elementToElement( { view: /.*/, model: ( viewElement, conversionApi ) => { + // Do not try to convert $comment fake element. + if ( viewElement.name == '$comment' ) { + return; + } + // Allow for fallback only if this element is not defined in data schema to make sure // that this will handle only custom elements not registered in the data schema. if ( dataSchema.getDefinitionsForView( viewElement.name ).size ) { diff --git a/packages/ckeditor5-html-support/tests/integrations/customelement.js b/packages/ckeditor5-html-support/tests/integrations/customelement.js index 1a01bd0792b..1a1f958a8bb 100644 --- a/packages/ckeditor5-html-support/tests/integrations/customelement.js +++ b/packages/ckeditor5-html-support/tests/integrations/customelement.js @@ -364,4 +364,17 @@ describe( 'CustomElementSupport', () => { expect( editor.getData() ).to.equal( 'bar' ); } ); } ); + + it( 'should not convert html comments as a custom element', () => { + dataFilter.allowElement( /.*/ ); + + editor.setData( 'bar' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: '', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( 'bar' ); + } ); } ); From e1e409a3a80a269c56af55a3d3b38f1d49309f57 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 31 May 2022 15:41:11 +0200 Subject: [PATCH 10/13] Docs. --- .../ckeditor5-html-support/src/integrations/customelement.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/ckeditor5-html-support/src/integrations/customelement.js b/packages/ckeditor5-html-support/src/integrations/customelement.js index b1357a1909b..25b2e426114 100644 --- a/packages/ckeditor5-html-support/src/integrations/customelement.js +++ b/packages/ckeditor5-html-support/src/integrations/customelement.js @@ -100,6 +100,8 @@ export default class CustomElementSupport extends Plugin { converterPriority: 'low' } ); + // Because this element is unsafe (DomConverter#unsafeElements), it will render as a transparent but it must + // be rendered anyway for the mapping between the model and the view to exist. conversion.for( 'editingDowncast' ).elementToElement( { model: { name: definition.model, @@ -130,6 +132,7 @@ export default class CustomElementSupport extends Plugin { domConverter.setContentOf( domElement, htmlContent ); // Unwrap the custom element content (it was stored in the attribute as the whole custom element). + // See the upcast conversion for the "htmlContent" attribute to learn more. const customElement = domElement.firstChild; customElement.remove(); From 26e1859821814f3bcc9428bdd85e0acc501bf448 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 31 May 2022 16:01:10 +0200 Subject: [PATCH 11/13] Docs. --- .../src/integrations/customelement.js | 1 + .../ckeditor5-html-support/src/integrations/mediaembed.js | 6 ++++++ .../tests/integrations/customelement.js | 2 ++ 3 files changed, 9 insertions(+) diff --git a/packages/ckeditor5-html-support/src/integrations/customelement.js b/packages/ckeditor5-html-support/src/integrations/customelement.js index 25b2e426114..056ed755113 100644 --- a/packages/ckeditor5-html-support/src/integrations/customelement.js +++ b/packages/ckeditor5-html-support/src/integrations/customelement.js @@ -49,6 +49,7 @@ export default class CustomElementSupport extends Plugin { isContent: true } ); + // Being executed on the low priority, it will catch all elements that were not caught by other converters. conversion.for( 'upcast' ).elementToElement( { view: /.*/, model: ( viewElement, conversionApi ) => { diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index e5aa0353107..52c34d66e51 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -19,10 +19,16 @@ import { updateViewAttributes } from '../conversionutils.js'; * @extends module:core/plugin~Plugin */ export default class MediaEmbedElementSupport extends Plugin { + /** + * @inheritDoc + */ static get requires() { return [ DataFilter ]; } + /** + * @inheritDoc + */ init() { const editor = this.editor; diff --git a/packages/ckeditor5-html-support/tests/integrations/customelement.js b/packages/ckeditor5-html-support/tests/integrations/customelement.js index 1a1f958a8bb..b6a686d6cd0 100644 --- a/packages/ckeditor5-html-support/tests/integrations/customelement.js +++ b/packages/ckeditor5-html-support/tests/integrations/customelement.js @@ -57,6 +57,7 @@ describe( 'CustomElementSupport', () => { } ); it( 'should not allow unknown custom element if allow-all is not enabled', () => { + // Note that this one does not match any element in the DataSchema. As a result, no upcast conversion will occur. dataFilter.allowElement( /custom-foo-element/ ); editor.setData( 'bar' ); @@ -121,6 +122,7 @@ describe( 'CustomElementSupport', () => { ' b ' ); + // Note: Some white spaces were trimmed in the data processor but this is still better than injecting NBSPs instead of white spaces. expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { data: ' Date: Tue, 31 May 2022 16:11:47 +0200 Subject: [PATCH 12/13] Gotta name 'em all. --- .../ckeditor5-html-support/src/integrations/codeblock.js | 7 +++++++ .../src/integrations/customelement.js | 7 +++++++ .../src/integrations/documentlist.js | 7 +++++++ .../ckeditor5-html-support/src/integrations/dualcontent.js | 7 +++++++ .../ckeditor5-html-support/src/integrations/heading.js | 7 +++++++ packages/ckeditor5-html-support/src/integrations/image.js | 7 +++++++ .../ckeditor5-html-support/src/integrations/mediaembed.js | 7 +++++++ packages/ckeditor5-html-support/src/integrations/script.js | 7 +++++++ packages/ckeditor5-html-support/src/integrations/style.js | 7 +++++++ packages/ckeditor5-html-support/src/integrations/table.js | 7 +++++++ .../ckeditor5-html-support/tests/integrations/codeblock.js | 4 ++++ .../tests/integrations/customelement.js | 4 ++++ .../tests/integrations/documentlist.js | 4 ++++ .../tests/integrations/dualcontent.js | 4 ++++ .../ckeditor5-html-support/tests/integrations/heading.js | 4 ++++ .../ckeditor5-html-support/tests/integrations/image.js | 4 ++++ .../tests/integrations/mediaembed.js | 4 ++++ .../ckeditor5-html-support/tests/integrations/script.js | 4 ++++ .../ckeditor5-html-support/tests/integrations/style.js | 4 ++++ .../ckeditor5-html-support/tests/integrations/table.js | 4 ++++ 20 files changed, 110 insertions(+) diff --git a/packages/ckeditor5-html-support/src/integrations/codeblock.js b/packages/ckeditor5-html-support/src/integrations/codeblock.js index bf727aa738c..2a501505d61 100644 --- a/packages/ckeditor5-html-support/src/integrations/codeblock.js +++ b/packages/ckeditor5-html-support/src/integrations/codeblock.js @@ -25,6 +25,13 @@ export default class CodeBlockElementSupport extends Plugin { return [ DataFilter ]; } + /** + * @inheritDoc + */ + static get pluginName() { + return 'CodeBlockElementSupport'; + } + /** * @inheritDoc */ diff --git a/packages/ckeditor5-html-support/src/integrations/customelement.js b/packages/ckeditor5-html-support/src/integrations/customelement.js index 056ed755113..2c0fbcb548a 100644 --- a/packages/ckeditor5-html-support/src/integrations/customelement.js +++ b/packages/ckeditor5-html-support/src/integrations/customelement.js @@ -27,6 +27,13 @@ export default class CustomElementSupport extends Plugin { return [ DataFilter, DataSchema ]; } + /** + * @inheritDoc + */ + static get pluginName() { + return 'CustomElementSupport'; + } + /** * @inheritDoc */ diff --git a/packages/ckeditor5-html-support/src/integrations/documentlist.js b/packages/ckeditor5-html-support/src/integrations/documentlist.js index 94fc688bffb..7e480e251b2 100644 --- a/packages/ckeditor5-html-support/src/integrations/documentlist.js +++ b/packages/ckeditor5-html-support/src/integrations/documentlist.js @@ -26,6 +26,13 @@ export default class DocumentListElementSupport extends Plugin { return [ DataFilter ]; } + /** + * @inheritDoc + */ + static get pluginName() { + return 'DocumentListElementSupport'; + } + /** * @inheritDoc */ diff --git a/packages/ckeditor5-html-support/src/integrations/dualcontent.js b/packages/ckeditor5-html-support/src/integrations/dualcontent.js index 9493d341b65..e2f68cbe223 100644 --- a/packages/ckeditor5-html-support/src/integrations/dualcontent.js +++ b/packages/ckeditor5-html-support/src/integrations/dualcontent.js @@ -43,6 +43,13 @@ export default class DualContentModelElementSupport extends Plugin { return [ DataFilter ]; } + /** + * @inheritDoc + */ + static get pluginName() { + return 'DualContentModelElementSupport'; + } + /** * @inheritDoc */ diff --git a/packages/ckeditor5-html-support/src/integrations/heading.js b/packages/ckeditor5-html-support/src/integrations/heading.js index 9f03858a76f..ea69095bcf4 100644 --- a/packages/ckeditor5-html-support/src/integrations/heading.js +++ b/packages/ckeditor5-html-support/src/integrations/heading.js @@ -24,6 +24,13 @@ export default class HeadingElementSupport extends Plugin { return [ DataSchema ]; } + /** + * @inheritDoc + */ + static get pluginName() { + return 'HeadingElementSupport'; + } + /** * @inheritDoc */ diff --git a/packages/ckeditor5-html-support/src/integrations/image.js b/packages/ckeditor5-html-support/src/integrations/image.js index b18a1ee9fa7..3e706e4abfa 100644 --- a/packages/ckeditor5-html-support/src/integrations/image.js +++ b/packages/ckeditor5-html-support/src/integrations/image.js @@ -28,6 +28,13 @@ export default class ImageElementSupport extends Plugin { return [ DataFilter ]; } + /** + * @inheritDoc + */ + static get pluginName() { + return 'ImageElementSupport'; + } + /** * @inheritDoc */ diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index 52c34d66e51..66995ba6575 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -26,6 +26,13 @@ export default class MediaEmbedElementSupport extends Plugin { return [ DataFilter ]; } + /** + * @inheritDoc + */ + static get pluginName() { + return 'MediaEmbedElementSupport'; + } + /** * @inheritDoc */ diff --git a/packages/ckeditor5-html-support/src/integrations/script.js b/packages/ckeditor5-html-support/src/integrations/script.js index 85c90e97dd8..b72ba9957e7 100644 --- a/packages/ckeditor5-html-support/src/integrations/script.js +++ b/packages/ckeditor5-html-support/src/integrations/script.js @@ -30,6 +30,13 @@ export default class ScriptElementSupport extends Plugin { return [ DataFilter ]; } + /** + * @inheritDoc + */ + static get pluginName() { + return 'ScriptElementSupport'; + } + /** * @inheritDoc */ diff --git a/packages/ckeditor5-html-support/src/integrations/style.js b/packages/ckeditor5-html-support/src/integrations/style.js index dcd59dcffb5..7e313e97eaa 100644 --- a/packages/ckeditor5-html-support/src/integrations/style.js +++ b/packages/ckeditor5-html-support/src/integrations/style.js @@ -30,6 +30,13 @@ export default class StyleElementSupport extends Plugin { return [ DataFilter ]; } + /** + * @inheritDoc + */ + static get pluginName() { + return 'StyleElementSupport'; + } + /** * @inheritDoc */ diff --git a/packages/ckeditor5-html-support/src/integrations/table.js b/packages/ckeditor5-html-support/src/integrations/table.js index 1fdcf2ca53c..f0911f86403 100644 --- a/packages/ckeditor5-html-support/src/integrations/table.js +++ b/packages/ckeditor5-html-support/src/integrations/table.js @@ -24,6 +24,13 @@ export default class TableElementSupport extends Plugin { return [ DataFilter ]; } + /** + * @inheritDoc + */ + static get pluginName() { + return 'TableElementSupport'; + } + /** * @inheritDoc */ diff --git a/packages/ckeditor5-html-support/tests/integrations/codeblock.js b/packages/ckeditor5-html-support/tests/integrations/codeblock.js index 3d0acde63bd..9174d07e640 100644 --- a/packages/ckeditor5-html-support/tests/integrations/codeblock.js +++ b/packages/ckeditor5-html-support/tests/integrations/codeblock.js @@ -36,6 +36,10 @@ describe( 'CodeBlockElementSupport', () => { return editor.destroy(); } ); + it( 'should be named', () => { + expect( editor.plugins.has( 'CodeBlockElementSupport' ) ).to.be.true; + } ); + it( 'should allow attributes', () => { dataFilter.allowElement( /^(pre|code)$/ ); dataFilter.allowAttributes( { name: /^(pre|code)$/, attributes: { 'data-foo': /[\s\S]+/ } } ); diff --git a/packages/ckeditor5-html-support/tests/integrations/customelement.js b/packages/ckeditor5-html-support/tests/integrations/customelement.js index b6a686d6cd0..62ba4b9f21b 100644 --- a/packages/ckeditor5-html-support/tests/integrations/customelement.js +++ b/packages/ckeditor5-html-support/tests/integrations/customelement.js @@ -42,6 +42,10 @@ describe( 'CustomElementSupport', () => { return editor.destroy(); } ); + it( 'should be named', () => { + expect( editor.plugins.has( 'CustomElementSupport' ) ).to.be.true; + } ); + it( 'should allow unknown custom element', () => { dataFilter.allowElement( /.*/ ); editor.setData( 'bar' ); diff --git a/packages/ckeditor5-html-support/tests/integrations/documentlist.js b/packages/ckeditor5-html-support/tests/integrations/documentlist.js index 5aff7a43ae1..7be59ae410b 100644 --- a/packages/ckeditor5-html-support/tests/integrations/documentlist.js +++ b/packages/ckeditor5-html-support/tests/integrations/documentlist.js @@ -42,6 +42,10 @@ describe( 'DocumentListElementSupport', () => { return editor.destroy(); } ); + it( 'should be named', () => { + expect( editor.plugins.has( 'DocumentListElementSupport' ) ).to.be.true; + } ); + describe( 'downcast', () => { it( 'should downcast list attributes', () => { setModelData( model, makeList( 'bulleted', 0, { attributes: { 'data-foo': 'foo', 'data-bar': 'bar' } }, [ diff --git a/packages/ckeditor5-html-support/tests/integrations/dualcontent.js b/packages/ckeditor5-html-support/tests/integrations/dualcontent.js index 1af64a67855..a69af2d758c 100644 --- a/packages/ckeditor5-html-support/tests/integrations/dualcontent.js +++ b/packages/ckeditor5-html-support/tests/integrations/dualcontent.js @@ -41,6 +41,10 @@ describe( 'DualContentModelElementSupport', () => { return editor.destroy(); } ); + it( 'should be named', () => { + expect( editor.plugins.has( 'DualContentModelElementSupport' ) ).to.be.true; + } ); + it( 'should be only applied to newly enabled elements', () => { model.schema.register( 'htmlDiv', {} ); dataFilter.allowElement( 'div' ); diff --git a/packages/ckeditor5-html-support/tests/integrations/heading.js b/packages/ckeditor5-html-support/tests/integrations/heading.js index 28a882739bb..1da40b25369 100644 --- a/packages/ckeditor5-html-support/tests/integrations/heading.js +++ b/packages/ckeditor5-html-support/tests/integrations/heading.js @@ -52,6 +52,10 @@ describe( 'HeadingElementSupport', () => { } ] ); } ); + it( 'should be named', () => { + expect( editor.plugins.has( 'HeadingElementSupport' ) ).to.be.true; + } ); + it( 'should register heading schemas', () => { expect( Array.from( dataSchema.getDefinitionsForView( 'h1' ) ) ).to.deep.include( { model: 'heading1', diff --git a/packages/ckeditor5-html-support/tests/integrations/image.js b/packages/ckeditor5-html-support/tests/integrations/image.js index 9f3dd26a4b2..6f9746ee81a 100644 --- a/packages/ckeditor5-html-support/tests/integrations/image.js +++ b/packages/ckeditor5-html-support/tests/integrations/image.js @@ -44,6 +44,10 @@ describe( 'ImageElementSupport', () => { return editor.destroy(); } ); + it( 'should be named', () => { + expect( editor.plugins.has( 'ImageElementSupport' ) ).to.be.true; + } ); + describe( 'BlockImage', () => { it( 'should allow attributes', () => { dataFilter.loadAllowedConfig( [ { diff --git a/packages/ckeditor5-html-support/tests/integrations/mediaembed.js b/packages/ckeditor5-html-support/tests/integrations/mediaembed.js index 782ccaab63e..e5562ea75db 100644 --- a/packages/ckeditor5-html-support/tests/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/tests/integrations/mediaembed.js @@ -38,6 +38,10 @@ describe( 'MediaEmbedElementSupport', () => { return editor.destroy(); } ); + it( 'should be named', () => { + expect( editor.plugins.has( 'MediaEmbedElementSupport' ) ).to.be.true; + } ); + it( 'should allow attributes', () => { dataFilter.loadAllowedConfig( [ { name: /^(figure|oembed)$/, diff --git a/packages/ckeditor5-html-support/tests/integrations/script.js b/packages/ckeditor5-html-support/tests/integrations/script.js index 0047c9fb561..88d75a492dc 100644 --- a/packages/ckeditor5-html-support/tests/integrations/script.js +++ b/packages/ckeditor5-html-support/tests/integrations/script.js @@ -39,6 +39,10 @@ describe( 'ScriptElementSupport', () => { return editor.destroy(); } ); + it( 'should be named', () => { + expect( editor.plugins.has( 'ScriptElementSupport' ) ).to.be.true; + } ); + it( 'should allow element', () => { editor.setData( `

Foo

` ); diff --git a/packages/ckeditor5-html-support/tests/integrations/style.js b/packages/ckeditor5-html-support/tests/integrations/style.js index af0e1bb90a7..cda34feff32 100644 --- a/packages/ckeditor5-html-support/tests/integrations/style.js +++ b/packages/ckeditor5-html-support/tests/integrations/style.js @@ -38,6 +38,10 @@ describe( 'StyleElementSupport', () => { return editor.destroy(); } ); + it( 'should be named', () => { + expect( editor.plugins.has( 'StyleElementSupport' ) ).to.be.true; + } ); + it( 'should allow element', () => { editor.setData( `

Foo

` ); diff --git a/packages/ckeditor5-html-support/tests/integrations/table.js b/packages/ckeditor5-html-support/tests/integrations/table.js index a5e2b755fd0..a99fa283fe2 100644 --- a/packages/ckeditor5-html-support/tests/integrations/table.js +++ b/packages/ckeditor5-html-support/tests/integrations/table.js @@ -41,6 +41,10 @@ describe( 'TableElementSupport', () => { return editor.destroy(); } ); + it( 'should be named', () => { + expect( editor.plugins.has( 'TableElementSupport' ) ).to.be.true; + } ); + it( 'should allow attributes', () => { dataFilter.loadAllowedConfig( [ { name: /^(figure|table|tbody|thead|tr|th|td)$/, From fa2aa8113f648bba87fbe96f3a0b9520bffcf75a Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 31 May 2022 16:13:00 +0200 Subject: [PATCH 13/13] Applied review suggestions. --- .../ckeditor5-html-support/src/integrations/customelement.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-html-support/src/integrations/customelement.js b/packages/ckeditor5-html-support/src/integrations/customelement.js index 2c0fbcb548a..a3d22a67aca 100644 --- a/packages/ckeditor5-html-support/src/integrations/customelement.js +++ b/packages/ckeditor5-html-support/src/integrations/customelement.js @@ -15,7 +15,7 @@ import DataFilter from '../datafilter'; import { setViewAttributes } from '../conversionutils'; /** - * Provides the General HTML Support for custom elements (not registered in the DataSchema). + * Provides the General HTML Support for custom elements (not registered in the {@link module:html-support/dataschema~DataSchema}). * * @extends module:core/plugin~Plugin */