[GHS] Editor crashes after inserting nested inline element with a class

[Mgsy]

📝 Provide detailed reproduction steps (if any)

1. Go to https://ckeditor.com/docs/ckeditor5/latest/features/general-html-support.html.
2. Insert the following content to the editor (via source editing):

<span>
    <span class="test">test</span>
</span>

✔️ Expected result

The content inserts properly.

❌ Actual result

The editor crashes.

📃 Other details

The issue occurs only if the parent element is the same element as the nested one and the nested one contains a class.

Error

emittermixin.js:232 Uncaught TypeError: target[key] is not iterable
    at mergeViewElementAttributes (conversionutils.js:48)
    at UpcastDispatcher.dispatcher.on.priority (converters.js:104)
    at UpcastDispatcher.fire (emittermixin.js:199)
    at UpcastDispatcher._convertItem (upcastdispatcher.js:249)
    at UpcastDispatcher._convertChildren (upcastdispatcher.js:282)
    at UpcastDispatcher.<anonymous> (upcasthelpers.js:439)
    at UpcastDispatcher.fire (emittermixin.js:199)
    at UpcastDispatcher._convertItem (upcastdispatcher.js:253)
    at UpcastDispatcher.convert (upcastdispatcher.js:207)
    at datacontroller.js:437

---

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

[shabab477]

Anyone working on this fix?

[shabab477]

Hi, we have some mission-critical content that can't be dropped in order to get through this bug. Is a PR welcome to fix this?

[shabab477]

@shahzebgit easiest fix should be just to do this at the source:

export function mergeViewElementAttributes( target, source ) {
	const result = cloneDeep( target );

	for ( const key in source ) {

		// Merge classes.
		if ( !Array.isArray( target[ key ] ) ) {
			continue;
		} 
		
		else if ( Array.isArray( source[ key ] ) ) {
			result[ key ] = Array.from( new Set( [ ...target[ key ], ...source[ key ] ] ) );
		}

		// Merge attributes or styles.
		else {
			result[ key ] = { ...target[ key ], ...source[ key ] };
		}
	}

	return result;
}

Any suggestion for a better fix? Looking to hear from the project maintainers too. @Mgsy

[theezeb]

@shabab477 yeah even i'm using something similar solution for now, but i think better solution would be to preserve the class even if it doesn't finds target classes to merge here.

if ( !Array.isArray( target[ key ] ) ) {
continue;
}

this here helps us to avoid the error for mean time.

I made little change for this, this helps it to let the classes get applied even if it doesn't have anything to merge with.

if (!Array.isArray(target[key])) {
result[key] = source[key];
}

[shabab477]

Yeah, this looks better, right now I am building from source and deploying in our app but would be great to have it in the upstream.

[theezeb]

Made a bit more changes hope this solves the issue here.

Here isEmpty method would really help to check if the target is empty(Obj) or not instead of checking for undefined.
will make changes if needed.

export function mergeViewElementAttributes( target, source ) {
	const result = cloneDeep( target );

	for ( const key in source ) {
		//If target is empty, just copy the source.
		if (target[key] === undefined)) {
			result[key] = source[key];
		}
		// Merge classes.
		else if ( Array.isArray( source[ key ] ) ) {
			result[ key ] = Array.from( new Set( [ ...target[ key ], ...source[ key ] ] ) );
		}

		// Merge attributes or styles.
		else {
			result[ key ] = { ...target[ key ], ...source[ key ] };
		}
	}

	return result;
}

[Reinmar]

We'll be working soon on a bug that throws the same error (#11450) so hopefully this issue will be resolved too.