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

@@ -11,6 +11,12 @@ import { ShiftEnterCommand } from './shiftentercommand.js';
 import { EnterObserver, type ViewDocumentEnterEvent } from './enterobserver.js';
 import { Plugin } from '@ckeditor/ckeditor5-core';
 
+import type {
+	ModelElement,
+	ModelNode,
+	ModelWriter
+} from '@ckeditor/ckeditor5-engine';
+
 /**
  * This plugin handles the <kbd>Shift</kbd>+<kbd>Enter</kbd> keystroke (soft line break) in the editor.
  *
@@ -44,6 +50,7 @@ export class ShiftEnter extends Plugin {
 		// Configure the schema.
 		schema.register( 'softBreak', {
 			allowWhere: '$text',
+			allowAttributesOf: '$text',
 			isInline: true
 		} );
 
@@ -63,6 +70,7 @@ export class ShiftEnter extends Plugin {
 		view.addObserver( EnterObserver );
 
 		editor.commands.add( 'shiftEnter', new ShiftEnterCommand( editor ) );
+		editor.model.document.registerPostFixer( writer => removeStaleSoftBreakAttributes( writer ) );
 
 		this.listenTo<ViewDocumentEnterEvent>( viewDocument, 'enter', ( evt, data ) => {
 			// When not in composition, we handle the action, so prevent the default one.
@@ -91,3 +99,47 @@ export class ShiftEnter extends Plugin {
 		} );
 	}
 }
+
+function removeStaleSoftBreakAttributes( writer: ModelWriter ): boolean {
+	const parentsToCheck = new Set<ModelElement>();
+
+	for ( const change of writer.model.document.differ.getChanges() ) {
+		if ( change.type == 'insert' || change.type == 'remove' ) {
+			if ( change.position.parent.is( 'element' ) ) {
+				parentsToCheck.add( change.position.parent );
+			}
+		} else if ( change.type == 'attribute' ) {
+			if ( change.range.start.parent.is( 'element' ) ) {
+				parentsToCheck.add( change.range.start.parent );
+			}
+		}
+	}
+
+	for ( const parent of parentsToCheck ) {
+		for ( const child of parent.getChildren() ) {
+			if ( !child.is( 'element', 'softBreak' ) ) {
+				continue;
+			}
+
+			const nextSibling = child.nextSibling;
+
+			if ( !nextSibling || nextSibling.is( 'element' ) ) {
+				continue;
+			}
+
+			for ( const [ key, value ] of child.getAttributes() ) {
+				if ( !hasSameAttribute( nextSibling, key, value ) ) {
+					writer.removeAttribute( key, child );
+
+					return true;
+				}
+			}
+		}
+	}
+
+	return false;
+}
+
+function hasSameAttribute( node: ModelNode | null, key: string, value: unknown ): boolean {
+	return node?.getAttribute( key ) === value;
+}

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 );
 	} );
 } );