Fixed selection attributes handling after softBreak.

.changelog/20260306121909_ck_19853_softBreakAndSelection.md

@@ -0,0 +1,11 @@
+---
+type: Fix
+scope:
+  - ckeditor5-engine
+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.

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

@@ -23,6 +23,7 @@ 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 +1158,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 +1198,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, 'left' );
 			}
 
 			// 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, 'right' );
 			}
 
 			// 6. If not found, selection should retrieve attributes from parent.
@@ -1247,37 +1238,80 @@ class LiveSelection extends ModelSelection {
  * 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`.
  */
-function getTextAttributes( node: ModelItem | null, schema: ModelSchema ): Iterable<[string, unknown]> | null {
-	if ( !node ) {
+function getTextAttributes(
+	startNode: ModelItem | null,
+	schema: ModelSchema,
+	scan: 'single' | 'left' | 'right' = 'single'
+): Iterable<[string, unknown]> | null {
+	if ( !startNode ) {
 		return null;
 	}
 
-	if ( node instanceof ModelTextProxy || node instanceof ModelText ) {
-		return node.getAttributes();
+	// This case can happen only for non-collapsed selection.
+	// It is already handled in `_getSurroundingAttributes` as `if ( value.type == 'text' )`).
+	// However this condition was also in `getTextAttributes` before. It's kept as a paranoid check.
+	/* istanbul ignore if -- @preserve */
+	if ( startNode instanceof ModelTextProxy ) {
+		return startNode.getAttributes();
 	}
 
-	if ( !schema.isInline( node ) ) {
-		return null;
-	}
+	for ( let node: ModelNode | null = startNode; node; node = getNextNode( node ) ) {
+		const attrs = getTextAttributesFromSingleNode( node );
 
-	// Stop on inline elements (such as `<softBreak>`) that are not objects (such as `<imageInline>` or `<mathml>`).
-	if ( !schema.isObject( node ) ) {
-		return [];
+		if ( attrs ) {
+			return attrs;
+		}
 	}
 
-	const attributes: Array<[string, unknown]> = [];
+	return null;
 
-	// 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 ] );
+	function getNextNode( node: ModelNode ) {
+		switch ( scan ) {
+			case 'single': return null;
+			case 'left': return node.previousSibling;
+			case 'right': return node.nextSibling;
 		}
 	}
 
-	return attributes;
+	function getTextAttributesFromSingleNode( node: ModelNode ) {
+		if ( node instanceof ModelText ) {
+			return node.getAttributes();
+		}
+
+		if ( node.is( 'element', 'softBreak' ) ) {
+			const attributes: Array<[ string, unknown ]> = [];
+
+			for ( const [ key, value ] of node.getAttributes() ) {
+				if ( schema.checkAttribute( '$text', key ) ) {
+					attributes.push( [ key, value ] );
+				}
+			}
+
+			return attributes;
+		}
+
+		if ( !schema.isInline( node ) ) {
+			return null;
+		}
+
+		if ( !schema.isObject( node ) ) {
+			return [];
+		}
+
+		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 ) &&
+				schema.getAttributeProperties( key ).copyFromObject !== false
+			) {
+				attributes.push( [ key, value ] );
+			}
+		}
+
+		return attributes;
+	}
 }
 
 /**

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

@@ -1406,23 +1406,69 @@ describe( 'DocumentSelection', () => {
 			} );
 		} );
 
-		// #7459
-		describe( 'ignores inline elements while reading surrounding attributes', () => {
+		// #7459 and #19853
+		describe( 'handles inline non-object elements while reading surrounding attributes', () => {
 			beforeEach( () => {
+				model.schema.extend( '$text', { allowAttributes: 'foo' } );
+
 				model.schema.register( 'softBreak', {
 					allowWhere: '$text',
+					allowAttributesOf: '$text',
 					isInline: true
 				} );
+
+				model.schema.register( 'inlineBoundary', {
+					allowWhere: '$text',
+					isInline: true
+				} );
+			} );
+
+			it( 'should not inherit attributes from a node before a softBreak when copyOnEnter is disabled', () => {
+				model.schema.setAttributeProperties( 'foo', { copyOnEnter: false } );
+				_setModelData( model, '<p><$text foo="true">Foo Bar.</$text><softBreak></softBreak>[]</p>' );
+
+				expect( selection.hasAttribute( 'foo' ) ).to.equal( false );
+			} );
+
+			it( 'should inherit attributes from a softBreak carrying copied attributes', () => {
+				model.schema.setAttributeProperties( 'bold', { copyOnEnter: true } );
+				_setModelData( model, '<p><$text bold="true">Foo Bar.</$text><softBreak bold="true"></softBreak>[]</p>' );
+
+				expect( selection.hasAttribute( 'bold' ) ).to.equal( true );
+			} );
+
+			it( 'should not inherit attributes from a node after a softBreak when copyOnEnter is disabled (override gravity)', () => {
+				model.schema.setAttributeProperties( 'foo', { copyOnEnter: false } );
+				_setModelData( model, '<p>[]<softBreak></softBreak><$text foo="true">Foo Bar.</$text></p>' );
+
+				const overrideGravityUid = selection._overrideGravity();
+
+				expect( selection.hasAttribute( 'foo' ) ).to.equal( false );
+
+				selection._restoreGravity( overrideGravityUid );
+			} );
+
+			it( 'should inherit attributes from a softBreak carrying copied attributes with override gravity', () => {
+				model.schema.setAttributeProperties( 'bold', { copyOnEnter: true } );
+				_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 );
 			} );
 
-			it( 'should not inherit attributes from a node before an inline element', () => {
-				_setModelData( model, '<p><$text bold="true">Foo Bar.</$text><softBreak></softBreak>[]</p>' );
+			it( 'should not inherit attributes from a node before a generic inline element even when copyOnEnter is enabled', () => {
+				model.schema.setAttributeProperties( 'bold', { copyOnEnter: true } );
+				_setModelData( model, '<p><$text bold="true">Foo Bar.</$text><inlineBoundary></inlineBoundary>[]</p>' );
 
 				expect( selection.hasAttribute( 'bold' ) ).to.equal( false );
 			} );
 
-			it( 'should not inherit attributes from a node after an inline element (override gravity)', () => {
-				_setModelData( model, '<p>[]<softBreak></softBreak><$text bold="true">Foo Bar.</$text></p>' );
+			it( 'should not inherit attributes after generic inline element with override gravity even when copyOnEnter is enabled', () => {
+				model.schema.setAttributeProperties( 'bold', { copyOnEnter: true } );
+				_setModelData( model, '<p>[]<inlineBoundary></inlineBoundary><$text bold="true">Foo Bar.</$text></p>' );
 
 				const overrideGravityUid = selection._overrideGravity();
 

packages/ckeditor5-enter/src/shiftenter.ts

@@ -44,6 +44,7 @@ export class ShiftEnter extends Plugin {
 		// Configure the schema.
 		schema.register( 'softBreak', {
 			allowWhere: '$text',
+			allowAttributesOf: '$text',
 			isInline: true
 		} );
 

packages/ckeditor5-enter/src/shiftentercommand.ts

@@ -98,12 +98,7 @@ function softBreakAction( model: Model, writer: ModelWriter, selection: ModelDoc
 	const isContainedWithinOneElement = ( startElement == endElement );
 
 	if ( isSelectionEmpty ) {
-		const attributesToCopy = getCopyOnEnterAttributes( model.schema, selection.getAttributes() );
-
-		insertBreak( model, writer, range.end );
-
-		writer.removeSelectionAttribute( selection.getAttributeKeys() );
-		writer.setSelectionAttribute( attributesToCopy );
+		insertBreak( model, writer, range.end, selection );
 	} else {
 		const leaveUnmerged = !( range.start.isAtStart && range.end.isAtEnd );
 
@@ -113,7 +108,7 @@ function softBreakAction( model: Model, writer: ModelWriter, selection: ModelDoc
 		//
 		// <h>x[xx]x</h>		-> <h>x^x</h>			-> <h>x<br>^x</h>
 		if ( isContainedWithinOneElement ) {
-			insertBreak( model, writer, selection.focus! );
+			insertBreak( model, writer, selection.focus!, selection );
 		}
 		// Selection over multiple elements.
 		//
@@ -134,8 +129,18 @@ function softBreakAction( model: Model, writer: ModelWriter, selection: ModelDoc
 	}
 }
 
-function insertBreak( model: Model, writer: ModelWriter, position: ModelPosition ): void {
+function insertBreak(
+	model: Model,
+	writer: ModelWriter,
+	position: ModelPosition,
+	selection: ModelDocumentSelection
+): void {
 	const breakLineElement = writer.createElement( 'softBreak' );
+	const attributesToCopy = Array.from( getCopyOnEnterAttributes( model.schema, selection.getAttributes() ) );
+
+	// Store copy-on-enter attributes on the soft break itself so the selection placed next to it
+	// can inherit them through the standard left/right gravity lookup.
+	writer.setAttributes( Object.fromEntries( attributesToCopy ), breakLineElement );
 
 	model.insertContent( breakLineElement, position );
 	writer.setSelection( breakLineElement, 'after' );

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

@@ -59,11 +59,11 @@ describe( 'ShiftEnter integration', () => {
 		);
 	} );
 
-	it( 'should not inherit text attributes before the "softBreak" element', () => {
+	it( 'should inherit only copyOnEnter text attributes before the "softBreak" element', () => {
 		_setModelData( model,
 			'<paragraph>' +
 				'<$text linkHref="foo" bold="true">Bolded link</$text>' +
-				'<softBreak></softBreak>' +
+				'<softBreak bold="true"></softBreak>' +
 				'F[]' +
 			'</paragraph>'
 		);
@@ -73,6 +73,6 @@ describe( 'ShiftEnter integration', () => {
 		const selection = model.document.selection;
 
 		expect( selection.hasAttribute( 'linkHref' ) ).to.equal( false );
-		expect( selection.hasAttribute( 'bold' ) ).to.equal( false );
+		expect( selection.hasAttribute( 'bold' ) ).to.equal( true );
 	} );
 } );

packages/ckeditor5-enter/tests/shiftentercommand.js

@@ -110,7 +110,7 @@ describe( 'ShiftEnterCommand', () => {
 				test(
 					'allowed attributes are copied',
 					'<p><$text foo="true">test[]</$text></p>',
-					'<p><$text foo="true">test</$text><softBreak></softBreak><$text foo="true">[]</$text></p>'
+					'<p><$text foo="true">test</$text><softBreak foo="true"></softBreak><$text foo="true">[]</$text></p>'
 				);
 
 				test(
@@ -122,7 +122,7 @@ describe( 'ShiftEnterCommand', () => {
 				test(
 					'only allowed attributes are copied from mix set',
 					'<p><$text bar="true" foo="true">test[]</$text></p>',
-					'<p><$text bar="true" foo="true">test</$text><softBreak></softBreak><$text foo="true">[]</$text></p>'
+					'<p><$text bar="true" foo="true">test</$text><softBreak foo="true"></softBreak><$text foo="true">[]</$text></p>'
 				);
 			} );
 		} );