Merge pull request #11235 from ckeditor/ck/11104-handling-style

Other (engine): The `<style>` element will not interfere with the editing experience. See #11104.

Feature (html-support): Added `<style>` tag support in General HTML Support feature. Closes #11104.

packages/ckeditor5-engine/src/view/domconverter.js

@@ -35,6 +35,7 @@ const NBSP_FILLER_REF = NBSP_FILLER( document ); // eslint-disable-line new-cap
 const MARKED_NBSP_FILLER_REF = MARKED_NBSP_FILLER( document ); // eslint-disable-line new-cap
 const UNSAFE_ATTRIBUTE_NAME_PREFIX = 'data-ck-unsafe-attribute-';
 const UNSAFE_ELEMENT_REPLACEMENT_ATTRIBUTE = 'data-ck-unsafe-element';
+const UNSAFE_ELEMENTS = [ 'script', 'style' ];
 
 /**
  * `DomConverter` is a set of tools to do transformations between DOM nodes and view nodes. It also handles
@@ -323,7 +324,7 @@ export default class DomConverter {
 
 			// There are certain nodes, that should be renamed to <span> in editing pipeline.
 			if ( this._shouldRenameElement( elementName ) ) {
-				logWarning( 'domconverter-unsafe-element-detected', { unsafeElement: currentNode } );
+				_logUnsafeElement( elementName );
 
 				currentNode.replaceWith( this._createReplacementDomElement( elementName, currentNode ) );
 			}
@@ -384,7 +385,7 @@ export default class DomConverter {
 			} else {
 				// Create DOM element.
 				if ( this._shouldRenameElement( viewNode.name ) ) {
-					logWarning( 'domconverter-unsafe-element-detected', { unsafeElement: viewNode } );
+					_logUnsafeElement( viewNode.name );
 
 					domElement = this._createReplacementDomElement( viewNode.name );
 				} else if ( viewNode.hasAttribute( 'xmlns' ) ) {
@@ -1546,7 +1547,9 @@ export default class DomConverter {
 	 * @returns {Boolean}
 	 */
 	_shouldRenameElement( elementName ) {
-		return this.renderingMode == 'editing' && elementName.toLowerCase() == 'script';
+		const name = elementName.toLowerCase();
+
+		return this.renderingMode === 'editing' && UNSAFE_ELEMENTS.includes( name );
 	}
 
 	/**
@@ -1626,6 +1629,20 @@ function hasBlockParent( domNode, blockElements ) {
 	return parent && parent.tagName && blockElements.includes( parent.tagName.toLowerCase() );
 }
 
+// Log to console the information about element that was replaced.
+// Check UNSAFE_ELEMENTS for all recognized unsafe elements.
+//
+// @param {String} elementName The name of the view element
+function _logUnsafeElement( elementName ) {
+	if ( elementName === 'script' ) {
+		logWarning( 'domconverter-unsafe-script-element-detected' );
+	}
+
+	if ( elementName === 'style' ) {
+		logWarning( 'domconverter-unsafe-style-element-detected' );
+	}
+}
+
 /**
  * Enum representing the type of the block filler.
  *
@@ -1640,13 +1657,17 @@ function hasBlockParent( domNode, blockElements ) {
  */
 
 /**
- * The {@link module:engine/view/domconverter~DomConverter} detected a `<script>` element that may disrupt the
- * {@glink framework/guides/architecture/editing-engine#editing-pipeline editing pipeline} of the editor. To avoid this,
- * the `<script>` element was renamed to `<span data-ck-unsafe-element="script"></span>`.
+ * While rendering the editor content, the {@link module:engine/view/domconverter~DomConverter} detected a `<script>` element that may
+ * disrupt the editing experience. To avoid this, the `<script>` element was replaced with `<span data-ck-unsafe-element="script"></span>`.
+ *
+ * @error domconverter-unsafe-script-element-detected
+ */
+
+/**
+ * While rendering the editor content, the {@link module:engine/view/domconverter~DomConverter} detected a `<style>` element that may affect
+ * the editing experience. To avoid this, the `<style>` element was replaced with `<span data-ck-unsafe-element="style"></span>`.
  *
- * @error domconverter-unsafe-element-detected
- * @param {module:engine/model/element~Element|HTMLElement} unsafeElement The editing view or DOM element
- * that was renamed.
+ * @error domconverter-unsafe-style-element-detected
  */
 
 /**

packages/ckeditor5-engine/tests/view/domconverter/domconverter.js

@@ -603,6 +603,16 @@ describe( 'DomConverter', () => {
 
 				expect( element.innerHTML ).to.equal( '<div>foo<script onclick="foo">bar</script></div>' );
 			} );
+
+			it( 'should keep style element', () => {
+				const element = document.createElement( 'p' );
+				const html = '<div>foo<style nonce="foo">bar</style></div>';
+
+				converter.renderingMode = 'data';
+				converter.setContentOf( element, html );
+
+				expect( element.innerHTML ).to.equal( '<div>foo<style nonce="foo">bar</style></div>' );
+			} );
 		} );
 
 		describe( 'editing pipeline', () => {
@@ -736,16 +746,28 @@ 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 = '<div>foo<script class="foo-class" style="foo-style" data-foo="bar">bar</script></div>';
 
 				converter.setContentOf( element, html );
 
 				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 = '<div>foo<style class="foo-class" nonce="foo-nonce" data-foo="bar">bar</style></div>';
+
+				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
 				);
 			} );
@@ -918,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();
@@ -966,5 +992,28 @@ describe( 'DomConverter', () => {
 				'<p>foo<span data-ck-unsafe-element="script" style="foo-style" data-foo="bar">bar</span></p>'
 			);
 		} );
+
+		it( 'should skip removing the (replacement) attribute representing the unsafe <style> tag', () => {
+			const domElement = document.createElement( 'p' );
+			const html = 'foo<style class="foo-class" style="foo-style" data-foo="bar">bar</style>';
+
+			converter.setContentOf( domElement, html );
+
+			expect( domElement.outerHTML ).to.equal(
+				'<p>foo<span data-ck-unsafe-element="style" class="foo-class" style="foo-style" data-foo="bar">bar</span></p>'
+			);
+
+			converter.removeDomElementAttribute( domElement.lastChild, 'data-ck-unsafe-element' );
+
+			expect( domElement.outerHTML ).to.equal(
+				'<p>foo<span data-ck-unsafe-element="style" class="foo-class" style="foo-style" data-foo="bar">bar</span></p>'
+			);
+
+			converter.removeDomElementAttribute( domElement.lastChild, 'class' );
+
+			expect( domElement.outerHTML ).to.equal(
+				'<p>foo<span data-ck-unsafe-element="style" style="foo-style" data-foo="bar">bar</span></p>'
+			);
+		} );
 	} );
 } );

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
 				);
 			} );

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 `<style>` element, as the utility method `setVewData` will fail becasuse of brace characters.
+				viewA._appendChild( new ViewText( viewDoc, '.foo { color: red; }' ) );
+				viewRoot._appendChild( viewA );
+
+				view.forceRender();
+
+				expect( window.spy.calledOnce ).to.be.false;
+				expect( getViewData( view ) ).to.equal( '<style>.foo { color: red; }</style>' );
+				expect( normalizeHtml( domRoot.innerHTML ) ).to.equal( '<span data-ck-unsafe-element="style">.foo { color: red; }</span>' );
+
+				delete window.spy;
+			} );
+
 			it( 'should rename attributes that can affect editing pipeline', () => {
 				setViewData( view,
 					'<container:p onclick="test">' +

packages/ckeditor5-html-support/src/generalhtmlsupport.js

@@ -17,6 +17,7 @@ import ImageElementSupport from './integrations/image';
 import MediaEmbedElementSupport from './integrations/mediaembed';
 import ScriptElementSupport from './integrations/script';
 import TableElementSupport from './integrations/table';
+import StyleElementSupport from './integrations/style';
 
 /**
  * The General HTML Support feature.
@@ -46,7 +47,8 @@ export default class GeneralHtmlSupport extends Plugin {
 			ImageElementSupport,
 			MediaEmbedElementSupport,
 			ScriptElementSupport,
-			TableElementSupport
+			TableElementSupport,
+			StyleElementSupport
 		];
 	}
 

packages/ckeditor5-html-support/src/integrations/style.js

@@ -0,0 +1,73 @@
+/**
+ * @license Copyright (c) 2003-2022, CKSource Holding sp. z o.o. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+/**
+ * @module html-support/integrations/style
+ */
+
+import { Plugin } from 'ckeditor5/src/core';
+import {
+	createObjectView,
+	modelToViewBlockAttributeConverter,
+	viewToModelBlockAttributeConverter,
+	viewToModelObjectConverter
+} from '../converters.js';
+
+import DataFilter from '../datafilter';
+
+/**
+ * Provides the General HTML Support for `style` elements.
+ *
+ * @extends module:core/plugin~Plugin
+ */
+export default class StyleElementSupport extends Plugin {
+	/**
+	 * @inheritDoc
+	 */
+	static get requires() {
+		return [ DataFilter ];
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	init() {
+		const dataFilter = this.editor.plugins.get( DataFilter );
+
+		dataFilter.on( 'register:style', ( evt, definition ) => {
+			const editor = this.editor;
+			const schema = editor.model.schema;
+			const conversion = editor.conversion;
+
+			schema.register( 'htmlStyle', definition.modelSchema );
+
+			schema.extend( 'htmlStyle', {
+				allowAttributes: [ 'htmlAttributes', 'htmlContent' ]
+			} );
+
+			editor.data.registerRawContentMatcher( {
+				name: 'style'
+			} );
+
+			conversion.for( 'upcast' ).elementToElement( {
+				view: 'style',
+				model: viewToModelObjectConverter( definition )
+			} );
+
+			conversion.for( 'upcast' ).add( viewToModelBlockAttributeConverter( definition, dataFilter ) );
+
+			conversion.for( 'downcast' ).elementToElement( {
+				model: 'htmlStyle',
+				view: ( modelElement, { writer } ) => {
+					return createObjectView( 'style', modelElement, writer );
+				}
+			} );
+
+			conversion.for( 'downcast' ).add( modelToViewBlockAttributeConverter( definition ) );
+
+			evt.stop();
+		} );
+	}
+}

packages/ckeditor5-html-support/src/schemadefinitions.js

@@ -822,7 +822,7 @@ export default {
 				inheritAllFrom: '$htmlObjectInline'
 			}
 		},
-		// TODO it could be probably represented as non-object element, although it has grafical representation,
+		// TODO it could be probably represented as non-object element, although it has graphical representation,
 		// so probably makes more sense to keep it as an object.
 		{
 			model: 'htmlProgress',
@@ -839,6 +839,14 @@ export default {
 				allowWhere: [ '$text', '$block' ],
 				isInline: true
 			}
+		},
+		{
+			model: 'htmlStyle',
+			view: 'style',
+			modelSchema: {
+				allowWhere: [ '$text', '$block' ],
+				isInline: true
+			}
 		}
 	]
 };