Fix inconsistent marker boundary order

.changelog/20260323163157_ck_19975_marker_boundary_order.md

@@ -0,0 +1,11 @@
+---
+type: Fix
+scope:
+  - ckeditor5-engine
+closes:
+  - 19975
+---
+
+Fixed the editing downcast order of adjacent marker UI boundaries so marker ends and starts are rendered consistently with the model and data output.
+
+The editing pipeline now uses stable marker ordering and preserves the expected boundary order when adjacent markers are added together or when the second adjacent marker is added later.

packages/ckeditor5-engine/src/controller/datacontroller.ts

@@ -18,6 +18,7 @@ import { Mapper } from '../conversion/mapper.js';
 
 import { DowncastDispatcher, type DowncastInsertEvent } from '../conversion/downcastdispatcher.js';
 import { insertAttributesAndChildren, insertText } from '../conversion/downcasthelpers.js';
+import { compareMarkersForDowncast } from '../conversion/comparemarkers.js';
 
 import {
 	UpcastDispatcher,
@@ -681,33 +682,7 @@ function _getMarkersRelativeToElement( element: ModelElement ): Map<string, Mode
 	// reverse DOM order, and intersecting ranges are in something approximating
 	// reverse DOM order (since reverse DOM order doesn't have a precise meaning
 	// when working with intersecting ranges).
-	result.sort( ( [ n1, r1 ], [ n2, r2 ] ) => {
-		if ( r1.end.compareWith( r2.start ) !== 'after' ) {
-			// m1.end <= m2.start -- m1 is entirely <= m2
-			return 1;
-		} else if ( r1.start.compareWith( r2.end ) !== 'before' ) {
-			// m1.start >= m2.end -- m1 is entirely >= m2
-			return -1;
-		} else {
-			// they overlap, so use their start positions as the primary sort key and
-			// end positions as the secondary sort key
-			switch ( r1.start.compareWith( r2.start ) ) {
-				case 'before':
-					return 1;
-				case 'after':
-					return -1;
-				default:
-					switch ( r1.end.compareWith( r2.end ) ) {
-						case 'before':
-							return 1;
-						case 'after':
-							return -1;
-						default:
-							return n2.localeCompare( n1 );
-					}
-			}
-		}
-	} );
+	result.sort( compareMarkersForDowncast );
 
 	return new Map( result );
 }

packages/ckeditor5-engine/src/conversion/comparemarkers.ts

@@ -0,0 +1,50 @@
+/**
+ * @license Copyright (c) 2003-2026, CKSource Holding sp. z o.o. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options
+ */
+
+// REVIEW: Let's call this file `comparemarkers.ts` (adjust module name and imports)
+/**
+ * @module engine/conversion/comparemarkers
+ */
+
+import type { ModelRange } from '../model/range.js';
+
+/**
+ * Sorts markers in a stable fashion so their addition order does not affect downcast output.
+ *
+ * Markers are ordered in reverse DOM order for non-intersecting ranges. For intersecting ranges,
+ * the start position is the primary sort key and the end position is the secondary sort key.
+ *
+ * @internal
+ */
+export function compareMarkersForDowncast(
+	[ name1, range1 ]: readonly [ string, ModelRange ],
+	[ name2, range2 ]: readonly [ string, ModelRange ]
+): number {
+	if ( range1.end.compareWith( range2.start ) !== 'after' ) {
+		// m1.end <= m2.start -- m1 is entirely <= m2.
+		return 1;
+	} else if ( range1.start.compareWith( range2.end ) !== 'before' ) {
+		// m1.start >= m2.end -- m1 is entirely >= m2.
+		return -1;
+	} else {
+		// They overlap, so use their start positions as the primary sort key and
+		// end positions as the secondary sort key.
+		switch ( range1.start.compareWith( range2.start ) ) {
+			case 'before':
+				return 1;
+			case 'after':
+				return -1;
+			default:
+				switch ( range1.end.compareWith( range2.end ) ) {
+					case 'before':
+						return 1;
+					case 'after':
+						return -1;
+					default:
+						return name2.localeCompare( name1 );
+				}
+		}
+	}
+}

packages/ckeditor5-engine/src/conversion/downcastdispatcher.ts

@@ -8,6 +8,7 @@
  */
 
 import { ModelConsumable } from './modelconsumable.js';
+import { compareMarkersForDowncast } from './comparemarkers.js';
 import { ModelRange } from '../model/range.js';
 
 import { EmitterMixin } from '@ckeditor/ckeditor5-utils';
@@ -201,7 +202,10 @@ export class DowncastDispatcher extends /* #__PURE__ */ EmitterMixin() {
 		}
 
 		// After the view is updated, convert markers which have changed.
