upcastAttributeToMarker converter overrides the conversion data object even when there is no conversion match

[ma2ciek]

This leads to a situation when adding a converter with the editor.conversion.dataToMarker upcast helper changes can change the conversion for some elements even if they do not match the view specified by the converter.

This behavior crashes the editor when the Table and either Track Changes or Comments are added to the editor (there're some additional prerequisites to reproduce the error though).

We should rethink if overriding the data properties for the whole element conversion in the  

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

Line 690 in 523f88c

data = Object.assign( data, conversionApi.convertChildren( data.viewItem, data.modelCursor ) );

is necessary. Currently adding any marker converter may change any element's upcast conversion - what's worse - it can be a race condition depending on the order of plugins in the editor.

As a temporary solution, we can check if any element can be consumed and return early for conversion calls for non-matching view elements - before the

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

Lines 685 to 707 in 523f88c

	// This converter wants to add a model element, marking a marker, before/after an element (or maybe even group of elements).
	// To do that, we can use `data.modelRange` which is set on an element (or a group of elements) that has been upcasted.
	// But, if the processed view element has not been upcasted yet (it does not have been converted), we need to
	// fire conversion for its children first, then we will have `data.modelRange` available.
	if ( !data.modelRange ) {
	data = Object.assign( data, conversionApi.convertChildren( data.viewItem, data.modelCursor ) );
	}
	if ( conversionApi.consumable.consume( data.viewItem, { attributes: attrName + '-end-after' } ) ) {
	addMarkerElements( data.modelRange.end, data.viewItem.getAttribute( attrName + '-end-after' ).split( ',' ) );
	}
	if ( conversionApi.consumable.consume( data.viewItem, { attributes: attrName + '-start-after' } ) ) {
	addMarkerElements( data.modelRange.end, data.viewItem.getAttribute( attrName + '-start-after' ).split( ',' ) );
	}
	if ( conversionApi.consumable.consume( data.viewItem, { attributes: attrName + '-end-before' } ) ) {
	addMarkerElements( data.modelRange.start, data.viewItem.getAttribute( attrName + '-end-before' ).split( ',' ) );
	}
	if ( conversionApi.consumable.consume( data.viewItem, { attributes: attrName + '-start-before' } ) ) {
	addMarkerElements( data.modelRange.start, data.viewItem.getAttribute( attrName + '-start-before' ).split( ',' ) );
	}

lines.

[CKEditorBot]

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

[CKEditorBot]

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).