Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GHS] custom elements #11744

Merged
merged 15 commits into from
May 31, 2022
5 changes: 5 additions & 0 deletions packages/ckeditor5-engine/src/conversion/upcasthelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,11 @@ export function convertText() {
return;
}

// Do not auto-paragraph whitespaces.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the following test:

it( 'nbsp between blocks is not ignored (between paragraphs)', () => {

We need to choose to either trim that nbsp in that case or correctly handle scenarios like:

<custom>x</custom>

<custom someattr></custom>

where the space (normal space) should be trimmed.

Unfortunately, since these are custom elements we can't easily make the decision on DOM->view conversion whether these are spaces between block or inline elements.

Additionally, unfortunately we lose the information whether this was an nbsp or normal space upon DOM->view conversion. Currently, all are normalized to normal spaces at this stage. if they weren't, the below code would actually correctly differentiate these situation.

Hence, we need to:

  • either lose intentional nbsps between blocks (may happen in HTML generated by external tools)
  • or make sure custom elements don't break things

if ( data.viewItem.data.trim().length == 0 ) {
return;
}

position = wrapInParagraph( position, writer );
}

Expand Down
12 changes: 10 additions & 2 deletions packages/ckeditor5-engine/src/view/domconverter.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const NBSP_FILLER_REF = NBSP_FILLER( document ); // eslint-disable-line new-cap
const MARKED_NBSP_FILLER_REF = MARKED_NBSP_FILLER( document ); // eslint-disable-line new-cap
const UNSAFE_ATTRIBUTE_NAME_PREFIX = 'data-ck-unsafe-attribute-';
const UNSAFE_ELEMENT_REPLACEMENT_ATTRIBUTE = 'data-ck-unsafe-element';
const UNSAFE_ELEMENTS = [ 'script', 'style' ];

/**
* `DomConverter` is a set of tools to do transformations between DOM nodes and view nodes. It also handles
Expand Down Expand Up @@ -127,6 +126,15 @@ export default class DomConverter {
'object', 'iframe', 'input', 'button', 'textarea', 'select', 'option', 'video', 'embed', 'audio', 'img', 'canvas'
];

/**
* A list of elements which may affect the editing experience. To avoid this, those elements are replaced with
* `<span data-ck-unsafe-element="[element name]"></span>` while rendering in the editing mode.
*
* @readonly
* @member {Array.<String>} module:engine/view/domconverter~DomConverter#unsafeElements
*/
this.unsafeElements = [ 'script', 'style' ];

/**
* The DOM-to-view mapping.
*
Expand Down Expand Up @@ -1564,7 +1572,7 @@ export default class DomConverter {
_shouldRenameElement( elementName ) {
const name = elementName.toLowerCase();

return this.renderingMode === 'editing' && UNSAFE_ELEMENTS.includes( name );
return this.renderingMode === 'editing' && this.unsafeElements.includes( name );
}

/**
Expand Down
27 changes: 27 additions & 0 deletions packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,33 @@ describe( 'DomConverter', () => {
);
} );

it( 'should replace registered custom element with span and add special data attribute', () => {
const viewScript = new ViewElement( viewDocument, 'custom-foo-element' );
const viewText = new ViewText( viewDocument, 'foo' );
const viewP = new ViewElement( viewDocument, 'p', { class: 'foo' } );

viewP._appendChild( viewScript );
viewP._appendChild( viewText );

converter = new DomConverter( viewDocument, {
renderingMode: 'editing'
} );

converter.unsafeElements.push( 'custom-foo-element' );

const domP = converter.viewToDom( viewP, document );

expect( domP ).to.be.an.instanceof( HTMLElement );
expect( domP.tagName ).to.equal( 'P' );
expect( domP.getAttribute( 'class' ) ).to.equal( 'foo' );
expect( domP.attributes.length ).to.equal( 1 );

expect( domP.childNodes.length ).to.equal( 2 );
expect( domP.childNodes[ 0 ].tagName ).to.equal( 'SPAN' );
expect( domP.childNodes[ 0 ].getAttribute( 'data-ck-unsafe-element' ) ).to.equal( 'custom-foo-element' );
expect( domP.childNodes[ 1 ].data ).to.equal( 'foo' );
} );

describe( 'unsafe attribute names that were declaratively permitted', () => {
let writer;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,31 +107,61 @@ describe( 'DomConverter – whitespace handling – integration', () => {
} );

// Controversial result. See https://github.com/ckeditor/ckeditor5-engine/issues/987.
it( 'nbsp between blocks is not ignored (between paragraphs)', () => {
// https://github.com/ckeditor/ckeditor5/pull/11744/files#r871377976.
it( 'nbsp between blocks is ignored (between paragraphs)', () => {
editor.setData( '<p>foo</p>&nbsp;<p>bar</p>' );

expect( getData( editor.model, { withoutSelection: true } ) )
.to.equal( '<paragraph>foo</paragraph><paragraph> </paragraph><paragraph>bar</paragraph>' );
.to.equal( '<paragraph>foo</paragraph><paragraph>bar</paragraph>' );

expect( editor.getData() ).to.equal( '<p>foo</p><p>&nbsp;</p><p>bar</p>' );
expect( editor.getData() ).to.equal( '<p>foo</p><p>bar</p>' );
} );

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

expect( getData( editor.model, { withoutSelection: true } ) )
.to.equal(
'<block>foo</block>' +
'<paragraph> </paragraph>' +
'<paragraph>bar</paragraph>'
);

expect( editor.getData() )
.to.equal(
'<block>foo</block>' +
'<p>&nbsp;</p>' +
'<p>bar</p>'
);
} );

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

expect( getData( editor.model, { withoutSelection: true } ) )
.to.equal(
'<paragraph>foo</paragraph>' +
'<custom-foo-element>a</custom-foo-element>' +
'<custom-foo-element>b</custom-foo-element>' +
'<paragraph>bar</paragraph>'
);

expect( editor.getData() )
.to.equal(
'<p>foo</p>' +
'<custom-foo-element>a</custom-foo-element>' +
'<custom-foo-element>b</custom-foo-element>' +
'<p>bar</p>'
);
} );
Expand Down
4 changes: 3 additions & 1 deletion packages/ckeditor5-html-support/src/generalhtmlsupport.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import ScriptElementSupport from './integrations/script';
import TableElementSupport from './integrations/table';
import StyleElementSupport from './integrations/style';
import DocumentListElementSupport from './integrations/documentlist';
import CustomElementSupport from './integrations/customelement';

/**
* The General HTML Support feature.
Expand Down Expand Up @@ -51,7 +52,8 @@ export default class GeneralHtmlSupport extends Plugin {
ScriptElementSupport,
TableElementSupport,
StyleElementSupport,
DocumentListElementSupport
DocumentListElementSupport,
CustomElementSupport
];
}

Expand Down
7 changes: 7 additions & 0 deletions packages/ckeditor5-html-support/src/integrations/codeblock.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ export default class CodeBlockElementSupport extends Plugin {
return [ DataFilter ];
}

/**
* @inheritDoc
*/
static get pluginName() {
return 'CodeBlockElementSupport';
}

/**
* @inheritDoc
*/
Expand Down
162 changes: 162 additions & 0 deletions packages/ckeditor5-html-support/src/integrations/customelement.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
/**
* @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/customelement
*/

import { Plugin } from 'ckeditor5/src/core';
import { UpcastWriter } from 'ckeditor5/src/engine';

import DataSchema from '../dataschema';
import DataFilter from '../datafilter';
import { setViewAttributes } from '../conversionutils';

/**
* Provides the General HTML Support for custom elements (not registered in the {@link module:html-support/dataschema~DataSchema}).
*
* @extends module:core/plugin~Plugin
*/
export default class CustomElementSupport extends Plugin {
/**
* @inheritDoc
*/
static get requires() {
return [ DataFilter, DataSchema ];
}

/**
* @inheritDoc
*/
static get pluginName() {
return 'CustomElementSupport';
}

/**
* @inheritDoc
*/
init() {
const dataFilter = this.editor.plugins.get( DataFilter );
const dataSchema = this.editor.plugins.get( DataSchema );

dataFilter.on( 'register:$customElement', ( evt, definition ) => {
evt.stop();

const editor = this.editor;
const schema = editor.model.schema;
const conversion = editor.conversion;
const unsafeElements = editor.editing.view.domConverter.unsafeElements;
const preLikeElements = editor.data.htmlProcessor.domConverter.preElements;

schema.register( definition.model, definition.modelSchema );
schema.extend( definition.model, {
allowAttributes: [ 'htmlElementName', 'htmlAttributes', 'htmlContent' ],
isContent: true
} );

// Being executed on the low priority, it will catch all elements that were not caught by other converters.
conversion.for( 'upcast' ).elementToElement( {
view: /.*/,
model: ( viewElement, conversionApi ) => {
// Do not try to convert $comment fake element.
if ( viewElement.name == '$comment' ) {
return;
}

// Allow for fallback only if this element is not defined in data schema to make sure
// that this will handle only custom elements not registered in the data schema.
if ( dataSchema.getDefinitionsForView( viewElement.name ).size ) {
return;
}

// Make sure that this element will not render in the editing view.
if ( !unsafeElements.includes( viewElement.name ) ) {
unsafeElements.push( viewElement.name );
}

// Make sure that whitespaces will not be trimmed or replaced by nbsps while stringify content.
if ( !preLikeElements.includes( viewElement.name ) ) {
preLikeElements.push( viewElement.name );
}

const modelElement = conversionApi.writer.createElement( definition.model, {
htmlElementName: viewElement.name
} );

const htmlAttributes = dataFilter.processViewAttributes( viewElement, conversionApi );

if ( htmlAttributes ) {
conversionApi.writer.setAttribute( 'htmlAttributes', htmlAttributes, modelElement );
}

// Store the whole element in the attribute so that DomConverter will be able to use the pre like element context.
const viewWriter = new UpcastWriter( viewElement.document );
const documentFragment = viewWriter.createDocumentFragment( viewElement );
const htmlContent = editor.data.processor.toData( documentFragment );

conversionApi.writer.setAttribute( 'htmlContent', htmlContent, modelElement );

// Consume the content of the element.
for ( const { item } of editor.editing.view.createRangeIn( viewElement ) ) {
conversionApi.consumable.consume( item, { name: true } );
}

return modelElement;
},
converterPriority: 'low'
} );

// Because this element is unsafe (DomConverter#unsafeElements), it will render as a transparent <span> but it must
// be rendered anyway for the mapping between the model and the view to exist.
conversion.for( 'editingDowncast' ).elementToElement( {
model: {
name: definition.model,
attributes: [ 'htmlElementName', 'htmlAttributes', 'htmlContent' ]
},
view: ( modelElement, { writer } ) => {
const viewName = modelElement.getAttribute( 'htmlElementName' );
const viewElement = writer.createRawElement( viewName );

if ( modelElement.hasAttribute( 'htmlAttributes' ) ) {
setViewAttributes( writer, modelElement.getAttribute( 'htmlAttributes' ), viewElement );
}

return viewElement;
}
} );

conversion.for( 'dataDowncast' ).elementToElement( {
model: {
name: definition.model,
attributes: [ 'htmlElementName', 'htmlAttributes', 'htmlContent' ]
},
view: ( modelElement, { writer } ) => {
const viewName = modelElement.getAttribute( 'htmlElementName' );
const htmlContent = modelElement.getAttribute( 'htmlContent' );

const viewElement = writer.createRawElement( viewName, null, ( domElement, domConverter ) => {
domConverter.setContentOf( domElement, htmlContent );

// Unwrap the custom element content (it was stored in the attribute as the whole custom element).
// See the upcast conversion for the "htmlContent" attribute to learn more.
const customElement = domElement.firstChild;

customElement.remove();

while ( customElement.firstChild ) {
domElement.appendChild( customElement.firstChild );
}
} );

if ( modelElement.hasAttribute( 'htmlAttributes' ) ) {
setViewAttributes( writer, modelElement.getAttribute( 'htmlAttributes' ), viewElement );
}

return viewElement;
}
} );
} );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ export default class DocumentListElementSupport extends Plugin {
return [ DataFilter ];
}

/**
* @inheritDoc
*/
static get pluginName() {
return 'DocumentListElementSupport';
}

/**
* @inheritDoc
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ export default class DualContentModelElementSupport extends Plugin {
return [ DataFilter ];
}

/**
* @inheritDoc
*/
static get pluginName() {
return 'DualContentModelElementSupport';
}

/**
* @inheritDoc
*/
Expand Down
7 changes: 7 additions & 0 deletions packages/ckeditor5-html-support/src/integrations/heading.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ export default class HeadingElementSupport extends Plugin {
return [ DataSchema ];
}

/**
* @inheritDoc
*/
static get pluginName() {
return 'HeadingElementSupport';
}

/**
* @inheritDoc
*/
Expand Down
7 changes: 7 additions & 0 deletions packages/ckeditor5-html-support/src/integrations/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ export default class ImageElementSupport extends Plugin {
return [ DataFilter ];
}

/**
* @inheritDoc
*/
static get pluginName() {
return 'ImageElementSupport';
}

/**
* @inheritDoc
*/
Expand Down
Loading