No longer crash during destroy() of MultiRootEditor or `InlineEdi…

…tor` when the editable was detached manually before destroying.
ckeditor · Dec 23, 2024 · ed194fe · ed194fe
1 parent c5143d0
commit ed194fe
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 3 deletions.
diff --git a/packages/ckeditor5-editor-inline/src/inlineeditorui.ts b/packages/ckeditor5-editor-inline/src/inlineeditorui.ts
@@ -114,7 +114,10 @@ export default class InlineEditorUI extends EditorUI {
 		const view = this.view;
 		const editingView = this.editor.editing.view;
 
-		editingView.detachDomRoot( view.editable.name! );
+		if ( editingView.getDomRoot( view.editable.name! ) ) {
+			editingView.detachDomRoot( view.editable.name! );
+		}
+
 		view.destroy();
 	}
 

diff --git a/packages/ckeditor5-editor-inline/tests/inlineeditorui.js b/packages/ckeditor5-editor-inline/tests/inlineeditorui.js
@@ -41,7 +41,9 @@ describe( 'InlineEditorUI', () => {
 	} );
 
 	afterEach( async () => {
-		await editor.destroy();
+		if ( editor ) {
+			await editor.destroy();
+		}
 	} );
 
 	describe( 'constructor()', () => {
@@ -328,6 +330,13 @@ describe( 'InlineEditorUI', () => {
 
 			sinon.assert.callOrder( parentDestroySpy, viewDestroySpy );
 		} );
+
+		it( 'should not crash if the editable element is not present', async () => {
+			editor.editing.view.detachDomRoot( editor.ui.view.editable.name );
+
+			await editor.destroy();
+			editor = null;
+		} );
 	} );
 
 	describe( 'element()', () => {

diff --git a/packages/ckeditor5-editor-multi-root/src/multirooteditorui.ts b/packages/ckeditor5-editor-multi-root/src/multirooteditorui.ts
@@ -157,7 +157,12 @@ export default class MultiRootEditorUI extends EditorUI {
 	 * @param editable Editable to remove from the editor UI.
 	 */
 	public removeEditable( editable: InlineEditableUIView ): void {
-		this.editor.editing.view.detachDomRoot( editable.name! );
+		const editingView = this.editor.editing.view;
+
+		if ( editingView.getDomRoot( editable.name! ) ) {
+			editingView.detachDomRoot( editable.name! );
+		}
+
 		editable.unbind( 'isFocused' );
 		this.removeEditableElement( editable.name! );
 	}

diff --git a/packages/ckeditor5-editor-multi-root/tests/multirooteditorui.js b/packages/ckeditor5-editor-multi-root/tests/multirooteditorui.js
@@ -423,5 +423,21 @@ describe( 'MultiRootEditorUI', () => {
 
 			sinon.assert.callOrder( parentDestroySpy, viewDestroySpy );
 		} );
+
+		// Some of integrations might detach the DOM editing view *before* destroying the editor.
+		// It happens quite often in the strict mode of the React integration. In such case, the editor
+		// component is being unmounted after editable component is detached from the DOM. In such scenario,
+		// the root doesn't contain the DOM editable anymore. This test ensures that the editor does not throw.
+		// Issue: https://github.com/ckeditor/ckeditor5/issues/16561
+		it( 'should not throw when trying to detach a DOM root that was not attached to editing view', async () => {
+			const newEditor = await MultiRootEditor.create( { foo: '', bar: '' } );
+			const editingView = newEditor.editing.view;
+
+			// Simulate unmounting the editable child component before the editor component.
+			editingView.detachDomRoot( 'foo' );
+
+			// This should not throw
+			await newEditor.destroy();
+		} );
 	} );
 } );