why does ckeditor complain with warnings in the console when the el is detached from the dom?

[thedavidmeister]

🐞 Is this a bug report or feature request? (choose one)

support request

💻 Version of CKEditor

ckeditor5 balloon

📋 Steps to reproduce

detach a DOM element from the DOM with an editor on it

✅ Expected result

element is detached without issue

❎ Actual result

ckeditor throws warnings in the console

📃 Other details that might be useful

[oskarwrobel]

When you remove element with editor from DOM you should first destroy editor instance by calling editorInstance#destroy(). Otherwise, there will be still some attached listeners reacting to document changes like selectionchange event. I guess that ContectualToolbar tries to reposition itself on selectionchange but because of it happens on detached element it logs a warning.

[oskarwrobel]

Please reopen this ticket if this is the other kind of problem.

[thedavidmeister]

@oskarwrobel why are event handlers on a detached DOM element a problem?

why should i destroy the editor if i plan to re-attach the DOM element?

[thedavidmeister]

@oskarwrobel i cannot re-open the ticket, i do not have access, i can only comment

[oskarwrobel]

@oskarwrobel i cannot re-open the ticket, i do not have access, i can only comment

My bad, sorry for that. I'm reopening it.

Please bring more details about this issue. What exactly you do to see the warning and what warning is it?

[Reinmar]

[OT] One of the pretty well known open source maintainers wrote on Twitter once that for years he's been asking people to reopen tickets if they disagree when they never could :D. [/OT]

[thedavidmeister]

i've added "reproduce this issue" to my list of things to do :)

[thedavidmeister]

@oskarwrobel it is this error code https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/framework/guides/support/error-codes.html#rect-source-not-in-dom

so i say that detaching DOM elements is part of normal DOM usage and not an error condition :)

if you cannot calculate a rectangle, then use NaN or null or 0 or something and gracefully handle that case in downstream plugin code, don't push destroying/rebuilding the editor onto the code managing DOM components (that probably knows nothing about ckeditor if it is decoupled).

i have a complicated, nested hierarchy of components in an SPA. Things somewhere near the top handle routing and passing data to nested components. The components that actually have a WYSIWYG on them are very low level, like a single div receiving a localised data object to edit - these low level components have no information about things like the current route/larger data structures or whether they are even in the DOM.

[Reinmar]

I guess we can remove this warning now. Or work, finally, on the ability to set log levels. Warnings like this and the one we had in selection renderer may sometimes help to debug some issue. Without them, there's just a black hole which is often hard to notice.

In this specific case, IIRC, we introduced this warning because we had some issue with the ways how this helper was used. @oleq could tell more.

BTW, there's also @pjasiun's https://github.com/ckeditor/ckeditor5/issues/383 but it's a much bigger topic. Although, perhaps it doesn't make sense to work first on log-levels and then on the rest and we should treat these topics holistically.

---

The second topic here is that we can't guarantee forever that CKEditor will work after being freely detached and attached to the DOM. In CKE5, fortunately, we don't use any iframes so far, but there will be types of editors which may do so. And iframes are very volatile – detaching them destroy them. There may be also other kinds of issues, listeners for instance, which may break when the editor is detached. I'm not sure whether it's realistic to expect that we'll secure the entire code for those scenarios.

Instead, I'd like to believe that CKEditor is used in a controlled environment. If it gets detached, a framework or simply an app should be able to tell when this happens and destroy it before that (or even right after). That's a part of a component's lifecycle. It shouldn't matter how deep that component is nested in some app. The framework should be able to handle such situations correctly. If not, then warnings on the console are the least problem. Memory leaks and more serious crashes are also on the horizon.

[Reinmar]

if you cannot calculate a rectangle, then use NaN or null or 0 or something and gracefully handle that case in downstream plugin code, don't push destroying/rebuilding the editor onto the code managing DOM components (that probably knows nothing about ckeditor if it is decoupled).

There may be no value that would ensure gracefull handling. E.g. 0,0 seems fine but it may cause editor to display some panels on the screen after you thought you removed that editor. So much more would need to be done to secure the code – we'd need to check whether the editor is still in the DOM. And such code would leak from this helper to the entire editor codebase.

In my opinion, it's exactly the app's job to maintain the components it uses. And there's no such thing as a decoupled editor – it's a child of some UI component so this component should destroy the editor. In other words, if that UI component instantiated the editor it should also care about destroying it.

[thedavidmeister]

@Reinmar thanks for the update, a few initial thoughts:

