From 3599a2440d6b5c297dd898b7da34dd72f7adf366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Thu, 3 Feb 2022 13:44:41 +0100 Subject: [PATCH 1/7] Add support for `'; + + converter.renderingMode = 'data'; + converter.setContentOf( element, html ); + + expect( element.innerHTML ).to.equal( '
foo
' ); + } ); } ); describe( 'editing pipeline', () => { @@ -736,7 +746,7 @@ describe( 'DomConverter', () => { ); } ); - it( 'should warn when an unsafe element was detected and renamed', () => { + it( 'should warn when an unsafe script element was detected and renamed', () => { const element = document.createElement( 'p' ); const html = '
foo
'; @@ -744,8 +754,20 @@ describe( 'DomConverter', () => { sinon.assert.calledOnce( warnStub ); sinon.assert.calledWithExactly( warnStub, - sinon.match( /^domconverter-unsafe-element-detected/ ), - sinon.match.has( 'unsafeElement', sinon.match.has( 'tagName', 'SCRIPT' ) ), + sinon.match( /^domconverter-unsafe-script-element-detected/ ), + sinon.match.string // Link to the documentation + ); + } ); + + it( 'should warn when an unsafe style element was detected and renamed', () => { + const element = document.createElement( 'p' ); + const html = '
foo
'; + + converter.setContentOf( element, html ); + + sinon.assert.calledOnce( warnStub ); + sinon.assert.calledWithExactly( warnStub, + sinon.match( /^domconverter-unsafe-style-element-detected/ ), sinon.match.string // Link to the documentation ); } ); diff --git a/packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js b/packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js index 6d671f1c76c..ac2d25fe839 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js @@ -324,8 +324,53 @@ describe( 'DomConverter', () => { sinon.assert.calledOnce( warnStub ); sinon.assert.calledWithExactly( warnStub, - sinon.match( /^domconverter-unsafe-element-detected/ ), - sinon.match.has( 'unsafeElement', sinon.match.has( 'name', 'script' ) ), + sinon.match( /^domconverter-unsafe-script-element-detected/ ), + sinon.match.string // Link to the documentation + ); + } ); + + it( 'should replace style with span and add special data attribute', () => { + const viewScript = new ViewElement( viewDocument, 'style' ); + const viewText = new ViewText( viewDocument, 'foo' ); + const viewP = new ViewElement( viewDocument, 'p', { class: 'foo' } ); + + viewP._appendChild( viewScript ); + viewP._appendChild( viewText ); + + converter = new DomConverter( viewDocument, { + renderingMode: 'editing' + } ); + + const domP = converter.viewToDom( viewP, document ); + + expect( domP ).to.be.an.instanceof( HTMLElement ); + expect( domP.tagName ).to.equal( 'P' ); + expect( domP.getAttribute( 'class' ) ).to.equal( 'foo' ); + expect( domP.attributes.length ).to.equal( 1 ); + + expect( domP.childNodes.length ).to.equal( 2 ); + expect( domP.childNodes[ 0 ].tagName ).to.equal( 'SPAN' ); + expect( domP.childNodes[ 0 ].getAttribute( 'data-ck-unsafe-element' ) ).to.equal( 'style' ); + expect( domP.childNodes[ 1 ].data ).to.equal( 'foo' ); + } ); + + it( 'should warn when an unsafe style was filtered out', () => { + const viewStyle = new ViewElement( viewDocument, 'style' ); + const viewText = new ViewText( viewDocument, 'foo' ); + const viewP = new ViewElement( viewDocument, 'p', { class: 'foo' } ); + + viewP._appendChild( viewStyle ); + viewP._appendChild( viewText ); + + converter = new DomConverter( viewDocument, { + renderingMode: 'editing' + } ); + + converter.viewToDom( viewP, document ); + + sinon.assert.calledOnce( warnStub ); + sinon.assert.calledWithExactly( warnStub, + sinon.match( /^domconverter-unsafe-style-element-detected/ ), sinon.match.string // Link to the documentation ); } ); diff --git a/packages/ckeditor5-html-support/tests/integrations/style.js b/packages/ckeditor5-html-support/tests/integrations/style.js new file mode 100644 index 00000000000..9da533a4665 --- /dev/null +++ b/packages/ckeditor5-html-support/tests/integrations/style.js @@ -0,0 +1,179 @@ +/** + * @license Copyright (c) 2003-2022, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import GeneralHtmlSupport from '../../src/generalhtmlsupport'; +import { getModelDataWithAttributes } from '../_utils/utils'; +import { getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; + +/* global console, document */ + +describe( 'StyleElementSupport', () => { + const STYLE = 'div { color: red; }'; + const CODE_CPP = 'cout << "Hello World" << endl;'; + + let editor, model, editorElement, dataFilter, warnStub; + + beforeEach( async () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + editor = await ClassicTestEditor.create( editorElement, { + plugins: [ Paragraph, GeneralHtmlSupport ] + } ); + model = editor.model; + dataFilter = editor.plugins.get( 'DataFilter' ); + + dataFilter.allowElement( 'style' ); + + warnStub = sinon.stub( console, 'warn' ); + } ); + + afterEach( () => { + warnStub.restore(); + editorElement.remove(); + + return editor.destroy(); + } ); + + it( 'should allow element', () => { + editor.setData( `

