Merge pull request #12641 from ckeditor/ck/12602-tooltip-manager-dest…

…ruction Fix (ui): UI tooltips should continue showing up until the last editor gets destroyed. Closes #12602. Internal (editor-classic,editor-inline,editor-balloon,editor-decoupled): Changed the order of destruction in respective `*EditorUI` classes to ensure the base `EditorUI` class is destroyed before the view of the editor.
ckeditor · Oct 14, 2022 · 5809d20 · 5809d20
2 parents 9c52490 + 88ec2a2
commit 5809d20
Show file tree

Hide file tree

Showing 20 changed files with 157 additions and 26 deletions.
diff --git a/docs/_snippets/examples/multi-root-editor.js b/docs/_snippets/examples/multi-root-editor.js
@@ -249,6 +249,8 @@ class MultirootEditorUI extends EditorUI {
 	 * @inheritDoc
 	 */
 	destroy() {
+		super.destroy();
+
 		const view = this.view;
 		const editingView = this.editor.editing.view;
 
@@ -257,8 +259,6 @@ class MultirootEditorUI extends EditorUI {
 		}
 
 		view.destroy();
-
-		super.destroy();
 	}
 
 	/**

diff --git a/docs/examples/framework/multi-root-editor.md b/docs/examples/framework/multi-root-editor.md
@@ -264,6 +264,8 @@ class MultirootEditorUI extends EditorUI {
 	 * @inheritDoc
 	 */
 	destroy() {
+		super.destroy();
+
 		const view = this.view;
 		const editingView = this.editor.editing.view;
 
@@ -272,8 +274,6 @@ class MultirootEditorUI extends EditorUI {
 		}
 
 		view.destroy();
-
-		super.destroy();
 	}
 
 	/**

diff --git a/docs/framework/guides/custom-editor-creator.md b/docs/framework/guides/custom-editor-creator.md
@@ -238,6 +238,8 @@ class MultirootEditorUI extends EditorUI {
 	 * @inheritDoc
 	 */
 	destroy() {
+		super.destroy();
+
 		const view = this.view;
 		const editingView = this.editor.editing.view;
 
@@ -246,8 +248,6 @@ class MultirootEditorUI extends EditorUI {
 		}
 
 		view.destroy();
-
-		super.destroy();
 	}
 
 	/**

diff --git a/packages/ckeditor5-core/tests/_utils-tests/classictesteditor.js b/packages/ckeditor5-core/tests/_utils-tests/classictesteditor.js
@@ -263,5 +263,17 @@ describe( 'ClassicTestEditor', () => {
 						} );
 				} );
 		} );
+
+		it( 'should call parent EditorUI#destroy() first before destroying the view', async () => {
+			const newEditor = await ClassicTestEditor.create( editorElement, { foo: 1 } );
+			const parentEditorUIPrototype = Object.getPrototypeOf( newEditor.ui.constructor.prototype );
+
+			const parentDestroySpy = testUtils.sinon.spy( parentEditorUIPrototype, 'destroy' );
+			const viewDestroySpy = testUtils.sinon.spy( newEditor.ui.view, 'destroy' );
+
+			await newEditor.destroy();
+
+			sinon.assert.callOrder( parentDestroySpy, viewDestroySpy );
+		} );
 	} );
 } );
diff --git a/packages/ckeditor5-core/tests/_utils/classictesteditor.js b/packages/ckeditor5-core/tests/_utils/classictesteditor.js
@@ -146,11 +146,11 @@ class ClassicTestEditorUI extends EditorUI {
 	 * @inheritDoc
 	 */
 	destroy() {
+		super.destroy();
+
 		this._elementReplacer.restore();
 
 		this._view.destroy();
-
-		super.destroy();
 	}
 }
 