-		for ( const change of differ.getMarkersToAdd() ) {
+		for ( const change of [ ...differ.getMarkersToAdd() ].sort( ( a, b ) => compareMarkersForDowncast(
+			[ a.name, a.range ],
+			[ b.name, b.range ]
+		) ) ) {
 			this._convertMarkerAdd( change.name, change.range, conversionApi );
 		}
 
@@ -230,7 +234,7 @@ export class DowncastDispatcher extends /* #__PURE__ */ EmitterMixin() {
 
 		this._convertInsert( range, conversionApi );
 
-		for ( const [ name, range ] of markers ) {
+		for ( const [ name, range ] of [ ...markers ].sort( compareMarkersForDowncast ) ) {
 			this._convertMarkerAdd( name, range, conversionApi );
 		}
 

packages/ckeditor5-engine/src/conversion/downcasthelpers.ts

@@ -1295,6 +1295,12 @@ export function insertStructure( elementCreator: DowncastStructureCreatorFunctio
  * @returns Insert element event converter.
  */
 export function insertUIElement( elementCreator: DowncastMarkerElementCreatorFunction ) {
+	// Marker UI boundaries use an internal custom property so the editing downcast can
+	// detect already-rendered closing boundaries at a shared model position. This is
+	// needed when a second adjacent marker is added later and its opening boundary must
+	// be inserted after the first marker's closing boundary.
+	const markerBoundaryType = Symbol( 'markerBoundaryType' );
+
 	return (
 		evt: EventInfo,
 		data: {
@@ -1335,16 +1341,39 @@ export function insertUIElement( elementCreator: DowncastMarkerElementCreatorFun
 		const mapper = conversionApi.mapper;
 		const viewWriter = conversionApi.writer;
 
-		// Add "opening" element.
-		viewWriter.insert( mapper.toViewPosition( markerRange.start ), viewStartElement );
-		conversionApi.mapper.bindElementToMarker( viewStartElement, data.markerName );
+		viewWriter.setCustomProperty( markerBoundaryType, 'opening', viewStartElement );
+		viewWriter.setCustomProperty( markerBoundaryType, 'closing', viewEndElement );
 
 		// Add "closing" element only if range is not collapsed.
 		if ( !markerRange.isCollapsed ) {
 			viewWriter.insert( mapper.toViewPosition( markerRange.end ), viewEndElement );
 			conversionApi.mapper.bindElementToMarker( viewEndElement, data.markerName );
 		}
 
+		// Add "opening" element after the closing one to keep the proper boundary order
+		// when adjacent markers share a position in the view.
+		let viewStartPosition = mapper.toViewPosition( markerRange.start );
+
+		while ( viewStartPosition.parent.is( '$text' ) && viewStartPosition.isAtEnd ) {
+			viewStartPosition = viewWriter.createPositionAfter( viewStartPosition.parent );
+		}
+
+		// If there are already closing marker boundaries at the same mapped position, place the
+		// opening boundary after them so closing boundaries remain first.
+		while ( true ) {
+			const nodeAfter = viewStartPosition.nodeAfter;
+
+			if ( !nodeAfter || !nodeAfter.is( 'uiElement' ) || nodeAfter.getCustomProperty( markerBoundaryType ) !== 'closing' ) {
+				break;
+			}
+
+			const nextViewPosition = viewWriter.createPositionAfter( nodeAfter );
+			viewStartPosition = nextViewPosition;
+		}
+
+		viewWriter.insert( viewStartPosition, viewStartElement );
+		conversionApi.mapper.bindElementToMarker( viewStartElement, data.markerName );
+
 		evt.stop();
 	};
 }

packages/ckeditor5-engine/tests/conversion/downcasthelpers.js

@@ -4162,6 +4162,65 @@ describe( 'DowncastHelpers', () => {
 				expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p></div>' );
 			} );
 
+			it( 'should keep adjacent marker boundaries in model order when markers are added together', () => {
+				downcastHelpers.markerToElement( {
+					model: 'marker',
+					view: ( data, { writer } ) => {
+						return writer.createUIElement( 'span', {
+							class: `${ data.markerName }-${ data.isOpening ? 'start' : 'end' }`
+						} );
+					}
+				} );
+
+				model.change( writer => {
+					writer.addMarker( 'marker:2', {
+						range: writer.createRange( writer.createPositionAt( modelElement, 3 ), writer.createPositionAt( modelElement, 4 ) ),
+						usingOperation: false
+					} );
+					writer.addMarker( 'marker:1', {
+						range: writer.createRange( writer.createPositionAt( modelElement, 2 ), writer.createPositionAt( modelElement, 3 ) ),
+						usingOperation: false
+					} );
+				} );
+
+				expect( viewToString( viewRoot ) ).to.equal(
+					'<div><p>fo' +
+					'<span class="marker:1-start"></span>o<span class="marker:1-end"></span><span class="marker:2-start"></span>b' +
+					'<span class="marker:2-end"></span>ar</p></div>'
+				);
+			} );
+
+			it( 'should keep adjacent marker boundaries in model order when the second marker is added later', () => {
+				downcastHelpers.markerToElement( {
+					model: 'marker',
+					view: ( data, { writer } ) => {
+						return writer.createUIElement( 'span', {
+							class: `${ data.markerName }-${ data.isOpening ? 'start' : 'end' }`
+						} );
+					}
+				} );
+
+				model.change( writer => {
+					writer.addMarker( 'marker:1', {
+						range: writer.createRange( writer.createPositionAt( modelElement, 2 ), writer.createPositionAt( modelElement, 3 ) ),
+						usingOperation: false
+					} );
+				} );
+
+				model.change( writer => {
+					writer.addMarker( 'marker:2', {
+						range: writer.createRange( writer.createPositionAt( modelElement, 3 ), writer.createPositionAt( modelElement, 4 ) ),
+						usingOperation: false
+					} );
+				} );
+
+				expect( viewToString( viewRoot ) ).to.equal(
+					'<div><p>fo' +
+					'<span class="marker:1-start"></span>o<span class="marker:1-end"></span><span class="marker:2-start"></span>b' +
+					'<span class="marker:2-end"></span>ar</p></div>'
+				);
+			} );
+
 			it( 'should not convert if consumable was consumed', () => {
 				sinon.spy( controller.downcastDispatcher, 'fire' );