Foo

` ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + `Foo` + ); + + expect( editor.getData() ).to.equal( `

Foo

` ); + } ); + + it( 'should allow attributes', () => { + dataFilter.allowAttributes( { name: 'style', attributes: [ 'type', 'nonce' ] } ); + + editor.setData( `

Foo

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

Foo

` ); + } ); + + it( 'should disallow attributes', () => { + dataFilter.allowAttributes( { name: 'style', attributes: true } ); + dataFilter.disallowAttributes( { name: 'style', attributes: 'nonce' } ); + + editor.setData( `

Foo

` ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: `Foo`, + attributes: { + 1: { + attributes: { + type: 'c++' + } + }, + 2: CODE_CPP + } + } ); + + expect( editor.getData() ).to.equal( `

Foo

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

FooBar

`, + model: + '' + + `FooBar` + + '' + }, { + name: 'section', + data: `

Foo

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

Foo

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

Foo

`, + model: + 'Foo' + + `` + } ]; + + for ( const { name, data, model: modelData } of testCases ) { + it( `should allow element inside ${ name }`, () => { + dataFilter.allowElement( 'article' ); + dataFilter.allowElement( 'section' ); + + editor.setData( data ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( modelData ); + + expect( editor.getData() ).to.equal( data ); + } ); + } + } ); + + it( 'should not consume attributes already consumed (downcast)', () => { + dataFilter.allowAttributes( { name: 'style', attributes: true } ); + + editor.conversion.for( 'downcast' ).add( dispatcher => { + dispatcher.on( 'attribute:htmlAttributes:htmlStyle', ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, evt.name ); + }, { priority: 'high' } ); + } ); + + editor.setData( `

Foo

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

Foo

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

Foo

` ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: `Foo`, + attributes: { + 1: { attributes: { type: 'text/css' } }, + 2: STYLE + } + } ); + } ); +} ); From 80a1c2186374cd0b072801349834680a20712884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Tue, 8 Feb 2022 16:07:20 +0100 Subject: [PATCH 4/7] Add missing tests. --- .../tests/view/domconverter/domconverter.js | 29 ++++++++++++++++++- .../ckeditor5-engine/tests/view/renderer.js | 24 ++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/domconverter/domconverter.js b/packages/ckeditor5-engine/tests/view/domconverter/domconverter.js index a6f2bfd1a7b..ecaa0c170e6 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/domconverter.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/domconverter.js @@ -940,7 +940,11 @@ describe( 'DomConverter', () => { .callsFake( () => {} ); console.warn - .withArgs( sinon.match( /^domconverter-unsafe-element-detected/ ) ) + .withArgs( sinon.match( /^domconverter-unsafe-script-element-detected/ ) ) + .callsFake( () => {} ); + + console.warn + .withArgs( sinon.match( /^domconverter-unsafe-style-element-detected/ ) ) .callsFake( () => {} ); console.warn.callThrough(); @@ -988,5 +992,28 @@ describe( 'DomConverter', () => { '

foobar

' ); } ); + + it( 'should skip removing the (replacement) attribute representing the unsafe '; + + converter.setContentOf( domElement, html ); + + expect( domElement.outerHTML ).to.equal( + '

foobar

' + ); + + converter.removeDomElementAttribute( domElement.lastChild, 'data-ck-unsafe-element' ); + + expect( domElement.outerHTML ).to.equal( + '

foobar

' + ); + + converter.removeDomElementAttribute( domElement.lastChild, 'class' ); + + expect( domElement.outerHTML ).to.equal( + '

foobar

' + ); + } ); } ); } ); diff --git a/packages/ckeditor5-engine/tests/view/renderer.js b/packages/ckeditor5-engine/tests/view/renderer.js index 3bef2589a8c..0809b129882 100644 --- a/packages/ckeditor5-engine/tests/view/renderer.js +++ b/packages/ckeditor5-engine/tests/view/renderer.js @@ -3981,7 +3981,11 @@ describe( 'Renderer', () => { .callsFake( () => {} ); console.warn - .withArgs( sinon.match( /^domconverter-unsafe-element-detected/ ) ) + .withArgs( sinon.match( /^domconverter-unsafe-script-element-detected/ ) ) + .callsFake( () => {} ); + + console.warn + .withArgs( sinon.match( /^domconverter-unsafe-style-element-detected/ ) ) .callsFake( () => {} ); console.warn.callThrough(); @@ -4027,6 +4031,24 @@ describe( 'Renderer', () => { delete window.spy; } ); + it( 'should replace style element with span and custom data attribute', () => { + window.spy = sinon.spy(); + + const viewA = new ViewElement( viewDoc, 'style' ); + + // Assign content of the `' ); + expect( normalizeHtml( domRoot.innerHTML ) ).to.equal( '.foo { color: red; }' ); + + delete window.spy; + } ); + it( 'should rename attributes that can affect editing pipeline', () => { setViewData( view, '' + From 612382dc8f1a66b6cbfad7ff402bea012f04044b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Wed, 9 Feb 2022 12:06:56 +0100 Subject: [PATCH 5/7] Improved manual test names and added the missing source editing button. --- .../ckeditor5-html-support/tests/manual/generalhtmlsupport.md | 1 - .../tests/manual/{features.html => ghs-all-features.html} | 0 .../tests/manual/{features.js => ghs-all-features.js} | 4 ++++ .../tests/manual/{features.md => ghs-all-features.md} | 0 .../{generalhtmlsupport.html => ghs-custom-config.html} | 0 .../manual/{generalhtmlsupport.js => ghs-custom-config.js} | 0 .../ckeditor5-html-support/tests/manual/ghs-custom-config.md | 3 +++ .../tests/manual/{no-features.html => ghs-no-features.html} | 0 .../tests/manual/{no-features.js => ghs-no-features.js} | 0 .../tests/manual/{no-features.md => ghs-no-features.md} | 0 10 files changed, 7 insertions(+), 1 deletion(-) delete mode 100644 packages/ckeditor5-html-support/tests/manual/generalhtmlsupport.md rename packages/ckeditor5-html-support/tests/manual/{features.html => ghs-all-features.html} (100%) rename packages/ckeditor5-html-support/tests/manual/{features.js => ghs-all-features.js} (96%) rename packages/ckeditor5-html-support/tests/manual/{features.md => ghs-all-features.md} (100%) rename packages/ckeditor5-html-support/tests/manual/{generalhtmlsupport.html => ghs-custom-config.html} (100%) rename packages/ckeditor5-html-support/tests/manual/{generalhtmlsupport.js => ghs-custom-config.js} (100%) create mode 100644 packages/ckeditor5-html-support/tests/manual/ghs-custom-config.md rename packages/ckeditor5-html-support/tests/manual/{no-features.html => ghs-no-features.html} (100%) rename packages/ckeditor5-html-support/tests/manual/{no-features.js => ghs-no-features.js} (100%) rename packages/ckeditor5-html-support/tests/manual/{no-features.md => ghs-no-features.md} (100%) diff --git a/packages/ckeditor5-html-support/tests/manual/generalhtmlsupport.md b/packages/ckeditor5-html-support/tests/manual/generalhtmlsupport.md deleted file mode 100644 index c357cf7f28a..00000000000 --- a/packages/ckeditor5-html-support/tests/manual/generalhtmlsupport.md +++ /dev/null @@ -1 +0,0 @@ -# General HTML Support diff --git a/packages/ckeditor5-html-support/tests/manual/features.html b/packages/ckeditor5-html-support/tests/manual/ghs-all-features.html similarity index 100% rename from packages/ckeditor5-html-support/tests/manual/features.html rename to packages/ckeditor5-html-support/tests/manual/ghs-all-features.html diff --git a/packages/ckeditor5-html-support/tests/manual/features.js b/packages/ckeditor5-html-support/tests/manual/ghs-all-features.js similarity index 96% rename from packages/ckeditor5-html-support/tests/manual/features.js rename to packages/ckeditor5-html-support/tests/manual/ghs-all-features.js index b0b65fb1a02..9e23800e727 100644 --- a/packages/ckeditor5-html-support/tests/manual/features.js +++ b/packages/ckeditor5-html-support/tests/manual/ghs-all-features.js @@ -26,6 +26,7 @@ import PageBreak from '@ckeditor/ckeditor5-page-break/src/pagebreak'; import Strikethrough from '@ckeditor/ckeditor5-basic-styles/src/strikethrough'; import Subscript from '@ckeditor/ckeditor5-basic-styles/src/subscript'; import Superscript from '@ckeditor/ckeditor5-basic-styles/src/superscript'; +import SourceEditing from '@ckeditor/ckeditor5-source-editing/src/sourceediting'; import TableCellProperties from '@ckeditor/ckeditor5-table/src/tablecellproperties'; import TableProperties from '@ckeditor/ckeditor5-table/src/tableproperties'; import TableCaption from '@ckeditor/ckeditor5-table/src/tablecaption'; @@ -46,9 +47,12 @@ ClassicEditor Alignment, IndentBlock, PageBreak, HorizontalLine, ImageUpload, CloudServices, + SourceEditing, GeneralHtmlSupport ], toolbar: [ + 'sourceEditing', + '|', 'heading', '|', 'bold', 'italic', 'strikethrough', 'underline', 'code', 'subscript', 'superscript', 'link', diff --git a/packages/ckeditor5-html-support/tests/manual/features.md b/packages/ckeditor5-html-support/tests/manual/ghs-all-features.md similarity index 100% rename from packages/ckeditor5-html-support/tests/manual/features.md rename to packages/ckeditor5-html-support/tests/manual/ghs-all-features.md diff --git a/packages/ckeditor5-html-support/tests/manual/generalhtmlsupport.html b/packages/ckeditor5-html-support/tests/manual/ghs-custom-config.html similarity index 100% rename from packages/ckeditor5-html-support/tests/manual/generalhtmlsupport.html rename to packages/ckeditor5-html-support/tests/manual/ghs-custom-config.html diff --git a/packages/ckeditor5-html-support/tests/manual/generalhtmlsupport.js b/packages/ckeditor5-html-support/tests/manual/ghs-custom-config.js similarity index 100% rename from packages/ckeditor5-html-support/tests/manual/generalhtmlsupport.js rename to packages/ckeditor5-html-support/tests/manual/ghs-custom-config.js diff --git a/packages/ckeditor5-html-support/tests/manual/ghs-custom-config.md b/packages/ckeditor5-html-support/tests/manual/ghs-custom-config.md new file mode 100644 index 00000000000..2721ff734e6 --- /dev/null +++ b/packages/ckeditor5-html-support/tests/manual/ghs-custom-config.md @@ -0,0 +1,3 @@ +# General HTML Support + +Test exploring a custom, complex config. diff --git a/packages/ckeditor5-html-support/tests/manual/no-features.html b/packages/ckeditor5-html-support/tests/manual/ghs-no-features.html similarity index 100% rename from packages/ckeditor5-html-support/tests/manual/no-features.html rename to packages/ckeditor5-html-support/tests/manual/ghs-no-features.html diff --git a/packages/ckeditor5-html-support/tests/manual/no-features.js b/packages/ckeditor5-html-support/tests/manual/ghs-no-features.js similarity index 100% rename from packages/ckeditor5-html-support/tests/manual/no-features.js rename to packages/ckeditor5-html-support/tests/manual/ghs-no-features.js diff --git a/packages/ckeditor5-html-support/tests/manual/no-features.md b/packages/ckeditor5-html-support/tests/manual/ghs-no-features.md similarity index 100% rename from packages/ckeditor5-html-support/tests/manual/no-features.md rename to packages/ckeditor5-html-support/tests/manual/ghs-no-features.md From 8e3fede7d01e620adfe61e2d1c14046c73449cfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barnas=CC=81?= Date: Wed, 9 Feb 2022 12:09:23 +0100 Subject: [PATCH 6/7] Review fixes. --- .../ckeditor5-engine/src/view/domconverter.js | 8 +++----- .../tests/integrations/style.js | 17 ++++++++--------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/domconverter.js b/packages/ckeditor5-engine/src/view/domconverter.js index ea692a41f24..4fd337a0892 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.js +++ b/packages/ckeditor5-engine/src/view/domconverter.js @@ -1547,7 +1547,7 @@ export default class DomConverter { * @returns {Boolean} */ _shouldRenameElement( elementName ) { - const name = elementName.toLocaleLowerCase(); + const name = elementName.toLowerCase(); return this.renderingMode === 'editing' && UNSAFE_ELEMENTS.includes( name ); } @@ -1662,15 +1662,13 @@ function _logUnsafeElement( elementName ) { * the `