Editor should be removable from dom right after it is destroyed

[engineering-this]

Type of report

Bug

Other details

Because of how components in modern JS frameworks are working we should address some issues with editor.

1. Warning [CKEDITOR] Error code: editor-incorrect-destroy.
   We should completely get rid of this warning. It is visible when part of destroy logic is executed after editor is removed from DOM. Cleaning logic should work regardless of iframe being in DOM or not.
   So we could do following:

   editor.destroy();
   editorWrapper.remove();
   

2. Errors are thrown when editor is destroyed and removed from DOM before it is fully initialized. Frameworks routing works in a way that component.destroy() is fired either right after or right before elements are removed from DOM. We should somehow allow this.

Currently all our integrations are affected by this.
React: ckeditor/ckeditor4-react#44
Vue: Requires user to add router guards.
Angular: classic editor is disabled, editor is rendered outside of a component and then appended to it - ugly hacks.

I have some ideas in mind I'll report here what works or doesn't.

[f1ames]

Warning [CKEDITOR] Error code: editor-incorrect-destroy.
We should completely get rid of this warning. It is visible when part of destroy logic is executed after editor is removed from DOM. Cleaning logic should work regardless of iframe being in DOM or not.

Do you mean that we should implement cleaning logic in a way that it works even after iframe is detached from DOM. Makes sense if it's doable ofc, but might cause some troubles when operating on detached elements (needs to be tested). OTOH, we can't just abort destroy chain if editor was detached because it will result in memory leaks.

If I understand correctly, it's not an issue for other editor types? Even though main editor element or it's parent could be detached too before destroying editor (doesn't break before there is no iframe?)?

Errors are thrown when editor is destroyed and removed from DOM before it is fully initialized. Frameworks routing works in a way that component.destroy() is fired either right after or right before elements are removed from DOM. We should somehow allow this.

Here, I'm not sure how we could handle this one 🤔 Ofc, we can prevent destroying editor or delay it after it's fully initialized, but I'm not sure if it will resolve issues we have in integrations?

[jacekbogdanski]

Do you mean that we should implement cleaning logic in a way that it works even after iframe is detached from DOM. Makes sense if it's doable ofc, but might cause some troubles when operating on detached elements (needs to be tested). OTOH, we can't just abort destroy chain if editor was detached because it will result in memory leaks.

I'm also wondering if we should treat this warning as an issue for integrations. It's a warning, not error, so if it's not causing any issues, maybe should be left as it is? There is a distinction between warning and error, we probably shouldn't treat them on the same level. Silencing warnings don't seem like a solution.

[engineering-this]

If I understand correctly, it's not an issue for other editor types? Even though main editor element or it's parent could be detached too before destroying editor (doesn't break before there is no iframe?)?

Inline (divarea) editors seems to be destroyed correctly, element references are sufficient. Iframe has one big problem - it doesn't operate when detached.

const iframe = document.createElement( 'iframe' );
console.log( iframe.contentDocument ); // null

document.body.append( iframe );
console.log( iframe.contentDocument ); // #document

document.body.removeChild( iframe );
console.log( iframe.contentDocument ); // null

Here, I'm not sure how we could handle this one 🤔 Ofc, we can prevent destroying editor or delay it after it's fully initialized, but I'm not sure if it will resolve issues we have in integrations?

It can be easily fixed for inline (divarea) editors. With just a few changes inline editor can be rendered outside of DOM. This protects editor from being removed by framework. Also to keep backwards compatibility, we should enable this way of rendering via config option which would be enabled by framework integrations. Possible cases: third party plugin search for button element based on it's ID.

For frame editors more research is needed.

[engineering-this]

[CKEDITOR] Error code: editor-incorrect-destroy.

Can be fixed easily. Look at code:

https://github.com/ckeditor/ckeditor-dev/blob/08ea0051cedd75c0bbb55d88d47fe9e05cddd1f5/plugins/wysiwygarea/plugin.js#L537-L539

Edit: Actually iframe custom data can be correctly cleaned so, there is no issue here. Just retrieving iframe should be corrected.

[f1ames]

Moving to the next iteration (4.13 since closing PR covers also feature part from #3124).