Fixed selection attributes handling after softBreak.

.changelog/20260306121909_ck_19853_softBreakAndSelection.md

@@ -0,0 +1,12 @@
+---
+type: Fix
+scope:
+  - ckeditor5-engine
+  - ckeditor5-enter
+closes:
+  - 19853
+---
+
+Improved document selection attribute inheritance around `<softBreak>` so returning the caret after a soft break restores expected formatting.
+
+When selection attributes are recalculated across `<softBreak>`, only attributes marked with `copyOnEnter` are inherited. Other inline non-object elements still act as hard boundaries.

.changelog/20260407170702_ck_19853_softBreakAndSelection.md

@@ -0,0 +1,10 @@
+---
+type: Fix
+scope:
+  - ckeditor5-engine
+  - ckeditor5-enter
+closes:
+  - https://github.com/ckeditor/ckeditor5/issues/1068
+---
+
+Inline formatting attributes (such as bold or link) are no longer split by a soft break (`<br>`). The `<br>` element now inherits applicable text attributes so that attribute elements in the view can wrap around it without being broken into separate segments.

packages/ckeditor5-engine/src/model/documentselection.ts

@@ -15,14 +15,14 @@ import {
 	type ModelSelectionChangeRangeEvent
 } from './selection.js';
 import { ModelText } from './text.js';
-import { ModelTextProxy } from './textproxy.js';
 
 import type { ModelDocumentChangeEvent, ModelDocument } from './document.js';
 import type { Model, ModelApplyOperationEvent } from './model.js';
 import type { Marker, MarkerCollectionUpdateEvent } from './markercollection.js';
 import { type Batch } from './batch.js';
 import { type ModelElement } from './element.js';
 import { type ModelItem } from './item.js';
+import { type ModelNode } from './node.js';
 import type { ModelPosition, ModelPositionOffset } from './position.js';
 import { type ModelRange } from './range.js';
 import { type ModelSchema } from './schema.js';
@@ -1157,7 +1157,7 @@ class LiveSelection extends ModelSelection {
 			return null;
 		}
 
