From 37d57c489dcba71e0a0498c4c2b720818130c6e7 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Wed, 15 Jan 2025 12:44:20 +0100 Subject: [PATCH 01/39] Init `EmptyBlocks` plugin prototype. --- .../ckeditor5-html-support/src/emptyblocks.ts | 100 ++++++++++++++++++ packages/ckeditor5-html-support/src/index.ts | 1 + 2 files changed, 101 insertions(+) create mode 100644 packages/ckeditor5-html-support/src/emptyblocks.ts diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts new file mode 100644 index 00000000000..c64e947e912 --- /dev/null +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -0,0 +1,100 @@ +/** + * @license Copyright (c) 2003-2025, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options + */ + +/** + * @module html-support/emptyblocks + */ + +import { Plugin } from 'ckeditor5/src/core.js'; +import type { UpcastElementEvent, Element } from 'ckeditor5/src/engine.js'; + +const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; + +/** + * This plugin allows for preserving empty block elements in the editor content instead of + * automatically filling them with block fillers (` `). + * + * Empty elements are detected during upcast and marked with a special attribute. + * During downcast, elements with this attribute have their `getFillerOffset` set to `null` + * which prevents adding block fillers. + * + * This is useful when you want to: + * + * * Preserve empty block elements exactly as they were in the source HTML + * * Allow for styling empty blocks with CSS (block fillers can interfere with height/margin) + * * Maintain compatibility with external systems that expect empty blocks to remain empty + * + * For example, this allows for HTML like: + * + * ```html + *

+ *

+ * + * ``` + * to remain empty instead of being converted to: + * + * ```html + *

 

+ *

 

+ *   + * ``` + */ +export default class EmptyBlocks extends Plugin { + /** + * @inheritDoc + */ + public static get pluginName() { + return 'EmptyBlocks' as const; + } + + /** + * @inheritDoc + */ + public static override get isOfficialPlugin(): true { + return true; + } + + /** + * @inheritDoc + */ + public init(): void { + const editor = this.editor; + const schema = editor.model.schema; + + // Register the attribute for block elements + schema.extend( '$block', { + allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] + } ); + + // Upcast conversion - detect empty elements + editor.conversion.for( 'upcast' ).add( dispatcher => { + dispatcher.on( 'element', ( evt, data, conversionApi ) => { + const { viewItem, modelRange } = data; + + if ( !viewItem.is( 'element' ) || !viewItem.isEmpty ) { + return; + } + + const modelElement = modelRange?.start.nodeAfter as Element; + + if ( modelElement && schema.isBlock( modelElement ) ) { + conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); + } + }, { priority: 'lowest' } ); + } ); + + // Data downcast conversion - prevent filler in empty elements + editor.conversion.for( 'dataDowncast' ).add( dispatcher => { + dispatcher.on( `attribute:${ EMPTY_BLOCK_MODEL_ATTRIBUTE }`, ( evt, data, conversionApi ) => { + const { item } = data; + const viewElement = conversionApi.mapper.toViewElement( item as Element ); + + if ( viewElement && data.attributeNewValue ) { + viewElement.getFillerOffset = () => null; + } + }, { priority: 'highest' } ); + } ); + } +} diff --git a/packages/ckeditor5-html-support/src/index.ts b/packages/ckeditor5-html-support/src/index.ts index 9521a658101..f144069b87f 100644 --- a/packages/ckeditor5-html-support/src/index.ts +++ b/packages/ckeditor5-html-support/src/index.ts @@ -13,6 +13,7 @@ export { default as DataSchema, type DataSchemaBlockElementDefinition } from './ export { default as HtmlComment } from './htmlcomment.js'; export { default as FullPage } from './fullpage.js'; export { default as HtmlPageDataProcessor } from './htmlpagedataprocessor.js'; +export { default as EmptyBlocks } from './emptyblocks.js'; export type { GeneralHtmlSupportConfig } from './generalhtmlsupportconfig.js'; export type { default as CodeBlockElementSupport } from './integrations/codeblock.js'; export type { default as CustomElementSupport } from './integrations/customelement.js'; From 9efcd016238fb16db8e61cff70a07e8bf8401cbb Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Wed, 15 Jan 2025 13:42:08 +0100 Subject: [PATCH 02/39] Add tests. --- .../tests/emptyblocks.js | 173 ++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 packages/ckeditor5-html-support/tests/emptyblocks.js diff --git a/packages/ckeditor5-html-support/tests/emptyblocks.js b/packages/ckeditor5-html-support/tests/emptyblocks.js new file mode 100644 index 00000000000..0c39d01863d --- /dev/null +++ b/packages/ckeditor5-html-support/tests/emptyblocks.js @@ -0,0 +1,173 @@ +/** + * @license Copyright (c) 2003-2025, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options + */ + +/* global document */ + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js'; +import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; + +import EmptyBlocks from '../src/emptyblocks.js'; +import { toWidget, viewToModelPositionOutsideModelElement } from '@ckeditor/ckeditor5-widget'; + +describe( 'EmptyBlocks', () => { + let editor, model, element; + + beforeEach( async () => { + element = document.createElement( 'div' ); + document.body.appendChild( element ); + + editor = await ClassicTestEditor.create( element, { + plugins: [ Paragraph, EmptyBlocks ] + } ); + + model = editor.model; + } ); + + afterEach( async () => { + element.remove(); + await editor.destroy(); + } ); + + it( 'should be named', () => { + expect( EmptyBlocks.pluginName ).to.equal( 'EmptyBlocks' ); + } ); + + it( 'should have `isOfficialPlugin` static flag set to `true`', () => { + expect( EmptyBlocks.isOfficialPlugin ).to.be.true; + } ); + + it( 'should be loaded', () => { + expect( editor.plugins.get( EmptyBlocks ) ).to.be.instanceOf( EmptyBlocks ); + } ); + + describe( 'schema', () => { + it( 'should allow htmlEmptyBlock attribute on $block', () => { + expect( model.schema.checkAttribute( [ '$block' ], 'htmlEmptyBlock' ) ).to.be.true; + } ); + } ); + + describe( 'upcast conversion', () => { + it( 'should set htmlEmptyBlock attribute on empty paragraph', () => { + editor.setData( '

' ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + ); + } ); + + it( 'should not set htmlEmptyBlock attribute on non-empty paragraph', () => { + editor.setData( '

foo

' ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + 'foo' + ); + } ); + + it( 'should set htmlEmptyBlock attribute on paragraph with whitespace', () => { + editor.setData( '

' ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + ); + } ); + + it( 'should not set htmlEmptyBlock attribute on empty inline elements', () => { + registerInlinePlaceholderWidget(); + + editor.setData( + '

' + + 'Hello' + + '' + + 'World' + + '

' + ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + 'HelloWorld' + ); + } ); + } ); + + describe( 'downcast conversion', () => { + it( 'should not return anything if blank paragraph in model (as it used to do)', () => { + setModelData( model, '' ); + + expect( editor.getData() ).to.equal( '' ); + } ); + + it( 'should add filler to normal empty paragraph', () => { + setModelData( model, 'Hello' ); + + expect( editor.getData() ).to.equal( '

Hello

 

' ); + } ); + + it( 'should preserve multiple empty paragraphs', () => { + setModelData( + model, + 'Hello' + + '' + + '' + + '' + ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + 'Hello' + + '' + + '' + + '' + ); + + expect( editor.getData() ).to.equal( '

Hello

' ); + } ); + + it( 'should preserve empty paragraphs mixed with non-empty ones', () => { + editor.setData( '

foo

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

foo

' ); + } ); + } ); + + function registerInlinePlaceholderWidget() { + model.schema.register( 'placeholder', { + inheritAllFrom: '$inlineObject', + allowAttributes: [ 'name' ] + } ); + + model.schema.extend( '$text', { allowIn: 'placeholder' } ); + + editor.editing.mapper.on( + 'viewToModelPosition', + viewToModelPositionOutsideModelElement( model, viewElement => viewElement.hasClass( 'placeholder' ) ) + ); + + editor.conversion.for( 'upcast' ).elementToElement( { + view: { + name: 'span', + classes: [ 'placeholder' ] + }, + model: ( _, { writer } ) => writer.createElement( 'placeholder' ) + } ); + + editor.conversion.for( 'editingDowncast' ).elementToElement( { + model: 'placeholder', + view: ( _, { writer } ) => toWidget( + writer.createContainerElement( 'span', { class: 'placeholder' } ), + writer + ) + } ); + + editor.conversion.for( 'dataDowncast' ).elementToElement( { + model: 'placeholder', + view: ( _, { writer } ) => writer.createContainerElement( 'span', { class: 'placeholder' } ) + } ); + } +} ); From fe051c72fdb135562e60890bca81d9f73aef5da9 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Fri, 17 Jan 2025 07:10:29 +0100 Subject: [PATCH 03/39] Minor fixes in docs after CR. --- .../ckeditor5-html-support/src/emptyblocks.ts | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts index c64e947e912..5b185c77eef 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -16,15 +16,11 @@ const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; * This plugin allows for preserving empty block elements in the editor content instead of * automatically filling them with block fillers (` `). * - * Empty elements are detected during upcast and marked with a special attribute. - * During downcast, elements with this attribute have their `getFillerOffset` set to `null` - * which prevents adding block fillers. - * * This is useful when you want to: * * * Preserve empty block elements exactly as they were in the source HTML - * * Allow for styling empty blocks with CSS (block fillers can interfere with height/margin) - * * Maintain compatibility with external systems that expect empty blocks to remain empty + * * Allow for styling empty blocks with CSS (block fillers can interfere with height/margin) + * * Maintain compatibility with external systems that expect empty blocks to remain empty * * For example, this allows for HTML like: * @@ -63,12 +59,12 @@ export default class EmptyBlocks extends Plugin { const editor = this.editor; const schema = editor.model.schema; - // Register the attribute for block elements + // Register the attribute for block elements. schema.extend( '$block', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); - // Upcast conversion - detect empty elements + // Upcast conversion - detect empty elements. editor.conversion.for( 'upcast' ).add( dispatcher => { dispatcher.on( 'element', ( evt, data, conversionApi ) => { const { viewItem, modelRange } = data; @@ -77,15 +73,15 @@ export default class EmptyBlocks extends Plugin { return; } - const modelElement = modelRange?.start.nodeAfter as Element; + const modelElement = modelRange && modelRange.start.nodeAfter as Element; if ( modelElement && schema.isBlock( modelElement ) ) { conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); } - }, { priority: 'lowest' } ); + } ); } ); - // Data downcast conversion - prevent filler in empty elements + // Data downcast conversion - prevent filler in empty elements. editor.conversion.for( 'dataDowncast' ).add( dispatcher => { dispatcher.on( `attribute:${ EMPTY_BLOCK_MODEL_ATTRIBUTE }`, ( evt, data, conversionApi ) => { const { item } = data; @@ -94,7 +90,7 @@ export default class EmptyBlocks extends Plugin { if ( viewElement && data.attributeNewValue ) { viewElement.getFillerOffset = () => null; } - }, { priority: 'highest' } ); + } ); } ); } } From e57d7f58b636dc9315ab0987f61ca119d9ce3331 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Fri, 17 Jan 2025 07:22:34 +0100 Subject: [PATCH 04/39] Adjust checks --- .../ckeditor5-html-support/src/emptyblocks.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts index 5b185c77eef..4cdc50596ea 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -8,7 +8,7 @@ */ import { Plugin } from 'ckeditor5/src/core.js'; -import type { UpcastElementEvent, Element } from 'ckeditor5/src/engine.js'; +import type { UpcastElementEvent, Element, DowncastDispatcher } from 'ckeditor5/src/engine.js'; const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; @@ -59,11 +59,15 @@ export default class EmptyBlocks extends Plugin { const editor = this.editor; const schema = editor.model.schema; - // Register the attribute for block elements. + // Register the attribute for block and container elements. schema.extend( '$block', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); + schema.extend( '$container', { + allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] + } ); + // Upcast conversion - detect empty elements. editor.conversion.for( 'upcast' ).add( dispatcher => { dispatcher.on( 'element', ( evt, data, conversionApi ) => { @@ -75,14 +79,14 @@ export default class EmptyBlocks extends Plugin { const modelElement = modelRange && modelRange.start.nodeAfter as Element; - if ( modelElement && schema.isBlock( modelElement ) ) { + if ( modelElement && schema.checkAttribute( modelElement, EMPTY_BLOCK_MODEL_ATTRIBUTE ) ) { conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); } } ); } ); // Data downcast conversion - prevent filler in empty elements. - editor.conversion.for( 'dataDowncast' ).add( dispatcher => { + const downcastDispatcher = ( dispatcher: DowncastDispatcher ) => { dispatcher.on( `attribute:${ EMPTY_BLOCK_MODEL_ATTRIBUTE }`, ( evt, data, conversionApi ) => { const { item } = data; const viewElement = conversionApi.mapper.toViewElement( item as Element ); @@ -91,6 +95,9 @@ export default class EmptyBlocks extends Plugin { viewElement.getFillerOffset = () => null; } } ); - } ); + }; + + editor.conversion.for( 'dataDowncast' ).add( downcastDispatcher ); + editor.conversion.for( 'editingDowncast' ).add( downcastDispatcher ); } } From 5e61837d549ecbd8c7d3f989b82c57573100c3d8 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Fri, 17 Jan 2025 07:59:28 +0100 Subject: [PATCH 05/39] Add table tests. --- .../ckeditor5-html-support/src/emptyblocks.ts | 6 +++ .../tests/emptyblocks.js | 47 ++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts index 4cdc50596ea..f7e020d7c6c 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -68,6 +68,12 @@ export default class EmptyBlocks extends Plugin { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); + if ( schema.isRegistered( 'tableCell' ) ) { + schema.extend( 'tableCell', { + allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] + } ); + } + // Upcast conversion - detect empty elements. editor.conversion.for( 'upcast' ).add( dispatcher => { dispatcher.on( 'element', ( evt, data, conversionApi ) => { diff --git a/packages/ckeditor5-html-support/tests/emptyblocks.js b/packages/ckeditor5-html-support/tests/emptyblocks.js index 0c39d01863d..7f53efa2a66 100644 --- a/packages/ckeditor5-html-support/tests/emptyblocks.js +++ b/packages/ckeditor5-html-support/tests/emptyblocks.js @@ -7,6 +7,7 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js'; +import TableEditing from '@ckeditor/ckeditor5-table/src/tableediting.js'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; import EmptyBlocks from '../src/emptyblocks.js'; @@ -20,7 +21,7 @@ describe( 'EmptyBlocks', () => { document.body.appendChild( element ); editor = await ClassicTestEditor.create( element, { - plugins: [ Paragraph, EmptyBlocks ] + plugins: [ Paragraph, TableEditing, EmptyBlocks ] } ); model = editor.model; @@ -136,6 +137,50 @@ describe( 'EmptyBlocks', () => { } ); } ); + describe( 'table integration', () => { + it( 'should set htmlEmptyBlock attribute on empty table cell', () => { + editor.setData( '
' ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '
' + ); + } ); + + it( 'should preserve empty table cells in data output', () => { + editor.setData( '
foo
' ); + + expect( editor.getData() ).to.equal( + '
foo
' + ); + } ); + + it( 'should preserve empty cells mixed with non-empty ones', () => { + editor.setData( '
foobar
' ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + + '' + + 'foo' + + '' + + 'bar' + + '' + + '
' + ); + + expect( editor.getData() ).to.equal( + '
foobar
' + ); + } ); + + it( 'should not set htmlEmptyBlock attribute on table cell with whitespace', () => { + editor.setData( '
' ); + + expect( editor.getData() ).to.equal( + '
 
' + ); + } ); + } ); + function registerInlinePlaceholderWidget() { model.schema.register( 'placeholder', { inheritAllFrom: '$inlineObject', From 99cef9416a05b8a9cded6368d0a96077f5dcc758 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Fri, 17 Jan 2025 08:39:38 +0100 Subject: [PATCH 06/39] Table autoparagraphing fixes. --- .../tests/emptyblocks.js | 20 +++++++++++++++---- .../table-cell-paragraph-post-fixer.ts | 2 +- .../src/converters/upcasttable.ts | 4 +++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-html-support/tests/emptyblocks.js b/packages/ckeditor5-html-support/tests/emptyblocks.js index 7f53efa2a66..44fcfbe219a 100644 --- a/packages/ckeditor5-html-support/tests/emptyblocks.js +++ b/packages/ckeditor5-html-support/tests/emptyblocks.js @@ -142,7 +142,7 @@ describe( 'EmptyBlocks', () => { editor.setData( '
' ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '
' + '
' ); } ); @@ -161,7 +161,7 @@ describe( 'EmptyBlocks', () => { '' + '' + 'foo' + - '' + + '' + 'bar' + '' + '
' @@ -172,11 +172,23 @@ describe( 'EmptyBlocks', () => { ); } ); - it( 'should not set htmlEmptyBlock attribute on table cell with whitespace', () => { + it( 'should set htmlEmptyBlock attribute on table cell with whitespace', () => { editor.setData( '
' ); expect( editor.getData() ).to.equal( - '
 
' + '
' + ); + } ); + + it( 'should not add auto paragraphs to empty table cells with htmlEmptyBlock', () => { + editor.setData( '
' ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '
' + ); + + expect( editor.getData() ).to.equal( + '
' ); } ); } ); diff --git a/packages/ckeditor5-table/src/converters/table-cell-paragraph-post-fixer.ts b/packages/ckeditor5-table/src/converters/table-cell-paragraph-post-fixer.ts index 4b0fead7782..b2db02c531e 100644 --- a/packages/ckeditor5-table/src/converters/table-cell-paragraph-post-fixer.ts +++ b/packages/ckeditor5-table/src/converters/table-cell-paragraph-post-fixer.ts @@ -100,7 +100,7 @@ function fixTableRow( tableRow: Element, writer: Writer ) { */ function fixTableCellContent( tableCell: Element, writer: Writer ) { // Insert paragraph to an empty table cell. - if ( tableCell.childCount == 0 ) { + if ( tableCell.childCount == 0 && !tableCell.hasAttribute( 'htmlEmptyBlock' ) ) { // @if CK_DEBUG_TABLE // console.log( 'Post-fixing table: insert paragraph in empty cell.' ); writer.insertElement( 'paragraph', tableCell ); diff --git a/packages/ckeditor5-table/src/converters/upcasttable.ts b/packages/ckeditor5-table/src/converters/upcasttable.ts index aed34ca13ab..ba24d372343 100644 --- a/packages/ckeditor5-table/src/converters/upcasttable.ts +++ b/packages/ckeditor5-table/src/converters/upcasttable.ts @@ -161,7 +161,9 @@ export function ensureParagraphInTableCell( elementName: string ) { // Ensure a paragraph in the model for empty table cells for converted table cells. if ( data.viewItem.isEmpty ) { - writer.insertElement( 'paragraph', modelCursor ); + if ( !tableCell.hasAttribute( 'htmlEmptyBlock' ) ) { + writer.insertElement( 'paragraph', modelCursor ); + } return; } From 2e5c624adf4d919cbda1bc52b8e91fe2dd10c2d7 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Fri, 17 Jan 2025 10:00:27 +0100 Subject: [PATCH 07/39] Add more tests. --- .../ckeditor5-html-support/src/emptyblocks.ts | 20 +++-- .../tests/emptyblocks.js | 78 ++++++++++++++++++- 2 files changed, 89 insertions(+), 9 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts index f7e020d7c6c..98069f07b20 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -68,14 +68,20 @@ export default class EmptyBlocks extends Plugin { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); - if ( schema.isRegistered( 'tableCell' ) ) { - schema.extend( 'tableCell', { - allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] - } ); - } - // Upcast conversion - detect empty elements. editor.conversion.for( 'upcast' ).add( dispatcher => { + // Prevent table cells from being filled with empty paragraphs. + // It has slightly higher priority to ensure that it runs before the next handler. + dispatcher.on( 'element', ( evt, data, conversionApi ) => { + const { viewItem, modelRange } = data; + const modelElement = modelRange && modelRange.start.nodeAfter as Element; + + if ( modelElement && modelElement.name === 'tableCell' && viewItem.isEmpty ) { + conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); + } + } ); + + // Handle other empty elements. dispatcher.on( 'element', ( evt, data, conversionApi ) => { const { viewItem, modelRange } = data; @@ -88,7 +94,7 @@ export default class EmptyBlocks extends Plugin { if ( modelElement && schema.checkAttribute( modelElement, EMPTY_BLOCK_MODEL_ATTRIBUTE ) ) { conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); } - } ); + }, { priority: 'lowest' } ); } ); // Data downcast conversion - prevent filler in empty elements. diff --git a/packages/ckeditor5-html-support/tests/emptyblocks.js b/packages/ckeditor5-html-support/tests/emptyblocks.js index 44fcfbe219a..78c35240115 100644 --- a/packages/ckeditor5-html-support/tests/emptyblocks.js +++ b/packages/ckeditor5-html-support/tests/emptyblocks.js @@ -8,23 +8,28 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js'; import TableEditing from '@ckeditor/ckeditor5-table/src/tableediting.js'; +import Heading from '@ckeditor/ckeditor5-heading/src/heading.js'; +import ListEditing from '@ckeditor/ckeditor5-list/src/list/listediting.js'; +import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote.js'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; +import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js'; import EmptyBlocks from '../src/emptyblocks.js'; import { toWidget, viewToModelPositionOutsideModelElement } from '@ckeditor/ckeditor5-widget'; describe( 'EmptyBlocks', () => { - let editor, model, element; + let editor, model, element, view; beforeEach( async () => { element = document.createElement( 'div' ); document.body.appendChild( element ); editor = await ClassicTestEditor.create( element, { - plugins: [ Paragraph, TableEditing, EmptyBlocks ] + plugins: [ Paragraph, TableEditing, EmptyBlocks, Heading, ListEditing, BlockQuote ] } ); model = editor.model; + view = editor.editing.view; } ); afterEach( async () => { @@ -45,9 +50,23 @@ describe( 'EmptyBlocks', () => { } ); describe( 'schema', () => { + it( 'should allow htmlEmptyBlock attribute on block elements', () => { + expect( model.schema.checkAttribute( [ 'paragraph' ], 'htmlEmptyBlock' ) ).to.be.true; + expect( model.schema.checkAttribute( [ 'heading1' ], 'htmlEmptyBlock' ) ).to.be.true; + } ); + + it( 'should not allow htmlEmptyBlock attribute on inline elements', () => { + model.schema.register( 'testInline', { isInline: true } ); + expect( model.schema.checkAttribute( [ 'testInline' ], 'htmlEmptyBlock' ) ).to.be.false; + } ); + it( 'should allow htmlEmptyBlock attribute on $block', () => { expect( model.schema.checkAttribute( [ '$block' ], 'htmlEmptyBlock' ) ).to.be.true; } ); + + it( 'should allow htmlEmptyBlock attribute on $container', () => { + expect( model.schema.checkAttribute( [ '$container' ], 'htmlEmptyBlock' ) ).to.be.true; + } ); } ); describe( 'upcast conversion', () => { @@ -193,6 +212,61 @@ describe( 'EmptyBlocks', () => { } ); } ); + describe( 'editing pipeline', () => { + it( 'should preserve empty paragraph in editing view', () => { + setModelData( model, '' ); + + expect( getViewData( view, { withoutSelection: true } ) ).to.equal( '

' ); + } ); + } ); + + describe( 'other block elements', () => { + it( 'should set htmlEmptyBlock on empty heading', () => { + editor.setData( '

' ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + ); + } ); + + it( 'should preserve empty heading in output', () => { + editor.setData( 'A

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

A

' ); + } ); + + it( 'should set htmlEmptyBlock on empty list item', () => { + editor.setData( '
' ); + + const modelData = getModelData( model, { withoutSelection: true } ); + const normalizedData = modelData.replace( / listItemId="[^"]+"/g, '' ); + + expect( normalizedData ) + .to.equal( '' ); + } ); + + it( 'should preserve mixed empty and non-empty block elements', () => { + editor.setData( '

foo

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

foo

' ); + } ); + + it( 'should handle nested empty blocks', () => { + editor.setData( '

A

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

A

' ); + } ); + } ); + function registerInlinePlaceholderWidget() { model.schema.register( 'placeholder', { inheritAllFrom: '$inlineObject', From 19c2a63b30e06c935ed3deed1efb0e2e5cf78408 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Fri, 17 Jan 2025 10:26:04 +0100 Subject: [PATCH 08/39] Code align improvements. --- .../ckeditor5-html-support/src/emptyblocks.ts | 47 ++++++++++++++----- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts index 98069f07b20..f56a4e43da9 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -56,6 +56,13 @@ export default class EmptyBlocks extends Plugin { * @inheritDoc */ public init(): void { + this._registerConverters(); + } + + /** + * Registers converters for handling empty block elements. + */ + private _registerConverters(): void { const editor = this.editor; const schema = editor.model.schema; @@ -68,20 +75,10 @@ export default class EmptyBlocks extends Plugin { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); - // Upcast conversion - detect empty elements. - editor.conversion.for( 'upcast' ).add( dispatcher => { - // Prevent table cells from being filled with empty paragraphs. - // It has slightly higher priority to ensure that it runs before the next handler. - dispatcher.on( 'element', ( evt, data, conversionApi ) => { - const { viewItem, modelRange } = data; - const modelElement = modelRange && modelRange.start.nodeAfter as Element; + // Upcast conversion - detect empty block elements and mark them as empty. + this._registerTableConverters(); - if ( modelElement && modelElement.name === 'tableCell' && viewItem.isEmpty ) { - conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); - } - } ); - - // Handle other empty elements. + editor.conversion.for( 'upcast' ).add( dispatcher => { dispatcher.on( 'element', ( evt, data, conversionApi ) => { const { viewItem, modelRange } = data; @@ -112,4 +109,28 @@ export default class EmptyBlocks extends Plugin { editor.conversion.for( 'dataDowncast' ).add( downcastDispatcher ); editor.conversion.for( 'editingDowncast' ).add( downcastDispatcher ); } + + /** + * Registers converters for handling empty table cells. + */ + private _registerTableConverters(): void { + const editor = this.editor; + + // Upcast conversion - detect empty table cells. + function registerTableCellUpcast( elementName: string ) { + editor.conversion.for( 'upcast' ).add( dispatcher => { + dispatcher.on( `element:${ elementName }`, ( evt, data, conversionApi ) => { + const { viewItem, modelRange } = data; + const modelElement = modelRange && modelRange.start.nodeAfter as Element; + + if ( modelElement && viewItem.isEmpty ) { + conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); + } + } ); + } ); + } + + registerTableCellUpcast( 'td' ); + registerTableCellUpcast( 'th' ); + } } From e6a035e6556cfbc8289c836a0fe373638a3b3c44 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 20 Jan 2025 09:43:33 +0100 Subject: [PATCH 09/39] Add consume downcasted attributes. --- .../ckeditor5-html-support/src/emptyblocks.ts | 10 ++++++++-- .../tests/emptyblocks.js | 19 ++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts index f56a4e43da9..42a21b02585 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -55,7 +55,7 @@ export default class EmptyBlocks extends Plugin { /** * @inheritDoc */ - public init(): void { + public afterInit(): void { this._registerConverters(); } @@ -97,8 +97,14 @@ export default class EmptyBlocks extends Plugin { // Data downcast conversion - prevent filler in empty elements. const downcastDispatcher = ( dispatcher: DowncastDispatcher ) => { dispatcher.on( `attribute:${ EMPTY_BLOCK_MODEL_ATTRIBUTE }`, ( evt, data, conversionApi ) => { + const { mapper, consumable } = conversionApi; const { item } = data; - const viewElement = conversionApi.mapper.toViewElement( item as Element ); + + if ( !consumable.consume( item, evt.name ) ) { + return; + } + + const viewElement = mapper.toViewElement( item as Element ); if ( viewElement && data.attributeNewValue ) { viewElement.getFillerOffset = () => null; diff --git a/packages/ckeditor5-html-support/tests/emptyblocks.js b/packages/ckeditor5-html-support/tests/emptyblocks.js index 78c35240115..88cda931b3e 100644 --- a/packages/ckeditor5-html-support/tests/emptyblocks.js +++ b/packages/ckeditor5-html-support/tests/emptyblocks.js @@ -111,7 +111,7 @@ describe( 'EmptyBlocks', () => { } ); } ); - describe( 'downcast conversion', () => { + describe( 'data downcast conversion', () => { it( 'should not return anything if blank paragraph in model (as it used to do)', () => { setModelData( model, '' ); @@ -154,6 +154,23 @@ describe( 'EmptyBlocks', () => { expect( editor.getData() ).to.equal( '

foo

' ); } ); + + it( 'should not set `getFillerOffset` if element is already consumed', () => { + editor.setData( '

foo

' ); + + editor.conversion.for( 'dataDowncast' ).add( dispatcher => { + dispatcher.on( 'attribute:htmlEmptyBlock', ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, 'attribute:htmlEmptyBlock' ); + }, { priority: 'highest' } ); + } ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + + 'foo' + ); + + expect( editor.getData() ).to.equal( '

 

foo

' ); + } ); } ); describe( 'table integration', () => { From df9a25b276766f0eeefbbcbe296dfb036c5c42d4 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 20 Jan 2025 10:08:50 +0100 Subject: [PATCH 10/39] Better code split. --- .../ckeditor5-html-support/src/emptyblocks.ts | 150 ++++++++++-------- 1 file changed, 86 insertions(+), 64 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts index 42a21b02585..8ffc7150b46 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -7,8 +7,9 @@ * @module html-support/emptyblocks */ -import { Plugin } from 'ckeditor5/src/core.js'; -import type { UpcastElementEvent, Element, DowncastDispatcher } from 'ckeditor5/src/engine.js'; +import type { PriorityString } from 'ckeditor5/src/utils.js'; +import { Plugin, type Editor } from 'ckeditor5/src/core.js'; +import type { UpcastElementEvent, Element, DowncastDispatcher, UpcastDispatcher } from 'ckeditor5/src/engine.js'; const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; @@ -56,17 +57,29 @@ export default class EmptyBlocks extends Plugin { * @inheritDoc */ public afterInit(): void { - this._registerConverters(); + this._registerDowncastConverters(); + this._registerUpcastConverters(); } /** - * Registers converters for handling empty block elements. + * Registers downcast converters for empty blocks handling. + * Sets up converters for both data and editing pipelines to prevent fillers in empty elements. */ - private _registerConverters(): void { - const editor = this.editor; + private _registerDowncastConverters(): void { + const { editor } = this; + + editor.conversion.for( 'dataDowncast' ).add( createEmptyBlocksDowncastDispatcher() ); + editor.conversion.for( 'editingDowncast' ).add( createEmptyBlocksDowncastDispatcher() ); + } + + /** + * Registers upcast converters for empty blocks handling. + * Extends schema to allow empty block attributes and sets up conversion for empty elements and table cells. + */ + private _registerUpcastConverters(): void { + const { editor } = this; const schema = editor.model.schema; - // Register the attribute for block and container elements. schema.extend( '$block', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); @@ -75,68 +88,77 @@ export default class EmptyBlocks extends Plugin { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); - // Upcast conversion - detect empty block elements and mark them as empty. - this._registerTableConverters(); - - editor.conversion.for( 'upcast' ).add( dispatcher => { - dispatcher.on( 'element', ( evt, data, conversionApi ) => { - const { viewItem, modelRange } = data; - - if ( !viewItem.is( 'element' ) || !viewItem.isEmpty ) { - return; - } - - const modelElement = modelRange && modelRange.start.nodeAfter as Element; + editor.conversion + .for( 'upcast' ) + .add( createEmptyBlocksUpcastDispatcher( editor, 'element', 'lowest' ) ); - if ( modelElement && schema.checkAttribute( modelElement, EMPTY_BLOCK_MODEL_ATTRIBUTE ) ) { - conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); - } - }, { priority: 'lowest' } ); - } ); - - // Data downcast conversion - prevent filler in empty elements. - const downcastDispatcher = ( dispatcher: DowncastDispatcher ) => { - dispatcher.on( `attribute:${ EMPTY_BLOCK_MODEL_ATTRIBUTE }`, ( evt, data, conversionApi ) => { - const { mapper, consumable } = conversionApi; - const { item } = data; + if ( schema.isRegistered( 'tableCell' ) ) { + schema.extend( 'tableCell', { + allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] + } ); - if ( !consumable.consume( item, evt.name ) ) { - return; - } + editor.conversion + .for( 'upcast' ) + .add( createEmptyBlocksUpcastDispatcher( editor, 'element:td' ) ) + .add( createEmptyBlocksUpcastDispatcher( editor, 'element:th' ) ); + } + } +} - const viewElement = mapper.toViewElement( item as Element ); +/** + * Creates a downcast dispatcher for handling empty blocks. + * The dispatcher prevents filler elements from being added to elements marked as empty blocks. + * + * @returns A function that sets up the downcast conversion dispatcher. + */ +function createEmptyBlocksDowncastDispatcher() { + return ( dispatcher: DowncastDispatcher ) => { + dispatcher.on( `attribute:${ EMPTY_BLOCK_MODEL_ATTRIBUTE }`, ( evt, data, conversionApi ) => { + const { mapper, consumable } = conversionApi; + const { item } = data; - if ( viewElement && data.attributeNewValue ) { - viewElement.getFillerOffset = () => null; - } - } ); - }; + if ( !consumable.consume( item, evt.name ) ) { + return; + } - editor.conversion.for( 'dataDowncast' ).add( downcastDispatcher ); - editor.conversion.for( 'editingDowncast' ).add( downcastDispatcher ); - } + const viewElement = mapper.toViewElement( item as Element ); - /** - * Registers converters for handling empty table cells. - */ - private _registerTableConverters(): void { - const editor = this.editor; - - // Upcast conversion - detect empty table cells. - function registerTableCellUpcast( elementName: string ) { - editor.conversion.for( 'upcast' ).add( dispatcher => { - dispatcher.on( `element:${ elementName }`, ( evt, data, conversionApi ) => { - const { viewItem, modelRange } = data; - const modelElement = modelRange && modelRange.start.nodeAfter as Element; - - if ( modelElement && viewItem.isEmpty ) { - conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); - } - } ); - } ); - } + if ( viewElement && data.attributeNewValue ) { + viewElement.getFillerOffset = () => null; + } + } ); + }; +} - registerTableCellUpcast( 'td' ); - registerTableCellUpcast( 'th' ); - } +/** + * Creates an upcast dispatcher for handling empty blocks. + * The dispatcher detects empty elements and marks them with the empty block attribute. + * + * @param editor - The editor instance. + * @param eventName - The event name to listen to during upcast conversion. + * @param priority - The priority of the conversion callback. + * @returns A function that sets up the upcast conversion dispatcher. + */ +function createEmptyBlocksUpcastDispatcher( + editor: Editor, + eventName: 'element' | `element:${ string }`, + priority: PriorityString = 'normal' +) { + const { schema } = editor.model; + + return ( dispatcher: UpcastDispatcher ) => { + dispatcher.on( eventName, ( evt, data, conversionApi ) => { + const { viewItem, modelRange } = data; + + if ( !viewItem.is( 'element' ) || !viewItem.isEmpty ) { + return; + } + + const modelElement = modelRange && modelRange.start.nodeAfter as Element; + + if ( modelElement && schema.checkAttribute( modelElement, EMPTY_BLOCK_MODEL_ATTRIBUTE ) && viewItem.isEmpty ) { + conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); + } + }, { priority } ); + }; } From 581252f010de1f9b3eccf5fbf4c66f151f224d83 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 20 Jan 2025 10:22:35 +0100 Subject: [PATCH 11/39] Improve priorities. --- packages/ckeditor5-html-support/src/emptyblocks.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts index 8ffc7150b46..8103760f229 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -7,7 +7,7 @@ * @module html-support/emptyblocks */ -import type { PriorityString } from 'ckeditor5/src/utils.js'; +import { priorities, type PriorityString } from 'ckeditor5/src/utils.js'; import { Plugin, type Editor } from 'ckeditor5/src/core.js'; import type { UpcastElementEvent, Element, DowncastDispatcher, UpcastDispatcher } from 'ckeditor5/src/engine.js'; @@ -99,8 +99,8 @@ export default class EmptyBlocks extends Plugin { editor.conversion .for( 'upcast' ) - .add( createEmptyBlocksUpcastDispatcher( editor, 'element:td' ) ) - .add( createEmptyBlocksUpcastDispatcher( editor, 'element:th' ) ); + .add( createEmptyBlocksUpcastDispatcher( editor, 'element:td', priorities.low + 1 ) ) + .add( createEmptyBlocksUpcastDispatcher( editor, 'element:th', priorities.low + 1 ) ); } } } @@ -142,7 +142,7 @@ function createEmptyBlocksDowncastDispatcher() { function createEmptyBlocksUpcastDispatcher( editor: Editor, eventName: 'element' | `element:${ string }`, - priority: PriorityString = 'normal' + priority: PriorityString ) { const { schema } = editor.model; From 182a3511c36eeb26dc5b9809f99d138d20c9ed6c Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 20 Jan 2025 10:58:55 +0100 Subject: [PATCH 12/39] Add data checks for list item tests. --- .../tests/emptyblocks.js | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-html-support/tests/emptyblocks.js b/packages/ckeditor5-html-support/tests/emptyblocks.js index 88cda931b3e..daba2521f8e 100644 --- a/packages/ckeditor5-html-support/tests/emptyblocks.js +++ b/packages/ckeditor5-html-support/tests/emptyblocks.js @@ -229,6 +229,22 @@ describe( 'EmptyBlocks', () => { } ); } ); + describe( 'lists integration', () => { + it( 'should set htmlEmptyBlock on empty list item', () => { + editor.setData( 'A
' ); + + const modelData = getModelData( model, { withoutSelection: true } ); + const normalizedData = modelData.replace( / listItemId="[^"]+"/g, '' ); + + expect( normalizedData ).to.equal( + 'A' + + '' + ); + + expect( editor.getData() ).to.equal( '

A

' ); + } ); + } ); + describe( 'editing pipeline', () => { it( 'should preserve empty paragraph in editing view', () => { setModelData( model, '' ); @@ -251,16 +267,6 @@ describe( 'EmptyBlocks', () => { expect( editor.getData() ).to.equal( '

A

' ); } ); - it( 'should set htmlEmptyBlock on empty list item', () => { - editor.setData( '
' ); - - const modelData = getModelData( model, { withoutSelection: true } ); - const normalizedData = modelData.replace( / listItemId="[^"]+"/g, '' ); - - expect( normalizedData ) - .to.equal( '' ); - } ); - it( 'should preserve mixed empty and non-empty block elements', () => { editor.setData( '

foo

' ); From 9a930db7aad259b49aa1a3ca83660dafc4d988c1 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 20 Jan 2025 11:44:54 +0100 Subject: [PATCH 13/39] Apply table CR remarks. --- .../ckeditor5-html-support/src/emptyblocks.ts | 104 +++++++++++------- .../tests/emptyblocks.js | 10 +- .../table-cell-paragraph-post-fixer.ts | 2 +- .../src/converters/upcasttable.ts | 4 +- 4 files changed, 72 insertions(+), 48 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts index 8103760f229..76dbfea290e 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -7,9 +7,16 @@ * @module html-support/emptyblocks */ -import { priorities, type PriorityString } from 'ckeditor5/src/utils.js'; import { Plugin, type Editor } from 'ckeditor5/src/core.js'; -import type { UpcastElementEvent, Element, DowncastDispatcher, UpcastDispatcher } from 'ckeditor5/src/engine.js'; +import type { + UpcastElementEvent, + Element, + DowncastDispatcher, + UpcastDispatcher, + DowncastAttributeEvent, + ElementCreatorFunction, + Node +} from 'ckeditor5/src/engine.js'; const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; @@ -57,28 +64,8 @@ export default class EmptyBlocks extends Plugin { * @inheritDoc */ public afterInit(): void { - this._registerDowncastConverters(); - this._registerUpcastConverters(); - } - - /** - * Registers downcast converters for empty blocks handling. - * Sets up converters for both data and editing pipelines to prevent fillers in empty elements. - */ - private _registerDowncastConverters(): void { - const { editor } = this; - - editor.conversion.for( 'dataDowncast' ).add( createEmptyBlocksDowncastDispatcher() ); - editor.conversion.for( 'editingDowncast' ).add( createEmptyBlocksDowncastDispatcher() ); - } - - /** - * Registers upcast converters for empty blocks handling. - * Extends schema to allow empty block attributes and sets up conversion for empty elements and table cells. - */ - private _registerUpcastConverters(): void { - const { editor } = this; - const schema = editor.model.schema; + const { model, conversion } = this.editor; + const schema = model.schema; schema.extend( '$block', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] @@ -88,23 +75,55 @@ export default class EmptyBlocks extends Plugin { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); - editor.conversion - .for( 'upcast' ) - .add( createEmptyBlocksUpcastDispatcher( editor, 'element', 'lowest' ) ); + // Downcasts. + conversion.for( 'dataDowncast' ).add( createEmptyBlocksDowncastDispatcher() ); + conversion.for( 'editingDowncast' ).add( createEmptyBlocksDowncastDispatcher() ); + // Upcasts. + conversion.for( 'upcast' ).add( createEmptyBlocksUpcastDispatcher( this.editor ) ); + + // Table related converters. if ( schema.isRegistered( 'tableCell' ) ) { schema.extend( 'tableCell', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); - editor.conversion - .for( 'upcast' ) - .add( createEmptyBlocksUpcastDispatcher( editor, 'element:td', priorities.low + 1 ) ) - .add( createEmptyBlocksUpcastDispatcher( editor, 'element:th', priorities.low + 1 ) ); + conversion.for( 'dataDowncast' ).elementToElement( { + model: 'paragraph', + view: convertEmptyBlockParagraphInTableCell(), + converterPriority: 'highest' + } ); } } } +/** + * Converts paragraphs in empty table cells during the downcast conversion. + */ +function convertEmptyBlockParagraphInTableCell(): ElementCreatorFunction { + return ( modelElement, { writer } ) => { + const parentCell = modelElement.parent; + + if ( !parentCell!.is( 'element', 'tableCell' ) ) { + return null; + } + + if ( parentCell.childCount != 1 || + !parentCell.hasAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE ) || + hasAnyAttribute( modelElement ) + ) { + return null; + } + + const viewElement = writer.createContainerElement( 'p' ); + + viewElement.getFillerOffset = () => null; + writer.setCustomProperty( 'dataPipeline:transparentRendering', true, viewElement ); + + return viewElement; + }; +} + /** * Creates a downcast dispatcher for handling empty blocks. * The dispatcher prevents filler elements from being added to elements marked as empty blocks. @@ -113,7 +132,7 @@ export default class EmptyBlocks extends Plugin { */ function createEmptyBlocksDowncastDispatcher() { return ( dispatcher: DowncastDispatcher ) => { - dispatcher.on( `attribute:${ EMPTY_BLOCK_MODEL_ATTRIBUTE }`, ( evt, data, conversionApi ) => { + dispatcher.on>( `attribute:${ EMPTY_BLOCK_MODEL_ATTRIBUTE }`, ( evt, data, conversionApi ) => { const { mapper, consumable } = conversionApi; const { item } = data; @@ -135,19 +154,13 @@ function createEmptyBlocksDowncastDispatcher() { * The dispatcher detects empty elements and marks them with the empty block attribute. * * @param editor - The editor instance. - * @param eventName - The event name to listen to during upcast conversion. - * @param priority - The priority of the conversion callback. * @returns A function that sets up the upcast conversion dispatcher. */ -function createEmptyBlocksUpcastDispatcher( - editor: Editor, - eventName: 'element' | `element:${ string }`, - priority: PriorityString -) { +function createEmptyBlocksUpcastDispatcher( editor: Editor ) { const { schema } = editor.model; return ( dispatcher: UpcastDispatcher ) => { - dispatcher.on( eventName, ( evt, data, conversionApi ) => { + dispatcher.on( 'element', ( evt, data, conversionApi ) => { const { viewItem, modelRange } = data; if ( !viewItem.is( 'element' ) || !viewItem.isEmpty ) { @@ -159,6 +172,15 @@ function createEmptyBlocksUpcastDispatcher( if ( modelElement && schema.checkAttribute( modelElement, EMPTY_BLOCK_MODEL_ATTRIBUTE ) && viewItem.isEmpty ) { conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); } - }, { priority } ); + }, { priority: 'lowest' } ); }; } + +/** + * Checks if an element has any attributes set. + */ +function hasAnyAttribute( element: Node ): boolean { + const iteratorItem = element.getAttributeKeys().next(); + + return !iteratorItem.done; +} diff --git a/packages/ckeditor5-html-support/tests/emptyblocks.js b/packages/ckeditor5-html-support/tests/emptyblocks.js index daba2521f8e..507af0a7547 100644 --- a/packages/ckeditor5-html-support/tests/emptyblocks.js +++ b/packages/ckeditor5-html-support/tests/emptyblocks.js @@ -178,7 +178,7 @@ describe( 'EmptyBlocks', () => { editor.setData( '
' ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '
' + '
' ); } ); @@ -197,7 +197,7 @@ describe( 'EmptyBlocks', () => { '' + '' + 'foo' + - '' + + '' + 'bar' + '' + '
' @@ -220,7 +220,11 @@ describe( 'EmptyBlocks', () => { editor.setData( '
' ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '
' + '' + + '' + + '' + + '' + + '
' ); expect( editor.getData() ).to.equal( diff --git a/packages/ckeditor5-table/src/converters/table-cell-paragraph-post-fixer.ts b/packages/ckeditor5-table/src/converters/table-cell-paragraph-post-fixer.ts index b2db02c531e..4b0fead7782 100644 --- a/packages/ckeditor5-table/src/converters/table-cell-paragraph-post-fixer.ts +++ b/packages/ckeditor5-table/src/converters/table-cell-paragraph-post-fixer.ts @@ -100,7 +100,7 @@ function fixTableRow( tableRow: Element, writer: Writer ) { */ function fixTableCellContent( tableCell: Element, writer: Writer ) { // Insert paragraph to an empty table cell. - if ( tableCell.childCount == 0 && !tableCell.hasAttribute( 'htmlEmptyBlock' ) ) { + if ( tableCell.childCount == 0 ) { // @if CK_DEBUG_TABLE // console.log( 'Post-fixing table: insert paragraph in empty cell.' ); writer.insertElement( 'paragraph', tableCell ); diff --git a/packages/ckeditor5-table/src/converters/upcasttable.ts b/packages/ckeditor5-table/src/converters/upcasttable.ts index ba24d372343..aed34ca13ab 100644 --- a/packages/ckeditor5-table/src/converters/upcasttable.ts +++ b/packages/ckeditor5-table/src/converters/upcasttable.ts @@ -161,9 +161,7 @@ export function ensureParagraphInTableCell( elementName: string ) { // Ensure a paragraph in the model for empty table cells for converted table cells. if ( data.viewItem.isEmpty ) { - if ( !tableCell.hasAttribute( 'htmlEmptyBlock' ) ) { - writer.insertElement( 'paragraph', modelCursor ); - } + writer.insertElement( 'paragraph', modelCursor ); return; } From 8187cc91d370af4128ff1050c29313f8b5642185 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 20 Jan 2025 13:31:50 +0100 Subject: [PATCH 14/39] Adjust docs. --- .../ckeditor5-html-support/src/emptyblocks.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts index 76dbfea290e..2f3c103cb04 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -76,11 +76,11 @@ export default class EmptyBlocks extends Plugin { } ); // Downcasts. - conversion.for( 'dataDowncast' ).add( createEmptyBlocksDowncastDispatcher() ); - conversion.for( 'editingDowncast' ).add( createEmptyBlocksDowncastDispatcher() ); + conversion.for( 'dataDowncast' ).add( createEmptyBlocksDowncastConverter() ); + conversion.for( 'editingDowncast' ).add( createEmptyBlocksDowncastConverter() ); // Upcasts. - conversion.for( 'upcast' ).add( createEmptyBlocksUpcastDispatcher( this.editor ) ); + conversion.for( 'upcast' ).add( createEmptyBlocksUpcastConverter( this.editor ) ); // Table related converters. if ( schema.isRegistered( 'tableCell' ) ) { @@ -125,12 +125,12 @@ function convertEmptyBlockParagraphInTableCell(): ElementCreatorFunction { } /** - * Creates a downcast dispatcher for handling empty blocks. + * Creates a downcast converter for handling empty blocks. * The dispatcher prevents filler elements from being added to elements marked as empty blocks. * * @returns A function that sets up the downcast conversion dispatcher. */ -function createEmptyBlocksDowncastDispatcher() { +function createEmptyBlocksDowncastConverter() { return ( dispatcher: DowncastDispatcher ) => { dispatcher.on>( `attribute:${ EMPTY_BLOCK_MODEL_ATTRIBUTE }`, ( evt, data, conversionApi ) => { const { mapper, consumable } = conversionApi; @@ -150,13 +150,13 @@ function createEmptyBlocksDowncastDispatcher() { } /** - * Creates an upcast dispatcher for handling empty blocks. + * Creates an upcast converter for handling empty blocks. * The dispatcher detects empty elements and marks them with the empty block attribute. * - * @param editor - The editor instance. - * @returns A function that sets up the upcast conversion dispatcher. + * @param editor The editor instance. + * @returns A function that sets up the upcast converter. */ -function createEmptyBlocksUpcastDispatcher( editor: Editor ) { +function createEmptyBlocksUpcastConverter( editor: Editor ) { const { schema } = editor.model; return ( dispatcher: UpcastDispatcher ) => { @@ -169,7 +169,7 @@ function createEmptyBlocksUpcastDispatcher( editor: Editor ) { const modelElement = modelRange && modelRange.start.nodeAfter as Element; - if ( modelElement && schema.checkAttribute( modelElement, EMPTY_BLOCK_MODEL_ATTRIBUTE ) && viewItem.isEmpty ) { + if ( modelElement && schema.checkAttribute( modelElement, EMPTY_BLOCK_MODEL_ATTRIBUTE ) ) { conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); } }, { priority: 'lowest' } ); From 99f91c7f42db292e85acf85b9ffb68ab520927e6 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 20 Jan 2025 13:48:30 +0100 Subject: [PATCH 15/39] Adjust table cell downcasts after CR. --- .../ckeditor5-html-support/src/emptyblocks.ts | 55 +++++++------------ .../tests/emptyblocks.js | 2 +- 2 files changed, 22 insertions(+), 35 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblocks.ts index 2f3c103cb04..b84749e1706 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblocks.ts @@ -14,8 +14,7 @@ import type { DowncastDispatcher, UpcastDispatcher, DowncastAttributeEvent, - ElementCreatorFunction, - Node + DowncastInsertEvent } from 'ckeditor5/src/engine.js'; const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; @@ -88,39 +87,36 @@ export default class EmptyBlocks extends Plugin { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); - conversion.for( 'dataDowncast' ).elementToElement( { - model: 'paragraph', - view: convertEmptyBlockParagraphInTableCell(), - converterPriority: 'highest' - } ); + conversion.for( 'dataDowncast' ).add( createEmptyBlockParagraphInTableCellConverter() ); } } } /** - * Converts paragraphs in empty table cells during the downcast conversion. + * Creates a downcast converter for handling empty block paragraphs in table cells. + * + * @returns A function that sets up the downcast conversion dispatcher. */ -function convertEmptyBlockParagraphInTableCell(): ElementCreatorFunction { - return ( modelElement, { writer } ) => { - const parentCell = modelElement.parent; - - if ( !parentCell!.is( 'element', 'tableCell' ) ) { - return null; - } +function createEmptyBlockParagraphInTableCellConverter() { + return ( dispatcher: DowncastDispatcher ) => { + dispatcher.on>( 'insert:paragraph', ( evt, data, conversionApi ) => { + const modelItem = data.item; + const parentCell = modelItem.parent; - if ( parentCell.childCount != 1 || - !parentCell.hasAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE ) || - hasAnyAttribute( modelElement ) - ) { - return null; - } + if ( !parentCell!.is( 'element', 'tableCell' ) ) { + return; + } - const viewElement = writer.createContainerElement( 'p' ); + if ( !parentCell.hasAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE ) ) { + return; + } - viewElement.getFillerOffset = () => null; - writer.setCustomProperty( 'dataPipeline:transparentRendering', true, viewElement ); + const viewElement = conversionApi.mapper.toViewElement( data.item )!; - return viewElement; + if ( viewElement.getCustomProperty( 'dataPipeline:transparentRendering' ) ) { + viewElement.getFillerOffset = () => null; + } + } ); }; } @@ -175,12 +171,3 @@ function createEmptyBlocksUpcastConverter( editor: Editor ) { }, { priority: 'lowest' } ); }; } - -/** - * Checks if an element has any attributes set. - */ -function hasAnyAttribute( element: Node ): boolean { - const iteratorItem = element.getAttributeKeys().next(); - - return !iteratorItem.done; -} diff --git a/packages/ckeditor5-html-support/tests/emptyblocks.js b/packages/ckeditor5-html-support/tests/emptyblocks.js index 507af0a7547..3118cce5dc4 100644 --- a/packages/ckeditor5-html-support/tests/emptyblocks.js +++ b/packages/ckeditor5-html-support/tests/emptyblocks.js @@ -235,7 +235,7 @@ describe( 'EmptyBlocks', () => { describe( 'lists integration', () => { it( 'should set htmlEmptyBlock on empty list item', () => { - editor.setData( 'A
' ); + editor.setData( '

A

' ); const modelData = getModelData( model, { withoutSelection: true } ); const normalizedData = modelData.replace( / listItemId="[^"]+"/g, '' ); From dcb6876e49d9e5ffbf173885390565401d762278 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 20 Jan 2025 20:45:29 +0100 Subject: [PATCH 16/39] Simplified implementation. --- .../src/augmentation.ts | 4 +- .../src/{emptyblocks.ts => emptyblock.ts} | 102 ++++++------------ packages/ckeditor5-html-support/src/index.ts | 2 +- .../ckeditor5-list/src/list/converters.ts | 2 +- .../src/converters/downcast.ts | 11 +- 5 files changed, 49 insertions(+), 72 deletions(-) rename packages/ckeditor5-html-support/src/{emptyblocks.ts => emptyblock.ts} (53%) diff --git a/packages/ckeditor5-html-support/src/augmentation.ts b/packages/ckeditor5-html-support/src/augmentation.ts index b927fd9f37f..46ac30b4538 100644 --- a/packages/ckeditor5-html-support/src/augmentation.ts +++ b/packages/ckeditor5-html-support/src/augmentation.ts @@ -19,7 +19,8 @@ import type { StyleElementSupport, TableElementSupport, HtmlComment, - FullPage + FullPage, + EmptyBlock } from './index.js'; declare module '@ckeditor/ckeditor5-core' { @@ -50,5 +51,6 @@ declare module '@ckeditor/ckeditor5-core' { [ TableElementSupport.pluginName ]: TableElementSupport; [ HtmlComment.pluginName ]: HtmlComment; [ FullPage.pluginName ]: FullPage; + [ EmptyBlock.pluginName ]: EmptyBlock; } } diff --git a/packages/ckeditor5-html-support/src/emptyblocks.ts b/packages/ckeditor5-html-support/src/emptyblock.ts similarity index 53% rename from packages/ckeditor5-html-support/src/emptyblocks.ts rename to packages/ckeditor5-html-support/src/emptyblock.ts index b84749e1706..fb7c4ef9062 100644 --- a/packages/ckeditor5-html-support/src/emptyblocks.ts +++ b/packages/ckeditor5-html-support/src/emptyblock.ts @@ -4,17 +4,17 @@ */ /** - * @module html-support/emptyblocks + * @module html-support/emptyblock */ -import { Plugin, type Editor } from 'ckeditor5/src/core.js'; +import { Plugin } from 'ckeditor5/src/core.js'; import type { UpcastElementEvent, Element, + Schema, DowncastDispatcher, UpcastDispatcher, - DowncastAttributeEvent, - DowncastInsertEvent + DowncastAttributeEvent } from 'ckeditor5/src/engine.js'; const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; @@ -44,12 +44,12 @@ const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; *   * ``` */ -export default class EmptyBlocks extends Plugin { +export default class EmptyBlock extends Plugin { /** * @inheritDoc */ public static get pluginName() { - return 'EmptyBlocks' as const; + return 'EmptyBlock' as const; } /** @@ -66,67 +66,23 @@ export default class EmptyBlocks extends Plugin { const { model, conversion } = this.editor; const schema = model.schema; - schema.extend( '$block', { - allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] - } ); - - schema.extend( '$container', { - allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] - } ); - - // Downcasts. - conversion.for( 'dataDowncast' ).add( createEmptyBlocksDowncastConverter() ); - conversion.for( 'editingDowncast' ).add( createEmptyBlocksDowncastConverter() ); - - // Upcasts. - conversion.for( 'upcast' ).add( createEmptyBlocksUpcastConverter( this.editor ) ); + schema.extend( '$block', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); + schema.extend( '$container', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); - // Table related converters. if ( schema.isRegistered( 'tableCell' ) ) { - schema.extend( 'tableCell', { - allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] - } ); - - conversion.for( 'dataDowncast' ).add( createEmptyBlockParagraphInTableCellConverter() ); + schema.extend( 'tableCell', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); } - } -} - -/** - * Creates a downcast converter for handling empty block paragraphs in table cells. - * - * @returns A function that sets up the downcast conversion dispatcher. - */ -function createEmptyBlockParagraphInTableCellConverter() { - return ( dispatcher: DowncastDispatcher ) => { - dispatcher.on>( 'insert:paragraph', ( evt, data, conversionApi ) => { - const modelItem = data.item; - const parentCell = modelItem.parent; - - if ( !parentCell!.is( 'element', 'tableCell' ) ) { - return; - } - - if ( !parentCell.hasAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE ) ) { - return; - } - - const viewElement = conversionApi.mapper.toViewElement( data.item )!; - if ( viewElement.getCustomProperty( 'dataPipeline:transparentRendering' ) ) { - viewElement.getFillerOffset = () => null; - } - } ); - }; + conversion.for( 'downcast' ).add( createEmptyBlockDowncastConverter() ); + conversion.for( 'upcast' ).add( createEmptyBlockUpcastConverter( schema ) ); + } } /** * Creates a downcast converter for handling empty blocks. - * The dispatcher prevents filler elements from being added to elements marked as empty blocks. - * - * @returns A function that sets up the downcast conversion dispatcher. + * This converter prevents filler elements from being added to elements marked as empty blocks. */ -function createEmptyBlocksDowncastConverter() { +function createEmptyBlockDowncastConverter() { return ( dispatcher: DowncastDispatcher ) => { dispatcher.on>( `attribute:${ EMPTY_BLOCK_MODEL_ATTRIBUTE }`, ( evt, data, conversionApi ) => { const { mapper, consumable } = conversionApi; @@ -147,14 +103,9 @@ function createEmptyBlocksDowncastConverter() { /** * Creates an upcast converter for handling empty blocks. - * The dispatcher detects empty elements and marks them with the empty block attribute. - * - * @param editor The editor instance. - * @returns A function that sets up the upcast converter. + * The converter detects empty elements and marks them with the empty block attribute. */ -function createEmptyBlocksUpcastConverter( editor: Editor ) { - const { schema } = editor.model; - +function createEmptyBlockUpcastConverter( schema: Schema ) { return ( dispatcher: UpcastDispatcher ) => { dispatcher.on( 'element', ( evt, data, conversionApi ) => { const { viewItem, modelRange } = data; @@ -163,10 +114,27 @@ function createEmptyBlocksUpcastConverter( editor: Editor ) { return; } + // Handle element itself. const modelElement = modelRange && modelRange.start.nodeAfter as Element; - if ( modelElement && schema.checkAttribute( modelElement, EMPTY_BLOCK_MODEL_ATTRIBUTE ) ) { - conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); + if ( !modelElement || !schema.checkAttribute( modelElement, EMPTY_BLOCK_MODEL_ATTRIBUTE ) ) { + return; + } + + conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); + + // Handle a auto-paragraphed bogus paragraph inside empty element. + if ( modelElement.childCount != 1 ) { + return; + } + + const firstModelChild = modelElement.getChild( 0 )!; + + if ( + firstModelChild.is( 'element', 'paragraph' ) && + schema.checkAttribute( firstModelChild, EMPTY_BLOCK_MODEL_ATTRIBUTE ) + ) { + conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, firstModelChild ); } }, { priority: 'lowest' } ); }; diff --git a/packages/ckeditor5-html-support/src/index.ts b/packages/ckeditor5-html-support/src/index.ts index f144069b87f..5550b17380a 100644 --- a/packages/ckeditor5-html-support/src/index.ts +++ b/packages/ckeditor5-html-support/src/index.ts @@ -13,7 +13,7 @@ export { default as DataSchema, type DataSchemaBlockElementDefinition } from './ export { default as HtmlComment } from './htmlcomment.js'; export { default as FullPage } from './fullpage.js'; export { default as HtmlPageDataProcessor } from './htmlpagedataprocessor.js'; -export { default as EmptyBlocks } from './emptyblocks.js'; +export { default as EmptyBlock } from './emptyblock.js'; export type { GeneralHtmlSupportConfig } from './generalhtmlsupportconfig.js'; export type { default as CodeBlockElementSupport } from './integrations/codeblock.js'; export type { default as CustomElementSupport } from './integrations/customelement.js'; diff --git a/packages/ckeditor5-list/src/list/converters.ts b/packages/ckeditor5-list/src/list/converters.ts index edc24ac8d97..adfafa8cea3 100644 --- a/packages/ckeditor5-list/src/list/converters.ts +++ b/packages/ckeditor5-list/src/list/converters.ts @@ -699,7 +699,7 @@ function shouldUseBogusParagraph( for ( const attributeKey of item.getAttributeKeys() ) { // Ignore selection attributes stored on block elements. - if ( attributeKey.startsWith( 'selection:' ) ) { + if ( attributeKey.startsWith( 'selection:' ) || attributeKey == 'htmlEmptyBlock' ) { continue; } diff --git a/packages/ckeditor5-table/src/converters/downcast.ts b/packages/ckeditor5-table/src/converters/downcast.ts index cc0f35f1874..5b7ce877c08 100644 --- a/packages/ckeditor5-table/src/converters/downcast.ts +++ b/packages/ckeditor5-table/src/converters/downcast.ts @@ -191,9 +191,16 @@ function toTableWidget( viewElement: ViewElement, writer: DowncastWriter ): View * Checks if an element has any attributes set. */ function hasAnyAttribute( element: Node ): boolean { - const iteratorItem = element.getAttributeKeys().next(); + for ( const attributeKey of element.getAttributeKeys() ) { + // Ignore selection attributes stored on block elements. + if ( attributeKey.startsWith( 'selection:' ) || attributeKey == 'htmlEmptyBlock' ) { + continue; + } + + return true; + } - return !iteratorItem.done; + return false; } export interface DowncastTableOptions { From 4b6b2d48e30b7e0fb20f14e834700c3ddac130bf Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Tue, 21 Jan 2025 15:21:24 +0100 Subject: [PATCH 17/39] Block filler handling adjusted to be able to detect presence of it while upcasting. --- .../ckeditor5-engine/src/view/domconverter.ts | 46 ++++++++++++++++--- .../ckeditor5-html-support/src/emptyblock.ts | 2 +- .../tests/{emptyblocks.js => emptyblock.js} | 14 +++--- 3 files changed, 48 insertions(+), 14 deletions(-) rename packages/ckeditor5-html-support/tests/{emptyblocks.js => emptyblock.js} (96%) diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts index 787d3deaacd..e9563072f6f 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.ts +++ b/packages/ckeditor5-engine/src/view/domconverter.ts @@ -1185,11 +1185,7 @@ export default class DomConverter { } // Special case for


in which
should be treated as filler even when we are not in the 'br' mode. See ckeditor5#5564. - if ( - ( domNode as DomElement ).tagName === 'BR' && - hasBlockParent( domNode, this.blockElements ) && - ( domNode as DomElement ).parentNode!.childNodes.length === 1 - ) { + if ( isOnlyBrInBlock( domNode as DomElement, this.blockElements ) ) { return true; } @@ -1373,7 +1369,14 @@ export default class DomConverter { }, inlineNodes: Array ): IterableIterator { - if ( this.isBlockFiller( domNode ) ) { + // Handle the editing view block filler. + // The data mode block filler is handled in this._processDomInlineNodes(). + if ( + this.blockFillerMode == 'br' && this.isBlockFiller( domNode ) || + // Special case for


in which
should be treated as filler even when we are not in the 'br' mode. + // See ckeditor5#5564. + isOnlyBrInBlock( domNode as DomElement, this.blockElements ) + ) { return null; } @@ -1557,6 +1560,13 @@ export default class DomConverter { // the inline filler is removed only after the data is initially processed (by the `.replace` above). See ckeditor5#692. data = getDataWithoutFiller( data ); + // Block filler handling. + // TODO handle MARKED_NBSP_FILLER + if ( this.blockFillerMode != 'br' && node.parent && isViewBlockFiller( node.parent, data, this.blockElements ) ) { + data = ''; + node.parent._setCustomProperty( '$hasBlockFiller', true ); + } + // At this point we should have removed all whitespaces from DOM text data. // // Now, We will reverse the process that happens in `_processDataFromViewText`. @@ -1879,6 +1889,30 @@ function hasBlockParent( domNode: DomNode, blockElements: ReadonlyArray return !!parent && !!( parent as DomElement ).tagName && blockElements.includes( ( parent as DomElement ).tagName.toLowerCase() ); } +/** + * TODO + */ +function isViewBlockFiller( parent: ViewNode | ViewDocumentFragment, data: string, blockElements: Array ): boolean { + return ( + data == '\u00A0' && + parent && + parent.is( 'element' ) && + parent.childCount == 1 && + blockElements.includes( parent.name ) + ); +} + +/** + * Special case for `


` in which `
` should be treated as filler even when we are not in the 'br' mode. See ckeditor5#5564. + */ +function isOnlyBrInBlock( domNode: DomElement, blockElements: Array ): boolean { + return ( + domNode.tagName === 'BR' && + hasBlockParent( domNode, blockElements ) && + domNode.parentNode!.childNodes.length === 1 + ); +} + /** * Log to console the information about element that was replaced. * Check UNSAFE_ELEMENTS for all recognized unsafe elements. diff --git a/packages/ckeditor5-html-support/src/emptyblock.ts b/packages/ckeditor5-html-support/src/emptyblock.ts index fb7c4ef9062..1dbd5736254 100644 --- a/packages/ckeditor5-html-support/src/emptyblock.ts +++ b/packages/ckeditor5-html-support/src/emptyblock.ts @@ -110,7 +110,7 @@ function createEmptyBlockUpcastConverter( schema: Schema ) { dispatcher.on( 'element', ( evt, data, conversionApi ) => { const { viewItem, modelRange } = data; - if ( !viewItem.is( 'element' ) || !viewItem.isEmpty ) { + if ( !viewItem.is( 'element' ) || !viewItem.isEmpty || viewItem.getCustomProperty( '$hasBlockFiller' ) ) { return; } diff --git a/packages/ckeditor5-html-support/tests/emptyblocks.js b/packages/ckeditor5-html-support/tests/emptyblock.js similarity index 96% rename from packages/ckeditor5-html-support/tests/emptyblocks.js rename to packages/ckeditor5-html-support/tests/emptyblock.js index 3118cce5dc4..db4a8fdc0a0 100644 --- a/packages/ckeditor5-html-support/tests/emptyblocks.js +++ b/packages/ckeditor5-html-support/tests/emptyblock.js @@ -14,10 +14,10 @@ import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote.js'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js'; -import EmptyBlocks from '../src/emptyblocks.js'; +import EmptyBlock from '../src/emptyblock.js'; import { toWidget, viewToModelPositionOutsideModelElement } from '@ckeditor/ckeditor5-widget'; -describe( 'EmptyBlocks', () => { +describe( 'EmptyBlock', () => { let editor, model, element, view; beforeEach( async () => { @@ -25,7 +25,7 @@ describe( 'EmptyBlocks', () => { document.body.appendChild( element ); editor = await ClassicTestEditor.create( element, { - plugins: [ Paragraph, TableEditing, EmptyBlocks, Heading, ListEditing, BlockQuote ] + plugins: [ Paragraph, TableEditing, EmptyBlock, Heading, ListEditing, BlockQuote ] } ); model = editor.model; @@ -38,15 +38,15 @@ describe( 'EmptyBlocks', () => { } ); it( 'should be named', () => { - expect( EmptyBlocks.pluginName ).to.equal( 'EmptyBlocks' ); + expect( EmptyBlock.pluginName ).to.equal( 'EmptyBlock' ); } ); it( 'should have `isOfficialPlugin` static flag set to `true`', () => { - expect( EmptyBlocks.isOfficialPlugin ).to.be.true; + expect( EmptyBlock.isOfficialPlugin ).to.be.true; } ); it( 'should be loaded', () => { - expect( editor.plugins.get( EmptyBlocks ) ).to.be.instanceOf( EmptyBlocks ); + expect( editor.plugins.get( EmptyBlock ) ).to.be.instanceOf( EmptyBlock ); } ); describe( 'schema', () => { @@ -245,7 +245,7 @@ describe( 'EmptyBlocks', () => { '' ); - expect( editor.getData() ).to.equal( '

A

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

A

' ); } ); } ); From 063f1f2ebd5128f0e06f9082c7d94826105d9fe2 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Tue, 21 Jan 2025 18:22:20 +0100 Subject: [PATCH 18/39] Fixed block filler handling. --- .../ckeditor5-engine/src/view/domconverter.ts | 70 ++++++++++++++----- .../tests/view/domconverter/dom-to-view.js | 10 +++ .../whitespace-handling-integration.js | 49 ++++++++++++- 3 files changed, 112 insertions(+), 17 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts index e9563072f6f..2cd45a80ef6 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.ts +++ b/packages/ckeditor5-engine/src/view/domconverter.ts @@ -725,6 +725,11 @@ export default class DomConverter { // Whitespace cleaning. this._processDomInlineNodes( null, inlineNodes, options ); + // This was a single block filler so just remove it. + if ( this.blockFillerMode == 'br' && isViewBrFiller( node ) ) { + return null; + } + // Text not got trimmed to an empty string so there is no result node. if ( node.is( '$text' ) && node.data.length == 0 ) { return null; @@ -770,7 +775,10 @@ export default class DomConverter { this._processDomInlineNodes( domElement, inlineNodes, options ); } - yield viewChild; + // Yield only if this is not a block filler. + if ( !( this.blockFillerMode == 'br' && isViewBrFiller( viewChild ) ) ) { + yield viewChild; + } // Trigger children handling. generator.next(); @@ -1184,7 +1192,8 @@ export default class DomConverter { return domNode.isEqualNode( BR_FILLER_REF ); } - // Special case for


in which
should be treated as filler even when we are not in the 'br' mode. See ckeditor5#5564. + // Special case for


in which
should be treated as filler even when we are not in the 'br' mode. + // See https://github.com/ckeditor/ckeditor5/issues/5564. if ( isOnlyBrInBlock( domNode as DomElement, this.blockElements ) ) { return true; } @@ -1369,14 +1378,9 @@ export default class DomConverter { }, inlineNodes: Array ): IterableIterator { - // Handle the editing view block filler. - // The data mode block filler is handled in this._processDomInlineNodes(). - if ( - this.blockFillerMode == 'br' && this.isBlockFiller( domNode ) || - // Special case for


in which
should be treated as filler even when we are not in the 'br' mode. - // See ckeditor5#5564. - isOnlyBrInBlock( domNode as DomElement, this.blockElements ) - ) { + // Special case for


in which
should be treated as filler even when we are not in the 'br' mode. + // See https://github.com/ckeditor/ckeditor5/issues/5564. + if ( this.blockFillerMode != 'br' && isOnlyBrInBlock( domNode as DomElement, this.blockElements ) ) { return null; } @@ -1561,10 +1565,20 @@ export default class DomConverter { data = getDataWithoutFiller( data ); // Block filler handling. - // TODO handle MARKED_NBSP_FILLER - if ( this.blockFillerMode != 'br' && node.parent && isViewBlockFiller( node.parent, data, this.blockElements ) ) { - data = ''; - node.parent._setCustomProperty( '$hasBlockFiller', true ); + if ( this.blockFillerMode != 'br' && node.parent ) { + if ( isViewMarkedNbspFiller( node.parent, data ) ) { + data = ''; + + // Mark block element as it has a block filler and remove the `` element. + if ( node.parent.parent ) { + node.parent.parent._setCustomProperty( '$hasBlockFiller', true ); + node.parent._remove(); + } + } + else if ( isViewNbspFiller( node.parent, data, this.blockElements ) ) { + data = ''; + node.parent._setCustomProperty( '$hasBlockFiller', true ); + } } // At this point we should have removed all whitespaces from DOM text data. @@ -1892,7 +1906,7 @@ function hasBlockParent( domNode: DomNode, blockElements: ReadonlyArray /** * TODO */ -function isViewBlockFiller( parent: ViewNode | ViewDocumentFragment, data: string, blockElements: Array ): boolean { +function isViewNbspFiller( parent: ViewNode | ViewDocumentFragment, data: string, blockElements: Array ): boolean { return ( data == '\u00A0' && parent && @@ -1903,9 +1917,33 @@ function isViewBlockFiller( parent: ViewNode | ViewDocumentFragment, data: strin } /** - * Special case for `


` in which `
` should be treated as filler even when we are not in the 'br' mode. See ckeditor5#5564. + * TODO + */ +function isViewMarkedNbspFiller( parent: ViewNode | ViewDocumentFragment, data: string ): boolean { + return ( + data == '\u00A0' && + parent && + parent.is( 'element', 'span' ) && + parent.childCount == 1 && + parent.hasAttribute( 'data-cke-filler' ) + ); +} + +/** + * TODO + */ +function isViewBrFiller( node: ViewNode ): boolean { + return ( + node.is( 'element', 'br' ) && + node.hasAttribute( 'data-cke-filler' ) + ); +} + +/** + * Special case for `


` in which `
` should be treated as filler even when we are not in the 'br' mode. */ function isOnlyBrInBlock( domNode: DomElement, blockElements: Array ): boolean { + // See https://github.com/ckeditor/ckeditor5/issues/5564. return ( domNode.tagName === 'BR' && hasBlockParent( domNode, blockElements ) && diff --git a/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js b/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js index 7e8c59409a4..4179bfdbb8d 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js @@ -171,6 +171,16 @@ describe( 'DomConverter', () => { expect( converter.domToView( domFiller ) ).to.be.null; } ); + it( 'should ignore a block filler inside a paragraph', () => { + // eslint-disable-next-line new-cap + const domFiller = BR_FILLER( document ); + const domP = createElement( document, 'p', undefined, [ domFiller ] ); + + const viewP = converter.domToView( domP ); + expect( viewP.is( 'element', 'p' ) ).to.be.true; + expect( viewP.childCount ).to.equal( 0 ); + } ); + it( 'should return null for empty text node', () => { const textNode = document.createTextNode( '' ); diff --git a/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js b/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js index 08843d51811..2c2c7fa0823 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js @@ -193,13 +193,60 @@ describe( 'DomConverter – whitespace handling – integration', () => { expect( editor.getData() ).to.equal( '

 foo 

' ); } ); - it( 'single nbsp inside blocks are ignored', () => { + it( 'single nbsp inside blocks are ignored (NBSP block filler)', () => { editor.setData( '

 

' ); expect( getData( editor.model, { withoutSelection: true } ) ) .to.equal( '' ); expect( editor.getData() ).to.equal( '' ); // trimmed + expect( editor.getData( { trim: false } ) ).to.equal( '

 

' ); + } ); + + it( 'nbsp with spaces inside blocks are ignored (NBSP block filler)', () => { + editor.setData( '

\n  \n

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( '' ); + + expect( editor.getData() ).to.equal( '' ); // trimmed + expect( editor.getData( { trim: false } ) ).to.equal( '

 

' ); + } ); + + it( 'single nbsp inside blocks are ignored (marked NBSP block filler)', () => { + editor.data.processor.useFillerType( 'marked' ); + + editor.conversion.for( 'upcast' ).add( dispatcher => { + dispatcher.on( 'element', ( evt, data ) => { + expect( data.viewItem.name ).to.not.equal( 'span' ); + } ); + } ); + + editor.setData( '

 

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( '' ); + + expect( editor.getData() ).to.equal( '' ); // trimmed + expect( editor.getData( { trim: false } ) ).to.equal( '

 

' ); + } ); + + it( 'nbsp with spaces inside blocks are ignored (marked NBSP block filler)', () => { + editor.data.processor.useFillerType( 'marked' ); + + editor.conversion.for( 'upcast' ).add( dispatcher => { + dispatcher.on( 'element', ( evt, data ) => { + expect( data.viewItem.name ).to.not.equal( 'span' ); + } ); + } ); + + editor.setData( '

\n  \n

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( '' ); + + expect( editor.getData() ).to.equal( '' ); // trimmed + expect( editor.getData( { trim: false } ) ).to.equal( '

 

' ); } ); it( 'all whitespaces together are ignored', () => { From bc6bca7f70420ceb65c933d7af88ceb7fecd90d5 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 22 Jan 2025 15:20:49 +0100 Subject: [PATCH 19/39] Added tests. --- .../whitespace-handling-integration.js | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js b/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js index 2c2c7fa0823..baac0b46f3d 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js @@ -3,12 +3,16 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options */ +/* globals document */ + import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor.js'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js'; import ImageInlineEditing from '@ckeditor/ckeditor5-image/src/image/imageinlineediting.js'; import ShiftEnter from '@ckeditor/ckeditor5-enter/src/shiftenter.js'; +import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement.js'; import { getData } from '../../../src/dev-utils/model.js'; +import { getFillerOffset } from '../../../src/index.js'; // NOTE: // dev utils' setData() loses white spaces so don't use it for tests here!!! @@ -925,6 +929,114 @@ describe( 'DomConverter – whitespace handling – integration', () => { '' ); } ); + + describe( 'text nodes parse and stringify', () => { + function testTexts( inputTexts, processedText, outputText ) { + if ( typeof inputTexts == 'string' ) { + inputTexts = [ inputTexts ]; + } + + outputText = outputText !== undefined ? outputText : inputTexts.join( '' ); + + it( `spaces in a text node: "${ inputTexts.join( '|' ) }" -> "${ processedText }" -> "${ outputText }"`, () => { + const domElement = createElement( document, 'p', {}, [] ); + + for ( const text of inputTexts ) { + domElement.appendChild( document.createTextNode( text.replace( /_/g, '\u00A0' ) ) ); + } + + const viewElement = editor.data.processor.domConverter.domToView( domElement ); + let viewData = ''; + + viewElement.getFillerOffset = getFillerOffset; + + for ( const child of viewElement.getChildren() ) { + viewData += child.data.replace( /\u00A0/g, '_' ); + } + + expect( viewData, 'processed' ).to.equal( processedText ); + + const outputDomElement = editor.data.processor.domConverter.viewToDom( viewElement ); + + expect( outputDomElement.innerHTML.replace( / /g, '_' ), 'output' ).to.equal( outputText ); + } ); + } + + // Block filler. + testTexts( '_', '', '_' ); + testTexts( ' _ ', '', '_' ); + testTexts( ' _ ', '', '_' ); + + // At the beginning. + testTexts( '_x', ' x' ); + testTexts( '_ x', ' x' ); + testTexts( '_ _x', ' x' ); + testTexts( '_ _ x', ' x' ); + + // At the end. + testTexts( 'x_', 'x ' ); + testTexts( 'x _', 'x ' ); + testTexts( 'x __', 'x ' ); + testTexts( 'x _ _', 'x ' ); + + // In the middle. + testTexts( 'x x', 'x x' ); + testTexts( 'x _x', 'x x' ); + testTexts( 'x _ x', 'x x' ); + testTexts( 'x _ _x', 'x x' ); + + // Complex. + testTexts( '_x_', ' x ' ); + testTexts( '_ x _x _', ' x x ' ); + testTexts( '_ _x x _', ' x x ' ); + testTexts( '_ _x x __', ' x x ' ); + testTexts( '_ _x _ _x_', ' x x ' ); + + // With hard   + testTexts( '_x', ' x' ); + testTexts( '__x', ' _x' ); + testTexts( '___x', ' __x' ); + testTexts( '__ x', ' _ x' ); + + testTexts( 'x_', 'x ' ); + testTexts( 'x__', 'x_ ' ); + testTexts( 'x___', 'x__ ' ); + + testTexts( 'x_x', 'x_x' ); + testTexts( 'x___x', 'x___x' ); + testTexts( 'x____x', 'x____x' ); + testTexts( 'x__ x', 'x__ x' ); + testTexts( 'x___ x', 'x___ x' ); + testTexts( 'x_ _x', 'x_ x' ); + testTexts( 'x __x', 'x _x' ); + testTexts( 'x _ x', 'x x' ); + testTexts( 'x __ _x', 'x _ x' ); + + // Two text nodes. + testTexts( [ 'x', 'y' ], 'xy' ); + testTexts( [ 'x ', 'y' ], 'x y' ); + testTexts( [ 'x _', 'y' ], 'x y' ); + testTexts( [ 'x __', 'y' ], 'x y' ); + testTexts( [ 'x _ _', 'y' ], 'x y', 'x _ _y' ); + + testTexts( [ 'x', ' y' ], 'x y' ); + testTexts( [ 'x_', ' y' ], 'x y' ); + testTexts( [ 'x _', ' y' ], 'x y' ); + testTexts( [ 'x __', ' y' ], 'x y' ); + testTexts( [ 'x _ _', ' y' ], 'x y' ); + + testTexts( [ 'x', ' _y' ], 'x y' ); + testTexts( [ 'x_', ' _y' ], 'x y' ); + testTexts( [ 'x _', ' _y' ], 'x y' ); + testTexts( [ 'x __', ' _y' ], 'x y' ); + testTexts( [ 'x _ _', ' _y' ], 'x y' ); + + // Some tests with hard   + testTexts( [ 'x', '_y' ], 'x_y' ); + testTexts( [ 'x_', 'y' ], 'x_y' ); + testTexts( [ 'x__', ' y' ], 'x_ y' ); + testTexts( [ 'x_ _', ' y' ], 'x_ y' ); + } ); } ); // https://github.com/ckeditor/ckeditor5/issues/1024 From e0d570debf45ef67862f1f8ecb72106fd3acd8ec Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 22 Jan 2025 17:37:52 +0100 Subject: [PATCH 20/39] Added code comments. --- packages/ckeditor5-engine/src/view/domconverter.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts index 2cd45a80ef6..9d4c5edcd96 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.ts +++ b/packages/ckeditor5-engine/src/view/domconverter.ts @@ -1880,7 +1880,7 @@ function forEachDomElementAncestor( element: DomElement, callback: ( node: DomEl } /** - * Checks if given node is a nbsp block filler. + * Checks if given DOM node is a nbsp block filler. * * A   is a block filler only if it is a single child of a block element. * @@ -1904,7 +1904,9 @@ function hasBlockParent( domNode: DomNode, blockElements: ReadonlyArray } /** - * TODO + * Checks if given view node is a nbsp block filler. + * + * A   is a block filler only if it is a single child of a block element. */ function isViewNbspFiller( parent: ViewNode | ViewDocumentFragment, data: string, blockElements: Array ): boolean { return ( @@ -1917,7 +1919,9 @@ function isViewNbspFiller( parent: ViewNode | ViewDocumentFragment, data: string } /** - * TODO + * Checks if given view node is a marked-nbsp block filler. + * + * A   is a block filler only if it is wrapped in `` element. */ function isViewMarkedNbspFiller( parent: ViewNode | ViewDocumentFragment, data: string ): boolean { return ( @@ -1930,7 +1934,9 @@ function isViewMarkedNbspFiller( parent: ViewNode | ViewDocumentFragment, data: } /** - * TODO + * Checks if given view node is a br block filler. + * + * A
is a block filler only if it has data-cke-filler attribute set. */ function isViewBrFiller( node: ViewNode ): boolean { return ( From b6ed3d56b7c521564f8adbd2b0bbb48c5dd52d31 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 22 Jan 2025 18:40:46 +0100 Subject: [PATCH 21/39] Added EmptyBlock integration tests. --- .../tests/list/integrations/emptyblock.js | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 packages/ckeditor5-list/tests/list/integrations/emptyblock.js diff --git a/packages/ckeditor5-list/tests/list/integrations/emptyblock.js b/packages/ckeditor5-list/tests/list/integrations/emptyblock.js new file mode 100644 index 00000000000..7b61f4258c7 --- /dev/null +++ b/packages/ckeditor5-list/tests/list/integrations/emptyblock.js @@ -0,0 +1,93 @@ +/** + * @license Copyright (c) 2003-2025, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options + */ + +import ListEditing from '../../../src/list/listediting.js'; + +import HeadingEditing from '@ckeditor/ckeditor5-heading/src/headingediting.js'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js'; +import EmptyBlock from '@ckeditor/ckeditor5-html-support/src/emptyblock.js'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils.js'; + +import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor.js'; +import { setupTestHelpers } from '../_utils/utils.js'; +import stubUid from '../_utils/uid.js'; + +describe( 'ListEditing - EmptyBlock integration', () => { + let editor, view, test; + + testUtils.createSinonSandbox(); + + beforeEach( async () => { + editor = await VirtualTestEditor.create( { + plugins: [ Paragraph, ListEditing, HeadingEditing, EmptyBlock ] + } ); + + view = editor.editing.view; + + // Stub `view.scrollToTheSelection` as it will fail on VirtualTestEditor without DOM. + sinon.stub( view, 'scrollToTheSelection' ).callsFake( () => {} ); + stubUid(); + + test = setupTestHelpers( editor ); + } ); + + afterEach( async () => { + await editor.destroy(); + } ); + + it( 'inside a plain li element', () => { + test.data( + '
    ' + + '
  • x
  • ' + + '
  •  
  • ' + + '
  • ' + + '
', + + 'x' + + '' + + '' + ); + } ); + + it( 'inside a paragraph in a li element', () => { + test.data( + '
    ' + + '
  • x

  • ' + + '
  •  

  • ' + + '
  • ' + + '
', + + 'x' + + '' + + '', + + '
    ' + + '
  • x
  • ' + + '
  •  
  • ' + + '
  • ' + + '
' + ); + } ); + + it( 'inside a heading in a li element', () => { + test.data( + '
    ' + + '
  • x

  • ' + + '
  •  

  • ' + + '
  • ' + + '
', + + 'x' + + '' + + '', + + '
    ' + + '
  • x

  • ' + + '
  •  

  • ' + + '
  • ' + + '
' + ); + } ); +} ); From 890234f2c4eeb426edddf6df4fc910540c18c6e1 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 22 Jan 2025 19:05:35 +0100 Subject: [PATCH 22/39] Added EmptyBlock integration tests. --- .../converters/table-cell-refresh-handler.js | 16 ++ packages/ckeditor5-table/tests/integration.js | 184 +++++++++++++++++- 2 files changed, 199 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-table/tests/converters/table-cell-refresh-handler.js b/packages/ckeditor5-table/tests/converters/table-cell-refresh-handler.js index b7e05539fb0..d7ae36a7169 100644 --- a/packages/ckeditor5-table/tests/converters/table-cell-refresh-handler.js +++ b/packages/ckeditor5-table/tests/converters/table-cell-refresh-handler.js @@ -218,6 +218,22 @@ describe( 'Table cell refresh handler', () => { expect( getViewForParagraph( table ) ).to.not.equal( previousView ); } ); + it( 'should not rename to

when setting a selection attribute on ', () => { + editor.setData( '

00

' ); + + const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); + + model.change( writer => { + writer.setAttribute( 'selection:bold', true, table.getNodeByPath( [ 0, 0, 0 ] ) ); + } ); + + expect( getViewData( view, { withoutSelection: true } ) ).to.equalMarkup( viewTable( [ + [ '00' ] + ], { asWidget: true } ) ); + expect( getViewForParagraph( table ) ).to.equal( previousView ); + } ); + it( 'should rename

to when removing one of two paragraphs inside table cell', () => { editor.setData( viewTable( [ [ '

00

foo

' ] ] ) ); diff --git a/packages/ckeditor5-table/tests/integration.js b/packages/ckeditor5-table/tests/integration.js index d1907adc3f6..378b7d48686 100644 --- a/packages/ckeditor5-table/tests/integration.js +++ b/packages/ckeditor5-table/tests/integration.js @@ -4,10 +4,12 @@ */ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js'; -import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; +import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; import BalloonToolbar from '@ckeditor/ckeditor5-ui/src/toolbar/balloon/balloontoolbar.js'; import ClipboardPipeline from '@ckeditor/ckeditor5-clipboard/src/clipboardpipeline.js'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js'; +import Heading from '@ckeditor/ckeditor5-heading/src/heading.js'; +import EmptyBlock from '@ckeditor/ckeditor5-html-support/src/emptyblock.js'; import global from '@ckeditor/ckeditor5-utils/src/dom/global.js'; import Table from '../src/table.js'; import TableToolbar from '../src/tabletoolbar.js'; @@ -99,4 +101,184 @@ describe( 'TableContentToolbar integration', () => { sinon.assert.notCalled( normalPrioritySpy ); } ); } ); + + describe( 'with the EmptyBlock', () => { + let editor, editorElement; + + beforeEach( async () => { + editorElement = global.document.createElement( 'div' ); + global.document.body.appendChild( editorElement ); + + editor = await ClassicTestEditor.create( editorElement, { + plugins: [ Table, Paragraph, Heading, EmptyBlock ] + } ); + } ); + + afterEach( async () => { + editorElement.remove(); + await editor.destroy(); + } ); + + it( 'plain content in table cell', () => { + editor.setData( + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
x
 
' + ); + + expect( getModelData( editor.model, { withoutSelection: true } ) ).to.equal( + '' + + '' + + '' + + 'x' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
x
 
' + + '
' + ); + } ); + + it( 'content in paragraph in a table cell', () => { + editor.setData( + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '

x

 

' + ); + + expect( getModelData( editor.model, { withoutSelection: true } ) ).to.equal( + '' + + '' + + '' + + 'x' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + ); + + expect( editor.getData() ).to.equal( + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
x
 
' + + '
' + ); + } ); + + it( 'content in heading in a table cell', () => { + editor.setData( + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '

x

 

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

x

 

' + + '
' + ); + } ); + } ); } ); From a6c2fd6a05f40b86737ed78f0b2dbdc7ea26c64e Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 22 Jan 2025 19:43:52 +0100 Subject: [PATCH 23/39] Updated tests. --- .../tests/emptyblock.js | 344 ++++++++++-------- 1 file changed, 198 insertions(+), 146 deletions(-) diff --git a/packages/ckeditor5-html-support/tests/emptyblock.js b/packages/ckeditor5-html-support/tests/emptyblock.js index db4a8fdc0a0..bd637f1b397 100644 --- a/packages/ckeditor5-html-support/tests/emptyblock.js +++ b/packages/ckeditor5-html-support/tests/emptyblock.js @@ -13,6 +13,7 @@ import ListEditing from '@ckeditor/ckeditor5-list/src/list/listediting.js'; import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote.js'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js'; +import { INLINE_FILLER } from '@ckeditor/ckeditor5-engine/src/view/filler.js'; import EmptyBlock from '../src/emptyblock.js'; import { toWidget, viewToModelPositionOutsideModelElement } from '@ckeditor/ckeditor5-widget'; @@ -69,90 +70,91 @@ describe( 'EmptyBlock', () => { } ); } ); - describe( 'upcast conversion', () => { - it( 'should set htmlEmptyBlock attribute on empty paragraph', () => { - editor.setData( '

' ); - - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + describe( 'data pipeline', () => { + it( 'should not affect paragraph with text', () => { + editor.setData( + '

foo

' ); - } ); - - it( 'should not set htmlEmptyBlock attribute on non-empty paragraph', () => { - editor.setData( '

foo

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

foo

' + ); } ); - it( 'should set htmlEmptyBlock attribute on paragraph with whitespace', () => { - editor.setData( '

' ); + it( 'should keep block filler in paragraph', () => { + editor.setData( + '

 

' + ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + '' ); - } ); - it( 'should not set htmlEmptyBlock attribute on empty inline elements', () => { - registerInlinePlaceholderWidget(); + expect( editor.getData( { trim: false } ) ).to.equal( + '

 

' + ); + } ); + it( 'should keep block filler surrounded with spaces in paragraph', () => { editor.setData( - '

' + - 'Hello' + - '' + - 'World' + - '

' + '

 

' ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - 'HelloWorld' + '' ); - } ); - } ); - describe( 'data downcast conversion', () => { - it( 'should not return anything if blank paragraph in model (as it used to do)', () => { - setModelData( model, '' ); - - expect( editor.getData() ).to.equal( '' ); + expect( editor.getData( { trim: false } ) ).to.equal( + '

 

' + ); } ); - it( 'should add filler to normal empty paragraph', () => { - setModelData( model, 'Hello' ); + it( 'should not inject block filler if loaded paragraph was empty', () => { + editor.setData( + '

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

Hello

 

' ); + expect( editor.getData( { trim: false } ) ).to.equal( + '

' + ); } ); - it( 'should preserve multiple empty paragraphs', () => { - setModelData( - model, - 'Hello' + - '' + - '' + - '' + it( 'should not inject block filler if loaded paragraph contains only spaces', () => { + editor.setData( + '

' ); expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - 'Hello' + - '' + - '' + '' ); - expect( editor.getData() ).to.equal( '

Hello

' ); + expect( editor.getData( { trim: false } ) ).to.equal( + '

' + ); } ); - it( 'should preserve empty paragraphs mixed with non-empty ones', () => { - editor.setData( '

foo

' ); + it( 'should not set htmlEmptyBlock attribute on empty inline elements', () => { + registerInlinePlaceholderWidget(); - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + - 'foo' + - '' + editor.setData( + '

' + + 'Hello' + + '' + + 'World' + + '

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

foo

' ); + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + 'HelloWorld' + ); } ); it( 'should not set `getFillerOffset` if element is already consumed', () => { @@ -171,126 +173,176 @@ describe( 'EmptyBlock', () => { expect( editor.getData() ).to.equal( '

 

foo

' ); } ); - } ); - - describe( 'table integration', () => { - it( 'should set htmlEmptyBlock attribute on empty table cell', () => { - editor.setData( '
' ); - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '
' - ); - } ); + describe( 'table integration', () => { + it( 'should preserve empty cell', () => { + editor.setData( + '
' + ); - it( 'should preserve empty table cells in data output', () => { - editor.setData( '
foo
' ); + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + + '' + + '
' + ); - expect( editor.getData() ).to.equal( - '
foo
' - ); - } ); + expect( editor.getData() ).to.equal( '
' ); + } ); - it( 'should preserve empty cells mixed with non-empty ones', () => { - editor.setData( '
foobar
' ); + it( 'should preserve empty cells mixed with non-empty ones', () => { + editor.setData( + '
foo 
' + ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + + '' + + 'foo' + + '' + + '' + + '' + + '
' + ); + + expect( editor.getData() ).to.equal( + '
foo 
' + ); + } ); - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + - '' + - 'foo' + - '' + - 'bar' + - '' + - '
' - ); + it( 'should preserve empty cells on table cell with whitespace', () => { + editor.setData( '
' ); - expect( editor.getData() ).to.equal( - '
foobar
' - ); - } ); + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + + '' + + '' + + '' + + '
' + ); - it( 'should set htmlEmptyBlock attribute on table cell with whitespace', () => { - editor.setData( '
' ); + expect( editor.getData() ).to.equal( + '
' + ); + } ); - expect( editor.getData() ).to.equal( - '
' - ); + it( 'should handle empty paragraph in a table cell', () => { + editor.setData( + '

 

' + ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + + '' + + '' + + '' + + '' + + '
' + ); + + expect( editor.getData() ).to.equal( + '
 
' + ); + } ); } ); - it( 'should not add auto paragraphs to empty table cells with htmlEmptyBlock', () => { - editor.setData( '
' ); - - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + - '' + - '' + - '' + - '
' - ); - - expect( editor.getData() ).to.equal( - '
' - ); + describe( 'lists integration', () => { + it( 'should preserve empty blocks in list', () => { + editor.setData( + '
    ' + + '
  • foo
  • ' + + '
  •  
  • ' + + '
  • ' + + '
  • ' + + '
' + ); + + expect( editor.getData() ).to.equal( + '
    ' + + '
  • foo
  • ' + + '
  •  
  • ' + + '
  • ' + + '
  • ' + + '
' + ); + } ); } ); - } ); - describe( 'lists integration', () => { - it( 'should set htmlEmptyBlock on empty list item', () => { - editor.setData( '

A

' ); - - const modelData = getModelData( model, { withoutSelection: true } ); - const normalizedData = modelData.replace( / listItemId="[^"]+"/g, '' ); - - expect( normalizedData ).to.equal( - 'A' + - '' - ); + describe( 'other block elements', () => { + it( 'should handle empty headings', () => { + editor.setData( + '

foo

' + + '

' + + '

' + + '

 

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

foo

' + + '

' + + '

' + + '

 

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

A

' ); + it( 'should handle nested empty blocks', () => { + editor.setData( + '

foo

' + + '
' + + '

' + + '

bar

' + + '

' + + '

 

' + + '
' + ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + 'foo' + + '
' + + '' + + 'bar' + + '' + + '' + + '
' + ); + + expect( editor.getData() ).to.equal( + '

foo

' + + '
' + + '

' + + '

bar

' + + '

' + + '

 

' + + '
' + ); + } ); } ); } ); describe( 'editing pipeline', () => { it( 'should preserve empty paragraph in editing view', () => { - setModelData( model, '' ); - - expect( getViewData( view, { withoutSelection: true } ) ).to.equal( '

' ); - } ); - } ); - - describe( 'other block elements', () => { - it( 'should set htmlEmptyBlock on empty heading', () => { - editor.setData( '

' ); - - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + setModelData( model, + '' + + '' ); - } ); - - it( 'should preserve empty heading in output', () => { - editor.setData( 'A

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

A

' ); - } ); - it( 'should preserve mixed empty and non-empty block elements', () => { - editor.setData( '

foo

' ); - - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - '' + - 'foo' + - '' + expect( getViewData( view ) ).to.equal( + '

[]

' + + '

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

foo

' ); - } ); - - it( 'should handle nested empty blocks', () => { - editor.setData( '

A

' ); + const domRoot = editor.editing.view.getDomRoot(); - expect( getModelData( model, { withoutSelection: true } ) ).to.equal( - 'A
' + expect( domRoot.innerHTML ).to.equal( + '

' + INLINE_FILLER + '

' + + '


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

A

' ); } ); } ); From 6326054243b0e11c27a500fbe6d3b319c213e0ca Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 22 Jan 2025 19:49:40 +0100 Subject: [PATCH 24/39] Typo fix. --- packages/ckeditor5-html-support/src/emptyblock.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-html-support/src/emptyblock.ts b/packages/ckeditor5-html-support/src/emptyblock.ts index 1dbd5736254..6248f59d907 100644 --- a/packages/ckeditor5-html-support/src/emptyblock.ts +++ b/packages/ckeditor5-html-support/src/emptyblock.ts @@ -123,7 +123,7 @@ function createEmptyBlockUpcastConverter( schema: Schema ) { conversionApi.writer.setAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, true, modelElement ); - // Handle a auto-paragraphed bogus paragraph inside empty element. + // Handle an auto-paragraphed bogus paragraph inside empty element. if ( modelElement.childCount != 1 ) { return; } From 3a489d0965691ac0bfd57440b693909d3f15a410 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Thu, 23 Jan 2025 10:32:58 +0100 Subject: [PATCH 25/39] Added missing dev dependency. --- packages/ckeditor5-table/package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-table/package.json b/packages/ckeditor5-table/package.json index 73089a5cfa3..b7a83c91b77 100644 --- a/packages/ckeditor5-table/package.json +++ b/packages/ckeditor5-table/package.json @@ -29,6 +29,7 @@ "@ckeditor/ckeditor5-dev-utils": "^46.0.0", "@ckeditor/ckeditor5-editor-classic": "44.1.0", "@ckeditor/ckeditor5-editor-multi-root": "44.1.0", + "@ckeditor/ckeditor5-heading": "44.1.0", "@ckeditor/ckeditor5-highlight": "44.1.0", "@ckeditor/ckeditor5-horizontal-line": "44.1.0", "@ckeditor/ckeditor5-html-support": "44.1.0", @@ -38,10 +39,10 @@ "@ckeditor/ckeditor5-list": "44.1.0", "@ckeditor/ckeditor5-media-embed": "44.1.0", "@ckeditor/ckeditor5-paragraph": "44.1.0", + "@ckeditor/ckeditor5-source-editing": "44.1.0", "@ckeditor/ckeditor5-theme-lark": "44.1.0", "@ckeditor/ckeditor5-typing": "44.1.0", "@ckeditor/ckeditor5-undo": "44.1.0", - "@ckeditor/ckeditor5-source-editing": "44.1.0", "json-diff": "^0.5.4", "typescript": "5.0.4", "webpack": "^5.94.0", From 48ff50584d141e6673bd282c2bde67dea5ee388d Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Fri, 24 Jan 2025 06:25:48 +0100 Subject: [PATCH 26/39] Fix typos. --- .../view/domconverter/whitespace-handling-integration.js | 6 +++--- packages/ckeditor5-html-support/src/emptyblock.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js b/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js index baac0b46f3d..1f3f2039386 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js @@ -197,7 +197,7 @@ describe( 'DomConverter – whitespace handling – integration', () => { expect( editor.getData() ).to.equal( '

 foo 

' ); } ); - it( 'single nbsp inside blocks are ignored (NBSP block filler)', () => { + it( 'single nbsp inside blocks is ignored (NBSP block filler)', () => { editor.setData( '

 

' ); expect( getData( editor.model, { withoutSelection: true } ) ) @@ -207,7 +207,7 @@ describe( 'DomConverter – whitespace handling – integration', () => { expect( editor.getData( { trim: false } ) ).to.equal( '

 

' ); } ); - it( 'nbsp with spaces inside blocks are ignored (NBSP block filler)', () => { + it( 'nbsp with spaces inside blocks is ignored (NBSP block filler)', () => { editor.setData( '

\n  \n

' ); expect( getData( editor.model, { withoutSelection: true } ) ) @@ -217,7 +217,7 @@ describe( 'DomConverter – whitespace handling – integration', () => { expect( editor.getData( { trim: false } ) ).to.equal( '

 

' ); } ); - it( 'single nbsp inside blocks are ignored (marked NBSP block filler)', () => { + it( 'single nbsp inside blocks is ignored (marked NBSP block filler)', () => { editor.data.processor.useFillerType( 'marked' ); editor.conversion.for( 'upcast' ).add( dispatcher => { diff --git a/packages/ckeditor5-html-support/src/emptyblock.ts b/packages/ckeditor5-html-support/src/emptyblock.ts index 6248f59d907..e7da548e9e9 100644 --- a/packages/ckeditor5-html-support/src/emptyblock.ts +++ b/packages/ckeditor5-html-support/src/emptyblock.ts @@ -25,9 +25,9 @@ const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; * * This is useful when you want to: * - * * Preserve empty block elements exactly as they were in the source HTML - * * Allow for styling empty blocks with CSS (block fillers can interfere with height/margin) - * * Maintain compatibility with external systems that expect empty blocks to remain empty + * * Preserve empty block elements exactly as they were in the source HTML + * * Allow for styling empty blocks with CSS (block fillers can interfere with height/margin) + * * Maintain compatibility with external systems that expect empty blocks to remain empty * * For example, this allows for HTML like: * From 192eba5a702c161fc4a590f588a20301f70c118a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Bagi=C5=84ski?= Date: Fri, 24 Jan 2025 06:49:39 +0100 Subject: [PATCH 27/39] Add clipboard integration to `EmptyBlock` plugin. (#17805) Add clipboard integration to `EmptyBlock` plugin. (#17805) --- .../ckeditor5-html-support/src/emptyblock.ts | 36 ++++++- .../tests/emptyblock.js | 93 ++++++++++++++++++- .../tests/manual/emptyblock.html | 84 +++++++++++++++++ .../tests/manual/emptyblock.js | 36 +++++++ .../tests/manual/emptyblock.md | 14 +++ 5 files changed, 261 insertions(+), 2 deletions(-) create mode 100644 packages/ckeditor5-html-support/tests/manual/emptyblock.html create mode 100644 packages/ckeditor5-html-support/tests/manual/emptyblock.js create mode 100644 packages/ckeditor5-html-support/tests/manual/emptyblock.md diff --git a/packages/ckeditor5-html-support/src/emptyblock.ts b/packages/ckeditor5-html-support/src/emptyblock.ts index e7da548e9e9..0895c76842a 100644 --- a/packages/ckeditor5-html-support/src/emptyblock.ts +++ b/packages/ckeditor5-html-support/src/emptyblock.ts @@ -7,6 +7,7 @@ * @module html-support/emptyblock */ +import type { ClipboardContentInsertionEvent, ClipboardPipeline } from 'ckeditor5/src/clipboard.js'; import { Plugin } from 'ckeditor5/src/core.js'; import type { UpcastElementEvent, @@ -63,7 +64,7 @@ export default class EmptyBlock extends Plugin { * @inheritDoc */ public afterInit(): void { - const { model, conversion } = this.editor; + const { model, conversion, plugins } = this.editor; const schema = model.schema; schema.extend( '$block', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); @@ -75,6 +76,39 @@ export default class EmptyBlock extends Plugin { conversion.for( 'downcast' ).add( createEmptyBlockDowncastConverter() ); conversion.for( 'upcast' ).add( createEmptyBlockUpcastConverter( schema ) ); + + if ( plugins.has( 'ClipboardPipeline' ) ) { + this._registerClipboardPastingHandler(); + } + } + + /** + * Handle clipboard paste events: + * + * * It does not affect *copying* content from the editor, only *pasting*. + * * When content is pasted from another editor instance with `

`, + * the ` ` filler is added, so the getData result is `

 

`. + * * When content is pasted from the same editor instance with `

`, + * the ` ` filler is not added, so the getData result is `

`. + * + * @internal + */ + private _registerClipboardPastingHandler() { + const clipboardPipeline: ClipboardPipeline = this.editor.plugins.get( 'ClipboardPipeline' ); + + this.listenTo( clipboardPipeline, 'contentInsertion', ( evt, data ) => { + if ( data.sourceEditorId === this.editor.id ) { + return; + } + + this.editor.model.change( writer => { + for ( const { item } of writer.createRangeIn( data.content ) ) { + if ( item.is( 'element' ) && item.hasAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE ) ) { + writer.removeAttribute( EMPTY_BLOCK_MODEL_ATTRIBUTE, item ); + } + } + } ); + } ); } } diff --git a/packages/ckeditor5-html-support/tests/emptyblock.js b/packages/ckeditor5-html-support/tests/emptyblock.js index bd637f1b397..4bc2a9281b7 100644 --- a/packages/ckeditor5-html-support/tests/emptyblock.js +++ b/packages/ckeditor5-html-support/tests/emptyblock.js @@ -11,6 +11,7 @@ import TableEditing from '@ckeditor/ckeditor5-table/src/tableediting.js'; import Heading from '@ckeditor/ckeditor5-heading/src/heading.js'; import ListEditing from '@ckeditor/ckeditor5-list/src/list/listediting.js'; import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote.js'; +import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard.js'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js'; import { INLINE_FILLER } from '@ckeditor/ckeditor5-engine/src/view/filler.js'; @@ -26,7 +27,7 @@ describe( 'EmptyBlock', () => { document.body.appendChild( element ); editor = await ClassicTestEditor.create( element, { - plugins: [ Paragraph, TableEditing, EmptyBlock, Heading, ListEditing, BlockQuote ] + plugins: [ Paragraph, TableEditing, EmptyBlock, Heading, ListEditing, BlockQuote, Clipboard ] } ); model = editor.model; @@ -346,6 +347,82 @@ describe( 'EmptyBlock', () => { } ); } ); + describe( 'clipboard pipeline', () => { + it( 'should not crash the editor if there is no clipboard plugin', async () => { + await editor.destroy(); + + editor = await ClassicTestEditor.create( element, { + plugins: [ EmptyBlock ] + } ); + + expect( editor.plugins.get( 'EmptyBlock' ) ).to.be.instanceOf( EmptyBlock ); + } ); + + describe( 'copying content', () => { + it( 'should not add block filler to copied empty paragraphs', () => { + const dataTransferMock = createDataTransfer(); + + setModelData( model, + '[' + + ']' + ); + + view.document.fire( 'copy', { + dataTransfer: dataTransferMock, + preventDefault: sinon.spy() + } ); + + expect( dataTransferMock.getData( 'text/html' ) ).to.equal( + '

 

' + ); + } ); + } ); + + describe( 'pasting content', () => { + it( 'should not add block filler if paste within editor', () => { + const dataTransferMock = createDataTransfer( { + 'application/ckeditor5-editor-id': editor.id, + 'text/html': '

Foo

' + } ); + + view.document.fire( 'paste', { + dataTransfer: dataTransferMock, + preventDefault: () => {}, + stopPropagation: () => {}, + method: 'paste' + } ); + + expect( getModelData( model ) ).to.equal( + '' + + 'Foo[]' + ); + + expect( editor.getData() ).to.equal( '

Foo

' ); + } ); + + it( 'should add block filler if paste from another editor', () => { + const dataTransferMock = createDataTransfer( { + 'application/ckeditor5-editor-id': 'it-is-absolutely-different-editor', + 'text/html': '

Foo

' + } ); + + view.document.fire( 'paste', { + dataTransfer: dataTransferMock, + preventDefault: () => {}, + stopPropagation: () => {}, + method: 'paste' + } ); + + expect( getModelData( model ) ).to.equal( + '' + + 'Foo[]' + ); + + expect( editor.getData() ).to.equal( '

 

Foo

' ); + } ); + } ); + } ); + function registerInlinePlaceholderWidget() { model.schema.register( 'placeholder', { inheritAllFrom: '$inlineObject', @@ -380,4 +457,18 @@ describe( 'EmptyBlock', () => { view: ( _, { writer } ) => writer.createContainerElement( 'span', { class: 'placeholder' } ) } ); } + + function createDataTransfer( data = {} ) { + const store = Object.create( data ); + + return { + setData( type, data ) { + store[ type ] = data; + }, + + getData( type ) { + return store[ type ]; + } + }; + } } ); diff --git a/packages/ckeditor5-html-support/tests/manual/emptyblock.html b/packages/ckeditor5-html-support/tests/manual/emptyblock.html new file mode 100644 index 00000000000..4b950a67a68 --- /dev/null +++ b/packages/ckeditor5-html-support/tests/manual/emptyblock.html @@ -0,0 +1,84 @@ + + + + + + +
+
+

Editor 1 (with EmptyBlocks)

+
+

This is editor 1

+

+

 

+

Some text

+ + + + + + + + + +
Table cell 1Table cell 2
+
    +
  • Item 1
  • +
  • +
  • Item 3
  • +
+
+
+ +
+

Editor 2 (without EmptyBlocks)

+
+

This is editor 2

+

+

 

+

Another text

+ + + + + + + + + +
Table cell 1Table cell 2
+
    +
  • Item 1
  • +
  • +
  • Item 3
  • +
+
+
+
+ +
+

Clipboard Preview (text/html)

+

+
diff --git a/packages/ckeditor5-html-support/tests/manual/emptyblock.js b/packages/ckeditor5-html-support/tests/manual/emptyblock.js new file mode 100644 index 00000000000..c1de6906a39 --- /dev/null +++ b/packages/ckeditor5-html-support/tests/manual/emptyblock.js @@ -0,0 +1,36 @@ +/** + * @license Copyright (c) 2003-2025, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options + */ + +/* global document */ + +import { ClassicEditor } from '@ckeditor/ckeditor5-editor-classic'; +import { SourceEditing } from '@ckeditor/ckeditor5-source-editing'; +import { Essentials } from '@ckeditor/ckeditor5-essentials'; +import { Paragraph } from '@ckeditor/ckeditor5-paragraph'; +import { Heading } from '@ckeditor/ckeditor5-heading'; +import { Table } from '@ckeditor/ckeditor5-table'; +import { List, ListProperties } from '@ckeditor/ckeditor5-list'; +import { Bold, Code, Italic } from '@ckeditor/ckeditor5-basic-styles'; +import EmptyBlock from '../../src/emptyblock.js'; + +const config = { + plugins: [ Table, Essentials, List, ListProperties, Bold, Italic, Code, Paragraph, Heading, SourceEditing, EmptyBlock ], + toolbar: [ 'sourceEditing', '|', 'insertTable', 'bulletedList', 'numberedList', 'bold', 'italic', 'heading' ] +}; + +ClassicEditor.create( document.getElementById( 'editor1' ), config ); +ClassicEditor.create( document.getElementById( 'editor2' ), { + ...config, + plugins: config.plugins.filter( plugin => plugin !== EmptyBlock ) +} ); + +const clipboardPreview = document.getElementById( 'clipboard-preview' ); + +function handleClipboardEvent( evt ) { + clipboardPreview.textContent = evt.clipboardData.getData( 'text/html' ); +} + +document.addEventListener( 'copy', handleClipboardEvent ); +document.addEventListener( 'cut', handleClipboardEvent ); diff --git a/packages/ckeditor5-html-support/tests/manual/emptyblock.md b/packages/ckeditor5-html-support/tests/manual/emptyblock.md new file mode 100644 index 00000000000..f7992de0ac4 --- /dev/null +++ b/packages/ckeditor5-html-support/tests/manual/emptyblock.md @@ -0,0 +1,14 @@ +# Empty Block Manual Test + +## Test scenario + +1. You should see two editors side by side and a clipboard preview area below them. +2. Both editors have the source editing plugin enabled. +3. The clipboard preview area shows the HTML content of the last clipboard operation (copy/cut). + +### Things to check + +1. If you copy `

 

` from the editor without the EmptyBlock plugin and then paste it to the editor with the EmptyBlock plugin, then the pasted content should be `

 

`. +2. If you copy `

 

` from the editor with the EmptyBlock plugin and then paste it to the editor without the EmptyBlock plugin, then the pasted content should be `

 

`. +3. If you copy `

` from the editor with the EmptyBlock plugin and then paste it to the editor without the EmptyBlock plugin, then the pasted content should be `

 

`. +4. If you copy `

` from the editor with the EmptyBlock plugin and then paste it to the same editor, then the pasted content should be `

`. From b6400a52906c56e2c56c509e82e58f21d9e6416a Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 27 Jan 2025 13:28:14 +0100 Subject: [PATCH 28/39] Assign editor in manual tests to window variables. --- .../tests/manual/emptyblock.js | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-html-support/tests/manual/emptyblock.js b/packages/ckeditor5-html-support/tests/manual/emptyblock.js index c1de6906a39..f855caa5059 100644 --- a/packages/ckeditor5-html-support/tests/manual/emptyblock.js +++ b/packages/ckeditor5-html-support/tests/manual/emptyblock.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options */ -/* global document */ +/* global document, window */ import { ClassicEditor } from '@ckeditor/ckeditor5-editor-classic'; import { SourceEditing } from '@ckeditor/ckeditor5-source-editing'; @@ -20,11 +20,20 @@ const config = { toolbar: [ 'sourceEditing', '|', 'insertTable', 'bulletedList', 'numberedList', 'bold', 'italic', 'heading' ] }; -ClassicEditor.create( document.getElementById( 'editor1' ), config ); -ClassicEditor.create( document.getElementById( 'editor2' ), { - ...config, - plugins: config.plugins.filter( plugin => plugin !== EmptyBlock ) -} ); +ClassicEditor + .create( document.getElementById( 'editor1' ), config ) + .then( instance => { + window.editor1 = instance; + } ); + +ClassicEditor + .create( document.getElementById( 'editor2' ), { + ...config, + plugins: config.plugins.filter( plugin => plugin !== EmptyBlock ) + } ) + .then( instance => { + window.editor2 = instance; + } ); const clipboardPreview = document.getElementById( 'clipboard-preview' ); From d4b60a84d6d5e809936d6376b14d77b66a7dc9f2 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 27 Jan 2025 14:00:19 +0100 Subject: [PATCH 29/39] Add inspector to manual demo. --- packages/ckeditor5-html-support/tests/manual/emptyblock.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-html-support/tests/manual/emptyblock.js b/packages/ckeditor5-html-support/tests/manual/emptyblock.js index f855caa5059..19acbeeb2ad 100644 --- a/packages/ckeditor5-html-support/tests/manual/emptyblock.js +++ b/packages/ckeditor5-html-support/tests/manual/emptyblock.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options */ -/* global document, window */ +/* global CKEditorInspector, document, window */ import { ClassicEditor } from '@ckeditor/ckeditor5-editor-classic'; import { SourceEditing } from '@ckeditor/ckeditor5-source-editing'; @@ -24,6 +24,7 @@ ClassicEditor .create( document.getElementById( 'editor1' ), config ) .then( instance => { window.editor1 = instance; + CKEditorInspector.attach( { 'With EmptyBlock plugin': instance } ); } ); ClassicEditor @@ -33,6 +34,7 @@ ClassicEditor } ) .then( instance => { window.editor2 = instance; + CKEditorInspector.attach( { 'Without EmptyBlock plugin': instance } ); } ); const clipboardPreview = document.getElementById( 'clipboard-preview' ); From aafc9b6044c2a01139d8ba5bfaf9c668a6f39780 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 28 Jan 2025 10:56:26 +0100 Subject: [PATCH 30/39] Add experimental warning. --- packages/ckeditor5-html-support/src/emptyblock.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblock.ts b/packages/ckeditor5-html-support/src/emptyblock.ts index 0895c76842a..fff277527ae 100644 --- a/packages/ckeditor5-html-support/src/emptyblock.ts +++ b/packages/ckeditor5-html-support/src/emptyblock.ts @@ -24,11 +24,18 @@ const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; * This plugin allows for preserving empty block elements in the editor content instead of * automatically filling them with block fillers (` `). * + * **Warning**: This is an experimental plugin. It may have bugs and breaking changes may be introduced without prior notice. + * + * Known limitations: + * * Empty blocks may not work correctly with revision history features. + * * Keyboard navigation through the document might behave unexpectedly, especially when + * navigating through structures like lists and tables. + * * This is useful when you want to: * - * * Preserve empty block elements exactly as they were in the source HTML - * * Allow for styling empty blocks with CSS (block fillers can interfere with height/margin) - * * Maintain compatibility with external systems that expect empty blocks to remain empty + * * Preserve empty block elements exactly as they were in the source HTML. + * * Allow for styling empty blocks with CSS (block fillers can interfere with height/margin). + * * Maintain compatibility with external systems that expect empty blocks to remain empty. * * For example, this allows for HTML like: * From 78347155f15f7dd6ea99d3468dd047ada621315b Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 28 Jan 2025 11:08:07 +0100 Subject: [PATCH 31/39] Add GHS empty div test. --- .../tests/emptyblock.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/packages/ckeditor5-html-support/tests/emptyblock.js b/packages/ckeditor5-html-support/tests/emptyblock.js index 4bc2a9281b7..754376ae2d8 100644 --- a/packages/ckeditor5-html-support/tests/emptyblock.js +++ b/packages/ckeditor5-html-support/tests/emptyblock.js @@ -12,6 +12,7 @@ import Heading from '@ckeditor/ckeditor5-heading/src/heading.js'; import ListEditing from '@ckeditor/ckeditor5-list/src/list/listediting.js'; import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote.js'; import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard.js'; +import GeneralHtmlSupport from '@ckeditor/ckeditor5-html-support/src/generalhtmlsupport.js'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js'; import { INLINE_FILLER } from '@ckeditor/ckeditor5-engine/src/view/filler.js'; @@ -423,6 +424,39 @@ describe( 'EmptyBlock', () => { } ); } ); + describe( 'GHS', () => { + let dataFilter; + + beforeEach( async () => { + await editor.destroy(); + + editor = await ClassicTestEditor.create( element, { + plugins: [ Paragraph, TableEditing, EmptyBlock, Heading, ListEditing, BlockQuote, Clipboard, GeneralHtmlSupport ] + } ); + + model = editor.model; + view = editor.editing.view; + + dataFilter = editor.plugins.get( 'DataFilter' ); + dataFilter.allowElement( /.*/ ); + } ); + + it( 'should handle empty div elements', () => { + editor.setData( + '
foo
' + ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + + 'foo' + ); + + expect( editor.getData() ).to.equal( + '
foo
' + ); + } ); + } ); + function registerInlinePlaceholderWidget() { model.schema.register( 'placeholder', { inheritAllFrom: '$inlineObject', From 72ca176c31156f5cf60b3ad73f6d3cc4a64a1e6b Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 28 Jan 2025 11:36:50 +0100 Subject: [PATCH 32/39] Fix self import. --- packages/ckeditor5-html-support/tests/emptyblock.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-html-support/tests/emptyblock.js b/packages/ckeditor5-html-support/tests/emptyblock.js index 754376ae2d8..2c35f2f4a87 100644 --- a/packages/ckeditor5-html-support/tests/emptyblock.js +++ b/packages/ckeditor5-html-support/tests/emptyblock.js @@ -12,7 +12,7 @@ import Heading from '@ckeditor/ckeditor5-heading/src/heading.js'; import ListEditing from '@ckeditor/ckeditor5-list/src/list/listediting.js'; import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote.js'; import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard.js'; -import GeneralHtmlSupport from '@ckeditor/ckeditor5-html-support/src/generalhtmlsupport.js'; +import GeneralHtmlSupport from '../src/generalhtmlsupport.js'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js'; import { INLINE_FILLER } from '@ckeditor/ckeditor5-engine/src/view/filler.js'; From 4f733a6365af9e3b222b2d0d1a9d94a053363c49 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 28 Jan 2025 15:13:39 +0100 Subject: [PATCH 33/39] Reorder docs. --- .../ckeditor5-html-support/src/emptyblock.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/ckeditor5-html-support/src/emptyblock.ts b/packages/ckeditor5-html-support/src/emptyblock.ts index fff277527ae..8daf5dcb570 100644 --- a/packages/ckeditor5-html-support/src/emptyblock.ts +++ b/packages/ckeditor5-html-support/src/emptyblock.ts @@ -21,21 +21,22 @@ import type { const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; /** - * This plugin allows for preserving empty block elements in the editor content instead of - * automatically filling them with block fillers (` `). + * This is experimental plugin that allows for preserving empty block elements + * in the editor content instead of automatically filling them with block fillers (` `). * - * **Warning**: This is an experimental plugin. It may have bugs and breaking changes may be introduced without prior notice. + * This is useful when you want to: + * + * * Preserve empty block elements exactly as they were in the source HTML. + * * Allow for styling empty blocks with CSS (block fillers can interfere with height/margin). + * * Maintain compatibility with external systems that expect empty blocks to remain empty. * * Known limitations: + * * * Empty blocks may not work correctly with revision history features. * * Keyboard navigation through the document might behave unexpectedly, especially when * navigating through structures like lists and tables. * - * This is useful when you want to: - * - * * Preserve empty block elements exactly as they were in the source HTML. - * * Allow for styling empty blocks with CSS (block fillers can interfere with height/margin). - * * Maintain compatibility with external systems that expect empty blocks to remain empty. + * **Warning**: This is an experimental plugin. It may have bugs and breaking changes may be introduced without prior notice. * * For example, this allows for HTML like: * From 66fa312d94945293f74995c200e4325b04c57261 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 28 Jan 2025 15:14:18 +0100 Subject: [PATCH 34/39] Reorder docs. --- packages/ckeditor5-html-support/src/emptyblock.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-html-support/src/emptyblock.ts b/packages/ckeditor5-html-support/src/emptyblock.ts index 8daf5dcb570..c3269517150 100644 --- a/packages/ckeditor5-html-support/src/emptyblock.ts +++ b/packages/ckeditor5-html-support/src/emptyblock.ts @@ -36,7 +36,6 @@ const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; * * Keyboard navigation through the document might behave unexpectedly, especially when * navigating through structures like lists and tables. * - * **Warning**: This is an experimental plugin. It may have bugs and breaking changes may be introduced without prior notice. * * For example, this allows for HTML like: * @@ -52,6 +51,8 @@ const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; *

 

*   * ``` + * + * **Warning**: This is an experimental plugin. It may have bugs and breaking changes may be introduced without prior notice. */ export default class EmptyBlock extends Plugin { /** From 5f486b77978ed191b51a1630ab4d81e01a7b8c87 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 28 Jan 2025 15:14:51 +0100 Subject: [PATCH 35/39] Fix newline. --- packages/ckeditor5-html-support/src/emptyblock.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/ckeditor5-html-support/src/emptyblock.ts b/packages/ckeditor5-html-support/src/emptyblock.ts index c3269517150..7a4dcd51ff3 100644 --- a/packages/ckeditor5-html-support/src/emptyblock.ts +++ b/packages/ckeditor5-html-support/src/emptyblock.ts @@ -36,7 +36,6 @@ const EMPTY_BLOCK_MODEL_ATTRIBUTE = 'htmlEmptyBlock'; * * Keyboard navigation through the document might behave unexpectedly, especially when * navigating through structures like lists and tables. * - * * For example, this allows for HTML like: * * ```html From d8cafb77ff95d4bf3d1cd8ba4ad06eb4ebc3dd87 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Thu, 30 Jan 2025 08:27:26 +0100 Subject: [PATCH 36/39] Add configuration for preserving block filler in editing view. --- .../ckeditor5-html-support/src/emptyblock.ts | 10 +++- .../src/emptyblockconfig.ts | 34 +++++++++++ .../tests/emptyblock.js | 59 +++++++++++++++---- .../tests/manual/emptyblock.html | 21 +++++++ .../tests/manual/emptyblock.js | 37 ++++++++++-- 5 files changed, 142 insertions(+), 19 deletions(-) create mode 100644 packages/ckeditor5-html-support/src/emptyblockconfig.ts diff --git a/packages/ckeditor5-html-support/src/emptyblock.ts b/packages/ckeditor5-html-support/src/emptyblock.ts index 7a4dcd51ff3..b1b545263e1 100644 --- a/packages/ckeditor5-html-support/src/emptyblock.ts +++ b/packages/ckeditor5-html-support/src/emptyblock.ts @@ -72,8 +72,9 @@ export default class EmptyBlock extends Plugin { * @inheritDoc */ public afterInit(): void { - const { model, conversion, plugins } = this.editor; + const { model, conversion, plugins, config } = this.editor; const schema = model.schema; + const preserveInEditingView = config.get( 'emptyBlock.preserveInEditingView' ); schema.extend( '$block', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); schema.extend( '$container', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); @@ -82,7 +83,12 @@ export default class EmptyBlock extends Plugin { schema.extend( 'tableCell', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); } - conversion.for( 'downcast' ).add( createEmptyBlockDowncastConverter() ); + if ( preserveInEditingView ) { + conversion.for( 'downcast' ).add( createEmptyBlockDowncastConverter() ); + } else { + conversion.for( 'dataDowncast' ).add( createEmptyBlockDowncastConverter() ); + } + conversion.for( 'upcast' ).add( createEmptyBlockUpcastConverter( schema ) ); if ( plugins.has( 'ClipboardPipeline' ) ) { diff --git a/packages/ckeditor5-html-support/src/emptyblockconfig.ts b/packages/ckeditor5-html-support/src/emptyblockconfig.ts new file mode 100644 index 00000000000..add924d7283 --- /dev/null +++ b/packages/ckeditor5-html-support/src/emptyblockconfig.ts @@ -0,0 +1,34 @@ +/** + * @license Copyright (c) 2003-2025, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options + */ + +/** + * @module html-support/emptyblockconfig + */ + +/** + * The configuration of the Empty Block feature. + * The option is used by the {@link module:html-support/emptyblock~EmptyBlock} feature. + * + * ```ts + * ClassicEditor + * .create( { + * emptyBlock: ... // Empty block feature config. + * } ) + * .then( ... ) + * .catch( ... ); + * ``` + * + * See {@link module:core/editor/editorconfig~EditorConfig all editor options}. + */ +export interface EmptyBlockConfig { + + /** + * When set to `true`, empty blocks will be preserved in the editing view. + * When `false` (default), empty blocks are only preserved in the data output. + * + * @default false + */ + preserveInEditingView?: boolean; +} diff --git a/packages/ckeditor5-html-support/tests/emptyblock.js b/packages/ckeditor5-html-support/tests/emptyblock.js index 2c35f2f4a87..f83cfd86b98 100644 --- a/packages/ckeditor5-html-support/tests/emptyblock.js +++ b/packages/ckeditor5-html-support/tests/emptyblock.js @@ -14,7 +14,6 @@ import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote.js'; import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard.js'; import GeneralHtmlSupport from '../src/generalhtmlsupport.js'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; -import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js'; import { INLINE_FILLER } from '@ckeditor/ckeditor5-engine/src/view/filler.js'; import EmptyBlock from '../src/emptyblock.js'; @@ -327,23 +326,61 @@ describe( 'EmptyBlock', () => { } ); } ); - describe( 'editing pipeline', () => { - it( 'should preserve empty paragraph in editing view', () => { - setModelData( model, - '' + - '' + describe( 'config.emptyBlock.preserveInEditingView', () => { + it( 'should preserve empty blocks in editing view when enabled', async () => { + await editor.destroy(); + + editor = await ClassicTestEditor.create( element, { + plugins: [ Paragraph, TableEditing, EmptyBlock, Heading, ListEditing, BlockQuote, Clipboard ], + emptyBlock: { + preserveInEditingView: true + } + } ); + + editor.setData( '

foo

' ); + + const domRoot = editor.editing.view.getDomRoot(); + + expect( domRoot.innerHTML ).to.equal( + '

' + INLINE_FILLER + '

' + + '

foo

' ); + } ); - expect( getViewData( view ) ).to.equal( - '

[]

' + - '

' + it( 'should not preserve empty blocks in editing view when disabled', async () => { + await editor.destroy(); + + editor = await ClassicTestEditor.create( element, { + plugins: [ Paragraph, TableEditing, EmptyBlock, Heading, ListEditing, BlockQuote, Clipboard ], + emptyBlock: { + preserveInEditingView: false + } + } ); + + editor.setData( '

foo

' ); + + const domRoot = editor.editing.view.getDomRoot(); + + expect( domRoot.innerHTML ).to.equal( + '


' + + '

foo

' ); + } ); + + it( 'should not preserve empty blocks in editing view by default', async () => { + await editor.destroy(); + + editor = await ClassicTestEditor.create( element, { + plugins: [ Paragraph, TableEditing, EmptyBlock, Heading, ListEditing, BlockQuote, Clipboard ] + } ); + + editor.setData( '

foo

' ); const domRoot = editor.editing.view.getDomRoot(); expect( domRoot.innerHTML ).to.equal( - '

' + INLINE_FILLER + '

' + - '


' + '


' + + '

foo

' ); } ); } ); diff --git a/packages/ckeditor5-html-support/tests/manual/emptyblock.html b/packages/ckeditor5-html-support/tests/manual/emptyblock.html index 4b950a67a68..32afae22c30 100644 --- a/packages/ckeditor5-html-support/tests/manual/emptyblock.html +++ b/packages/ckeditor5-html-support/tests/manual/emptyblock.html @@ -23,9 +23,20 @@ background: #f0f0f0; white-space: pre-wrap; } + + .controls { + margin-bottom: 20px; + } +
+ +
+

Editor 1 (with EmptyBlocks)

@@ -34,6 +45,11 @@

Editor 1 (with EmptyBlocks)

 

Some text

+
    +
  • +
  • +
  • +
@@ -59,6 +75,11 @@

Editor 2 (without EmptyBlocks)

 

Another text

+
    +
  • +
  • +
  • +
Table cell 1
diff --git a/packages/ckeditor5-html-support/tests/manual/emptyblock.js b/packages/ckeditor5-html-support/tests/manual/emptyblock.js index 19acbeeb2ad..97f74a65dca 100644 --- a/packages/ckeditor5-html-support/tests/manual/emptyblock.js +++ b/packages/ckeditor5-html-support/tests/manual/emptyblock.js @@ -20,12 +20,24 @@ const config = { toolbar: [ 'sourceEditing', '|', 'insertTable', 'bulletedList', 'numberedList', 'bold', 'italic', 'heading' ] }; -ClassicEditor - .create( document.getElementById( 'editor1' ), config ) - .then( instance => { - window.editor1 = instance; - CKEditorInspector.attach( { 'With EmptyBlock plugin': instance } ); - } ); +const preserveEmptyBlocksCheckbox = document.getElementById( 'preserve-empty-blocks' ); + +function createEditor1( preserveInEditingView ) { + return ClassicEditor + .create( document.getElementById( 'editor1' ), { + ...config, + emptyBlock: { + preserveInEditingView + } + } ) + .then( instance => { + window.editor1 = instance; + CKEditorInspector.attach( { 'With EmptyBlock plugin': instance } ); + } ); +} + +// Initial editor creation +createEditor1( preserveEmptyBlocksCheckbox.checked ); ClassicEditor .create( document.getElementById( 'editor2' ), { @@ -45,3 +57,16 @@ function handleClipboardEvent( evt ) { document.addEventListener( 'copy', handleClipboardEvent ); document.addEventListener( 'cut', handleClipboardEvent ); + +preserveEmptyBlocksCheckbox.addEventListener( 'change', async () => { + const editorElement = document.getElementById( 'editor1' ); + const editorData = window.editor1.getData(); + + await window.editor1.destroy(); + + // Restore any content that was in the editor + editorElement.innerHTML = editorData; + + // Create new editor instance with updated configuration + await createEditor1( preserveEmptyBlocksCheckbox.checked ); +} ); From f72b720185cd172d21204fbdb642981ca7192048 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Fri, 31 Jan 2025 15:37:47 +0100 Subject: [PATCH 37/39] Apply CR remarks. --- .../ckeditor5-html-support/src/emptyblock.ts | 14 ++++---- .../src/emptyblockconfig.ts | 34 ------------------- .../src/generalhtmlsupportconfig.ts | 16 +++++++++ .../tests/emptyblock.js | 12 ++++--- .../tests/manual/emptyblock.js | 6 ++-- 5 files changed, 34 insertions(+), 48 deletions(-) delete mode 100644 packages/ckeditor5-html-support/src/emptyblockconfig.ts diff --git a/packages/ckeditor5-html-support/src/emptyblock.ts b/packages/ckeditor5-html-support/src/emptyblock.ts index b1b545263e1..861213d53f9 100644 --- a/packages/ckeditor5-html-support/src/emptyblock.ts +++ b/packages/ckeditor5-html-support/src/emptyblock.ts @@ -74,7 +74,7 @@ export default class EmptyBlock extends Plugin { public afterInit(): void { const { model, conversion, plugins, config } = this.editor; const schema = model.schema; - const preserveInEditingView = config.get( 'emptyBlock.preserveInEditingView' ); + const preserveInEditingView = config.get( 'htmlSupport.emptyBlock.preserveInEditingView' ); schema.extend( '$block', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); schema.extend( '$container', { allowAttributes: [ EMPTY_BLOCK_MODEL_ATTRIBUTE ] } ); @@ -99,13 +99,11 @@ export default class EmptyBlock extends Plugin { /** * Handle clipboard paste events: * - * * It does not affect *copying* content from the editor, only *pasting*. - * * When content is pasted from another editor instance with `

`, - * the ` ` filler is added, so the getData result is `

 

`. - * * When content is pasted from the same editor instance with `

`, - * the ` ` filler is not added, so the getData result is `

`. - * - * @internal + * * It does not affect *copying* content from the editor, only *pasting*. + * * When content is pasted from another editor instance with `

`, + * the ` ` filler is added, so the getData result is `

 

`. + * * When content is pasted from the same editor instance with `

`, + * the ` ` filler is not added, so the getData result is `

`. */ private _registerClipboardPastingHandler() { const clipboardPipeline: ClipboardPipeline = this.editor.plugins.get( 'ClipboardPipeline' ); diff --git a/packages/ckeditor5-html-support/src/emptyblockconfig.ts b/packages/ckeditor5-html-support/src/emptyblockconfig.ts deleted file mode 100644 index add924d7283..00000000000 --- a/packages/ckeditor5-html-support/src/emptyblockconfig.ts +++ /dev/null @@ -1,34 +0,0 @@ -/** - * @license Copyright (c) 2003-2025, CKSource Holding sp. z o.o. All rights reserved. - * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options - */ - -/** - * @module html-support/emptyblockconfig - */ - -/** - * The configuration of the Empty Block feature. - * The option is used by the {@link module:html-support/emptyblock~EmptyBlock} feature. - * - * ```ts - * ClassicEditor - * .create( { - * emptyBlock: ... // Empty block feature config. - * } ) - * .then( ... ) - * .catch( ... ); - * ``` - * - * See {@link module:core/editor/editorconfig~EditorConfig all editor options}. - */ -export interface EmptyBlockConfig { - - /** - * When set to `true`, empty blocks will be preserved in the editing view. - * When `false` (default), empty blocks are only preserved in the data output. - * - * @default false - */ - preserveInEditingView?: boolean; -} diff --git a/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts b/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts index 6f45970f71b..a67046ac38c 100644 --- a/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts +++ b/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts @@ -80,4 +80,20 @@ export interface GeneralHtmlSupportConfig { * ``` */ allowEmpty?: Array; + + /** + * The configuration of allowed empty block elements that should not be removed. + * + * The option is used by the {@link module:html-support/emptyblock~EmptyBlock} feature. + */ + emptyBlock?: { + + /** + * When set to `true`, empty blocks will be preserved in the editing view. + * When `false` (default), empty blocks are only preserved in the data output. + * + * @default false + */ + preserveInEditingView?: boolean; + }; } diff --git a/packages/ckeditor5-html-support/tests/emptyblock.js b/packages/ckeditor5-html-support/tests/emptyblock.js index f83cfd86b98..919ee47d6c9 100644 --- a/packages/ckeditor5-html-support/tests/emptyblock.js +++ b/packages/ckeditor5-html-support/tests/emptyblock.js @@ -332,8 +332,10 @@ describe( 'EmptyBlock', () => { editor = await ClassicTestEditor.create( element, { plugins: [ Paragraph, TableEditing, EmptyBlock, Heading, ListEditing, BlockQuote, Clipboard ], - emptyBlock: { - preserveInEditingView: true + htmlSupport: { + emptyBlock: { + preserveInEditingView: true + } } } ); @@ -352,8 +354,10 @@ describe( 'EmptyBlock', () => { editor = await ClassicTestEditor.create( element, { plugins: [ Paragraph, TableEditing, EmptyBlock, Heading, ListEditing, BlockQuote, Clipboard ], - emptyBlock: { - preserveInEditingView: false + htmlSupport: { + emptyBlock: { + preserveInEditingView: false + } } } ); diff --git a/packages/ckeditor5-html-support/tests/manual/emptyblock.js b/packages/ckeditor5-html-support/tests/manual/emptyblock.js index 97f74a65dca..1aa3741fb3d 100644 --- a/packages/ckeditor5-html-support/tests/manual/emptyblock.js +++ b/packages/ckeditor5-html-support/tests/manual/emptyblock.js @@ -26,8 +26,10 @@ function createEditor1( preserveInEditingView ) { return ClassicEditor .create( document.getElementById( 'editor1' ), { ...config, - emptyBlock: { - preserveInEditingView + htmlSupport: { + emptyBlock: { + preserveInEditingView + } } } ) .then( instance => { From 39be496022680706fff9a661f489f5b6eecf87cd Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Fri, 31 Jan 2025 15:52:40 +0100 Subject: [PATCH 38/39] Apply CR remark. --- .../ckeditor5-html-support/src/generalhtmlsupportconfig.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts b/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts index a67046ac38c..5f486da7b77 100644 --- a/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts +++ b/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts @@ -82,7 +82,8 @@ export interface GeneralHtmlSupportConfig { allowEmpty?: Array; /** - * The configuration of allowed empty block elements that should not be removed. + * Whether a filler text (non-breaking space entity —  ) will be inserted into empty block elements in HTML output. + * This is used to render block elements properly with line-height. * * The option is used by the {@link module:html-support/emptyblock~EmptyBlock} feature. */ From 73d1dbc6615aba21f317257e1fcdd00230a5dff6 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Fri, 31 Jan 2025 16:01:31 +0100 Subject: [PATCH 39/39] Fix typo. --- packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts b/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts index 5f486da7b77..898dabd64ba 100644 --- a/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts +++ b/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts @@ -82,7 +82,7 @@ export interface GeneralHtmlSupportConfig { allowEmpty?: Array; /** - * Whether a filler text (non-breaking space entity —  ) will be inserted into empty block elements in HTML output. + * Whether a filler text (non-breaking space entity — ` `) will be inserted into empty block elements in HTML output. * This is used to render block elements properly with line-height. * * The option is used by the {@link module:html-support/emptyblock~EmptyBlock} feature.
Table cell 1