From 3437e1cf4effe4f6f092b82c6a632ef1ab29b4e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Mon, 19 Jul 2021 11:28:30 +0200 Subject: [PATCH 01/17] MediaEmbed (oembed) integration with tests. --- .../src/generalhtmlsupport.js | 4 +- .../src/integrations/mediaembed.js | 67 +++++++++++++++++++ .../src/schemadefinitions.js | 8 +++ .../tests/manual/oembed.html | 30 +++++++++ .../tests/manual/oembed.js | 53 +++++++++++++++ .../tests/manual/oembed.md | 17 +++++ 6 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 packages/ckeditor5-html-support/src/integrations/mediaembed.js create mode 100644 packages/ckeditor5-html-support/tests/manual/oembed.html create mode 100644 packages/ckeditor5-html-support/tests/manual/oembed.js create mode 100644 packages/ckeditor5-html-support/tests/manual/oembed.md diff --git a/packages/ckeditor5-html-support/src/generalhtmlsupport.js b/packages/ckeditor5-html-support/src/generalhtmlsupport.js index 1a39a377584..8a5fb91d4bb 100644 --- a/packages/ckeditor5-html-support/src/generalhtmlsupport.js +++ b/packages/ckeditor5-html-support/src/generalhtmlsupport.js @@ -10,6 +10,7 @@ import { Plugin } from 'ckeditor5/src/core'; import DataFilter from './datafilter'; +import MediaEmbedElementSupport from './integrations/mediaembed'; import TableElementSupport from './integrations/table'; import CodeBlockElementSupport from './integrations/codeblock'; import DualContentModelElementSupport from './integrations/dualcontent'; @@ -38,7 +39,8 @@ export default class GeneralHtmlSupport extends Plugin { DataFilter, TableElementSupport, CodeBlockElementSupport, - DualContentModelElementSupport + DualContentModelElementSupport, + MediaEmbedElementSupport ]; } diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js new file mode 100644 index 00000000000..de4fa39f280 --- /dev/null +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -0,0 +1,67 @@ +/** + * @license Copyright (c) 2003-2021, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module html-support/integrations/mediaembed + */ + +import { Plugin } from 'ckeditor5/src/core'; + +import DataFilter from '../datafilter'; + +/** + * Provides the General HTML Support integration with {@link module:media-embed/mediaembed~MediaEmbed Media Embed} feature. + * + * @extends module:core/plugin~Plugin + */ +export default class MediaEmbedElementSupport extends Plugin { + static get requires() { + return [ DataFilter ]; + } + + init() { + const editor = this.editor; + + if ( !editor.plugins.has( 'MediaEmbed' ) ) { + return; + } + + const schema = editor.model.schema; + const conversion = editor.conversion; + const dataFilter = this.editor.plugins.get( DataFilter ); + + dataFilter.on( 'register:oembed', ( evt, definition ) => { + if ( definition.model !== 'htmlOembed' ) { + return; + } + + schema.extend( 'media', { + allowAttributes: [ + 'htmlAttributes', + // Map of attributes on the containing `
`. + 'htmlFigureAttributes' + ] + } ); + + conversion.for( 'upcast' ).add( viewToModelOembedAttributesConverter() ); + + evt.stop(); + } ); + + dataFilter.on( 'register:figure', () => { + // conversion.for( 'upcast' ).add( consumeTableFigureConverter() ); + } ); + } +} + +function viewToModelOembedAttributesConverter() { + return dispatcher => { + dispatcher.on( 'element:oembed', ( evt, data, conversionApi ) => { + debugger; + }, { priority: 'high' } ); + }; +} + +// function consumeTableFigureConverter diff --git a/packages/ckeditor5-html-support/src/schemadefinitions.js b/packages/ckeditor5-html-support/src/schemadefinitions.js index 691a96bd4db..25282ee1326 100644 --- a/packages/ckeditor5-html-support/src/schemadefinitions.js +++ b/packages/ckeditor5-html-support/src/schemadefinitions.js @@ -788,6 +788,14 @@ export default { inheritAllFrom: '$htmlObjectInline' } }, + { + model: 'htmlOembed', + view: 'oembed', + isObject: true, + modelSchema: { + inheritAllFrom: '$htmlObjectInline' + } + }, { model: 'htmlAudio', view: 'audio', diff --git a/packages/ckeditor5-html-support/tests/manual/oembed.html b/packages/ckeditor5-html-support/tests/manual/oembed.html new file mode 100644 index 00000000000..b7719393b5d --- /dev/null +++ b/packages/ckeditor5-html-support/tests/manual/oembed.html @@ -0,0 +1,30 @@ +
+

Media with previews

+
+ +
+
+ +
+
+ +
+

Generic media

+
+ +
+
+ +
+
+ +
+
+ + +
+
+ +
+
diff --git a/packages/ckeditor5-html-support/tests/manual/oembed.js b/packages/ckeditor5-html-support/tests/manual/oembed.js new file mode 100644 index 00000000000..fe5bac73760 --- /dev/null +++ b/packages/ckeditor5-html-support/tests/manual/oembed.js @@ -0,0 +1,53 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* globals console:false, window, document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; +import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset'; +import Strikethrough from '@ckeditor/ckeditor5-basic-styles/src/strikethrough'; +import MediaEmbed from '@ckeditor/ckeditor5-media-embed/src/mediaembed'; +import MediaEmbedToolbar from '@ckeditor/ckeditor5-media-embed/src/mediaembedtoolbar'; +import SourceEditing from '@ckeditor/ckeditor5-source-editing/src/sourceediting'; + +import GeneralHtmlSupport from '../../src/generalhtmlsupport'; + +ClassicEditor + .create( document.querySelector( '#editor' ), { + plugins: [ + ArticlePluginSet, + GeneralHtmlSupport, + SourceEditing, + Strikethrough, + MediaEmbed, + MediaEmbedToolbar + ], + image: { toolbar: [ 'toggleImageCaption', 'imageTextAlternative' ] }, + toolbar: [ 'mediaEmbed', '|', 'bold', 'italic', 'strikethrough', '|', 'sourceEditing' ], + mediaEmbed: { + previewsInData: true, + toolbar: [ 'blockQuote' ] + }, + htmlSupport: { + allow: [ + { + name: /^(figure|table|tbody|thead|tr|th|td|caption|figcaption|oembed)$/, + attributes: [ 'data-validation-allow', 'data-validation-disallow' ] + } + ], + disallow: [ + { + name: /^(figure|table|tbody|thead|tr|th|td|caption|figcaption|oembed)$/, + attributes: 'data-validation-disallow' + } + ] + } + } ) + .then( editor => { + window.editor = editor; + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/packages/ckeditor5-html-support/tests/manual/oembed.md b/packages/ckeditor5-html-support/tests/manual/oembed.md new file mode 100644 index 00000000000..1adced20e65 --- /dev/null +++ b/packages/ckeditor5-html-support/tests/manual/oembed.md @@ -0,0 +1,17 @@ +## Table + +Verify if every table structure element: + +* `figure` +* `table` +* `tbody` +* `thead` +* `caption` +* `figurecaption` +* `tr` +* `th` +* `td` + +**Case 1:** Has its own `data-validation-allow` attribute and blue border. + +**Case 2:** Doesn't have `data-validation-disallow` attribute or red border. From ee7c40ce4423e7b4d00fcbbacbfc356fa395cda6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Mon, 19 Jul 2021 12:54:46 +0200 Subject: [PATCH 02/17] Upcast and downcast allowed attributes, strip off disallowed ones. --- packages/ckeditor5-html-support/package.json | 1 + .../src/integrations/mediaembed.js | 72 ++++++++++++++++--- .../tests/manual/oembed.html | 5 +- .../tests/manual/oembed.js | 2 +- 4 files changed, 67 insertions(+), 13 deletions(-) diff --git a/packages/ckeditor5-html-support/package.json b/packages/ckeditor5-html-support/package.json index b1a9e0c7a09..32300e7ff1a 100644 --- a/packages/ckeditor5-html-support/package.json +++ b/packages/ckeditor5-html-support/package.json @@ -41,6 +41,7 @@ "@ckeditor/ckeditor5-indent": "^29.0.0", "@ckeditor/ckeditor5-link": "^29.0.0", "@ckeditor/ckeditor5-list": "^29.0.0", + "@ckeditor/ckeditor5-media-embed": "^29.0.0", "@ckeditor/ckeditor5-page-break": "^29.0.0", "@ckeditor/ckeditor5-paragraph": "^29.0.0", "@ckeditor/ckeditor5-source-editing": "^29.0.0", diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index de4fa39f280..a0f4b4cdfa1 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -8,6 +8,8 @@ */ import { Plugin } from 'ckeditor5/src/core'; +import { disallowedAttributesConverter } from '../converters'; +import { setViewAttributes } from '../conversionutils.js'; import DataFilter from '../datafilter'; @@ -40,28 +42,78 @@ export default class MediaEmbedElementSupport extends Plugin { schema.extend( 'media', { allowAttributes: [ 'htmlAttributes', - // Map of attributes on the containing `
`. 'htmlFigureAttributes' ] } ); - conversion.for( 'upcast' ).add( viewToModelOembedAttributesConverter() ); + conversion.for( 'upcast' ).add( disallowedAttributesConverter( definition, dataFilter ) ); + conversion.for( 'upcast' ).add( viewToModelOembedAttributesConverter( dataFilter ) ); + conversion.for( 'dataDowncast' ).add( modelToViewOembedAttributeConverter() ); evt.stop(); } ); - - dataFilter.on( 'register:figure', () => { - // conversion.for( 'upcast' ).add( consumeTableFigureConverter() ); - } ); } } -function viewToModelOembedAttributesConverter() { +function viewToModelOembedAttributesConverter( dataFilter ) { return dispatcher => { dispatcher.on( 'element:oembed', ( evt, data, conversionApi ) => { - debugger; - }, { priority: 'high' } ); + const viewOembedElement = data.viewItem; + + preserveElementAttributes( viewOembedElement, 'htmlAttributes' ); + + const viewFigureElement = viewOembedElement.parent; + if ( viewFigureElement.is( 'element', 'figure' ) ) { + preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); + } + + function preserveElementAttributes( viewElement, attributeName ) { + const viewAttributes = dataFilter._consumeAllowedAttributes( viewElement, conversionApi ); + + if ( viewAttributes ) { + conversionApi.writer.setAttribute( attributeName, viewAttributes, data.modelRange ); + } + } + }, + // Low priority to let other converters prepare the modelRange for us. + { priority: 'low' } ); }; } -// function consumeTableFigureConverter +function modelToViewOembedAttributeConverter() { + return dispatcher => { + addAttributeConversionDispatcherHandler( 'oembed', 'htmlAttributes' ); + addAttributeConversionDispatcherHandler( 'figure', 'htmlFigureAttributes' ); + + function addAttributeConversionDispatcherHandler( elementName, attributeName ) { + dispatcher.on( `attribute:${ attributeName }:media`, ( evt, data, conversionApi ) => { + if ( !conversionApi.consumable.consume( data.item, evt.name ) ) { + return; + } + + const containerElement = conversionApi.mapper.toViewElement( data.item ); + const viewElement = getDescendantElement( conversionApi, containerElement, elementName ); + + setViewAttributes( conversionApi.writer, data.attributeNewValue, viewElement ); + } ); + } + }; +} + +// Returns the first view element descendant matching the given view name. +// Includes view element itself. +// +// @private +// @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi +// @param {module:engine/view/element~Element} containerElement +// @param {String} elementName +// @returns {module:engine/view/element~Element|null} +function getDescendantElement( conversionApi, containerElement, elementName ) { + const range = conversionApi.writer.createRangeOn( containerElement ); + + for ( const { item } of range.getWalker() ) { + if ( item.is( 'element', elementName ) ) { + return item; + } + } +} diff --git a/packages/ckeditor5-html-support/tests/manual/oembed.html b/packages/ckeditor5-html-support/tests/manual/oembed.html index b7719393b5d..62f63d95d8f 100644 --- a/packages/ckeditor5-html-support/tests/manual/oembed.html +++ b/packages/ckeditor5-html-support/tests/manual/oembed.html @@ -1,7 +1,8 @@

Media with previews

-
- +
+
diff --git a/packages/ckeditor5-html-support/tests/manual/oembed.js b/packages/ckeditor5-html-support/tests/manual/oembed.js index fe5bac73760..d9f75c16559 100644 --- a/packages/ckeditor5-html-support/tests/manual/oembed.js +++ b/packages/ckeditor5-html-support/tests/manual/oembed.js @@ -27,7 +27,7 @@ ClassicEditor image: { toolbar: [ 'toggleImageCaption', 'imageTextAlternative' ] }, toolbar: [ 'mediaEmbed', '|', 'bold', 'italic', 'strikethrough', '|', 'sourceEditing' ], mediaEmbed: { - previewsInData: true, + // previewsInData: true, toolbar: [ 'blockQuote' ] }, htmlSupport: { From 9f6f1f1acdaabea80e9e8dbb25514efe0be04980 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Thu, 22 Jul 2021 14:15:41 +0200 Subject: [PATCH 03/17] Use MediaEmbed config to create converters. --- .../src/integrations/mediaembed.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index a0f4b4cdfa1..c432808923b 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -26,7 +26,9 @@ export default class MediaEmbedElementSupport extends Plugin { init() { const editor = this.editor; - if ( !editor.plugins.has( 'MediaEmbed' ) ) { + // Stop here if MediaEmbed plugin is not provided or the integrator wants to output markup with previews as + // we do not support filtering previews. + if ( !editor.plugins.has( 'MediaEmbed' ) || editor.config.get( 'mediaEmbed.previewsInData' ) ) { return; } @@ -34,7 +36,9 @@ export default class MediaEmbedElementSupport extends Plugin { const conversion = editor.conversion; const dataFilter = this.editor.plugins.get( DataFilter ); - dataFilter.on( 'register:oembed', ( evt, definition ) => { + const mediaElementName = editor.config.get( 'mediaEmbed.elementName' ); + + dataFilter.on( `register:${ mediaElementName }`, ( evt, definition ) => { if ( definition.model !== 'htmlOembed' ) { return; } @@ -47,7 +51,7 @@ export default class MediaEmbedElementSupport extends Plugin { } ); conversion.for( 'upcast' ).add( disallowedAttributesConverter( definition, dataFilter ) ); - conversion.for( 'upcast' ).add( viewToModelOembedAttributesConverter( dataFilter ) ); + conversion.for( 'upcast' ).add( viewToModelOembedAttributesConverter( dataFilter, mediaElementName ) ); conversion.for( 'dataDowncast' ).add( modelToViewOembedAttributeConverter() ); evt.stop(); @@ -55,9 +59,9 @@ export default class MediaEmbedElementSupport extends Plugin { } } -function viewToModelOembedAttributesConverter( dataFilter ) { +function viewToModelOembedAttributesConverter( dataFilter, mediaElementName ) { return dispatcher => { - dispatcher.on( 'element:oembed', ( evt, data, conversionApi ) => { + dispatcher.on( `element:${ mediaElementName }`, ( evt, data, conversionApi ) => { const viewOembedElement = data.viewItem; preserveElementAttributes( viewOembedElement, 'htmlAttributes' ); @@ -80,9 +84,9 @@ function viewToModelOembedAttributesConverter( dataFilter ) { }; } -function modelToViewOembedAttributeConverter() { +function modelToViewOembedAttributeConverter( mediaElementName ) { return dispatcher => { - addAttributeConversionDispatcherHandler( 'oembed', 'htmlAttributes' ); + addAttributeConversionDispatcherHandler( mediaElementName, 'htmlAttributes' ); addAttributeConversionDispatcherHandler( 'figure', 'htmlFigureAttributes' ); function addAttributeConversionDispatcherHandler( elementName, attributeName ) { From 636504d809284d4e8861b825237cf86cf4407bf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Mon, 26 Jul 2021 13:27:12 +0200 Subject: [PATCH 04/17] Renaming of the tests. --- .../src/integrations/mediaembed.js | 62 +- .../manual/{oembed.html => mediaembed.html} | 11 + .../tests/manual/{oembed.js => mediaembed.js} | 14 +- .../tests/manual/mediaembed.md | 3 + .../tests/manual/oembed.md | 17 - .../tests/mediaembed.js | 1241 +++++++++++++++++ 6 files changed, 1315 insertions(+), 33 deletions(-) rename packages/ckeditor5-html-support/tests/manual/{oembed.html => mediaembed.html} (95%) rename packages/ckeditor5-html-support/tests/manual/{oembed.js => mediaembed.js} (80%) create mode 100644 packages/ckeditor5-html-support/tests/manual/mediaembed.md delete mode 100644 packages/ckeditor5-html-support/tests/manual/oembed.md create mode 100644 packages/ckeditor5-html-support/tests/mediaembed.js diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index c432808923b..a6aa33fd299 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -12,6 +12,8 @@ import { disallowedAttributesConverter } from '../converters'; import { setViewAttributes } from '../conversionutils.js'; import DataFilter from '../datafilter'; +import DataSchema from '../dataschema'; +import { getDataFromElement } from 'ckeditor5/src/utils'; /** * Provides the General HTML Support integration with {@link module:media-embed/mediaembed~MediaEmbed Media Embed} feature. @@ -35,9 +37,20 @@ export default class MediaEmbedElementSupport extends Plugin { const schema = editor.model.schema; const conversion = editor.conversion; const dataFilter = this.editor.plugins.get( DataFilter ); + // const dataSchema = this.editor.plugins.get( DataSchema ); const mediaElementName = editor.config.get( 'mediaEmbed.elementName' ); + // Add dynamically schema definition for a given elementName. + // dataSchema.registerBlockElement( { + // model: 'htmlOembed2', + // view: mediaElementName, + // isObject: true, + // modelSchema: { + // inheritAllFrom: '$htmlObjectInline' + // } + // } ); + dataFilter.on( `register:${ mediaElementName }`, ( evt, definition ) => { if ( definition.model !== 'htmlOembed' ) { return; @@ -52,7 +65,7 @@ export default class MediaEmbedElementSupport extends Plugin { conversion.for( 'upcast' ).add( disallowedAttributesConverter( definition, dataFilter ) ); conversion.for( 'upcast' ).add( viewToModelOembedAttributesConverter( dataFilter, mediaElementName ) ); - conversion.for( 'dataDowncast' ).add( modelToViewOembedAttributeConverter() ); + conversion.for( 'dataDowncast' ).add( modelToViewOembedAttributeConverter( mediaElementName ) ); evt.stop(); } ); @@ -61,14 +74,24 @@ export default class MediaEmbedElementSupport extends Plugin { function viewToModelOembedAttributesConverter( dataFilter, mediaElementName ) { return dispatcher => { - dispatcher.on( `element:${ mediaElementName }`, ( evt, data, conversionApi ) => { - const viewOembedElement = data.viewItem; + dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => { + if ( data.viewItem.getChild( 0 ).name === 'oembed' ) { + // Since we are converting to attribute we need a range on which we will set the attribute. + // If the range is not created yet, let's create it by converting children of the current node first. + if ( !data.modelRange ) { + // Convert children and set conversion result as a current data. + Object.assign( data, conversionApi.convertChildren( data.viewItem, data.modelCursor ) ); + } - preserveElementAttributes( viewOembedElement, 'htmlAttributes' ); + const viewOembedElement = data.viewItem.getChild( 0 ); + preserveElementAttributes( viewOembedElement, 'htmlAttributes' ); - const viewFigureElement = viewOembedElement.parent; - if ( viewFigureElement.is( 'element', 'figure' ) ) { - preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); + const viewFigureElement = viewOembedElement.parent; + if ( viewFigureElement.is( 'element', 'figure' ) ) { + preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); + } + + conversionApi.consumable.consume( data.viewItem, { name: true } ); } function preserveElementAttributes( viewElement, attributeName ) { @@ -78,9 +101,28 @@ function viewToModelOembedAttributesConverter( dataFilter, mediaElementName ) { conversionApi.writer.setAttribute( attributeName, viewAttributes, data.modelRange ); } } - }, - // Low priority to let other converters prepare the modelRange for us. - { priority: 'low' } ); + } ); + + // dispatcher.on( `element:${ mediaElementName }`, ( evt, data, conversionApi ) => { + // const viewOembedElement = data.viewItem; + + // preserveElementAttributes( viewOembedElement, 'htmlAttributes' ); + + // const viewFigureElement = viewOembedElement.parent; + // if ( viewFigureElement.is( 'element', 'figure' ) ) { + // preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); + // } + + // function preserveElementAttributes( viewElement, attributeName ) { + // const viewAttributes = dataFilter._consumeAllowedAttributes( viewElement, conversionApi ); + + // if ( viewAttributes ) { + // conversionApi.writer.setAttribute( attributeName, viewAttributes, data.modelRange ); + // } + // } + // }, + // // Low priority to let other converters prepare the modelRange for us. + // { priority: 'low' } ); }; } diff --git a/packages/ckeditor5-html-support/tests/manual/oembed.html b/packages/ckeditor5-html-support/tests/manual/mediaembed.html similarity index 95% rename from packages/ckeditor5-html-support/tests/manual/oembed.html rename to packages/ckeditor5-html-support/tests/manual/mediaembed.html index 62f63d95d8f..6756e15688c 100644 --- a/packages/ckeditor5-html-support/tests/manual/oembed.html +++ b/packages/ckeditor5-html-support/tests/manual/mediaembed.html @@ -1,30 +1,41 @@

Media with previews

+
+
+ + +
+

Generic media

+
+
+
+
+
diff --git a/packages/ckeditor5-html-support/tests/manual/oembed.js b/packages/ckeditor5-html-support/tests/manual/mediaembed.js similarity index 80% rename from packages/ckeditor5-html-support/tests/manual/oembed.js rename to packages/ckeditor5-html-support/tests/manual/mediaembed.js index d9f75c16559..59ff9125593 100644 --- a/packages/ckeditor5-html-support/tests/manual/oembed.js +++ b/packages/ckeditor5-html-support/tests/manual/mediaembed.js @@ -6,29 +6,31 @@ /* globals console:false, window, document */ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; -import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset'; -import Strikethrough from '@ckeditor/ckeditor5-basic-styles/src/strikethrough'; import MediaEmbed from '@ckeditor/ckeditor5-media-embed/src/mediaembed'; import MediaEmbedToolbar from '@ckeditor/ckeditor5-media-embed/src/mediaembedtoolbar'; import SourceEditing from '@ckeditor/ckeditor5-source-editing/src/sourceediting'; +import Essentials from '@ckeditor/ckeditor5-essentials/src/essentials'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import GeneralHtmlSupport from '../../src/generalhtmlsupport'; ClassicEditor .create( document.querySelector( '#editor' ), { plugins: [ - ArticlePluginSet, GeneralHtmlSupport, + Essentials, + Paragraph, SourceEditing, - Strikethrough, MediaEmbed, MediaEmbedToolbar ], image: { toolbar: [ 'toggleImageCaption', 'imageTextAlternative' ] }, - toolbar: [ 'mediaEmbed', '|', 'bold', 'italic', 'strikethrough', '|', 'sourceEditing' ], + toolbar: [ 'mediaEmbed', '|', 'sourceEditing' ], mediaEmbed: { // previewsInData: true, - toolbar: [ 'blockQuote' ] + + // elementName: 'xemebd', + toolbar: [ 'mediaEmbed' ] }, htmlSupport: { allow: [ diff --git a/packages/ckeditor5-html-support/tests/manual/mediaembed.md b/packages/ckeditor5-html-support/tests/manual/mediaembed.md new file mode 100644 index 00000000000..21fea1d54d5 --- /dev/null +++ b/packages/ckeditor5-html-support/tests/manual/mediaembed.md @@ -0,0 +1,3 @@ +## Media Embed + +### TODO ### diff --git a/packages/ckeditor5-html-support/tests/manual/oembed.md b/packages/ckeditor5-html-support/tests/manual/oembed.md deleted file mode 100644 index 1adced20e65..00000000000 --- a/packages/ckeditor5-html-support/tests/manual/oembed.md +++ /dev/null @@ -1,17 +0,0 @@ -## Table - -Verify if every table structure element: - -* `figure` -* `table` -* `tbody` -* `thead` -* `caption` -* `figurecaption` -* `tr` -* `th` -* `td` - -**Case 1:** Has its own `data-validation-allow` attribute and blue border. - -**Case 2:** Doesn't have `data-validation-disallow` attribute or red border. diff --git a/packages/ckeditor5-html-support/tests/mediaembed.js b/packages/ckeditor5-html-support/tests/mediaembed.js new file mode 100644 index 00000000000..dc36d12496d --- /dev/null +++ b/packages/ckeditor5-html-support/tests/mediaembed.js @@ -0,0 +1,1241 @@ +/** + * @license Copyright (c) 2003-2021, CKSource - Frederico Knabben. 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 Table from '@ckeditor/ckeditor5-table/src/table'; +import TableCaption from '@ckeditor/ckeditor5-table/src/tablecaption'; +import GeneralHtmlSupport from '../src/generalhtmlsupport'; +import { getModelDataWithAttributes } from './_utils/utils'; +import { range } from 'lodash-es'; + +/* global document */ + +describe( 'MediaEmbedElementSupport', () => { + let editor, model, editorElement, dataFilter; + + beforeEach( () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor + .create( editorElement, { + plugins: [ Table, TableCaption, Paragraph, GeneralHtmlSupport ] + } ) + .then( newEditor => { + editor = newEditor; + model = editor.model; + + dataFilter = editor.plugins.get( 'DataFilter' ); + } ); + } ); + + afterEach( () => { + editorElement.remove(); + + return editor.destroy(); + } ); + + it( 'should allow attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + attributes: /^data-.*$/ + } ] ); + + const expectedHtml = + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
2.12.22.3
' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1' + + '' + + '' + + '2' + + '' + + '' + + '3' + + '' + + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '' + + '' + + '2.1' + + '' + + '' + + '2.2' + + '' + + '' + + '2.3' + + '' + + '' + + '
', + attributes: { + 1: { + attributes: { + 'data-table': 'table' + } + }, + 2: { + attributes: { + 'data-figure': 'figure' + } + }, + 3: { + attributes: { + 'data-tbody': 'tbody' + } + }, + 4: { + attributes: { + 'data-thead': 'thead' + } + }, + 5: { + attributes: { + 'data-tr': 'tr' + } + }, + 6: { + attributes: { + 'data-th': 'th' + } + }, + 7: { + attributes: { + 'data-th': 'th' + } + }, + 8: { + attributes: { + 'data-th': 'th' + } + }, + 9: { + attributes: { + 'data-tr': 'tr' + } + }, + 10: { + attributes: { + 'data-td': 'td' + } + }, + 11: { + attributes: { + 'data-td': 'td' + } + }, + 12: { + attributes: { + 'data-td': 'td' + } + }, + 13: { + attributes: { + 'data-tr': 'tr' + } + }, + 14: { + attributes: { + 'data-td': 'td' + } + }, + 15: { + attributes: { + 'data-td': 'td' + } + }, + 16: { + attributes: { + 'data-td': 'td' + } + } + } + } ); + + expect( editor.getData() ).to.equal( expectedHtml ); + } ); + + it( 'should allow classes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + classes: 'foobar' + } ] ); + + const expectedHtml = + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
2.12.22.3
' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1' + + '' + + '' + + '2' + + '' + + '' + + '3' + + '' + + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '' + + '' + + '2.1' + + '' + + '' + + '2.2' + + '' + + '' + + '2.3' + + '' + + '' + + '
', + attributes: range( 1, 17 ).reduce( ( attributes, index ) => { + attributes[ index ] = { + classes: [ 'foobar' ] + }; + return attributes; + }, {} ) + } ); + + expect( editor.getData() ).to.equal( expectedHtml ); + } ); + + it( 'should allow styles', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + styles: 'color' + } ] ); + + const expectedHtml = + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
2.12.22.3
' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1' + + '' + + '' + + '2' + + '' + + '' + + '3' + + '' + + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '' + + '' + + '2.1' + + '' + + '' + + '2.2' + + '' + + '' + + '2.3' + + '' + + '' + + '
', + attributes: range( 1, 17 ).reduce( ( attributes, index ) => { + attributes[ index ] = { + styles: { + color: 'red' + } + }; + return attributes; + }, {} ) + } ); + + expect( editor.getData() ).to.equal( expectedHtml ); + } ); + + it( 'should disallow attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + attributes: /^data-.*$/ + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + attributes: /^data-.*$/ + } ] ); + + editor.setData( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
2.12.22.3
' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1' + + '' + + '' + + '2' + + '' + + '' + + '3' + + '' + + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '' + + '' + + '2.1' + + '' + + '' + + '2.2' + + '' + + '' + + '2.3' + + '' + + '' + + '
', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
2.12.22.3
' + + '
' + ); + } ); + + it( 'should disallow classes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + classes: 'foobar' + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + classes: 'foobar' + } ] ); + + editor.setData( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
2.12.22.3
' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1' + + '' + + '' + + '2' + + '' + + '' + + '3' + + '' + + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '' + + '' + + '2.1' + + '' + + '' + + '2.2' + + '' + + '' + + '2.3' + + '' + + '' + + '
', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
2.12.22.3
' + + '
' + ); + } ); + + it( 'should disallow styles', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + styles: 'color' + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + styles: 'color' + } ] ); + + editor.setData( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
2.12.22.3
' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1' + + '' + + '' + + '2' + + '' + + '' + + '3' + + '' + + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '' + + '' + + '2.1' + + '' + + '' + + '2.2' + + '' + + '' + + '2.3' + + '' + + '' + + '
', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
2.12.22.3
' + + '
' + ); + } ); + + it( 'should not set attributes on non existing figure', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|table|tbody|tr|td)$/, + attributes: true + } ] ); + + editor.setData( + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
1.11.21.3
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '
', + attributes: { + 1: { + attributes: { 'data-foo': 'foo' } + } + } + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
1.11.21.3
' + + '
' + ); + } ); + + it( 'should not break figure integration for other features', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|figcaption|table|tbody|tr|td)$/, + attributes: /^data-.*$/ + } ] ); + + const expectedHtml = + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
1.11.21.3
' + + '
' + + '
' + + '
foobar
' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '
' + + '' + + 'foobar' + + '', + attributes: { + 1: { + attributes: { + 'data-figure': 'table' + } + }, + 2: { + attributes: { + 'data-figure': 'standalone' + } + }, + 3: { + attributes: { + 'data-figcaption': 'figcaption' + } + } + } + } ); + + expect( editor.getData() ).to.equal( expectedHtml ); + } ); + + it( 'should not consume attributes already consumed (downcast)', () => { + [ + 'htmlAttributes', + 'htmlFigureAttributes', + 'htmlTbodyAttributes', + 'htmlTheadAttributes' + ].forEach( attributeName => { + editor.conversion.for( 'downcast' ).add( dispatcher => { + dispatcher.on( `attribute:${ attributeName }:table`, ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, evt.name ); + }, { priority: 'high' } ); + } ); + } ); + + dataFilter.allowElement( /^(figure|table|tbody|thead)$/ ); + dataFilter.allowAttributes( { + name: /^(figure|table|tbody|thead)$/, + attributes: { 'data-foo': true } + } ); + + editor.setData( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
' + + '
' + ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
' + + '
' + ); + } ); + + describe( 'TableCaption', () => { + // Sanity tests verifying if table caption is correctly handled by default converters. + + it( 'should allow attributes (caption)', () => { + dataFilter.loadAllowedConfig( [ { + // caption is changed to figcaption by TableCaption feature. + name: /^(caption|figcaption)$/, + attributes: 'data-foo', + styles: 'color', + classes: 'foobar' + } ] ); + + editor.setData( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
caption
1.11.21.3
' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '' + + '
caption
', + attributes: { + 1: { + attributes: { 'data-foo': 'foo' }, + styles: { color: 'red' }, + classes: [ 'foobar' ] + } + } + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
1.11.21.3
' + + '
caption
' + + '
' + ); + } ); + + it( 'should disallow attributes (caption)', () => { + dataFilter.loadAllowedConfig( [ { + // caption is changed to figcaption by TableCaption feature. + name: /^(caption|figcaption)$/, + attributes: 'data-foo', + styles: 'color', + classes: 'foobar' + } ] ); + + dataFilter.loadDisallowedConfig( [ { + // caption is changed to figcaption by TableCaption feature. + name: /^(caption|figcaption)$/, + attributes: 'data-foo', + styles: 'color', + classes: 'foobar' + } ] ); + + editor.setData( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
caption
1.11.21.3
' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '' + + '
caption
', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
1.11.21.3
' + + '
caption
' + + '
' + ); + } ); + + it( 'should allow attributes (figcaption)', () => { + dataFilter.loadAllowedConfig( [ { + name: 'figcaption', + attributes: 'data-foo', + styles: 'color', + classes: 'foobar' + } ] ); + + editor.setData( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
1.11.21.3
' + + '
caption
' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '' + + '
caption
', + attributes: { + 1: { + attributes: { 'data-foo': 'foo' }, + styles: { color: 'red' }, + classes: [ 'foobar' ] + } + } + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
1.11.21.3
' + + '
caption
' + + '
' + ); + } ); + + it( 'should disallow attributes (figcaption)', () => { + dataFilter.loadAllowedConfig( [ { + name: 'figcaption', + attributes: 'data-foo', + styles: 'color', + classes: 'foobar' + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: 'figcaption', + attributes: 'data-foo', + styles: 'color', + classes: 'foobar' + } ] ); + + editor.setData( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
1.11.21.3
' + + '
caption
' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '' + + '
caption
', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
1.11.21.3
' + + '
caption
' + + '
' + ); + } ); + + it( 'should handle mixed allowed and disallowed attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + attributes: /^data-.*$/, + classes: [ 'allow', 'disallow' ], + styles: [ 'color', 'background' ] + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + attributes: 'data-disallow', + classes: 'disallow', + styles: 'background' + } ] ); + + /* eslint-disable max-len */ + editor.setData( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
2.12.22.3
' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1' + + '' + + '' + + '2' + + '' + + '' + + '3' + + '' + + '' + + '' + + '' + + '1.1' + + '' + + '' + + '1.2' + + '' + + '' + + '1.3' + + '' + + '' + + '' + + '' + + '2.1' + + '' + + '' + + '2.2' + + '' + + '' + + '2.3' + + '' + + '' + + '
', + attributes: range( 1, 17 ).reduce( ( attributes, index ) => { + attributes[ index ] = { + attributes: { + 'data-allow': 'allow' + }, + styles: { + color: 'red' + }, + classes: [ 'allow' ] + }; + return attributes; + }, {} ) + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
123
1.11.21.3
2.12.22.3
' + + '
' + ); + /* eslint-enable max-len */ + } ); + } ); +} ); From cf016933beabcb6c3d4b8c7b55d2ebec78abf8fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Mon, 26 Jul 2021 13:27:43 +0200 Subject: [PATCH 05/17] First batch of tests - Media Feature is available. --- .../src/integrations/mediaembed.js | 34 +- .../tests/mediaembed.js | 1049 ++--------------- 2 files changed, 85 insertions(+), 998 deletions(-) diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index a6aa33fd299..7be306474f9 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -103,26 +103,28 @@ function viewToModelOembedAttributesConverter( dataFilter, mediaElementName ) { } } ); - // dispatcher.on( `element:${ mediaElementName }`, ( evt, data, conversionApi ) => { - // const viewOembedElement = data.viewItem; + // Handle media elements without `
` container. + // TODO: Cleanup of those two handlers + dispatcher.on( `element:${ mediaElementName }`, ( evt, data, conversionApi ) => { + const viewOembedElement = data.viewItem; - // preserveElementAttributes( viewOembedElement, 'htmlAttributes' ); + preserveElementAttributes( viewOembedElement, 'htmlAttributes' ); - // const viewFigureElement = viewOembedElement.parent; - // if ( viewFigureElement.is( 'element', 'figure' ) ) { - // preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); - // } + const viewFigureElement = viewOembedElement.parent; + if ( viewFigureElement.is( 'element', 'figure' ) ) { + preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); + } - // function preserveElementAttributes( viewElement, attributeName ) { - // const viewAttributes = dataFilter._consumeAllowedAttributes( viewElement, conversionApi ); + function preserveElementAttributes( viewElement, attributeName ) { + const viewAttributes = dataFilter._consumeAllowedAttributes( viewElement, conversionApi ); - // if ( viewAttributes ) { - // conversionApi.writer.setAttribute( attributeName, viewAttributes, data.modelRange ); - // } - // } - // }, - // // Low priority to let other converters prepare the modelRange for us. - // { priority: 'low' } ); + if ( viewAttributes ) { + conversionApi.writer.setAttribute( attributeName, viewAttributes, data.modelRange ); + } + } + }, + // Low priority to let other converters prepare the modelRange for us. + { priority: 'low' } ); }; } diff --git a/packages/ckeditor5-html-support/tests/mediaembed.js b/packages/ckeditor5-html-support/tests/mediaembed.js index dc36d12496d..a256b963331 100644 --- a/packages/ckeditor5-html-support/tests/mediaembed.js +++ b/packages/ckeditor5-html-support/tests/mediaembed.js @@ -7,13 +7,14 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictest import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import Table from '@ckeditor/ckeditor5-table/src/table'; import TableCaption from '@ckeditor/ckeditor5-table/src/tablecaption'; +import MediaEmbed from '@ckeditor/ckeditor5-media-embed/src/mediaembed'; import GeneralHtmlSupport from '../src/generalhtmlsupport'; import { getModelDataWithAttributes } from './_utils/utils'; import { range } from 'lodash-es'; /* global document */ -describe( 'MediaEmbedElementSupport', () => { +describe.only( 'MediaEmbedElementSupport', () => { let editor, model, editorElement, dataFilter; beforeEach( () => { @@ -22,7 +23,7 @@ describe( 'MediaEmbedElementSupport', () => { return ClassicTestEditor .create( editorElement, { - plugins: [ Table, TableCaption, Paragraph, GeneralHtmlSupport ] + plugins: [ Table, TableCaption, Paragraph, GeneralHtmlSupport, MediaEmbed ] } ) .then( newEditor => { editor = newEditor; @@ -40,153 +41,29 @@ describe( 'MediaEmbedElementSupport', () => { it( 'should allow attributes', () => { dataFilter.loadAllowedConfig( [ { - name: /^(figure|table|tbody|thead|tr|th|td)$/, + name: /^(figure|oembed)$/, attributes: /^data-.*$/ } ] ); const expectedHtml = - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
2.12.22.3
' + + '
' + + '' + '
'; editor.setData( expectedHtml ); expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { data: - '' + - '' + - '' + - '1' + - '' + - '' + - '2' + - '' + - '' + - '3' + - '' + - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '' + - '' + - '2.1' + - '' + - '' + - '2.2' + - '' + - '' + - '2.3' + - '' + - '' + - '
', + '', attributes: { 1: { attributes: { - 'data-table': 'table' + 'data-oembed': 'data-oembed-value' } }, 2: { attributes: { - 'data-figure': 'figure' - } - }, - 3: { - attributes: { - 'data-tbody': 'tbody' - } - }, - 4: { - attributes: { - 'data-thead': 'thead' - } - }, - 5: { - attributes: { - 'data-tr': 'tr' - } - }, - 6: { - attributes: { - 'data-th': 'th' - } - }, - 7: { - attributes: { - 'data-th': 'th' - } - }, - 8: { - attributes: { - 'data-th': 'th' - } - }, - 9: { - attributes: { - 'data-tr': 'tr' - } - }, - 10: { - attributes: { - 'data-td': 'td' - } - }, - 11: { - attributes: { - 'data-td': 'td' - } - }, - 12: { - attributes: { - 'data-td': 'td' - } - }, - 13: { - attributes: { - 'data-tr': 'tr' - } - }, - 14: { - attributes: { - 'data-td': 'td' - } - }, - 15: { - attributes: { - 'data-td': 'td' - } - }, - 16: { - attributes: { - 'data-td': 'td' + 'data-figure': 'data-figure-value' } } } @@ -197,75 +74,21 @@ describe( 'MediaEmbedElementSupport', () => { it( 'should allow classes', () => { dataFilter.loadAllowedConfig( [ { - name: /^(figure|table|tbody|thead|tr|th|td)$/, + name: /^(figure|oembed)$/, classes: 'foobar' } ] ); const expectedHtml = - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
2.12.22.3
' + + '
' + + '' + '
'; editor.setData( expectedHtml ); expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { data: - '' + - '' + - '' + - '1' + - '' + - '' + - '2' + - '' + - '' + - '3' + - '' + - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '' + - '' + - '2.1' + - '' + - '' + - '2.2' + - '' + - '' + - '2.3' + - '' + - '' + - '
', - attributes: range( 1, 17 ).reduce( ( attributes, index ) => { + '', + attributes: range( 1, 3 ).reduce( ( attributes, index ) => { attributes[ index ] = { classes: [ 'foobar' ] }; @@ -278,75 +101,21 @@ describe( 'MediaEmbedElementSupport', () => { it( 'should allow styles', () => { dataFilter.loadAllowedConfig( [ { - name: /^(figure|table|tbody|thead|tr|th|td)$/, + name: /^(figure|oembed)$/, styles: 'color' } ] ); const expectedHtml = - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
2.12.22.3
' + + '
' + + '' + '
'; editor.setData( expectedHtml ); expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { data: - '' + - '' + - '' + - '1' + - '' + - '' + - '2' + - '' + - '' + - '3' + - '' + - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '' + - '' + - '2.1' + - '' + - '' + - '2.2' + - '' + - '' + - '2.3' + - '' + - '' + - '
', - attributes: range( 1, 17 ).reduce( ( attributes, index ) => { + '', + attributes: range( 1, 3 ).reduce( ( attributes, index ) => { attributes[ index ] = { styles: { color: 'red' @@ -361,388 +130,133 @@ describe( 'MediaEmbedElementSupport', () => { it( 'should disallow attributes', () => { dataFilter.loadAllowedConfig( [ { - name: /^(figure|table|tbody|thead|tr|th|td)$/, + name: /^(figure|oembed)$/, attributes: /^data-.*$/ } ] ); dataFilter.loadDisallowedConfig( [ { - name: /^(figure|table|tbody|thead|tr|th|td)$/, + name: /^(figure|oembed)$/, attributes: /^data-.*$/ } ] ); editor.setData( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
2.12.22.3
' + + '
' + + '' + '
' ); expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { data: - '' + - '' + - '' + - '1' + - '' + - '' + - '2' + - '' + - '' + - '3' + - '' + - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '' + - '' + - '2.1' + - '' + - '' + - '2.2' + - '' + - '' + - '2.3' + - '' + - '' + - '
', + '', attributes: {} } ); expect( editor.getData() ).to.equal( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
2.12.22.3
' + + '
' + + '' + '
' ); } ); it( 'should disallow classes', () => { dataFilter.loadAllowedConfig( [ { - name: /^(figure|table|tbody|thead|tr|th|td)$/, + name: /^(figure|oembed)$/, classes: 'foobar' } ] ); dataFilter.loadDisallowedConfig( [ { - name: /^(figure|table|tbody|thead|tr|th|td)$/, + name: /^(figure|oembed)$/, classes: 'foobar' } ] ); editor.setData( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
2.12.22.3
' + + '
' + + '' + '
' ); expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { data: - '' + - '' + - '' + - '1' + - '' + - '' + - '2' + - '' + - '' + - '3' + - '' + - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '' + - '' + - '2.1' + - '' + - '' + - '2.2' + - '' + - '' + - '2.3' + - '' + - '' + - '
', + '', attributes: {} } ); expect( editor.getData() ).to.equal( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
2.12.22.3
' + + '
' + + '' + '
' ); } ); it( 'should disallow styles', () => { dataFilter.loadAllowedConfig( [ { - name: /^(figure|table|tbody|thead|tr|th|td)$/, + name: /^(figure|oembed)$/, styles: 'color' } ] ); dataFilter.loadDisallowedConfig( [ { - name: /^(figure|table|tbody|thead|tr|th|td)$/, + name: /^(figure|oembed)$/, styles: 'color' } ] ); editor.setData( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
2.12.22.3
' + + '
' + + '' + '
' ); expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { data: - '' + - '' + - '' + - '1' + - '' + - '' + - '2' + - '' + - '' + - '3' + - '' + - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '' + - '' + - '2.1' + - '' + - '' + - '2.2' + - '' + - '' + - '2.3' + - '' + - '' + - '
', + '', attributes: {} } ); expect( editor.getData() ).to.equal( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
2.12.22.3
' + + '
' + + '' + '
' ); } ); it( 'should not set attributes on non existing figure', () => { dataFilter.loadAllowedConfig( [ { - name: /^(figure|table|tbody|tr|td)$/, + name: /^(figure|oembed)$/, attributes: true } ] ); editor.setData( - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
1.11.21.3
' + '' ); expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { data: - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '
', + '', attributes: { 1: { - attributes: { 'data-foo': 'foo' } + attributes: { + 'data-foo': 'foo', + 'url': 'https://www.youtube.com/watch?v=ZVv7UMQPEWk' + } } } } ); expect( editor.getData() ).to.equal( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
1.11.21.3
' + + '
' + + '' + '
' ); } ); it( 'should not break figure integration for other features', () => { dataFilter.loadAllowedConfig( [ { - name: /^(figure|figcaption|table|tbody|tr|td)$/, + name: /^(figure|figcaption|oembed)$/, attributes: /^data-.*$/ } ] ); const expectedHtml = - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
1.11.21.3
' + + '
' + + '' + '
' + '
' + '
foobar
' + @@ -752,26 +266,14 @@ describe( 'MediaEmbedElementSupport', () => { expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { data: - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '
' + + '' + '' + 'foobar' + '', attributes: { 1: { attributes: { - 'data-figure': 'table' + 'data-figure': 'oembed' } }, 2: { @@ -793,449 +295,32 @@ describe( 'MediaEmbedElementSupport', () => { it( 'should not consume attributes already consumed (downcast)', () => { [ 'htmlAttributes', - 'htmlFigureAttributes', - 'htmlTbodyAttributes', - 'htmlTheadAttributes' + 'htmlFigureAttributes' ].forEach( attributeName => { - editor.conversion.for( 'downcast' ).add( dispatcher => { - dispatcher.on( `attribute:${ attributeName }:table`, ( evt, data, conversionApi ) => { - conversionApi.consumable.consume( data.item, evt.name ); - }, { priority: 'high' } ); - } ); + editor.conversion.for( 'downcast' ) + .add( dispatcher => { + dispatcher.on( `attribute:${ attributeName }:media`, ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, evt.name ); + }, { priority: 'high' } ); + } ); } ); - dataFilter.allowElement( /^(figure|table|tbody|thead)$/ ); + dataFilter.allowElement( /^(figure|oembed)$/ ); dataFilter.allowAttributes( { - name: /^(figure|table|tbody|thead)$/, + name: /^(figure|oembed)$/, attributes: { 'data-foo': true } } ); editor.setData( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
' + + '
' + + '' + '
' ); expect( editor.getData() ).to.equal( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
' + + '
' + + '' + '
' ); } ); - - describe( 'TableCaption', () => { - // Sanity tests verifying if table caption is correctly handled by default converters. - - it( 'should allow attributes (caption)', () => { - dataFilter.loadAllowedConfig( [ { - // caption is changed to figcaption by TableCaption feature. - name: /^(caption|figcaption)$/, - attributes: 'data-foo', - styles: 'color', - classes: 'foobar' - } ] ); - - editor.setData( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
caption
1.11.21.3
' + - '
' - ); - - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '' + - '
caption
', - attributes: { - 1: { - attributes: { 'data-foo': 'foo' }, - styles: { color: 'red' }, - classes: [ 'foobar' ] - } - } - } ); - - expect( editor.getData() ).to.equal( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
1.11.21.3
' + - '
caption
' + - '
' - ); - } ); - - it( 'should disallow attributes (caption)', () => { - dataFilter.loadAllowedConfig( [ { - // caption is changed to figcaption by TableCaption feature. - name: /^(caption|figcaption)$/, - attributes: 'data-foo', - styles: 'color', - classes: 'foobar' - } ] ); - - dataFilter.loadDisallowedConfig( [ { - // caption is changed to figcaption by TableCaption feature. - name: /^(caption|figcaption)$/, - attributes: 'data-foo', - styles: 'color', - classes: 'foobar' - } ] ); - - editor.setData( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
caption
1.11.21.3
' + - '
' - ); - - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '' + - '
caption
', - attributes: {} - } ); - - expect( editor.getData() ).to.equal( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
1.11.21.3
' + - '
caption
' + - '
' - ); - } ); - - it( 'should allow attributes (figcaption)', () => { - dataFilter.loadAllowedConfig( [ { - name: 'figcaption', - attributes: 'data-foo', - styles: 'color', - classes: 'foobar' - } ] ); - - editor.setData( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
1.11.21.3
' + - '
caption
' + - '
' - ); - - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '' + - '
caption
', - attributes: { - 1: { - attributes: { 'data-foo': 'foo' }, - styles: { color: 'red' }, - classes: [ 'foobar' ] - } - } - } ); - - expect( editor.getData() ).to.equal( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
1.11.21.3
' + - '
caption
' + - '
' - ); - } ); - - it( 'should disallow attributes (figcaption)', () => { - dataFilter.loadAllowedConfig( [ { - name: 'figcaption', - attributes: 'data-foo', - styles: 'color', - classes: 'foobar' - } ] ); - - dataFilter.loadDisallowedConfig( [ { - name: 'figcaption', - attributes: 'data-foo', - styles: 'color', - classes: 'foobar' - } ] ); - - editor.setData( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
1.11.21.3
' + - '
caption
' + - '
' - ); - - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '' + - '
caption
', - attributes: {} - } ); - - expect( editor.getData() ).to.equal( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
1.11.21.3
' + - '
caption
' + - '
' - ); - } ); - - it( 'should handle mixed allowed and disallowed attributes', () => { - dataFilter.loadAllowedConfig( [ { - name: /^(figure|table|tbody|thead|tr|th|td)$/, - attributes: /^data-.*$/, - classes: [ 'allow', 'disallow' ], - styles: [ 'color', 'background' ] - } ] ); - - dataFilter.loadDisallowedConfig( [ { - name: /^(figure|table|tbody|thead|tr|th|td)$/, - attributes: 'data-disallow', - classes: 'disallow', - styles: 'background' - } ] ); - - /* eslint-disable max-len */ - editor.setData( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
2.12.22.3
' + - '
' - ); - - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '' + - '' + - '' + - '1' + - '' + - '' + - '2' + - '' + - '' + - '3' + - '' + - '' + - '' + - '' + - '1.1' + - '' + - '' + - '1.2' + - '' + - '' + - '1.3' + - '' + - '' + - '' + - '' + - '2.1' + - '' + - '' + - '2.2' + - '' + - '' + - '2.3' + - '' + - '' + - '
', - attributes: range( 1, 17 ).reduce( ( attributes, index ) => { - attributes[ index ] = { - attributes: { - 'data-allow': 'allow' - }, - styles: { - color: 'red' - }, - classes: [ 'allow' ] - }; - return attributes; - }, {} ) - } ); - - expect( editor.getData() ).to.equal( - '
' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '
123
1.11.21.3
2.12.22.3
' + - '
' - ); - /* eslint-enable max-len */ - } ); - } ); } ); From bb02c40d2139c27eabe0b9598bf63a5e948dd12e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Mon, 26 Jul 2021 13:54:44 +0200 Subject: [PATCH 06/17] Add tests for GHS without Media Embed feature. --- .../tests/mediaembed.js | 868 ++++++++++++------ 1 file changed, 612 insertions(+), 256 deletions(-) diff --git a/packages/ckeditor5-html-support/tests/mediaembed.js b/packages/ckeditor5-html-support/tests/mediaembed.js index a256b963331..8d0c3a3b580 100644 --- a/packages/ckeditor5-html-support/tests/mediaembed.js +++ b/packages/ckeditor5-html-support/tests/mediaembed.js @@ -14,313 +14,669 @@ import { range } from 'lodash-es'; /* global document */ -describe.only( 'MediaEmbedElementSupport', () => { - let editor, model, editorElement, dataFilter; - - beforeEach( () => { - editorElement = document.createElement( 'div' ); - document.body.appendChild( editorElement ); - - return ClassicTestEditor - .create( editorElement, { - plugins: [ Table, TableCaption, Paragraph, GeneralHtmlSupport, MediaEmbed ] - } ) - .then( newEditor => { - editor = newEditor; - model = editor.model; - - dataFilter = editor.plugins.get( 'DataFilter' ); +describe( 'MediaEmbedElementSupport', () => { + describe( 'MediaEmbed feature is available', () => { + let editor, model, editorElement, dataFilter; + + beforeEach( () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor + .create( editorElement, { + plugins: [ Table, TableCaption, Paragraph, GeneralHtmlSupport, MediaEmbed ] + } ) + .then( newEditor => { + editor = newEditor; + model = editor.model; + + dataFilter = editor.plugins.get( 'DataFilter' ); + } ); + } ); + + afterEach( () => { + editorElement.remove(); + + return editor.destroy(); + } ); + + it( 'should allow attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + attributes: /^data-.*$/ + } ] ); + + const expectedHtml = + '
' + + '' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: { + 1: { + attributes: { + 'data-oembed': 'data-oembed-value' + } + }, + 2: { + attributes: { + 'data-figure': 'data-figure-value' + } + } + } } ); - } ); - afterEach( () => { - editorElement.remove(); + expect( editor.getData() ).to.equal( expectedHtml ); + } ); - return editor.destroy(); - } ); + it( 'should allow classes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + classes: 'foobar' + } ] ); + + const expectedHtml = + '
' + + '' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: range( 1, 3 ).reduce( ( attributes, index ) => { + attributes[ index ] = { + classes: [ 'foobar' ] + }; + return attributes; + }, {} ) + } ); + + expect( editor.getData() ).to.equal( expectedHtml ); + } ); - it( 'should allow attributes', () => { - dataFilter.loadAllowedConfig( [ { - name: /^(figure|oembed)$/, - attributes: /^data-.*$/ - } ] ); + it( 'should allow styles', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + styles: 'color' + } ] ); - const expectedHtml = - '
' + - '' + - '
'; + const expectedHtml = + '
' + + '' + + '
'; - editor.setData( expectedHtml ); + editor.setData( expectedHtml ); - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: '', - attributes: { - 1: { - attributes: { - 'data-oembed': 'data-oembed-value' - } - }, - 2: { - attributes: { - 'data-figure': 'data-figure-value' - } - } - } + attributes: range( 1, 3 ).reduce( ( attributes, index ) => { + attributes[ index ] = { + styles: { + color: 'red' + } + }; + return attributes; + }, {} ) + } ); + + expect( editor.getData() ).to.equal( expectedHtml ); } ); - expect( editor.getData() ).to.equal( expectedHtml ); - } ); + it( 'should disallow attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + attributes: /^data-.*$/ + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|oembed)$/, + attributes: /^data-.*$/ + } ] ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: {} + } ); - it( 'should allow classes', () => { - dataFilter.loadAllowedConfig( [ { - name: /^(figure|oembed)$/, - classes: 'foobar' - } ] ); - - const expectedHtml = - '
' + - '' + - '
'; - - editor.setData( expectedHtml ); - - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '', - attributes: range( 1, 3 ).reduce( ( attributes, index ) => { - attributes[ index ] = { - classes: [ 'foobar' ] - }; - return attributes; - }, {} ) + expect( editor.getData() ).to.equal( + '
' + + '' + + '
' + ); } ); - expect( editor.getData() ).to.equal( expectedHtml ); - } ); + it( 'should disallow classes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + classes: 'foobar' + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|oembed)$/, + classes: 'foobar' + } ] ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: {} + } ); - it( 'should allow styles', () => { - dataFilter.loadAllowedConfig( [ { - name: /^(figure|oembed)$/, - styles: 'color' - } ] ); - - const expectedHtml = - '
' + - '' + - '
'; - - editor.setData( expectedHtml ); - - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '', - attributes: range( 1, 3 ).reduce( ( attributes, index ) => { - attributes[ index ] = { - styles: { - color: 'red' - } - }; - return attributes; - }, {} ) + expect( editor.getData() ).to.equal( + '
' + + '' + + '
' + ); } ); - expect( editor.getData() ).to.equal( expectedHtml ); - } ); + it( 'should disallow styles', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + styles: 'color' + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|oembed)$/, + styles: 'color' + } ] ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '
' + ); + } ); - it( 'should disallow attributes', () => { - dataFilter.loadAllowedConfig( [ { - name: /^(figure|oembed)$/, - attributes: /^data-.*$/ - } ] ); + it( 'should not set attributes on non existing figure', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + attributes: true + } ] ); + + editor.setData( + '' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: { + 1: { + attributes: { + 'data-foo': 'foo', + 'url': 'https://www.youtube.com/watch?v=ZVv7UMQPEWk' + } + } + } + } ); - dataFilter.loadDisallowedConfig( [ { - name: /^(figure|oembed)$/, - attributes: /^data-.*$/ - } ] ); + expect( editor.getData() ).to.equal( + '
' + + '' + + '
' + ); + } ); - editor.setData( - '
' + - '' + - '
' - ); + it( 'should not break figure integration for other features', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|figcaption|oembed)$/, + attributes: /^data-.*$/ + } ] ); + + const expectedHtml = + '
' + + '' + + '
' + + '
' + + '
foobar
' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + 'foobar' + + '', + attributes: { + 1: { + attributes: { + 'data-figure': 'oembed' + } + }, + 2: { + attributes: { + 'data-figure': 'standalone' + } + }, + 3: { + attributes: { + 'data-figcaption': 'figcaption' + } + } + } + } ); - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '', - attributes: {} + expect( editor.getData() ).to.equal( expectedHtml ); } ); - expect( editor.getData() ).to.equal( - '
' + - '' + - '
' - ); + it( 'should not consume attributes already consumed (downcast)', () => { + [ + 'htmlAttributes', + 'htmlFigureAttributes' + ].forEach( attributeName => { + editor.conversion.for( 'downcast' ) + .add( dispatcher => { + dispatcher.on( `attribute:${ attributeName }:media`, ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, evt.name ); + }, { priority: 'high' } ); + } ); + } ); + + dataFilter.allowElement( /^(figure|oembed)$/ ); + dataFilter.allowAttributes( { + name: /^(figure|oembed)$/, + attributes: { 'data-foo': true } + } ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '
' + ); + } ); } ); - it( 'should disallow classes', () => { - dataFilter.loadAllowedConfig( [ { - name: /^(figure|oembed)$/, - classes: 'foobar' - } ] ); + // Even though the support of media embed feature by GHS alone seems lacking (e.g. extra paragraphs in the model, output) + // we still wanted to keep track of the state of that kind of conversion. + describe( 'Oembed supported solely by GHS', () => { + let editor, model, editorElement, dataFilter; - dataFilter.loadDisallowedConfig( [ { - name: /^(figure|oembed)$/, - classes: 'foobar' - } ] ); + beforeEach( () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); - editor.setData( - '
' + - '' + - '
' - ); + return ClassicTestEditor + .create( editorElement, { + plugins: [ Table, TableCaption, Paragraph, GeneralHtmlSupport ] + } ) + .then( newEditor => { + editor = newEditor; + model = editor.model; - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '', - attributes: {} + dataFilter = editor.plugins.get( 'DataFilter' ); + } ); } ); - expect( editor.getData() ).to.equal( - '
' + - '' + + afterEach( () => { + editorElement.remove(); + + return editor.destroy(); + } ); + + it( 'should allow attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + attributes: /^data-.*$/ + } ] ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '' + + '', + attributes: { + 1: { + attributes: { + 'data-figure': 'data-figure-value' + } + }, + 2: { + attributes: { + 'data-oembed': 'data-oembed-value' + } + }, + 3: '' + } + } ); + + expect( editor.getData() ).to.equal( + '
' + + '

' + + '' + + '

' + + '
' + ); + } ); + + it( 'should allow classes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + classes: [ 'media', 'foobar' ] + } ] ); + + editor.setData( + '
' + + '' + '
' - ); - } ); + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '' + + '', + attributes: { + 1: { + classes: [ 'media', 'foobar' ] + }, + 2: { + classes: [ 'foobar' ] + }, + 3: '' + } + } ); - it( 'should disallow styles', () => { - dataFilter.loadAllowedConfig( [ { - name: /^(figure|oembed)$/, - styles: 'color' - } ] ); + expect( editor.getData() ).to.equal( + '

' + ); + } ); - dataFilter.loadDisallowedConfig( [ { - name: /^(figure|oembed)$/, - styles: 'color' - } ] ); + it( 'should allow styles', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + styles: 'color' + } ] ); - editor.setData( + const inputHtml = '
' + '' + - '
' - ); + '
'; - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '', - attributes: {} - } ); + editor.setData( inputHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '' + + '', + attributes: { + 1: { + styles: { color: 'red' } + }, + 2: { + styles: { color: 'red' } + }, + 3: '' + } + } ); - expect( editor.getData() ).to.equal( - '
' + - '' + - '
' - ); - } ); + expect( editor.getData() ).to.equal( + '

' + ); + } ); - it( 'should not set attributes on non existing figure', () => { - dataFilter.loadAllowedConfig( [ { - name: /^(figure|oembed)$/, - attributes: true - } ] ); - - editor.setData( - '' - ); - - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '', - attributes: { - 1: { - attributes: { - 'data-foo': 'foo', - 'url': 'https://www.youtube.com/watch?v=ZVv7UMQPEWk' - } + it( 'should disallow attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + attributes: /^data-.*$/ + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|oembed)$/, + attributes: /^data-.*$/ + } ] ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: { + 1: '' } - } + } ); + + expect( editor.getData() ).to.equal( + '

' + ); } ); - expect( editor.getData() ).to.equal( - '
' + - '' + - '
' - ); - } ); + it( 'should disallow classes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + classes: 'foobar' + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|oembed)$/, + classes: 'foobar' + } ] ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: { + 1: '' + } + } ); - it( 'should not break figure integration for other features', () => { - dataFilter.loadAllowedConfig( [ { - name: /^(figure|figcaption|oembed)$/, - attributes: /^data-.*$/ - } ] ); - - const expectedHtml = - '
' + - '' + - '
' + - '
' + - '
foobar
' + - '
'; + expect( editor.getData() ).to.equal( + '

' + ); + } ); - editor.setData( expectedHtml ); - - expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { - data: - '' + - '' + - 'foobar' + - '', - attributes: { - 1: { - attributes: { - 'data-figure': 'oembed' - } - }, - 2: { - attributes: { - 'data-figure': 'standalone' - } - }, - 3: { - attributes: { - 'data-figcaption': 'figcaption' - } + it( 'should disallow styles', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + styles: 'color' + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|oembed)$/, + styles: 'color' + } ] ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: { + 1: '' } - } + } ); + + expect( editor.getData() ).to.equal( + '

' + ); } ); - expect( editor.getData() ).to.equal( expectedHtml ); - } ); + it( 'should not set attributes on non existing figure', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|oembed)$/, + attributes: true + } ] ); + + editor.setData( + '' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: { + 1: { + attributes: { + 'data-foo': 'foo', + 'url': 'https://www.youtube.com/watch?v=ZVv7UMQPEWk' + } + }, + 2: '' + } + } ); - it( 'should not consume attributes already consumed (downcast)', () => { - [ - 'htmlAttributes', - 'htmlFigureAttributes' - ].forEach( attributeName => { - editor.conversion.for( 'downcast' ) - .add( dispatcher => { - dispatcher.on( `attribute:${ attributeName }:media`, ( evt, data, conversionApi ) => { - conversionApi.consumable.consume( data.item, evt.name ); - }, { priority: 'high' } ); - } ); + expect( editor.getData() ).to.equal( + '

' + ); } ); - dataFilter.allowElement( /^(figure|oembed)$/ ); - dataFilter.allowAttributes( { - name: /^(figure|oembed)$/, - attributes: { 'data-foo': true } + it( 'should not break figure integration for other features', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|figcaption|oembed)$/, + attributes: /^data-.*$/ + } ] ); + + const expectedHtml = + '
' + + '' + + '
' + + '
' + + '
foobar
' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '' + + '' + + '' + + 'foobar' + + '', + attributes: { + 1: { + attributes: { + 'data-figure': 'oembed' + } + }, + 2: '', + 3: { + attributes: { + 'data-figure': 'standalone' + } + }, + 4: { + attributes: { + 'data-figcaption': 'figcaption' + } + } + } + } ); + + expect( editor.getData() ).to.equal( + '
' + + '

' + + '' + + '

' + + '
' + + '
' + + '
foobar
' + + '
' + ); } ); - editor.setData( - '
' + - '' + - '
' - ); + it( 'should not consume attributes already consumed (downcast)', () => { + [ + 'htmlAttributes', + 'htmlFigureAttributes' + ].forEach( attributeName => { + editor.conversion.for( 'downcast' ) + .add( dispatcher => { + dispatcher.on( `attribute:${ attributeName }:htmlOembed`, ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, evt.name ); + }, { priority: 'high' } ); + } ) + .add( dispatcher => { + dispatcher.on( `attribute:${ attributeName }:htmlFigure`, ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, evt.name ); + }, { priority: 'high' } ); + } ); + } ); - expect( editor.getData() ).to.equal( - '
' + - '' + - '
' - ); + dataFilter.allowElement( /^(figure|oembed)$/ ); + dataFilter.allowAttributes( { + name: /^(figure|oembed)$/, + attributes: { 'data-foo': true } + } ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( editor.getData() ).to.equal( + '

' + ); + } ); } ); } ); From 5ee9edbb1dbdca24834adee804aa9cb2b6665be5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Mon, 26 Jul 2021 14:52:35 +0200 Subject: [PATCH 07/17] Add support for custom media embed element names. Clean up manual test. --- .../src/integrations/mediaembed.js | 23 +++--- .../tests/manual/mediaembed.html | 72 ++++++++----------- .../tests/manual/mediaembed.js | 43 +++++++++-- 3 files changed, 81 insertions(+), 57 deletions(-) diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index 7be306474f9..492ddc12fd4 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -13,7 +13,6 @@ import { setViewAttributes } from '../conversionutils.js'; import DataFilter from '../datafilter'; import DataSchema from '../dataschema'; -import { getDataFromElement } from 'ckeditor5/src/utils'; /** * Provides the General HTML Support integration with {@link module:media-embed/mediaembed~MediaEmbed Media Embed} feature. @@ -37,19 +36,19 @@ export default class MediaEmbedElementSupport extends Plugin { const schema = editor.model.schema; const conversion = editor.conversion; const dataFilter = this.editor.plugins.get( DataFilter ); - // const dataSchema = this.editor.plugins.get( DataSchema ); + const dataSchema = this.editor.plugins.get( DataSchema ); const mediaElementName = editor.config.get( 'mediaEmbed.elementName' ); - // Add dynamically schema definition for a given elementName. - // dataSchema.registerBlockElement( { - // model: 'htmlOembed2', - // view: mediaElementName, - // isObject: true, - // modelSchema: { - // inheritAllFrom: '$htmlObjectInline' - // } - // } ); + // Overwrite GHS schema definition for a given elementName. + dataSchema.registerInlineElement( { + model: 'htmlOembed', + view: mediaElementName, + isObject: true, + modelSchema: { + inheritAllFrom: '$htmlObjectInline' + } + } ); dataFilter.on( `register:${ mediaElementName }`, ( evt, definition ) => { if ( definition.model !== 'htmlOembed' ) { @@ -75,7 +74,7 @@ export default class MediaEmbedElementSupport extends Plugin { function viewToModelOembedAttributesConverter( dataFilter, mediaElementName ) { return dispatcher => { dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => { - if ( data.viewItem.getChild( 0 ).name === 'oembed' ) { + if ( data.viewItem.getChild( 0 ).name === mediaElementName ) { // Since we are converting to attribute we need a range on which we will set the attribute. // If the range is not created yet, let's create it by converting children of the current node first. if ( !data.modelRange ) { diff --git a/packages/ckeditor5-html-support/tests/manual/mediaembed.html b/packages/ckeditor5-html-support/tests/manual/mediaembed.html index 6756e15688c..1cd873bcc7e 100644 --- a/packages/ckeditor5-html-support/tests/manual/mediaembed.html +++ b/packages/ckeditor5-html-support/tests/manual/mediaembed.html @@ -1,42 +1,32 @@ -
-

Media with previews

- -
- -
- -
- -
- - - -
- -
- -

Generic media

- -
- -
- -
- -
- -
- -
- -
- - -
- -
- -
+
+
+

The default oembed element

+ +
+
+ +
+ +
+ +
+
+
+
+

Custom media element

+ +
+
+ +
+ +
+ +
+
+
diff --git a/packages/ckeditor5-html-support/tests/manual/mediaembed.js b/packages/ckeditor5-html-support/tests/manual/mediaembed.js index 59ff9125593..027e68ab423 100644 --- a/packages/ckeditor5-html-support/tests/manual/mediaembed.js +++ b/packages/ckeditor5-html-support/tests/manual/mediaembed.js @@ -27,21 +27,56 @@ ClassicEditor image: { toolbar: [ 'toggleImageCaption', 'imageTextAlternative' ] }, toolbar: [ 'mediaEmbed', '|', 'sourceEditing' ], mediaEmbed: { - // previewsInData: true, + toolbar: [ 'mediaEmbed' ] + }, + htmlSupport: { + allow: [ + { + name: /^(figure|oembed)$/, + attributes: [ 'data-validation-allow', 'data-validation-disallow' ] + } + ], + disallow: [ + { + name: /^(figure|oembed)$/, + attributes: 'data-validation-disallow' + } + ] + } + } ) + .then( editor => { + window.editor = editor; + } ) + .catch( err => { + console.error( err.stack ); + } ); - // elementName: 'xemebd', +ClassicEditor + .create( document.querySelector( '#editor-custom-element-name' ), { + plugins: [ + GeneralHtmlSupport, + Essentials, + Paragraph, + SourceEditing, + MediaEmbed, + MediaEmbedToolbar + ], + image: { toolbar: [ 'toggleImageCaption', 'imageTextAlternative' ] }, + toolbar: [ 'mediaEmbed', '|', 'sourceEditing' ], + mediaEmbed: { + elementName: 'custom-oembed', toolbar: [ 'mediaEmbed' ] }, htmlSupport: { allow: [ { - name: /^(figure|table|tbody|thead|tr|th|td|caption|figcaption|oembed)$/, + name: /^(figure|custom-oembed)$/, attributes: [ 'data-validation-allow', 'data-validation-disallow' ] } ], disallow: [ { - name: /^(figure|table|tbody|thead|tr|th|td|caption|figcaption|oembed)$/, + name: /^(figure|custom-oembed)$/, attributes: 'data-validation-disallow' } ] From 9f66b70c0a8ba6e69e91b76e6c4e59fc378d9aab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Mon, 26 Jul 2021 15:06:41 +0200 Subject: [PATCH 08/17] Cleanup of the converters. --- .../src/integrations/mediaembed.js | 38 ++++++------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index 492ddc12fd4..875f7a40541 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -63,29 +63,28 @@ export default class MediaEmbedElementSupport extends Plugin { } ); conversion.for( 'upcast' ).add( disallowedAttributesConverter( definition, dataFilter ) ); - conversion.for( 'upcast' ).add( viewToModelOembedAttributesConverter( dataFilter, mediaElementName ) ); - conversion.for( 'dataDowncast' ).add( modelToViewOembedAttributeConverter( mediaElementName ) ); + conversion.for( 'upcast' ).add( viewToModelMediaAttributesConverter( dataFilter, mediaElementName ) ); + conversion.for( 'dataDowncast' ).add( modelToViewMediaAttributeConverter( mediaElementName ) ); evt.stop(); } ); } } -function viewToModelOembedAttributesConverter( dataFilter, mediaElementName ) { +function viewToModelMediaAttributesConverter( dataFilter, mediaElementName ) { return dispatcher => { dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => { if ( data.viewItem.getChild( 0 ).name === mediaElementName ) { - // Since we are converting to attribute we need a range on which we will set the attribute. // If the range is not created yet, let's create it by converting children of the current node first. if ( !data.modelRange ) { // Convert children and set conversion result as a current data. Object.assign( data, conversionApi.convertChildren( data.viewItem, data.modelCursor ) ); } - const viewOembedElement = data.viewItem.getChild( 0 ); - preserveElementAttributes( viewOembedElement, 'htmlAttributes' ); + const viewMediaElement = data.viewItem.getChild( 0 ); + preserveElementAttributes( viewMediaElement, 'htmlAttributes' ); - const viewFigureElement = viewOembedElement.parent; + const viewFigureElement = viewMediaElement.parent; if ( viewFigureElement.is( 'element', 'figure' ) ) { preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); } @@ -103,31 +102,18 @@ function viewToModelOembedAttributesConverter( dataFilter, mediaElementName ) { } ); // Handle media elements without `
` container. - // TODO: Cleanup of those two handlers dispatcher.on( `element:${ mediaElementName }`, ( evt, data, conversionApi ) => { - const viewOembedElement = data.viewItem; + const viewMediaElement = data.viewItem; + const viewAttributes = dataFilter._consumeAllowedAttributes( viewMediaElement, conversionApi ); - preserveElementAttributes( viewOembedElement, 'htmlAttributes' ); - - const viewFigureElement = viewOembedElement.parent; - if ( viewFigureElement.is( 'element', 'figure' ) ) { - preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); - } - - function preserveElementAttributes( viewElement, attributeName ) { - const viewAttributes = dataFilter._consumeAllowedAttributes( viewElement, conversionApi ); - - if ( viewAttributes ) { - conversionApi.writer.setAttribute( attributeName, viewAttributes, data.modelRange ); - } + if ( viewAttributes ) { + conversionApi.writer.setAttribute( 'htmlAttributes', viewAttributes, data.modelRange ); } - }, - // Low priority to let other converters prepare the modelRange for us. - { priority: 'low' } ); + } ); }; } -function modelToViewOembedAttributeConverter( mediaElementName ) { +function modelToViewMediaAttributeConverter( mediaElementName ) { return dispatcher => { addAttributeConversionDispatcherHandler( mediaElementName, 'htmlAttributes' ); addAttributeConversionDispatcherHandler( 'figure', 'htmlFigureAttributes' ); From 0be2bcceba541e89c31b1a8a92ae77ccebb6eb0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Mon, 26 Jul 2021 15:14:08 +0200 Subject: [PATCH 09/17] Add tests for custom media element. --- .../tests/mediaembed.js | 320 +++++++++++++++++- 1 file changed, 316 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-html-support/tests/mediaembed.js b/packages/ckeditor5-html-support/tests/mediaembed.js index 8d0c3a3b580..16d67ac4cdf 100644 --- a/packages/ckeditor5-html-support/tests/mediaembed.js +++ b/packages/ckeditor5-html-support/tests/mediaembed.js @@ -5,8 +5,6 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; -import Table from '@ckeditor/ckeditor5-table/src/table'; -import TableCaption from '@ckeditor/ckeditor5-table/src/tablecaption'; import MediaEmbed from '@ckeditor/ckeditor5-media-embed/src/mediaembed'; import GeneralHtmlSupport from '../src/generalhtmlsupport'; import { getModelDataWithAttributes } from './_utils/utils'; @@ -24,7 +22,7 @@ describe( 'MediaEmbedElementSupport', () => { return ClassicTestEditor .create( editorElement, { - plugins: [ Table, TableCaption, Paragraph, GeneralHtmlSupport, MediaEmbed ] + plugins: [ Paragraph, GeneralHtmlSupport, MediaEmbed ] } ) .then( newEditor => { editor = newEditor; @@ -326,6 +324,320 @@ describe( 'MediaEmbedElementSupport', () => { } ); } ); + describe( 'MediaEmbed feature with custom element name', () => { + let editor, model, editorElement, dataFilter; + + beforeEach( () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor + .create( editorElement, { + plugins: [ Paragraph, GeneralHtmlSupport, MediaEmbed ], + mediaEmbed: { + elementName: 'custom-oembed' + } + } ) + .then( newEditor => { + editor = newEditor; + model = editor.model; + + dataFilter = editor.plugins.get( 'DataFilter' ); + } ); + } ); + + afterEach( () => { + editorElement.remove(); + + return editor.destroy(); + } ); + + it( 'should allow attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|custom-oembed)$/, + attributes: /^data-.*$/ + } ] ); + + const expectedHtml = + '
' + + '' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: { + 1: { + attributes: { + 'data-oembed': 'data-oembed-value' + } + }, + 2: { + attributes: { + 'data-figure': 'data-figure-value' + } + } + } + } ); + + expect( editor.getData() ).to.equal( expectedHtml ); + } ); + + it( 'should allow classes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|custom-oembed)$/, + classes: 'foobar' + } ] ); + + const expectedHtml = + '
' + + '' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: range( 1, 3 ).reduce( ( attributes, index ) => { + attributes[ index ] = { + classes: [ 'foobar' ] + }; + return attributes; + }, {} ) + } ); + + expect( editor.getData() ).to.equal( expectedHtml ); + } ); + + it( 'should allow styles', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|custom-oembed)$/, + styles: 'color' + } ] ); + + const expectedHtml = + '
' + + '' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: range( 1, 3 ).reduce( ( attributes, index ) => { + attributes[ index ] = { + styles: { + color: 'red' + } + }; + return attributes; + }, {} ) + } ); + + expect( editor.getData() ).to.equal( expectedHtml ); + } ); + + it( 'should disallow attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|custom-oembed)$/, + attributes: /^data-.*$/ + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|custom-oembed)$/, + attributes: /^data-.*$/ + } ] ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '
' + ); + } ); + + it( 'should disallow classes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|custom-oembed)$/, + classes: 'foobar' + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|custom-oembed)$/, + classes: 'foobar' + } ] ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '
' + ); + } ); + + it( 'should disallow styles', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|custom-oembed)$/, + styles: 'color' + } ] ); + + dataFilter.loadDisallowedConfig( [ { + name: /^(figure|custom-oembed)$/, + styles: 'color' + } ] ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '
' + ); + } ); + + it( 'should not set attributes on non existing figure', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|custom-oembed)$/, + attributes: true + } ] ); + + editor.setData( + '' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '', + attributes: { + 1: { + attributes: { + 'data-foo': 'foo', + 'url': 'https://www.youtube.com/watch?v=ZVv7UMQPEWk' + } + } + } + } ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '
' + ); + } ); + + it( 'should not break figure integration for other features', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|figcaption|custom-oembed)$/, + attributes: /^data-.*$/ + } ] ); + + const expectedHtml = + '
' + + '' + + '
' + + '
' + + '
foobar
' + + '
'; + + editor.setData( expectedHtml ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + 'foobar' + + '', + attributes: { + 1: { + attributes: { + 'data-figure': 'oembed' + } + }, + 2: { + attributes: { + 'data-figure': 'standalone' + } + }, + 3: { + attributes: { + 'data-figcaption': 'figcaption' + } + } + } + } ); + + expect( editor.getData() ).to.equal( expectedHtml ); + } ); + + it( 'should not consume attributes already consumed (downcast)', () => { + [ + 'htmlAttributes', + 'htmlFigureAttributes' + ].forEach( attributeName => { + editor.conversion.for( 'downcast' ) + .add( dispatcher => { + dispatcher.on( `attribute:${ attributeName }:media`, ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, evt.name ); + }, { priority: 'high' } ); + } ); + } ); + + dataFilter.allowElement( /^(figure|custom-oembed)$/ ); + dataFilter.allowAttributes( { + name: /^(figure|custom-oembed)$/, + attributes: { 'data-foo': true } + } ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '
' + ); + } ); + } ); + // Even though the support of media embed feature by GHS alone seems lacking (e.g. extra paragraphs in the model, output) // we still wanted to keep track of the state of that kind of conversion. describe( 'Oembed supported solely by GHS', () => { @@ -337,7 +649,7 @@ describe( 'MediaEmbedElementSupport', () => { return ClassicTestEditor .create( editorElement, { - plugins: [ Table, TableCaption, Paragraph, GeneralHtmlSupport ] + plugins: [ Paragraph, GeneralHtmlSupport ] } ) .then( newEditor => { editor = newEditor; From 8fbff38b82a361c8a257f64dd4e4a9f561985422 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Mon, 26 Jul 2021 16:48:56 +0200 Subject: [PATCH 10/17] Remove unused code. --- .../src/integrations/mediaembed.js | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index 875f7a40541..067abdbd99e 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -37,7 +37,6 @@ export default class MediaEmbedElementSupport extends Plugin { const conversion = editor.conversion; const dataFilter = this.editor.plugins.get( DataFilter ); const dataSchema = this.editor.plugins.get( DataSchema ); - const mediaElementName = editor.config.get( 'mediaEmbed.elementName' ); // Overwrite GHS schema definition for a given elementName. @@ -74,24 +73,25 @@ export default class MediaEmbedElementSupport extends Plugin { function viewToModelMediaAttributesConverter( dataFilter, mediaElementName ) { return dispatcher => { dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => { - if ( data.viewItem.getChild( 0 ).name === mediaElementName ) { - // If the range is not created yet, let's create it by converting children of the current node first. - if ( !data.modelRange ) { - // Convert children and set conversion result as a current data. - Object.assign( data, conversionApi.convertChildren( data.viewItem, data.modelCursor ) ); - } + const viewFigureElement = data.viewItem; + const viewMediaElement = Array.from( viewFigureElement.getChildren() ) + .find( item => item.is( 'element', mediaElementName ) ); - const viewMediaElement = data.viewItem.getChild( 0 ); - preserveElementAttributes( viewMediaElement, 'htmlAttributes' ); - - const viewFigureElement = viewMediaElement.parent; - if ( viewFigureElement.is( 'element', 'figure' ) ) { - preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); - } + if ( !viewMediaElement ) { + return; + } - conversionApi.consumable.consume( data.viewItem, { name: true } ); + // If the range is not created yet, let's create it by converting children of the current node first. + if ( !data.modelRange ) { + // Convert children and set conversion result as a current data. + Object.assign( data, conversionApi.convertChildren( viewFigureElement, data.modelCursor ) ); } + preserveElementAttributes( viewMediaElement, 'htmlAttributes' ); + preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); + + conversionApi.consumable.consume( viewFigureElement, { name: true } ); + function preserveElementAttributes( viewElement, attributeName ) { const viewAttributes = dataFilter._consumeAllowedAttributes( viewElement, conversionApi ); From 81b002c1d5165de21c28a401a6b8223cc2597af2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Tue, 27 Jul 2021 10:12:17 +0200 Subject: [PATCH 11/17] Changes in tests and conversion code. --- .../src/integrations/mediaembed.js | 46 +++++++++++++------ .../tests/manual/mediaembed.html | 2 + .../tests/mediaembed.js | 2 +- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index 067abdbd99e..635d69b62a9 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -8,9 +8,10 @@ */ import { Plugin } from 'ckeditor5/src/core'; +import { first } from 'ckeditor5/src/utils'; + import { disallowedAttributesConverter } from '../converters'; import { setViewAttributes } from '../conversionutils.js'; - import DataFilter from '../datafilter'; import DataSchema from '../dataschema'; @@ -40,17 +41,13 @@ export default class MediaEmbedElementSupport extends Plugin { const mediaElementName = editor.config.get( 'mediaEmbed.elementName' ); // Overwrite GHS schema definition for a given elementName. - dataSchema.registerInlineElement( { - model: 'htmlOembed', - view: mediaElementName, - isObject: true, - modelSchema: { - inheritAllFrom: '$htmlObjectInline' - } + dataSchema.registerBlockElement( { + model: 'media', + view: mediaElementName } ); dataFilter.on( `register:${ mediaElementName }`, ( evt, definition ) => { - if ( definition.model !== 'htmlOembed' ) { + if ( definition.model !== 'media' ) { return; } @@ -74,22 +71,41 @@ function viewToModelMediaAttributesConverter( dataFilter, mediaElementName ) { return dispatcher => { dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => { const viewFigureElement = data.viewItem; + + // Convert only "media figure" elements. + if ( !conversionApi.consumable.test( viewFigureElement, { name: true, class: 'media' } ) ) { + return; + } + + // Find media element. const viewMediaElement = Array.from( viewFigureElement.getChildren() ) .find( item => item.is( 'element', mediaElementName ) ); - if ( !viewMediaElement ) { + // Do not convert if media element is absent or was already converted. + if ( !viewMediaElement || !conversionApi.consumable.test( viewMediaElement, { name: true } ) ) { return; } - // If the range is not created yet, let's create it by converting children of the current node first. - if ( !data.modelRange ) { - // Convert children and set conversion result as a current data. - Object.assign( data, conversionApi.convertChildren( viewFigureElement, data.modelCursor ) ); + // Convert view figure to model figure. + const conversionResult = conversionApi.convertItem( viewMediaElement, data.modelCursor ); + + // Get media element from conversion result. + const modelMediaElement = first( conversionResult.modelRange.getItems() ); + + // When media wasn't successfully converted then finish conversion. + if ( !modelMediaElement ) { + return; } + // Convert the rest of the figure element's children as an media children. + conversionApi.convertChildren( viewFigureElement, conversionApi.writer.createPositionAt( modelMediaElement, 'end' ) ); + + conversionApi.updateConversionResult( modelMediaElement, data ); + preserveElementAttributes( viewMediaElement, 'htmlAttributes' ); preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); + // Consume the figure to prevent converting it to `htmlFigure` by default GHS converters. conversionApi.consumable.consume( viewFigureElement, { name: true } ); function preserveElementAttributes( viewElement, attributeName ) { @@ -99,7 +115,7 @@ function viewToModelMediaAttributesConverter( dataFilter, mediaElementName ) { conversionApi.writer.setAttribute( attributeName, viewAttributes, data.modelRange ); } } - } ); + }, { priority: 'high' } ); // Handle media elements without `
` container. dispatcher.on( `element:${ mediaElementName }`, ( evt, data, conversionApi ) => { diff --git a/packages/ckeditor5-html-support/tests/manual/mediaembed.html b/packages/ckeditor5-html-support/tests/manual/mediaembed.html index 1cd873bcc7e..7ef93179379 100644 --- a/packages/ckeditor5-html-support/tests/manual/mediaembed.html +++ b/packages/ckeditor5-html-support/tests/manual/mediaembed.html @@ -13,6 +13,7 @@

The default oembed element

+

Custom media element

@@ -28,5 +29,6 @@

Custom media element

+ diff --git a/packages/ckeditor5-html-support/tests/mediaembed.js b/packages/ckeditor5-html-support/tests/mediaembed.js index 16d67ac4cdf..8cc253ce5b6 100644 --- a/packages/ckeditor5-html-support/tests/mediaembed.js +++ b/packages/ckeditor5-html-support/tests/mediaembed.js @@ -12,7 +12,7 @@ import { range } from 'lodash-es'; /* global document */ -describe( 'MediaEmbedElementSupport', () => { +describe.only( 'MediaEmbedElementSupport', () => { describe( 'MediaEmbed feature is available', () => { let editor, model, editorElement, dataFilter; From 4d4981a0abc9e836861102a71272533c54e1eb7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Tue, 27 Jul 2021 11:18:38 +0200 Subject: [PATCH 12/17] Cleaned up the converters. --- .../src/integrations/mediaembed.js | 84 +++++++++---------- 1 file changed, 38 insertions(+), 46 deletions(-) diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index 635d69b62a9..3d6be8c9edb 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -8,7 +8,6 @@ */ import { Plugin } from 'ckeditor5/src/core'; -import { first } from 'ckeditor5/src/utils'; import { disallowedAttributesConverter } from '../converters'; import { setViewAttributes } from '../conversionutils.js'; @@ -69,64 +68,57 @@ export default class MediaEmbedElementSupport extends Plugin { function viewToModelMediaAttributesConverter( dataFilter, mediaElementName ) { return dispatcher => { - dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => { - const viewFigureElement = data.viewItem; + // Here we want to be the first to convert (and consume) the figure element, otherwise GHS can pick it up and + // convert it to generic `htmlFigure`. + dispatcher.on( 'element:figure', upcastFigure, { priority: 'high' } ); - // Convert only "media figure" elements. - if ( !conversionApi.consumable.test( viewFigureElement, { name: true, class: 'media' } ) ) { - return; - } - - // Find media element. - const viewMediaElement = Array.from( viewFigureElement.getChildren() ) - .find( item => item.is( 'element', mediaElementName ) ); - - // Do not convert if media element is absent or was already converted. - if ( !viewMediaElement || !conversionApi.consumable.test( viewMediaElement, { name: true } ) ) { - return; - } + // Handle media elements without `
` container. + dispatcher.on( `element:${ mediaElementName }`, upcastMedia ); + }; - // Convert view figure to model figure. - const conversionResult = conversionApi.convertItem( viewMediaElement, data.modelCursor ); + function upcastFigure( evt, data, conversionApi ) { + const viewFigureElement = data.viewItem; - // Get media element from conversion result. - const modelMediaElement = first( conversionResult.modelRange.getItems() ); + // Convert only "media figure" elements. + if ( !conversionApi.consumable.test( viewFigureElement, { name: true, class: 'media' } ) ) { + return; + } - // When media wasn't successfully converted then finish conversion. - if ( !modelMediaElement ) { - return; - } + // Find media element. + const viewMediaElement = Array.from( viewFigureElement.getChildren() ) + .find( item => item.is( 'element', mediaElementName ) ); - // Convert the rest of the figure element's children as an media children. - conversionApi.convertChildren( viewFigureElement, conversionApi.writer.createPositionAt( modelMediaElement, 'end' ) ); + // Do not convert if media element is absent or was already converted. + if ( !viewMediaElement || !conversionApi.consumable.test( viewMediaElement, { name: true } ) ) { + return; + } - conversionApi.updateConversionResult( modelMediaElement, data ); + // Convert just the media element. + Object.assign( data, conversionApi.convertItem( viewMediaElement, data.modelCursor ) ); - preserveElementAttributes( viewMediaElement, 'htmlAttributes' ); - preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); + preserveElementAttributes( viewMediaElement, 'htmlAttributes' ); + preserveElementAttributes( viewFigureElement, 'htmlFigureAttributes' ); - // Consume the figure to prevent converting it to `htmlFigure` by default GHS converters. - conversionApi.consumable.consume( viewFigureElement, { name: true } ); + // Consume the figure to prevent converting it to `htmlFigure` by default GHS converters. + conversionApi.consumable.consume( viewFigureElement, { name: true } ); - function preserveElementAttributes( viewElement, attributeName ) { - const viewAttributes = dataFilter._consumeAllowedAttributes( viewElement, conversionApi ); + function preserveElementAttributes( viewElement, attributeName ) { + const viewAttributes = dataFilter._consumeAllowedAttributes( viewElement, conversionApi ); - if ( viewAttributes ) { - conversionApi.writer.setAttribute( attributeName, viewAttributes, data.modelRange ); - } + if ( viewAttributes ) { + conversionApi.writer.setAttribute( attributeName, viewAttributes, data.modelRange ); } - }, { priority: 'high' } ); + } + } - // Handle media elements without `
` container. - dispatcher.on( `element:${ mediaElementName }`, ( evt, data, conversionApi ) => { - const viewMediaElement = data.viewItem; - const viewAttributes = dataFilter._consumeAllowedAttributes( viewMediaElement, conversionApi ); + function upcastMedia( evt, data, conversionApi ) { + const viewMediaElement = data.viewItem; + const viewAttributes = dataFilter._consumeAllowedAttributes( viewMediaElement, conversionApi ); - if ( viewAttributes ) { - conversionApi.writer.setAttribute( 'htmlAttributes', viewAttributes, data.modelRange ); - } - } ); - }; + if ( viewAttributes ) { + conversionApi.writer.setAttribute( 'htmlAttributes', viewAttributes, data.modelRange ); + } + } } function modelToViewMediaAttributeConverter( mediaElementName ) { From 048e5febbc90578cb60ab808b825309b65315d15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Tue, 27 Jul 2021 13:29:39 +0200 Subject: [PATCH 13/17] Add missing test for overriding converters. --- .../src/integrations/mediaembed.js | 4 +- .../tests/mediaembed.js | 88 ++++++++++++++++++- 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index 3d6be8c9edb..f4a50d5bb3b 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -80,7 +80,7 @@ function viewToModelMediaAttributesConverter( dataFilter, mediaElementName ) { const viewFigureElement = data.viewItem; // Convert only "media figure" elements. - if ( !conversionApi.consumable.test( viewFigureElement, { name: true, class: 'media' } ) ) { + if ( !conversionApi.consumable.test( viewFigureElement, { name: true, classes: 'media' } ) ) { return; } @@ -89,7 +89,7 @@ function viewToModelMediaAttributesConverter( dataFilter, mediaElementName ) { .find( item => item.is( 'element', mediaElementName ) ); // Do not convert if media element is absent or was already converted. - if ( !viewMediaElement || !conversionApi.consumable.test( viewMediaElement, { name: true } ) ) { + if ( !viewMediaElement ) { return; } diff --git a/packages/ckeditor5-html-support/tests/mediaembed.js b/packages/ckeditor5-html-support/tests/mediaembed.js index 8cc253ce5b6..1ca0b324645 100644 --- a/packages/ckeditor5-html-support/tests/mediaembed.js +++ b/packages/ckeditor5-html-support/tests/mediaembed.js @@ -12,7 +12,7 @@ import { range } from 'lodash-es'; /* global document */ -describe.only( 'MediaEmbedElementSupport', () => { +describe( 'MediaEmbedElementSupport', () => { describe( 'MediaEmbed feature is available', () => { let editor, model, editorElement, dataFilter; @@ -291,6 +291,49 @@ describe.only( 'MediaEmbedElementSupport', () => { expect( editor.getData() ).to.equal( expectedHtml ); } ); + it( 'should not consume media figure element that is already consumed (upcast)', () => { + editor.conversion.for( 'upcast' ) + .add( dispatcher => { + dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.viewItem, { name: true, classes: 'media' } ); + }, { priority: 'highest' } ); + } ); + + dataFilter.allowElement( /^(figure|oembed)$/ ); + dataFilter.allowAttributes( { + name: /^(figure|oembed)$/, + attributes: { 'data-foo': true } + } ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( editor.getData() ).to.equal( '' ); + } ); + + it( 'should not consume media element that is already consumed (upcast)', () => { + dataFilter.allowElement( /^(figure|oembed)$/ ); + dataFilter.allowAttributes( { + name: /^(figure|oembed)$/, + attributes: { 'data-foo': true } + } ); + + editor.setData( + '
' + + '

foobar

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

foobar

' + + '
' + ); + } ); + it( 'should not consume attributes already consumed (downcast)', () => { [ 'htmlAttributes', @@ -605,6 +648,49 @@ describe.only( 'MediaEmbedElementSupport', () => { expect( editor.getData() ).to.equal( expectedHtml ); } ); + it( 'should not consume media figure element that is already consumed (upcast)', () => { + editor.conversion.for( 'upcast' ) + .add( dispatcher => { + dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.viewItem, { name: true, classes: 'media' } ); + }, { priority: 'highest' } ); + } ); + + dataFilter.allowElement( /^(figure|custom-oembed)$/ ); + dataFilter.allowAttributes( { + name: /^(figure|custom-oembed)$/, + attributes: { 'data-foo': true } + } ); + + editor.setData( + '
' + + '' + + '
' + ); + + expect( editor.getData() ).to.equal( '' ); + } ); + + it( 'should not consume media element that is already consumed (upcast)', () => { + dataFilter.allowElement( /^(figure|custom-oembed)$/ ); + dataFilter.allowAttributes( { + name: /^(figure|custom-oembed)$/, + attributes: { 'data-foo': true } + } ); + + editor.setData( + '
' + + '

foobar

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

foobar

' + + '
' + ); + } ); + it( 'should not consume attributes already consumed (downcast)', () => { [ 'htmlAttributes', From 34b2aae24e2e31b7deb00677272b5fe67c6ac02a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Tue, 27 Jul 2021 13:44:53 +0200 Subject: [PATCH 14/17] Clean up manual test. --- .../tests/manual/mediaembed.html | 46 +++++++++++-------- .../tests/manual/mediaembed.js | 24 ++++++++-- .../tests/manual/mediaembed.md | 2 +- 3 files changed, 49 insertions(+), 23 deletions(-) diff --git a/packages/ckeditor5-html-support/tests/manual/mediaembed.html b/packages/ckeditor5-html-support/tests/manual/mediaembed.html index 7ef93179379..5c6c3ef73a3 100644 --- a/packages/ckeditor5-html-support/tests/manual/mediaembed.html +++ b/packages/ckeditor5-html-support/tests/manual/mediaembed.html @@ -1,34 +1,44 @@
-

The default oembed element

+

The default oembed element

-
- -
- -
- +
+
+

Expected output

+ + +
-

Custom media element

+

Custom media element

-
- -
- -
- +
+
+

Expected output

+ + +
diff --git a/packages/ckeditor5-html-support/tests/manual/mediaembed.js b/packages/ckeditor5-html-support/tests/manual/mediaembed.js index 027e68ab423..867fe90f7af 100644 --- a/packages/ckeditor5-html-support/tests/manual/mediaembed.js +++ b/packages/ckeditor5-html-support/tests/manual/mediaembed.js @@ -33,13 +33,21 @@ ClassicEditor allow: [ { name: /^(figure|oembed)$/, - attributes: [ 'data-validation-allow', 'data-validation-disallow' ] + attributes: [ 'data-validation-allow', 'data-validation-disallow' ], + classes: [ 'allowed-class' ], + styles: { + color: 'blue' + } } ], disallow: [ { name: /^(figure|oembed)$/, - attributes: 'data-validation-disallow' + attributes: 'data-validation-disallow', + classes: [ 'disallowed-class' ], + styles: { + color: 'red' + } } ] } @@ -71,13 +79,21 @@ ClassicEditor allow: [ { name: /^(figure|custom-oembed)$/, - attributes: [ 'data-validation-allow', 'data-validation-disallow' ] + attributes: [ 'data-validation-allow', 'data-validation-disallow' ], + classes: [ 'allowed-class' ], + styles: { + color: 'blue' + } } ], disallow: [ { name: /^(figure|custom-oembed)$/, - attributes: 'data-validation-disallow' + attributes: 'data-validation-disallow', + classes: [ 'disallowed-class' ], + styles: { + color: 'red' + } } ] } diff --git a/packages/ckeditor5-html-support/tests/manual/mediaembed.md b/packages/ckeditor5-html-support/tests/manual/mediaembed.md index 21fea1d54d5..5e95a04b061 100644 --- a/packages/ckeditor5-html-support/tests/manual/mediaembed.md +++ b/packages/ckeditor5-html-support/tests/manual/mediaembed.md @@ -1,3 +1,3 @@ ## Media Embed -### TODO ### +Compare the output (using e.g. Source Editing plugin) with the expected result below the editor. From 4fcd62faae757b07006e59b7d9a9ce255fc41382 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Tue, 27 Jul 2021 16:14:14 +0200 Subject: [PATCH 15/17] Make CI happy with Vimeo link. --- .../tests/manual/mediaembed.html | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/ckeditor5-html-support/tests/manual/mediaembed.html b/packages/ckeditor5-html-support/tests/manual/mediaembed.html index 5c6c3ef73a3..48452b303f2 100644 --- a/packages/ckeditor5-html-support/tests/manual/mediaembed.html +++ b/packages/ckeditor5-html-support/tests/manual/mediaembed.html @@ -1,3 +1,16 @@ + + + + +

The default oembed element

From 9bd0a9eb9e5ad8f71ed9f1f2a85bd32cfb56efc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Wed, 28 Jul 2021 09:58:18 +0200 Subject: [PATCH 16/17] Update comment. --- packages/ckeditor5-html-support/src/integrations/mediaembed.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.js b/packages/ckeditor5-html-support/src/integrations/mediaembed.js index f4a50d5bb3b..cd2e7d32d8d 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.js +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.js @@ -88,7 +88,7 @@ function viewToModelMediaAttributesConverter( dataFilter, mediaElementName ) { const viewMediaElement = Array.from( viewFigureElement.getChildren() ) .find( item => item.is( 'element', mediaElementName ) ); - // Do not convert if media element is absent or was already converted. + // Do not convert if media element is absent. if ( !viewMediaElement ) { return; } From c8f57d31b911f6d1faa249136d04de224c4fd476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Wed, 28 Jul 2021 13:41:52 +0200 Subject: [PATCH 17/17] Remove unnecesary rule. --- packages/ckeditor5-html-support/tests/manual/mediaembed.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-html-support/tests/manual/mediaembed.js b/packages/ckeditor5-html-support/tests/manual/mediaembed.js index 867fe90f7af..72c83af5c33 100644 --- a/packages/ckeditor5-html-support/tests/manual/mediaembed.js +++ b/packages/ckeditor5-html-support/tests/manual/mediaembed.js @@ -33,7 +33,7 @@ ClassicEditor allow: [ { name: /^(figure|oembed)$/, - attributes: [ 'data-validation-allow', 'data-validation-disallow' ], + attributes: [ 'data-validation-allow' ], classes: [ 'allowed-class' ], styles: { color: 'blue' @@ -79,7 +79,7 @@ ClassicEditor allow: [ { name: /^(figure|custom-oembed)$/, - attributes: [ 'data-validation-allow', 'data-validation-disallow' ], + attributes: [ 'data-validation-allow' ], classes: [ 'allowed-class' ], styles: { color: 'blue'