-		let attrs = null;
+		let attrs: Iterable<[string, unknown]> | null = null;
 
 		if ( !this.isCollapsed ) {
 			// 1. If selection is a range...
@@ -1197,22 +1197,12 @@ class LiveSelection extends ModelSelection {
 			// 4. If not, try to find the first character on the left, that is in the same node.
 			// When gravity is overridden then don't take node before into consideration.
 			if ( !this.isGravityOverridden && !attrs ) {
-				let node = nodeBefore;
-
-				while ( node && !attrs ) {
-					node = node.previousSibling;
-					attrs = getTextAttributes( node, schema );
-				}
+				attrs = getTextAttributes( nodeBefore, schema, 'backward' );
 			}
 
 			// 5. If not found, try to find the first character on the right, that is in the same node.
 			if ( !attrs ) {
-				let node = nodeAfter;
-
-				while ( node && !attrs ) {
-					node = node.nextSibling;
-					attrs = getTextAttributes( node, schema );
-				}
+				attrs = getTextAttributes( nodeAfter, schema, 'forward' );
 			}
 
 			// 6. If not found, selection should retrieve attributes from parent.
@@ -1244,40 +1234,70 @@ class LiveSelection extends ModelSelection {
 /**
  * Helper function for {@link module:engine/model/liveselection~LiveSelection#_updateAttributes}.
  *
- * It checks if the passed model item is a text node (or text proxy) and, if so, returns it's attributes.
- * If not, it checks if item is an inline object and does the same. Otherwise it returns `null`.
+ * It checks if the passed model node is a text node and, if so, returns its attributes.
+ * If not, it checks if item is an inline element and does the same. Otherwise, it returns `null`.
  */
-function getTextAttributes( node: ModelItem | null, schema: ModelSchema ): Iterable<[string, unknown]> | null {
-	if ( !node ) {
+function getTextAttributes(
+	startNode: ModelNode | null,
+	schema: ModelSchema,
+	scan: 'self' | 'backward' | 'forward' = 'self'
+): Iterable<[string, unknown]> | null {
+	if ( !startNode ) {
 		return null;
 	}
 
-	if ( node instanceof ModelTextProxy || node instanceof ModelText ) {
-		return node.getAttributes();
-	}
+	for ( const node of siblingNodes( startNode, scan ) ) {
+		if ( !node ) {
+			return null;
+		}
 
-	if ( !schema.isInline( node ) ) {
-		return null;
+		if ( node instanceof ModelText ) {
+			return node.getAttributes();
+		}
+
+		if ( !schema.isInline( node ) ) {
+			continue;
+		}
+
+		const isObject = schema.isObject( node );
+		const attributes: Array<[string, unknown]> = [];
+
+		// Collect all attributes that can be applied to the text node.
+		for ( const [ key, value ] of node.getAttributes() ) {
+			if (
+				schema.checkAttribute( '$text', key ) &&
+				( !isObject || schema.getAttributeProperties( key ).copyFromObject !== false )
+			) {
+				attributes.push( [ key, value ] );
+			}
+		}
+
+		return attributes;
 	}
 
-	// Stop on inline elements (such as `<softBreak>`) that are not objects (such as `<imageInline>` or `<mathml>`).
-	if ( !schema.isObject( node ) ) {
-		return [];
+	return null;
+}
+
+/**
+ * Returns sibling nodes from the given start node.
+ */
+function* siblingNodes( startNode: ModelNode, scan: 'self' | 'backward' | 'forward' ) {
+	if ( scan == 'self' ) {
+		yield startNode;
 	}
+	else {
+		let node: ModelNode | null = startNode;
 
-	const attributes: Array<[string, unknown]> = [];
+		while ( node ) {
+			node = scan == 'backward' ?
+				node.previousSibling :
+				node.nextSibling;
 
-	// Collect all attributes that can be applied to the text node.
-	for ( const [ key, value ] of node.getAttributes() ) {
-		if (
-			schema.checkAttribute( '$text', key ) &&
-			schema.getAttributeProperties( key ).copyFromObject !== false
-		) {
-			attributes.push( [ key, value ] );
+			yield node;
 		}
 	}
 
-	return attributes;
+	return null;
 }
 
 /**

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

@@ -92,13 +92,22 @@ ViewContainerElement.prototype.is = function( this: ViewContainerElement, type:
  */
 export function getViewFillerOffset( this: ViewContainerElement ): number | null {
 	const children = [ ...this.getChildren() ];
-	const lastChild = children[ this.childCount - 1 ];
+	let lastChild: ViewNode | undefined = 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;
 	}
 
+	// Check if there is a `<br>` inside an attribute element at the end of container.
+	while ( lastChild && lastChild.is( 'attributeElement' ) ) {
+		lastChild = lastChild.getChild( lastChild.childCount - 1 );
+
+		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' ) ) {

packages/ckeditor5-engine/tests/model/documentselection.js

@@ -1122,6 +1122,30 @@ describe( 'DocumentSelection', () => {
 					new ModelElement( 'p', { p: true } ),
 					new ModelText( 'e', { e: true } )
 				] );
+
+				// Just to make sure how model looks.
+				expect( _getModelData( model, { withoutSelection: true } ) ).to.equal(
+					'<p p="true"></p>' +
+
+					'<$text a="true">a</$text>' +
+
+					'<p p="true"></p>' +
+
+					'<$text b="true">b</$text>' +
+					'<$text c="true">c</$text>' +
+
+					'<p>' +
+						'<$text d="true">d</$text>' +
+					'</p>' +
+
+					'<p p="true"></p>' +
+
+					'<$text e="true">e</$text>' +
+
+					// Left-overs from parent describe.
+					'<p>foobar</p>' +
+					'<p></p>'
+				);
 			} );
 
 			it( 'if selection is a range, should find first character in it and copy it\'s attributes', () => {
@@ -1407,10 +1431,11 @@ describe( 'DocumentSelection', () => {
 		} );
 
 		// #7459
-		describe( 'ignores inline elements while reading surrounding attributes', () => {
+		describe( 'softBreak integration', () => {
 			beforeEach( () => {
 				model.schema.register( 'softBreak', {
 					allowWhere: '$text',
+					allowAttributesOf: '$text',
 					isInline: true
 				} );
 			} );
@@ -1430,6 +1455,22 @@ describe( 'DocumentSelection', () => {
 
 				selection._restoreGravity( overrideGravityUid );
 			} );
+
+			it( 'should inherit attributes from an inline element', () => {
+				_setModelData( model, '<p><$text bold="true">Foo Bar.</$text><softBreak bold="true"></softBreak>[]</p>' );
+
+				expect( selection.hasAttribute( 'bold' ) ).to.equal( true );
+			} );
+
+			it( 'should inherit attributes from an inline element (override gravity)', () => {
+				_setModelData( model, '<p>[]<softBreak bold="true"></softBreak><$text bold="true">Foo Bar.</$text></p>' );
+
+				const overrideGravityUid = selection._overrideGravity();
+
+				expect( selection.hasAttribute( 'bold' ) ).to.equal( true );
+
+				selection._restoreGravity( overrideGravityUid );
+			} );
 		} );
 
 		// #14106

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

@@ -79,36 +79,87 @@ describe( 'ContainerElement', () => {
 
 		// 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', () => {
+			it( 'should return null for a container with text content only', () => {
 				expect( _parseView( '<container:p>Foo.</container:p>' ).getFillerOffset() ).to.equals( null );
 			} );
 
-			it( 'returns offset of the last child which is the <br> element (1)', () => {
-				expect( _parseView( '<container:p><empty:br></empty:br></container:p>' ).getFillerOffset() ).to.equals( 1 );
+			it( 'should return offset after a <br> element that is the only child', () => {
+				expect( _parseView( '<container:p><empty:br/></container:p>' ).getFillerOffset() ).to.equals( 1 );
 			} );
 
-			it( 'returns offset of the last child which is the <br> element (2)', () => {
-				expect( _parseView( '<container:p>Foo.<empty:br></empty:br></container:p>' ).getFillerOffset() ).to.equals( 2 );
+			it( 'should return offset after a <br> element that follows text', () => {
+				expect( _parseView( '<container:p>Foo.<empty:br/></container:p>' ).getFillerOffset() ).to.equals( 2 );
 			} );
 
-			it( 'always returns the last <br> element in the container', () => {
-				expect( _parseView( '<container:p>Foo.<empty:br></empty:br><empty:br></empty:br></container:p>' ).getFillerOffset() )
+			it( 'should return offset after the last <br> element when there are consecutive <br> elements', () => {
+				expect( _parseView( '<container:p>Foo.<empty:br/><empty:br/></container:p>' ).getFillerOffset() )
 					.to.equals( 3 );
 			} );
 
-			it( 'works fine with non-empty container with multi <br> elements', () => {
-				expect( _parseView( '<container:p>Foo.<empty:br></empty:br>Bar.<empty:br></empty:br></container:p>' ).getFillerOffset() )
-					.to.equals( 4 );
+			it( 'should return offset after the last <br> element when text nodes are between <br> elements', () => {
+				expect( _parseView( '<container:p>Foo.<empty:br/>Bar.<empty:br/></container:p>' ).getFillerOffset() ).to.equals( 4 );
 			} );
 
-			it( 'ignores the ui elements', () => {
-				expect( _parseView( '<container:p><ui:span></ui:span><empty:br></empty:br></container:p>' ).getFillerOffset() )
-					.to.equals( 2 );
+			it( 'should return offset after a <br> element ignoring preceding ui elements', () => {
+				expect( _parseView( '<container:p><ui:span></ui:span><empty:br/></container:p>' ).getFillerOffset() ).to.equals( 2 );
 			} );
 
-			it( 'empty element must be the <br> element', () => {
-				expect( _parseView( '<container:p>Foo<empty:img></empty:img></container:p>' ).getFillerOffset() )
-					.to.equals( null );
+			it( 'should return null when the last empty element is not a <br> element', () => {
+				expect( _parseView( '<container:p>Foo<empty:img/></container:p>' ).getFillerOffset() ).to.equals( null );
+			} );
+		} );
+
+		// Block filler is required after the `<br>` element if the element is the last child in the container
+		// even when nested in an attribute element.
+		describe( 'for <br> elements in container inside an attribute element', () => {
+			it( 'should return null when the attribute element contains text only', () => {
+				expect(
+					_parseView( '<container:p><attribute:b>Foo.</attribute:b></container:p>' ).getFillerOffset()
+				).to.equals( null );
+			} );
+
+			it( 'should return offset after the attribute element that contains only a <br> element', () => {
+				expect(
+					_parseView( '<container:p><attribute:b><empty:br/></attribute:b></container:p>' ).getFillerOffset()
+				).to.equals( 1 );
+			} );
+
+			it( 'should return offset after the attribute element that ends with a <br> element', () => {
+				expect(
+					_parseView( '<container:p><attribute:b>Foo.<empty:br/></attribute:b></container:p>' ).getFillerOffset()
+				).to.equals( 1 );
+			} );
+
+			it( 'should return offset after the attribute element with deeply nested <br> element', () => {
+				expect(
+					_parseView(
+						'<container:p><attribute:b><attribute:i>Foo.<empty:br/></attribute:i></attribute:b></container:p>'
+					).getFillerOffset()
+				).to.equals( 1 );
+			} );
+
+			it( 'should return offset after the attribute element that ends with consecutive <br> elements', () => {
+				expect(
+					_parseView( '<container:p><attribute:b>Foo.<empty:br/><empty:br/></attribute:b></container:p>' ).getFillerOffset()
+				).to.equals( 1 );
+			} );
+
+			it( 'should return offset after the attribute element with text nodes between <br> elements', () => {
+				expect(
+					_parseView( '<container:p><attribute:b>Foo.<empty:br/>Bar.<empty:br/></attribute:b></container:p>' ).getFillerOffset()
+				).to.equals( 1 );
+			} );
+
+			it( 'should return offset after the attribute element with a <br> element ignoring ui elements', () => {
+				expect(
+					_parseView( '<container:p><attribute:b><ui:span></ui:span><empty:br/></attribute:b></container:p>' ).getFillerOffset()
+				).to.equals( 1 );
+			} );
+
+			it( 'should return null when the last empty element inside the attribute element is not a <br> element', () => {
+				expect(
+					_parseView( '<container:p><attribute:b>Foo<empty:img/></attribute:b></container:p>' ).getFillerOffset()
+				).to.equals( null );
 			} );
 		} );
 	} );