• "lifecycle protocols" are considered an anti-pattern in hoplon and generally frowned upon (e.g. because they introduce leaky abstractions), i know they are a fundamental concept in some frameworks, but not all
• event listeners don't break AFAIK when detaching elements from the DOM, events triggered while the element is detached simply do not bubble, re-attaching the element allows events to bubble once more. jQuery (for example) provides logic to normalise bubbling detached events. Can you provide an example of an event listener breaking due to detaching a DOM element?
• i think you're double dipping a bit conceptually w.r.t "child of some component" given https://github.com/ckeditor/ckeditor5/issues/727 (i'd be much more on board with what you are saying here if i could indeed push everything related to a single editor instance inside my components)
• in the case of iframes, the warning can easily be re-introduced when needed as it would actually impact that editor, and the warning would probably be different (e.g. something specific to detaching iframes), i don't see a reason to keep generic "harmless" error messages around "just in case"
• yes, if this is just debug then it makes sense to use debug or info level console logs rather than warn
• 0,0 was just one suggestion and i agree not the best, NaN or null or undefined seems more appropriate to me as they are unambiguously not numbers and so could represent "not in DOM" or "do not handle this" or whatever, without needing to poll the DOM or do anything else complicated downstream
• ckeditor is the only library i've encountered so far that cannot handle detaching/reattaching elements without a full destroy/rebuild loop
• hoplon avoids memory leaks of detached elements by caching and recycling them, FYI, it's because I want to use the native memory leak mitigation strategy of the framework that I would like to see this working :)

[thedavidmeister]

@Reinmar think of it this way:

i have component A that creates the editor, but component A is added/removed in the DOM by component B, which is added/removed in the DOM by component C, .... all the way up to component N at the top of the app watching routes.

component A can certainly build/destroy the WYSIWYG based on what it is doing, but if it is detached from the DOM itself by component C via. component B then how can A respond to such a thing and do something to the WYSIWYG in response to what component C is responsible for?

[Reinmar]

"lifecycle protocols" are considered an anti-pattern in hoplon and generally frowned upon (e.g. because they introduce leaky abstractions), i know they are a fundamental concept in some frameworks, but not all

...

hoplon avoids memory leaks of detached elements by caching and recycling them, FYI

Unfortunately, I don't know hoplon and I can't quickly learn its concepts so it's hard for me to respond. But it seems to be a typical problem of paradigm collisions. The touch points are hard to get right if two libs work conceptually different.

One thing which I find interesting here is – if you'll need to instantiate (at the same time) N components using the editor (N instances) and then just remove them, when will these editors be destroyed? From what you wrote it seems that never, because lifecycle doesn't matter. So I can see here a memory leak – these editors will occupy the memory forever (because they'll be retained due to DOM listeners).

So, either I don't understand something here (which is very probable given I've never worked with hoplon) or there's a serious design problem.

event listeners don't break AFAIK when detaching elements from the DOM, events triggered while the element is detached simply do not bubble, re-attaching the element allows events to bubble once more. jQuery (for example) provides logic to normalise bubbling detached events. Can you provide an example of an event listener breaking due to detaching a DOM element?

What if the editor needs to listen on window/documents events? Not all events bubble. Those listeners will be retained. Of course, we may review them and secure them. TBH, it's even possible that all of them will work fine now and that there are not that many of them. But I've learnt how awfully complex your app becomes if you start considering all possible integration problems. We've been there. And complexity means bugs. So I'd rather push some responsibility to the API consumers because they know their cases. They know how the editor is integrated and can create proper abstraction. And this abstraction will be easier than what we'd need to consider internally to secure ourselves from all possible issues, in all possible frameworks, apps, environments and scenarios. It requires work, of course. But I'd like to believe that this is a better choice.

ckeditor is the only library i've encountered so far that cannot handle detaching/reattaching elements without a full destroy/rebuild loop

Isn't it also the most complex one? :P I often find that people underestimate the complexity behind libs like CKEditor. I don't want brag here about it, cause that's not the point. It's just about the scale of the problem that we need to consider. I'm 6 years into that and (unfortunately) I've seen things that cannot be unseen :D

E.g. we could agree today to simply stop logging such warnings and handle it gracefully downstream. But I kinda expect where it leads.

First of all, let's not depreciate a value of such harmless messages. There are bugs which make you spend entire days on them and we only started adding such logs because of them. Because previously we didn't and it made us suffer :P. Of course, here we're talking about debugging, not consumer-level communication. So we all agree that warnings like this could be hidden behind a flag. This is clear.

