Merge pull request #9897 from ckeditor/ck/8485

Fix (engine): Fixed downcast conversion of collapsed markers at the conversion range boundary. Closes #8485.

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

@@ -251,6 +251,7 @@ export default class DataController {
 		// For document fragment, simply take the markers assigned to this document fragment.
 		// For model root, all markers in that root will be taken.
 		// For model element, we need to check which markers are intersecting with this element and relatively modify the markers' ranges.
+		// Collapsed markers at element boundary, although considered as not intersecting with the element, will also be returned.
 		const markers = modelElementOrFragment.is( 'documentFragment' ) ?
 			Array.from( modelElementOrFragment.markers ) :
 			_getMarkersRelativeToElement( modelElementOrFragment );
@@ -528,8 +529,9 @@ mix( DataController, ObservableMixin );
 
 // Helper function for downcast conversion.
 //
-// Takes a document element (element that is added to a model document) and checks which markers are inside it
-// and which markers are containing it. If the marker is intersecting with element, the intersection is returned.
+// Takes a document element (element that is added to a model document) and checks which markers are inside it. If the marker is collapsed
+// at element boundary, it is considered as contained inside the element and marker range is returned. Otherwise, if the marker is
+// intersecting with the element, the intersection is returned.
 function _getMarkersRelativeToElement( element ) {
 	const result = [];
 	const doc = element.root.document;
@@ -541,10 +543,19 @@ function _getMarkersRelativeToElement( element ) {
 	const elementRange = ModelRange._createIn( element );
 
 	for ( const marker of doc.model.markers ) {
-		const intersection = elementRange.getIntersection( marker.getRange() );
+		const markerRange = marker.getRange();
 
-		if ( intersection ) {
-			result.push( [ marker.name, intersection ] );
+		const isMarkerCollapsed = markerRange.isCollapsed;
+		const isMarkerAtElementBoundary = markerRange.start.isEqual( elementRange.start ) || markerRange.end.isEqual( elementRange.end );
+
+		if ( isMarkerCollapsed && isMarkerAtElementBoundary ) {
+			result.push( [ marker.name, markerRange ] );
+		} else {
+			const updatedMarkerRange = elementRange.getIntersection( markerRange );
+
+			if ( updatedMarkerRange ) {
+				result.push( [ marker.name, updatedMarkerRange ] );
+			}
 		}
 	}
 

packages/ckeditor5-engine/tests/controller/datacontroller.js

@@ -599,12 +599,13 @@ describe( 'DataController', () => {
 	describe( 'toView()', () => {
 		beforeEach( () => {
 			schema.register( 'paragraph', { inheritAllFrom: '$block' } );
-			schema.register( 'div' );
+			schema.register( 'div', { inheritAllFrom: '$block' } );
 
 			schema.extend( '$block', { allowIn: 'div' } );
-			schema.extend( 'div', { allowIn: '$root' } );
+			schema.extend( 'div', { allowIn: 'div' } );
 
 			downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
+			downcastHelpers.elementToElement( { model: 'div', view: 'div' } );
 		} );
 
 		it( 'should use #viewDocument as a parent for returned document fragments', () => {
@@ -691,6 +692,97 @@ describe( 'DataController', () => {
 			);
 		} );
 
+		// See https://github.com/ckeditor/ckeditor5/issues/8485.
+		it( 'should convert collapsed markers at element boundary', () => {
+			const modelElement = parseModel( '<div><paragraph>foo</paragraph></div>', schema );
+			const modelRoot = model.document.getRoot();
+
+			downcastHelpers.markerToData( { model: 'marker:a' } );
+			downcastHelpers.markerToData( { model: 'marker:b' } );
+
+			const modelParagraph = modelElement.getChild( 0 );
+
+			model.change( writer => {
+				writer.insert( modelElement, modelRoot, 0 );
+
+				const rangeAtStart = writer.createRange( writer.createPositionFromPath( modelParagraph, [ 0 ] ) );
+				const rangeAtEnd = writer.createRange( writer.createPositionFromPath( modelParagraph, [ 3 ] ) );
+
+				writer.addMarker( 'marker:a', { range: rangeAtStart, usingOperation: true } );
+				writer.addMarker( 'marker:b', { range: rangeAtEnd, usingOperation: true } );
+			} );
+
+			const viewElement = data.toView( modelParagraph );
+
+			expect( stringifyView( viewElement ) ).to.equal(
+				'<marker:a-start></marker:a-start><marker:a-end></marker:a-end>' +
+				'foo' +
+				'<marker:b-start></marker:b-start><marker:b-end></marker:b-end>'
+			);
+		} );
+
+		// See https://github.com/ckeditor/ckeditor5/issues/8485.
+		it( 'should convert collapsed markers at element boundary in a deeply nested element', () => {
+			const modelElement = parseModel( '<div><div><div><div><paragraph>foo</paragraph></div></div></div></div>', schema );
+			const modelRoot = model.document.getRoot();
+
+			downcastHelpers.markerToData( { model: 'marker:a' } );
+			downcastHelpers.markerToData( { model: 'marker:b' } );
+
+			const modelParagraph = modelElement.getChild( 0 ).getChild( 0 ).getChild( 0 ).getChild( 0 );
+
+			model.change( writer => {
+				writer.insert( modelElement, modelRoot, 0 );
+
+				const rangeAtStart = writer.createRange( writer.createPositionFromPath( modelParagraph, [ 0 ] ) );
+				const rangeAtEnd = writer.createRange( writer.createPositionFromPath( modelParagraph, [ 3 ] ) );
+
+				writer.addMarker( 'marker:a', { range: rangeAtStart, usingOperation: true } );
+				writer.addMarker( 'marker:b', { range: rangeAtEnd, usingOperation: true } );
+			} );
+
+			const viewElement = data.toView( modelElement );
+
+			expect( stringifyView( viewElement ) ).to.equal(
+				'<div><div><div><p>' +
+				'<marker:a-start></marker:a-start><marker:a-end></marker:a-end>' +
+				'foo' +
+				'<marker:b-start></marker:b-start><marker:b-end></marker:b-end>' +
+				'</p></div></div></div>'
+			);
+		} );
+
+		// See https://github.com/ckeditor/ckeditor5/issues/8485.
+		it( 'should skip collapsed markers at other element\'s boundaries', () => {
+			const modelElement = parseModel( '<div><paragraph>foo</paragraph><paragraph>bar</paragraph></div>', schema );
+			const modelRoot = model.document.getRoot();
+
+			downcastHelpers.markerToData( { model: 'marker:a' } );
+			downcastHelpers.markerToData( { model: 'marker:b' } );
+
+			const modelP1 = modelElement.getChild( 0 );
+			const modelP2 = modelElement.getChild( 1 );
+
+			model.change( writer => {
+				writer.insert( modelElement, modelRoot, 0 );
+
+				const rangeA = writer.createRange( writer.createPositionFromPath( modelP1, [ 0 ] ) );
+				const rangeB = writer.createRange( writer.createPositionFromPath( modelP2, [ 0 ] ) );
+
+				writer.addMarker( 'marker:a', { range: rangeA, usingOperation: true } );
+				writer.addMarker( 'marker:b', { range: rangeB, usingOperation: true } );
+			} );
+
+			const viewElementP1 = data.toView( modelP1 );
+			const viewElementP2 = data.toView( modelP2 );
+
+			// The `marker:b` should not be present as it belongs to other element.
+			expect( stringifyView( viewElementP1 ) ).to.equal( '<marker:a-start></marker:a-start><marker:a-end></marker:a-end>foo' );
+
+			// The `marker:a` should not be present as it belongs to other element.
+			expect( stringifyView( viewElementP2 ) ).to.equal( '<marker:b-start></marker:b-start><marker:b-end></marker:b-end>bar' );
+		} );
+
 		it( 'should keep view-model mapping', () => {
 			const modelDocumentFragment = parseModel( '<paragraph>foo</paragraph><paragraph>bar</paragraph>', schema );
 			const viewDocumentFragment = data.toView( modelDocumentFragment );