From a445f1ffb0ffae0dd23ca6f1932eaa9adaa3c9a7 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Thu, 7 Jun 2018 09:40:10 +0200 Subject: [PATCH 1/2] Block filler will be inserted to container if its last child is the
element. --- src/view/containerelement.js | 10 +++++++++- tests/view/containerelement.js | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/view/containerelement.js b/src/view/containerelement.js index 3735a0e9d..16ed2cb9b 100644 --- a/src/view/containerelement.js +++ b/src/view/containerelement.js @@ -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 the `
` element if the element is the last child in the 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; diff --git a/tests/view/containerelement.js b/tests/view/containerelement.js index 131cadf16..d3a29dc1d 100644 --- a/tests/view/containerelement.js +++ b/tests/view/containerelement.js @@ -59,5 +59,40 @@ describe( 'ContainerElement', () => { it( 'should return null if element is not empty', () => { expect( parse( 'foo' ).getFillerOffset() ).to.be.null; } ); + + // Block filler is required after the `
` element if the element is the last child in the container. See #1422. + describe( 'for
elements in container', () => { + it( 'returns null because container does not need the block filler', () => { + expect( parse( 'Foo.' ).getFillerOffset() ).to.equals( null ); + } ); + + it( 'returns offset of the last child which is the
element (1)', () => { + expect( parse( '' ).getFillerOffset() ).to.equals( 1 ); + } ); + + it( 'returns offset of the last child which is the
element (2)', () => { + expect( parse( 'Foo.' ).getFillerOffset() ).to.equals( 2 ); + } ); + + it( 'always returns the last
element in the container', () => { + expect( parse( 'Foo.' ).getFillerOffset() ) + .to.equals( 3 ); + } ); + + it( 'works fine with non-empty container with multi
elements', () => { + expect( parse( 'Foo.Bar.' ).getFillerOffset() ) + .to.equals( 4 ); + } ); + + it( 'ignores the ui elements', () => { + expect( parse( '' ).getFillerOffset() ) + .to.equals( 2 ); + } ); + + it( 'empty element must be the
element', () => { + expect( parse( 'Foo' ).getFillerOffset() ) + .to.equals( null ); + } ); + } ); } ); } ); From a0c8bf061f0acf9dff89e92e096e65a749a67285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 7 Jun 2018 11:15:48 +0200 Subject: [PATCH 2/2] Wording. --- src/view/containerelement.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/view/containerelement.js b/src/view/containerelement.js index 16ed2cb9b..3c0aad494 100644 --- a/src/view/containerelement.js +++ b/src/view/containerelement.js @@ -83,7 +83,7 @@ function getFillerOffset() { const children = [ ...this.getChildren() ]; const lastChild = children[ this.childCount - 1 ]; - // Block filler is required after the `
` element if the element is the last child in the container. See #1422. + // Block filler is required after a `
` if it's the last element in its container. See #1422. if ( lastChild && lastChild.is( 'element', 'br' ) ) { return this.childCount; }