Fix for #3812 breaks CKEditor in IE when editor instance is no longer in DOM

[Marcos Wright Kuhns]

The IE-specific fix for #3812 causes another bug if the CKEditor container isn't a descendant of document.body. This bug only occurs in IE (because we use the moveToElementText function).

Attached is a test HTML file as well as a potential patch. The main shortcoming of the patch is that it does scroll to the top of the page. If someone has a better fix that would be great!

[Marcos Wright Kuhns]

An html file that can be used to reproduce the error in IE

[Marcos Wright Kuhns]

A better patch for this bug that also stops unexpected window scrolling.

[Alfonso Martínez de Lizarrondo]

What's the use case for allowing CKEditor to be moved out of the document DOM?

Whenever it's moved in the DOM the inner iframe will reload in Firefox, Safari and I think also Opera, so it's just better to destroy the editor and then recreate a new instance at the new location because that's in fact the only option that works.

[Marcos Wright Kuhns]

The test case I attached is completely contrived. The actual use-case where we're encountering the bug is this:
1) We load the CKEditor into a modal dialog box using AJAX. So the editorInstance.container.ancestors() == [form#dialog_form, div#modal_dialog, body, html]
2) When the user clicks the "Save" button in the dialog box, we save the CKEditor data via an AJAX POST, then remove the modal dialog from the DOM using the removeChild function. Now the editorInstance.container.ancestors() == [form#dialog_form, div#modal_dialog]
3) Finally, we have a cleanup function that removes old CKEditor instances. This function calls editor.destroy() on all CKEDITOR.instances that don't have document.body as an ancestor. This is where we encounter the error.

I realize that we could work around this error by calling the editor.destroy() function before we remove the modal dialog from the DOM, but because we use this pattern many different places in app, that would be a fairly significant change & require us to touch many points in our codebase. Also, beyond our specific case, I don't think it's a safe to assume that the CKEditor will always belong to the DOM when the destroy function is called.

Does this make sense?

[Frederico Caldeira Knabben]

It makes sense.

[Frederico Caldeira Knabben]

#5727 may be fixed by this one.

[Alfonso Martínez de Lizarrondo]

Updated patch

[Alfonso Martínez de Lizarrondo]

That solution is very good and fixes the original problem, but the patch was outdated and more important: now the elementsPath gives an error for all the browsers, the revised patch fixes also that issue.

[Alfonso Martínez de Lizarrondo]

Fixed with [5681]