Merge pull request #8858 from ckeditor/i/8788

Fix (page-break): Dropping an image on the page break widget should not crash the editor. Closes #8788.

packages/ckeditor5-page-break/src/pagebreakediting.js

@@ -66,16 +66,19 @@ export default class PageBreakEditing extends Plugin {
 			view: ( modelElement, { writer } ) => {
 				const label = t( 'Page break' );
 				const viewWrapper = writer.createContainerElement( 'div' );
-				const viewLabelElement = writer.createContainerElement( 'span' );
-				const innerText = writer.createText( t( 'Page break' ) );
+				const viewLabelElement = writer.createUIElement(
+					'span',
+					{ class: 'page-break__label' },
+					function( domDocument ) {
+						const domElement = this.toDomElement( domDocument );
+						domElement.innerText = t( 'Page break' );
+
+						return domElement;
+					}
+				);
 
 				writer.addClass( 'page-break', viewWrapper );
-				writer.setCustomProperty( 'pageBreak', true, viewWrapper );
-
-				writer.addClass( 'page-break__label', viewLabelElement );
-
 				writer.insert( writer.createPositionAt( viewWrapper, 0 ), viewLabelElement );
-				writer.insert( writer.createPositionAt( viewLabelElement, 0 ), innerText );
 
 				return toPageBreakWidget( viewWrapper, writer, label );
 			}

packages/ckeditor5-page-break/tests/pagebreakediting.js

@@ -50,6 +50,17 @@ describe( 'PageBreakEditing', () => {
 		expect( editor.commands.get( 'pageBreak' ) ).to.be.instanceOf( PageBreakCommand );
 	} );
 
+	// https://github.com/ckeditor/ckeditor5/issues/8788.
+	// Proper integration testing of this is too complex.
+	// Making sure the label is no longer a regular text element should be enough.
+	it( 'should have label as a UIElement', () => {
+		setModelData( model, '[<pageBreak></pageBreak>]' );
+		const textNode = viewDocument.getRoot().getChild( 0 ).getChild( 0 );
+
+		expect( textNode.is( 'uiElement' ) ).to.be.true;
+		expect( textNode.hasClass( 'page-break__label' ) ).to.be.true;
+	} );
+
 	describe( 'conversion in data pipeline', () => {
 		describe( 'model to view', () => {
 			it( 'should convert', () => {
@@ -252,7 +263,12 @@ describe( 'PageBreakEditing', () => {
 			it( 'should convert', () => {
 				setModelData( model, '<pageBreak></pageBreak>' );
 
+				// The page break label should be an UI element, thus should not be rendered by default.
 				expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
+					'<div class="ck-widget page-break" contenteditable="false"><span class="page-break__label"></span></div>'
+				);
+
+				expect( getViewData( view, { withoutSelection: true, renderUIElements: true } ) ).to.equal(
 					'<div class="ck-widget page-break" contenteditable="false"><span class="page-break__label">Page break</span></div>'
 				);
 			} );