Then we come to handling it gracefully on our side – as I explained above already, I'm not sure about this. That's just one small issue now but there will be more. If we'll agree once that this is our duty to secure the editor from being detached, then we're sold already. And don't get me wrong – I'd love to secure the editor. But since it means additional work and additional complexity (which means more work and bugs) I want to wait and see to learn how popular this problem is. Dealing with the community requires assertiveness. There are millions of frameworks out there and developers want us to handle all of them, in the way those frameworks expect the other libs to work. We can't satisfy everyone. I remember one day when a team of devs working on a new browser (a fork of Firefox I think) expected from us that we'll add support for their product. The problem was that one of the first things they decided to do was to break compatibility with their mother-browser by doing some completely unnecessary changes to some globals (UA, I think). Breaking CKEditor 4 should've been their least problem, in fact :D And I haven't heard about them since then.

---

I went full-on here not to depreciate and ignore your feedback. It's the opposite – I really appreciate it and we'll (at least try to) do changes that we can do. We'll also keep it in the back of our heads when making next decisions (e.g. about those error logging mechanisms in #383). But, at the same time, I wanted to explain why we're reluctant initially to make conceptual changes. It's not that we don't care. It's that we're scared :P

[thedavidmeister]

@Reinmar i appreciate your work, you guys have been super responsive across multiple issues, it's encouraging me to dig deeper and find solutions :)

i am still a bit skeptical about the need to listen to any and all arbitrary events including window events while detached. Given that no user input can be provided while detached, it seems reasonable that the WYSIWYG be disabled globally and events ignored/throttled/filtered or similar until reattached.

i also don't think what we're discussing here is quite an "apples to apples" comparison to that dead browser fork example:

• Hoplon has been around and actively developed since at least 2013 and is relatively well known in the clojure community
• Hoplon is mostly a thin functional wrapper around jQuery and introduces little in the way of "magic" that should/could break 3rd party libraries
• Detaching/attaching DOM elements is native browser functionality in all major browsers and always has been AFAIK

I feel like perhaps you are imagining Hoplon to be doing something more exotic and complex than it is.

Ckeditor certainly is complex, if i didn't think so i'd be building my own editor right now ;)

yes, i agree there are a paradigm mismatches here (e.g. functional programming in cljs vs. your heavy use of OO), but i do really want to get it working because i like a lot of things about both hoplon and ckeditor :)

I don't think you need to understand that much about Hoplon for this discussion. it is a functional jQuery wrapper that aims to mimic HTML in structure (so there are functions called div, span, etc.), which means it is largely declarative so concepts like "lifecycles" don't fit well.

How Hoplon handles low level stuff like CPU/memory management is definitely interesting (e.g. a DAG propagating values compared to diffing a virtual DOM) but a better fit for the Hoplon github/slack IMO. Unless there's reason to believe that the WYSIWYG is particularly heavy memory-wise it seems like an academic/premature optimisation to worry about a handful of disabled editor instances sitting in memory.

Let's just focus on the idea that:

• there may be legitimate reasons to detach elements from the DOM (I googled and found other people in stack overflow/forums coming up against this in older versions of ckeditor too, e.g. when trying to re-arrange elements)
• not every framework/app has a formal set of lifecycle protocols to procedurally co-ordinate build/destroy methods in the way you are describing

what could be helpful in this case, is agreeing that at least the destroy API method should always work on detached editors - that way it is possible to do cleanup in the background or "out of order" based on high level application state rather than needing to co-ordinate things strictly with fine grained DOM management.

[thedavidmeister]

ugh, i tried to destroy and rebuild the editor and started getting this error again https://github.com/ckeditor/ckeditor5/issues/706 - i'll start digging into that

[thedavidmeister]

I thought a bit more about this.

Given that:

• ckeditor considers detaching from the DOM an unrecoverable error state
• not all frameworks provide a "lifecycle" to hook into (i haven't tried but imagine you'd run into similar problems with something like https://cycle.js.org/) in order to destroy at the exact right moment
• the only current information we get about the error state is a somewhat cryptic warning in the console about measuring rectangles
• ckeditor can break in subtle ways after being re-attached to the DOM without obvious explanation

Maybe ckeditor should just destroy itself with a clear debug message (not error/warning, just debug/info) in the console the moment it is detached from the DOM or otherwise put in an unrecoverable state. A callback option might also be nice.

Then:

• it should be very obvious to developers that they are doing something that needs careful handling
• developers only need to handle building new editors as needed, not manually managing the editor state based on logic known only to those quite familiar with ckeditor code
• there is no possibility of ckeditor attempting to continue to function after having been placed in an explicitly unrecoverable state
• ckeditor should overall become simpler to maintain

[CKEditorBot]

There has been no activity on this issue for the past two years. 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.

[thedavidmeister]

it's not stale, the logic still stands

[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 still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.