diff --git a/packages/ckeditor5-core/tests/editor/editorui.js b/packages/ckeditor5-core/tests/editor/editorui.js
@@ -23,7 +23,7 @@ describe( 'EditorUI', () => {
 
 	beforeEach( () => {
 		editor = new Editor();
-		ui = new EditorUI( editor );
+		editor.ui = ui = new EditorUI( editor );
 	} );
 
 	afterEach( () => {
@@ -461,7 +461,7 @@ describe( 'EditorUI', () => {
 
 			it( 'should do nothing if no toolbars were registered', () => {
 				const editor = new Editor();
-				const ui = new EditorUI( editor );
+				const ui = editor.ui = new EditorUI( editor );
 				const editingArea = document.createElement( 'div' );
 				document.body.appendChild( editingArea );
 

diff --git a/packages/ckeditor5-editor-balloon/src/ballooneditorui.js b/packages/ckeditor5-editor-balloon/src/ballooneditorui.js
@@ -86,13 +86,13 @@ export default class BalloonEditorUI extends EditorUI {
 	 * @inheritDoc
 	 */
 	destroy() {
+		super.destroy();
+
 		const view = this.view;
 		const editingView = this.editor.editing.view;
 
 		editingView.detachDomRoot( view.editable.name );
 		view.destroy();
-
-		super.destroy();
 	}
 
 	/**

diff --git a/packages/ckeditor5-editor-balloon/tests/ballooneditorui.js b/packages/ckeditor5-editor-balloon/tests/ballooneditorui.js
@@ -186,6 +186,18 @@ describe( 'BalloonEditorUI', () => {
 						} );
 				} );
 		} );
+
+		it( 'should call parent EditorUI#destroy() first before destroying the view', async () => {
+			const newEditor = await VirtualBalloonTestEditor.create( '' );
+			const parentEditorUIPrototype = Object.getPrototypeOf( newEditor.ui.constructor.prototype );
+
+			const parentDestroySpy = testUtils.sinon.spy( parentEditorUIPrototype, 'destroy' );
+			const viewDestroySpy = testUtils.sinon.spy( newEditor.ui.view, 'destroy' );
+
+			await newEditor.destroy();
+
+			sinon.assert.callOrder( parentDestroySpy, viewDestroySpy );
+		} );
 	} );
 
 	describe( 'element()', () => {

diff --git a/packages/ckeditor5-editor-classic/src/classiceditorui.js b/packages/ckeditor5-editor-classic/src/classiceditorui.js
@@ -114,14 +114,14 @@ export default class ClassicEditorUI extends EditorUI {
 	 * @inheritDoc
 	 */
 	destroy() {
+		super.destroy();
+
 		const view = this.view;
 		const editingView = this.editor.editing.view;
 
 		this._elementReplacer.restore();
 		editingView.detachDomRoot( view.editable.name );
 		view.destroy();
-
-		super.destroy();
 	}
 
 	/**

diff --git a/packages/ckeditor5-editor-classic/tests/classiceditorui.js b/packages/ckeditor5-editor-classic/tests/classiceditorui.js
@@ -297,6 +297,18 @@ describe( 'ClassicEditorUI', () => {
 						} );
 				} );
 		} );
+
+		it( 'should call parent EditorUI#destroy() first before destroying the view', async () => {
+			const newEditor = await VirtualClassicTestEditor.create( '' );
+			const parentEditorUIPrototype = Object.getPrototypeOf( newEditor.ui.constructor.prototype );
+
+			const parentDestroySpy = testUtils.sinon.spy( parentEditorUIPrototype, 'destroy' );
+			const viewDestroySpy = testUtils.sinon.spy( newEditor.ui.view, 'destroy' );
+
+			await newEditor.destroy();
+
+			sinon.assert.callOrder( parentDestroySpy, viewDestroySpy );
+		} );
 	} );
 
 	describe( 'view()', () => {

diff --git a/packages/ckeditor5-editor-decoupled/src/decouplededitorui.js b/packages/ckeditor5-editor-decoupled/src/decouplededitorui.js
@@ -80,13 +80,13 @@ export default class DecoupledEditorUI extends EditorUI {
 	 * @inheritDoc
 	 */
 	destroy() {
+		super.destroy();
+
 		const view = this.view;
 		const editingView = this.editor.editing.view;
 
 		editingView.detachDomRoot( view.editable.name );
 		view.destroy();
-
-		super.destroy();
 	}
 
 	/**

diff --git a/packages/ckeditor5-editor-decoupled/tests/decouplededitorui.js b/packages/ckeditor5-editor-decoupled/tests/decouplededitorui.js
@@ -238,6 +238,18 @@ describe( 'DecoupledEditorUI', () => {
 						} );
 				} );
 		} );
+
+		it( 'should call parent EditorUI#destroy() first before destroying the view', async () => {
+			const newEditor = await VirtualDecoupledTestEditor.create( '' );
+			const parentEditorUIPrototype = Object.getPrototypeOf( newEditor.ui.constructor.prototype );
+
+			const parentDestroySpy = testUtils.sinon.spy( parentEditorUIPrototype, 'destroy' );
+			const viewDestroySpy = testUtils.sinon.spy( newEditor.ui.view, 'destroy' );
+
+			await newEditor.destroy();
+
+			sinon.assert.callOrder( parentDestroySpy, viewDestroySpy );
+		} );
 	} );
 
 	describe( 'element()', () => {

diff --git a/packages/ckeditor5-editor-inline/src/inlineeditorui.js b/packages/ckeditor5-editor-inline/src/inlineeditorui.js
@@ -96,13 +96,13 @@ export default class InlineEditorUI extends EditorUI {
 	 * @inheritDoc
 	 */
 	destroy() {
+		super.destroy();
+
 		const view = this.view;
 		const editingView = this.editor.editing.view;
 
 		editingView.detachDomRoot( view.editable.name );
 		view.destroy();
-
-		super.destroy();
 	}
 
 	/**

diff --git a/packages/ckeditor5-editor-inline/tests/inlineeditorui.js b/packages/ckeditor5-editor-inline/tests/inlineeditorui.js
@@ -323,6 +323,18 @@ describe( 'InlineEditorUI', () => {
 						} );
 				} );
 		} );
+
+		it( 'should call parent EditorUI#destroy() first before destroying the view', async () => {
+			const newEditor = await VirtualInlineTestEditor.create( '' );
+			const parentEditorUIPrototype = Object.getPrototypeOf( newEditor.ui.constructor.prototype );
+
+			const parentDestroySpy = testUtils.sinon.spy( parentEditorUIPrototype, 'destroy' );
+			const viewDestroySpy = testUtils.sinon.spy( newEditor.ui.view, 'destroy' );
+
+			await newEditor.destroy();
+
+			sinon.assert.callOrder( parentDestroySpy, viewDestroySpy );
+		} );
 	} );
 
 	describe( 'element()', () => {

diff --git a/packages/ckeditor5-find-and-replace/tests/manual/multiroot.js b/packages/ckeditor5-find-and-replace/tests/manual/multiroot.js
@@ -138,6 +138,8 @@ class MultirootEditorUI extends EditorUI {
 	}
 
 	destroy() {
+		super.destroy();
+
 		const view = this.view;
 		const editingView = this.editor.editing.view;
 
@@ -146,8 +148,6 @@ class MultirootEditorUI extends EditorUI {
 		}
 
 		view.destroy();
-
-		super.destroy();
 	}
 
 	_initToolbar() {

diff --git a/packages/ckeditor5-ui/docs/_snippets/examples/bootstrap-ui-inner.js b/packages/ckeditor5-ui/docs/_snippets/examples/bootstrap-ui-inner.js
@@ -174,14 +174,14 @@ class BootstrapEditorUI extends EditorUI {
 	}
 
 	destroy() {
+		super.destroy();
+
 		// Restore the original editor#element.
 		this._elementReplacer.restore();
 
 		// Destroy the view.
 		this._view.editable.destroy();
 		this._view.destroy();
-
-		super.destroy();
 	}
 
 	// This method activates Bold, Italic, Underline, Undo and Redo buttons in the toolbar.

diff --git a/packages/ckeditor5-ui/docs/examples/custom-ui.md b/packages/ckeditor5-ui/docs/examples/custom-ui.md
@@ -187,14 +187,14 @@ class BootstrapEditorUI extends EditorUI {
 	}
 
 	destroy() {
+		super.destroy();
+
 		// Restore the original editor#element.
 		this._elementReplacer.restore();
 
 		// Destroy the view.
 		this._view.editable.destroy();
 		this._view.destroy();
-
-		super.destroy();
 	}
 
 	// This method activates Bold, Italic, Underline, Undo and Redo buttons in the toolbar.

diff --git a/packages/ckeditor5-ui/docs/framework/guides/external-ui.md b/packages/ckeditor5-ui/docs/framework/guides/external-ui.md
@@ -309,14 +309,14 @@ class BootstrapEditorUI extends EditorUI {
 	}
 
 	destroy() {
+		super.destroy();
+
 		// Restore the original editor#element.
 		this._elementReplacer.restore();
 
 		// Destroy the view.
 		this._view.editable.destroy();
 		this._view.destroy();
-
-		super.destroy();
 	}
 
 	// This method activates Bold, Italic, Underline, Undo and Redo buttons in the toolbar.

diff --git a/packages/ckeditor5-ui/src/tooltipmanager.ts b/packages/ckeditor5-ui/src/tooltipmanager.ts
@@ -214,9 +214,17 @@ export default class TooltipManager extends DomEmitter {
 	 * @param {module:core/editor/editor~Editor} editor The editor the manager was created for.
 	 */
 	public destroy( editor: Editor ): void {
+		const editorBodyViewCollection = editor.ui.view && editor.ui.view.body;
+
 		TooltipManager._editors.delete( editor );
 		this.stopListening( editor.ui );
 
+		// Prevent the balloon panel from being destroyed in the EditorUI#destroy() cascade. It should be destroyed along
+		// with the last editor only (https://github.com/ckeditor/ckeditor5/issues/12602).
+		if ( editorBodyViewCollection && editorBodyViewCollection.has( this.balloonPanelView ) ) {
+			editorBodyViewCollection.remove( this.balloonPanelView );
+		}
+
 		if ( !TooltipManager._editors.size ) {
 			this._unpinTooltip();
 			this.balloonPanelView.destroy();