Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #1426 from ckeditor/t/1422
Browse files Browse the repository at this point in the history
Fix: Block filler will be inserted into the container if its last child is a `<br>` element. Closes #1422.
  • Loading branch information
Reinmar authored Jun 7, 2018
2 parents 62a0324 + a0c8bf0 commit ba3d641
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
10 changes: 9 additions & 1 deletion src/view/containerelement.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,15 @@ export default class ContainerElement extends Element {
//
// @returns {Number|null} Block filler offset or `null` if block filler is not needed.
function getFillerOffset() {
for ( const child of this.getChildren() ) {
const children = [ ...this.getChildren() ];
const lastChild = children[ this.childCount - 1 ];

// Block filler is required after a `<br>` if it's the last element in its container. See #1422.
if ( lastChild && lastChild.is( 'element', 'br' ) ) {
return this.childCount;
}

for ( const child of children ) {
// If there's any non-UI element – don't render the bogus.
if ( !child.is( 'uiElement' ) ) {
return null;
Expand Down
35 changes: 35 additions & 0 deletions tests/view/containerelement.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,40 @@ describe( 'ContainerElement', () => {
it( 'should return null if element is not empty', () => {
expect( parse( '<container:p>foo</container:p>' ).getFillerOffset() ).to.be.null;
} );

// Block filler is required after the `<br>` element if the element is the last child in the container. See #1422.
describe( 'for <br> elements in container', () => {
it( 'returns null because container does not need the block filler', () => {
expect( parse( '<container:p>Foo.</container:p>' ).getFillerOffset() ).to.equals( null );
} );

it( 'returns offset of the last child which is the <br> element (1)', () => {
expect( parse( '<container:p><empty:br></empty:br></container:p>' ).getFillerOffset() ).to.equals( 1 );
} );

it( 'returns offset of the last child which is the <br> element (2)', () => {
expect( parse( '<container:p>Foo.<empty:br></empty:br></container:p>' ).getFillerOffset() ).to.equals( 2 );
} );

it( 'always returns the last <br> element in the container', () => {
expect( parse( '<container:p>Foo.<empty:br></empty:br><empty:br></empty:br></container:p>' ).getFillerOffset() )
.to.equals( 3 );
} );

it( 'works fine with non-empty container with multi <br> elements', () => {
expect( parse( '<container:p>Foo.<empty:br></empty:br>Bar.<empty:br></empty:br></container:p>' ).getFillerOffset() )
.to.equals( 4 );
} );

it( 'ignores the ui elements', () => {
expect( parse( '<container:p><ui:span></ui:span><empty:br></empty:br></container:p>' ).getFillerOffset() )
.to.equals( 2 );
} );

it( 'empty element must be the <br> element', () => {
expect( parse( '<container:p>Foo<empty:img></empty:img></container:p>' ).getFillerOffset() )
.to.equals( null );
} );
} );
} );
} );

0 comments on commit ba3d641

Please sign in to comment.