Merge pull request #15691 from ckeditor/ck/14028

Fix (engine): An inline filler should be rendered after a BR just before a block filler so that scrolling to selection could properly find the client rect. Closes #14028.

packages/ckeditor5-engine/src/view/renderer.ts

@@ -22,6 +22,7 @@ import {
 	isNode,
 	isText,
 	remove,
+	indexOf,
 	type DiffResult,
 	type ObservableChangeEvent
 } from '@ckeditor/ckeditor5-utils';
@@ -524,18 +525,18 @@ export default class Renderer extends ObservableMixin() {
 			return false;
 		}
 
-		// We have block filler, we do not need inline one.
-		if ( selectionOffset === selectionParent.getFillerOffset!() ) {
-			return false;
-		}
-
 		const nodeBefore = selectionPosition.nodeBefore;
 		const nodeAfter = selectionPosition.nodeAfter;
 
 		if ( nodeBefore instanceof ViewText || nodeAfter instanceof ViewText ) {
 			return false;
 		}
 
+		// We have block filler, we do not need inline one.
+		if ( selectionOffset === selectionParent.getFillerOffset!() && ( !nodeBefore || !nodeBefore.is( 'element', 'br' ) ) ) {
+			return false;
+		}
+
 		// Do not use inline filler while typing outside inline elements on Android.
 		// The deleteContentBackward would remove part of the inline filler instead of removing last letter in a link.
 		if ( env.isAndroid && ( nodeBefore || nodeAfter ) ) {
@@ -1147,15 +1148,21 @@ function sameNodes( domConverter: DomConverter, actualDomChild: DomNode, expecte
  * caret to the new line. A quick fix is as simple as force–refreshing the selection with the same range.
  */
 function fixGeckoSelectionAfterBr( focus: ReturnType<DomConverter[ 'viewPositionToDom' ]>, domSelection: DomSelection ) {
-	const parent = focus!.parent;
+	let parent = focus!.parent;
+	let offset = focus!.offset;
+
+	if ( isText( parent ) && isInlineFiller( parent ) ) {
+		offset = indexOf( parent ) + 1;
+		parent = parent.parentNode!;
+	}
 
 	// This fix works only when the focus point is at the very end of an element.
 	// There is no point in running it in cases unrelated to the browser bug.
-	if ( parent.nodeType != Node.ELEMENT_NODE || focus!.offset != parent.childNodes.length - 1 ) {
+	if ( parent.nodeType != Node.ELEMENT_NODE || offset != parent.childNodes.length - 1 ) {
 		return;
 	}
 
-	const childAtOffset = parent.childNodes[ focus!.offset ];
+	const childAtOffset = parent.childNodes[ offset ];
 
 	// To stay on the safe side, the fix being as specific as possible, it targets only the
 	// selection which is at the very end of the element and preceded by <br />.

packages/ckeditor5-engine/tests/view/renderer.js

@@ -769,6 +769,56 @@ describe( 'Renderer', () => {
 			renderAndExpectNoChanges( renderer, domRoot );
 		} );
 
+		// See https://github.com/ckeditor/ckeditor5/issues/14028.
+		it( 'should add and remove inline filler in case <p><br>[]</p>', () => {
+			const domSelection = document.getSelection();
+
+			// Step 1: <p><br>{}</p>
+			const { view: viewP, selection: newSelection } = parse(
+				'<container:p><empty:br/>[]</container:p>' );
+
+			viewRoot._appendChild( viewP );
+			selection._setTo( newSelection );
+
+			renderer.markToSync( 'children', viewRoot );
+			renderer.render();
+
+			const domP = domRoot.childNodes[ 0 ];
+
+			expect( domP.childNodes.length ).to.equal( 3 );
+			expect( domP.childNodes[ 0 ].tagName.toLowerCase() ).to.equal( 'br' );
+			expect( domP.childNodes[ 1 ].data ).to.equal( INLINE_FILLER );
+			expect( domConverter.isBlockFiller( domP.childNodes[ 2 ] ) ).to.be.true;
+
+			expect( domSelection.rangeCount ).to.equal( 1 );
+			expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domP.childNodes[ 1 ] );
+			expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( INLINE_FILLER_LENGTH );
+			expect( domSelection.getRangeAt( 0 ).collapsed ).to.be.true;
+
+			// Step 2: No mutation on second render
+			renderer.markToSync( 'children', viewP );
+			renderAndExpectNoChanges( renderer, domRoot );
+
+			// Step 3: <p>{}<br></p>
+			selection._setTo( ViewRange._createFromParentsAndOffsets(
+				viewP.getChild( 0 ), 0, viewP.getChild( 0 ), 0 ) );
+
+			renderer.render();
+
+			expect( domP.childNodes.length ).to.equal( 2 );
+			expect( domP.childNodes[ 0 ].tagName.toLowerCase() ).to.equal( 'br' );
+			expect( domConverter.isBlockFiller( domP.childNodes[ 1 ] ) ).to.be.true;
+
+			expect( domSelection.rangeCount ).to.equal( 1 );
+			expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domP.childNodes[ 0 ].childNodes[ 0 ] );
+			expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( INLINE_FILLER_LENGTH );
+			expect( domSelection.getRangeAt( 0 ).collapsed ).to.be.true;
+
+			// Step 4: No mutation on second render
+			renderer.markToSync( 'children', viewP );
+			renderAndExpectNoChanges( renderer, domRoot );
+		} );
+
 		it( 'should not add inline filler in case <p><b>foo</b>[]</p> on Android', () => {
 			testUtils.sinon.stub( env, 'isAndroid' ).value( true );
 

packages/ckeditor5-enter/tests/shiftenter-integration.js

@@ -11,6 +11,7 @@ import LinkEditing from '@ckeditor/ckeditor5-link/src/linkediting.js';
 import Delete from '@ckeditor/ckeditor5-typing/src/delete.js';
 import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting.js';
 import ShiftEnter from '../src/shiftenter.js';
+import { INLINE_FILLER } from '@ckeditor/ckeditor5-engine/src/view/filler.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';
@@ -45,13 +46,22 @@ describe( 'ShiftEnter integration', () => {
 		expect( getViewData( editor.editing.view, options ) ).to.equal( '<p>First line.<br></br>Second line.</p>' );
 	} );
 
-	it( 'BLOCK_FILLER should be inserted after <br> in the paragraph', () => {
+	it( 'BLOCK_FILLER should be inserted after <br> in the paragraph (data pipeline)', () => {
 		setModelData( model, '<paragraph>[]</paragraph>' );
 
 		editor.execute( 'shiftEnter' );
 
 		expect( editor.getData( { trim: 'none' } ) ).to.equal( '<p><br>&nbsp;</p>' );
-		expect( editor.ui.view.editable.element.innerHTML ).to.equal( '<p><br><br data-cke-filler="true"></p>' );
+	} );
+
+	it( 'INLINE_FILLER should be inserted before last <br> (BLOCK_FILLER) in the paragraph (editing pipeline)', () => {
+		setModelData( model, '<paragraph>[]</paragraph>' );
+
+		editor.execute( 'shiftEnter' );
+
+		expect( editor.ui.view.editable.element.innerHTML ).to.equal(
+			`<p><br>${ INLINE_FILLER }<br data-cke-filler="true"></p>`
+		);
 	} );
 
 	it( 'should not inherit text attributes before the "softBreak" element', () => {