Merge pull request #9462 from ckeditor/i/9460

Feature (engine): Enable marker downcast for document fragments. Closes #9460.

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

@@ -247,14 +247,16 @@ export default class DataController {
 		// We have no view controller and rendering to DOM in DataController so view.change() block is not used here.
 		this.downcastDispatcher.convertInsert( modelRange, viewWriter );
 
-		if ( !modelElementOrFragment.is( 'documentFragment' ) ) {
-			// Then, if a document element is converted, convert markers.
-			// From all document markers, get those, which "intersect" with the converter element.
-			const markers = _getMarkersRelativeToElement( modelElementOrFragment );
-
-			for ( const [ name, range ] of markers ) {
-				this.downcastDispatcher.convertMarkerAdd( name, range, viewWriter );
-			}
+		// Convert markers.
+		// 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.
+		const markers = modelElementOrFragment.is( 'documentFragment' ) ?
+			Array.from( modelElementOrFragment.markers ) :
+			_getMarkersRelativeToElement( modelElementOrFragment );
+
+		for ( const [ name, range ] of markers ) {
+			this.downcastDispatcher.convertMarkerAdd( name, range, viewWriter );
 		}
 
 		// Clean `conversionApi`.

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

@@ -390,8 +390,8 @@ export default class DowncastDispatcher {
 	 * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify the view document.
 	 */
 	convertMarkerAdd( markerName, markerRange, writer ) {
-		// Do not convert if range is in graveyard or not in the document (e.g. in DocumentFragment).
-		if ( !markerRange.root.document || markerRange.root.rootName == '$graveyard' ) {
+		// Do not convert if range is in graveyard.
+		if ( markerRange.root.rootName == '$graveyard' ) {
 			return;
 		}
 
@@ -445,8 +445,8 @@ export default class DowncastDispatcher {
 	 * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify the view document.
 	 */
 	convertMarkerRemove( markerName, markerRange, writer ) {
-		// Do not convert if range is in graveyard or not in the document (e.g. in DocumentFragment).
-		if ( !markerRange.root.document || markerRange.root.rootName == '$graveyard' ) {
+		// Do not convert if range is in graveyard.
+		if ( markerRange.root.rootName == '$graveyard' ) {
 			return;
 		}
 

packages/ckeditor5-engine/src/model/model.js

@@ -95,21 +95,30 @@ export default class Model {
 		this.schema.register( '$root', {
 			isLimit: true
 		} );
+
 		this.schema.register( '$block', {
 			allowIn: '$root',
 			isBlock: true
 		} );
+
 		this.schema.register( '$text', {
 			allowIn: '$block',
 			isInline: true,
 			isContent: true
 		} );
+
 		this.schema.register( '$clipboardHolder', {
 			allowContentOf: '$root',
 			isLimit: true
 		} );
 		this.schema.extend( '$text', { allowIn: '$clipboardHolder' } );
 
+		this.schema.register( '$documentFragment', {
+			allowContentOf: '$root',
+			isLimit: true
+		} );
+		this.schema.extend( '$text', { allowIn: '$documentFragment' } );
+
 		// An element needed by the `upcastElementToMarker` converter.
 		// This element temporarily represents a marker boundary during the conversion process and is removed
 		// at the end of the conversion. `UpcastDispatcher` or at least `Conversion` class looks like a

packages/ckeditor5-engine/src/model/schema.js

@@ -1328,10 +1328,6 @@ export class SchemaContext {
 			context = context.getAncestors( { includeSelf: true } );
 		}
 
-		if ( context[ 0 ] && typeof context[ 0 ] != 'string' && context[ 0 ].is( 'documentFragment' ) ) {
-			context.shift();
-		}
-
 		this._items = context.map( mapContextItem );
 	}
 
@@ -1708,9 +1704,9 @@ function getValues( obj ) {
 }
 
 function mapContextItem( ctxItem ) {
-	if ( typeof ctxItem == 'string' ) {
+	if ( typeof ctxItem == 'string' || ctxItem.is( 'documentFragment' ) ) {
 		return {
-			name: ctxItem,
+			name: typeof ctxItem == 'string' ? ctxItem : '$documentFragment',
 
 			* getAttributeKeys() {},
 

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

@@ -654,18 +654,24 @@ describe( 'DataController', () => {
 			expect( stringifyView( viewDocumentFragment ) ).to.equal( 'f<span class="a">oo</span>' );
 		} );
 
-		it( 'should convert a document fragment', () => {
+		it( 'should convert a document fragment and its markers', () => {
+			downcastHelpers.markerToData( { model: 'foo' } );
 			const modelDocumentFragment = parseModel( '<paragraph>foo</paragraph><paragraph>bar</paragraph>', schema );
+
+			const range = model.createRange(
+				model.createPositionAt( modelDocumentFragment.getChild( 0 ), 1 ),
+				model.createPositionAt( modelDocumentFragment.getChild( 1 ), 2 )
+			);
+			modelDocumentFragment.markers.set( 'foo:bar', range );
+
 			const viewDocumentFragment = data.toView( modelDocumentFragment );
 
 			expect( viewDocumentFragment ).to.be.instanceOf( ViewDocumentFragment );
 			expect( viewDocumentFragment ).to.have.property( 'childCount', 2 );
 
-			const viewElement = viewDocumentFragment.getChild( 0 );
-
-			expect( viewElement.name ).to.equal( 'p' );
-			expect( viewElement.childCount ).to.equal( 1 );
-			expect( viewElement.getChild( 0 ).data ).to.equal( 'foo' );
+			expect( stringifyView( viewDocumentFragment ) ).to.equal(
+				'<p>f<foo-start name="bar"></foo-start>oo</p><p>ba<foo-end name="bar"></foo-end>r</p>'
+			);
 		} );
 
 		it( 'should keep view-model mapping', () => {

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

@@ -9,6 +9,7 @@ import Mapper from '../../src/conversion/mapper';
 import Model from '../../src/model/model';
 import ModelText from '../../src/model/text';
 import ModelElement from '../../src/model/element';
+import ModelDocumentFragment from '../../src/model/documentfragment';
 import ModelRange from '../../src/model/range';
 
 import View from '../../src/view/view';
@@ -500,21 +501,22 @@ describe( 'DowncastDispatcher', () => {
 			expect( spy.calledOnce ).to.be.true;
 		} );
 
-		it( 'should not convert marker if it is in graveyard', () => {
-			const gyRange = model.createRange( model.createPositionAt( doc.graveyard, 0 ), model.createPositionAt( doc.graveyard, 0 ) );
+		it( 'should convert marker in document fragment', () => {
+			const text = new ModelText( 'foo' );
+			const docFrag = new ModelDocumentFragment( text );
+			const eleRange = model.createRange( model.createPositionAt( docFrag, 1 ), model.createPositionAt( docFrag, 2 ) );
 			sinon.spy( dispatcher, 'fire' );
 
-			dispatcher.convertMarkerAdd( 'name', gyRange );
+			dispatcher.convertMarkerAdd( 'name', eleRange );
 
-			expect( dispatcher.fire.called ).to.be.false;
+			expect( dispatcher.fire.called ).to.be.true;
 		} );
 
-		it( 'should not convert marker if it is not in model root', () => {
-			const element = new ModelElement( 'element', null, new ModelText( 'foo' ) );
-			const eleRange = model.createRange( model.createPositionAt( element, 1 ), model.createPositionAt( element, 2 ) );
+		it( 'should not convert marker if it is in graveyard', () => {
+			const gyRange = model.createRange( model.createPositionAt( doc.graveyard, 0 ), model.createPositionAt( doc.graveyard, 0 ) );
 			sinon.spy( dispatcher, 'fire' );
 
-			dispatcher.convertMarkerAdd( 'name', eleRange );
+			dispatcher.convertMarkerAdd( 'name', gyRange );
 
 			expect( dispatcher.fire.called ).to.be.false;
 		} );
@@ -662,14 +664,15 @@ describe( 'DowncastDispatcher', () => {
 			expect( dispatcher.fire.called ).to.be.false;
 		} );
 
-		it( 'should not convert marker if it is not in model root', () => {
-			const element = new ModelElement( 'element', null, new ModelText( 'foo' ) );
-			const eleRange = model.createRange( model.createPositionAt( element, 1 ), model.createPositionAt( element, 2 ) );
+		it( 'should convert marker in document fragment', () => {
+			const text = new ModelText( 'foo' );
+			const docFrag = new ModelDocumentFragment( text );
+			const eleRange = model.createRange( model.createPositionAt( docFrag, 1 ), model.createPositionAt( docFrag, 2 ) );
 			sinon.spy( dispatcher, 'fire' );
 
 			dispatcher.convertMarkerRemove( 'name', eleRange );
 
-			expect( dispatcher.fire.called ).to.be.false;
+			expect( dispatcher.fire.called ).to.be.true;
 		} );
 
 		it( 'should fire conversion for the range', () => {

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

@@ -54,6 +54,13 @@ describe( 'Model', () => {
 			expect( schema.checkChild( [ '$clipboardHolder' ], '$block' ) ).to.be.true;
 		} );
 
+		it( 'registers $documentFragment to the schema', () => {
+			expect( schema.isRegistered( '$documentFragment' ) ).to.be.true;
+			expect( schema.isLimit( '$documentFragment' ) ).to.be.true;
+			expect( schema.checkChild( [ '$documentFragment' ], '$text' ) ).to.be.true;
+			expect( schema.checkChild( [ '$documentFragment' ], '$block' ) ).to.be.true;
+		} );
+
 		it( 'registers $marker to the schema', () => {
 			model.document.createRoot( '$anywhere', 'anywhere' );
 			schema.register( 'anything' );

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

@@ -3233,33 +3233,32 @@ describe( 'SchemaContext', () => {
 			expect( ctx ).to.equal( previousCtx );
 		} );
 
-		it( 'filters out DocumentFragment when it is a first item of context - array', () => {
+		it( 'creates context in DocumentFragment - array with string', () => {
 			const ctx = new SchemaContext( [ new DocumentFragment(), 'paragraph' ] );
 
-			expect( ctx.length ).to.equal( 1 );
-			expect( Array.from( ctx.getNames() ) ).to.deep.equal( [ 'paragraph' ] );
+			expect( ctx.length ).to.equal( 2 );
+			expect( Array.from( ctx.getNames() ) ).to.deep.equal( [ '$documentFragment', 'paragraph' ] );
 		} );
 
-		it( 'filters out DocumentFragment when it is a first item of context - element', () => {
+		it( 'creates context in DocumentFragment - element', () => {
 			const p = new Element( 'paragraph' );
 			const docFrag = new DocumentFragment();
 			docFrag._appendChild( p );
 
 			const ctx = new SchemaContext( p );
 
-			expect( ctx.length ).to.equal( 1 );
-			expect( Array.from( ctx.getNames() ) ).to.deep.equal( [ 'paragraph' ] );
+			expect( ctx.length ).to.equal( 2 );
+			expect( Array.from( ctx.getNames() ) ).to.deep.equal( [ '$documentFragment', 'paragraph' ] );
 		} );
 
-		it( 'filters out DocumentFragment when it is a first item of context - position', () => {
+		it( 'creates context in DocumentFragment - position', () => {
 			const p = new Element( 'paragraph' );
-			const docFrag = new DocumentFragment();
-			docFrag._appendChild( p );
-
-			const ctx = new SchemaContext( new Position( docFrag, [ 0, 0 ] ) );
+			const docFrag = new DocumentFragment( p );
+			const pos = Position._createAt( docFrag.getChild( 0 ), 0 );
+			const ctx = new SchemaContext( pos );
 
-			expect( ctx.length ).to.equal( 1 );
-			expect( Array.from( ctx.getNames() ) ).to.deep.equal( [ 'paragraph' ] );
+			expect( ctx.length ).to.equal( 2 );
+			expect( Array.from( ctx.getNames() ) ).to.deep.equal( [ '$documentFragment', 'paragraph' ] );
 		} );
 	} );