Input: DesignMode "thread safety" bug

[FlowIT-JIT]

The following example demonstrates how an input control in DesignMode can break when toggling DesignMode on and off. This may also happen when changing properties such as Height, Resizable, and Maximizable.

With React and other frameworks performing re-renders, we currently cannot guarantee that the bug won't happen at some point. We should look into this.

// Warmup - load editor resources (works fine)
i = new Fit.Controls.Input();
i.DesignMode(true);
i.Render(document.body);

// Thread safety bug - editor breaks when editor is destroyed/recreated due to its async nature
setTimeout(function()
{
    i = new Fit.Controls.Input();
    i.DesignMode(true);
    i.Render(document.body);
    i.DesignMode(false);i.DesignMode(true);
    i.DesignMode(false);i.DesignMode(true);
    i.DesignMode(false);i.DesignMode(true);
}, 3000);

Make sure we don't cause memory leaks! Inspect CKEditor.instances and make sure it does not contain destroyed instances.

[FlowIT-JIT]

After experimenting quite a bit, it turns out not to be as simple as one could hope - allowing for DesignMode to be disabled while editor is loading and initializing.

CKEditor loads async. - first resources are loaded async., then a CKEditor instance is created which internally seems to be using setTimeout(..) quite a bit, so that's also async. Furthermore editor initialization may be postponed by the Input control until it is rooted in DOM and made visible in the UI, since CKEditor relies on control being part of DOM and render tree. So a lot of asynchronous operations.

Since we can't expect CKEditor to behave as expected if suddently destroyed while loading and initializing, we must wait for the editor instance to finish loading. That means DesignMode(false) and Dispose() are not allowed to interupt editor initialization. And what's more, if Dispose() is in fact invoked while editor is initializing, we must ensure that the control is removed from the user interface immediately but kept in memory until it is safe to fully dispose of all resources. But as mentioned the control must also remain in DOM and the render tree - otherwise CKEditor might fail as it relies on the ability to measure dimensions. We can temporarily place the control off-screen, outside the viewport.

DesignMode(false) might remove the <textarea> element which CKEditor relies on, and replace it with an <input> element, if it returns to single line editing (depending on whether MultiLine(true) was invoked prior to DesignMode(true). This will also cause CKEditor initialization to break. The same applies to MultiLine(false) which also disables DesignMode.

On the positive side we should be able to handle changes to DesignMode such as Height, Resizable, and Maximizable, as it merely requires a reload of the editor, which can be postponed until initialization is complete.

So this leaves us with mainly one problem: DesignMode can't be disabled while loading/initializing, and after all that's probably a fairly rare usecase - at least its an indicator of poor app design. Therefore a fairly simple solution would be to simply discard calls to DesignMode(bool) and MultiLine(bool) while editor is loading/initializing. This would allow the features to be used in response to user interactions (e.g. toggling DesignMode using a button), while preventing poor app design from breaking the app. We should issue a warning to the browser console when such API calls are ignored since it's bad practice.

One could argue that DesignMode(false) could simply be async. like the suggested fix for Dispose(), but DesignMode() (getter) must never return an incorrect value compared to the state set by the app - so DesignMode(false) must result in DesignMode() returning False. This change would likely require that internal code stop using DesignMode() since we internally relies on the current control state, not the state applied by the app. So this change is more complicated, but might be possible. Do take care not to complicate this part of the solution though as disabling the editor while loading is not really a likely usecase, which Dispose() actually is, as we could imagine a Single Page Application loading data and the UI async., which might allow a user to navigate away from a view at any time, including while editor is loading, in which case the editor needs to be disposable.

So in short: Fix Height(..), Maximizable(..), and Resizable(..) by force-reloading editor when ready, make Dispose() remove control from visible part of UI and remove it from memory when ready, and discard calls to DesignMode(false) and MultiLine(false) while editor is loading and make sure to issue a warning.

[FlowIT-JIT]

Fixed as suggested.