Ckeditor5: Should editor initialization crash still add DOM to the host page

[mlewand]

📝 Provide a description of the new feature

I'd like to discuss whether or not it is OK for our editor to leave it's DOM in case of a crashed initialization.

For instance if you use following code:

ClassicEditor.create( '<p>Hello world!</p>', {
    initialData: '<p>Pure evil</p>'
} );

We'll throw an cancel the initialization, which is understandable as this kind of initialization doesn't make much sense.

However despite not create an editor we leave following markup in the document's body (prefer demos? see this codepen):

<div class="ck ck-reset_all ck-body ck-rounded-corners" dir="ltr">

    <div class="ck ck-balloon-panel ck-balloon-panel_arrow_nw ck-balloon-panel_with-arrow" style="top: 0px; left: 0px;">

        <div class="ck ck-balloon-rotator" z-index="-1">

            <div class="ck-balloon-rotator__navigation ck-hidden"><button class="ck ck-button ck-off" type="button"
                    tabindex="-1" aria-labelledby="ck-editor__aria-label_e5c47478618b1083c761d4232bcf0bb4c"><svg
                        class="ck ck-icon ck-button__icon" viewBox="0 0 20 20">
                        <path
                            d="M11.463 5.187a.888.888 0 1 1 1.254 1.255L9.16 10l3.557 3.557a.888.888 0 1 1-1.254 1.255L7.26 10.61a.888.888 0 0 1 .16-1.382l4.043-4.042z">
                        </path>
                    </svg><span class="ck ck-tooltip ck-tooltip_s"><span
                            class="ck ck-tooltip__text">Previous</span></span><span class="ck ck-button__label"
                        id="ck-editor__aria-label_e5c47478618b1083c761d4232bcf0bb4c">Previous</span></button><span
                    class="ck-balloon-rotator__counter"></span><button class="ck ck-button ck-off" type="button"
                    tabindex="-1" aria-labelledby="ck-editor__aria-label_e3e7a322ec3119ce3cf20dde591e8387e"><svg
                        class="ck ck-icon ck-button__icon" viewBox="0 0 20 20">
                        <path
                            d="M8.537 14.813a.888.888 0 1 1-1.254-1.255L10.84 10 7.283 6.442a.888.888 0 1 1 1.254-1.255L12.74 9.39a.888.888 0 0 1-.16 1.382l-4.043 4.042z">
                        </path>
                    </svg><span class="ck ck-tooltip ck-tooltip_s"><span
                            class="ck ck-tooltip__text">Next</span></span><span class="ck ck-button__label"
                        id="ck-editor__aria-label_e3e7a322ec3119ce3cf20dde591e8387e">Next</span></button></div>

            <div class="ck-balloon-rotator__content"></div>
        </div>
    </div>

    <div class="ck-fake-panel ck-hidden" style="top: 0px; left: 0px; width: 0px; height: 0px;"></div>
</div>

I faced this while doing test cleanups for #6002 (there are 200+ tests leaking DOM elements). Some leaks are caused by behavior described in this issue.

---

If you'd like to see this feature implemented, add a 👍 reaction to this post.

[mlewand]

For me, case where the editor crashes should not happen and we should not worry about this case. But as I mentioned I wanted to hear other opinions.

[jodator]

It would be probably very nice of the editor to clean up in Edtior.create() scenario. But then we also have crashes happening when the editor is running.

The problem is that such a situation will cause problems buuuut it shouldn't happen in production. Only in the tests or in development setup. So any leaks will happen in any other app that crashes.

That reminds me that we have Watchdog - if editor restarts because of watchdog then we probably should extra cleanup.

So automatic cleanup on .create() will not be reliable anyway and a better thing would be to clean up the test in the first place.

If, however, removing created dom element could be easy (fast to implement) I see some benefit in that.

[Reinmar]

It would be probably very nice of the editor to clean up in Edtior.create() scenario. But then we also have crashes happening when the editor is running.

That exactly. And it's really hard (if not nearly impossible) to clean up in such cases.

[ma2ciek]

For me, case where the editor crashes should not happen and we should not worry about this case.

So we should kill the Watchdog feature if that's true :smile: (if you mean all crashes, not only initialization crashes)

In the case of initialization crashes I think that cleaning DOM is valuable only for our tests to make the cleanup. So I'm fine with the test util that takes care about cleaning DOM.

[mlewand]

@ma2ciek I meant initialization crash. In a sense that if you add editor to your project and push to production without checking whether you initialized it correctly - you're doing something wrong. 😄