Skip to content

Commit

Permalink
Merge pull request #11744 from ckeditor/ck/11432-ghs-custom-elements
Browse files Browse the repository at this point in the history
Feature (html-support): Custom elements should be preserved by the General HTML Support feature. Closes #11432.

Fix (engine): Whitespaces between block elements should not trigger auto-paragraphing. Closes #11248.
  • Loading branch information
oleq authored May 31, 2022
2 parents 82bf6c6 + fa2aa81 commit efd6f84
Show file tree
Hide file tree
Showing 26 changed files with 742 additions and 9 deletions.
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.
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

0 comments on commit efd6f84

Please sign in to comment.