Merge pull request #3128 from ckeditor/t/3115

Fix for: Errors thrown when Editor is detached and destroyed during initialization

CHANGES.md

@@ -36,6 +36,9 @@ Fixed Issues:
 * [#453](https://github.com/ckeditor/ckeditor-dev/issues/453): Fixed: [Link](https://ckeditor.com/cke4/addon/link) dialog has invalid width when editor is maximized and browser window resized.
 * [#2138](https://github.com/ckeditor/ckeditor-dev/issues/2138): Fixed: Email address containing question mark is mishandled by [Link](https://ckeditor.com/cke4/addon/link) plugin.
 * [#917](https://github.com/ckeditor/ckeditor-dev/issues/917): Fixed: [`CKEDITOR.plugins.addExternal`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_plugins.html#method-addExternal) method does not load a plugin if the path does not contain slash at the end.
+* [#14613](https://dev.ckeditor.com/ticket/14613): Fixed: Race condition when loading plugins for already destroyed editor instance throws an error.
+* [#2257](https://github.com/ckeditor/ckeditor-dev/issues/2257): Fixed: Editor throws an exception when destroyed shortly after it was created.
+* [#3115](https://github.com/ckeditor/ckeditor-dev/issues/3115): Fixed: Destroying editor during initialization throws an error.
 
 API Changes:
 
@@ -47,6 +50,7 @@ API Changes:
 * [#3247](https://github.com/ckeditor/ckeditor-dev/issues/3247): Extended [`CKEDITOR.tools.bind()`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_tools.html#method-bind) method to accept arguments for bound functions.
 * [#3326](https://github.com/ckeditor/ckeditor-dev/issues/3326): Added [`CKEDITOR.dom.text#isEmpty()`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_dom_text.html#method-isEmpty) method.
 * [#2423](https://github.com/ckeditor/ckeditor-dev/issues/2423): Added the [`CKEDITOR.plugins.dialog.getModel`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_dialog.html#method-getModel) and [`CKEDITOR.plugins.dialog.getMode`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_dialog.html#method-getMode) methods with their [`CKEDITOR.plugin.definition`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_dialog_definition.html) counterparts allowing to get dialog subject of change.
+* [#3124](https://github.com/ckeditor/ckeditor-dev/issues/3124): Added the [`CKEDITOR.dom.element#isDetached()`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_dom_element.html#method-isDetached) method.
 
 ## CKEditor 4.12.1
 

core/creators/inline.js

@@ -93,11 +93,16 @@
 
 		// Handle editor destroying.
 		editor.on( 'destroy', function() {
+			var container = editor.container;
 			// Remove container from DOM if inline-textarea editor.
 			// Show <textarea> back again.
+			// Editor can be destroyed before container is created (#3115).
+			if ( textarea && container ) {
+				container.clearCustomData();
+				container.remove();
+			}
+
 			if ( textarea ) {
-				editor.container.clearCustomData();
-				editor.container.remove();
 				textarea.show();
 			}
 

core/creators/themedui.js

@@ -239,6 +239,9 @@ CKEDITOR.replaceClass = 'ckeditor';
 
 			// Delay to avoid race conditions (setMode inside setMode).
 			setTimeout( function() {
+				if ( editor.isDestroyed() || editor.isDetached() ) {
+					return;
+				}
 				editor.fire( 'mode' );
 				callback && callback.call( editor );
 			}, 0 );
@@ -343,6 +346,10 @@ CKEDITOR.replaceClass = 'ckeditor';
 
 		// Once the editor is loaded, start the UI.
 		editor.on( 'loaded', function() {
+			if ( editor.isDestroyed() || editor.isDetached() ) {
+				return;
+			}
+
 			loadTheme( editor );
 
 			if ( mode == CKEDITOR.ELEMENT_MODE_REPLACE && editor.config.autoUpdateElement && element.$.form )

core/dom/domobject.js

@@ -126,15 +126,24 @@ CKEDITOR.dom.domObject.prototype = ( function() {
 		 * references left after the object is no longer needed.
 		 */
 		removeAllListeners: function() {
-			var nativeListeners = this.getCustomData( '_cke_nativeListeners' );
-			for ( var eventName in nativeListeners ) {
-				var listener = nativeListeners[ eventName ];
-				if ( this.$.detachEvent )
-					this.$.detachEvent( 'on' + eventName, listener );
-				else if ( this.$.removeEventListener )
-					this.$.removeEventListener( eventName, listener, false );
-
-				delete nativeListeners[ eventName ];
+			try {
+				var nativeListeners = this.getCustomData( '_cke_nativeListeners' );
+				for ( var eventName in nativeListeners ) {
+					var listener = nativeListeners[ eventName ];
+					if ( this.$.detachEvent ) {
+						this.$.detachEvent( 'on' + eventName, listener );
+					} else if ( this.$.removeEventListener ) {
+						this.$.removeEventListener( eventName, listener, false );
+					}
+
+					delete nativeListeners[ eventName ];
+				}
+			// Catch Edge `Permission denied` error which occurs randomly. Since the error is quite
+			// random, catching allows to continue the code execution and cleanup (#3419).
+			} catch ( error ) {
+				if ( !CKEDITOR.env.edge || error.number !== -2146828218 ) {
+					throw( error );
+				}
 			}
 
 			// Remove events from events object so fire() method will not call
@@ -244,7 +253,7 @@ CKEDITOR.dom.domObject.prototype = ( function() {
 		// Clear all event listeners
 		this.removeAllListeners();
 
-		var expandoNumber = this.$[ 'data-cke-expando' ];
+		var expandoNumber = this.getUniqueId();
 		expandoNumber && delete customData[ expandoNumber ];
 	};
 

core/dom/element.js

@@ -2161,6 +2161,29 @@ CKEDITOR.dom.element.clearMarkers = function( database, element, removeFromDatab
 			} else {
 				nativeElement[ nativeElement[ eventName ] ? eventName : handlerName ]( evt );
 			}
+		},
+
+		/**
+		 * Checks if element is detached from DOM.
+		 *
+		 * @since 4.13.0
+		 * @returns {Boolean} Whether element is detached from DOM.
+		 */
+		isDetached: function() {
+			var doc = this.getDocument(),
+				docEl = doc.getDocumentElement();
+
+			// It is not in the document.
+			if ( !docEl.equals( this ) && !docEl.contains( this ) ) {
+				return true;
+			}
+
+			// Check if the `window` exists in this document. The `window` is null for detached documents.
+			if ( !CKEDITOR.env.ie || CKEDITOR.env.version > 8 ) {
+				return !doc.$.defaultView;
+			}
+
+			return false;
 		}
 	} );
 

core/editable.js

@@ -534,19 +534,29 @@
 			 * Detaches this editable object from the DOM (removes classes, listeners, etc.)
 			 */
 			detach: function() {
-				// Cleanup the element.
-				this.removeClass( 'cke_editable' );
-
 				this.status = 'detached';
 
-				// Save the editor reference which will be lost after
-				// calling detach from super class.
-				var editor = this.editor;
+				// Update the editor cached data with current data.
+				this.editor.setData( this.editor.getData(), {
+					internal: true
+				} );
 
-				this._.detach();
+				this.clearListeners();
 
-				delete editor.document;
-				delete editor.window;
+				// Edge randomly throws permission denied when trying to access native elements of detached editor (#3115, #3419).
+				try {
+					this._.cleanCustomData();
+				} catch ( error ) {
+					if ( !CKEDITOR.env.ie || error.number !== -2146828218 ) {
+						throw( error );
+					}
+				}
+
+				this.editor.fire( 'contentDomUnload' );
+
+				delete this.editor.document;
+				delete this.editor.window;
+				delete this.editor;
 			},
 
 			/**
@@ -1222,44 +1232,59 @@
 						return false;
 					}, this, null, 100 ); // Later is better – do not override existing listeners.
 				}
+			},
+
+			/**
+			 * @inheritdoc CKEDITOR.dom.domObject#getUniqueId
+			 */
+			getUniqueId: function() {
+				var expandoNumber;
+				// Editable is cached unlike other elements, so we can use it to store expando number.
+				// We need it to properly cleanup custom data in case of permission denied
+				// thrown by Edge when accessing native element of detached editable (#3115).
+				try {
+					this._.expandoNumber = expandoNumber = CKEDITOR.dom.domObject.prototype.getUniqueId.call( this );
+				} catch ( e ) {
+					expandoNumber = this._ && this._.expandoNumber;
+				}
+
+				return expandoNumber;
 			}
 		},
 
 		_: {
-			detach: function() {
+			cleanCustomData: function() {
 				// Update the editor cached data with current data.
-				this.editor.setData( this.editor.getData(), 0, 1 );
-
-				this.clearListeners();
+				this.removeClass( 'cke_editable' );
 				this.restoreAttrs();
 
 				// Cleanup our custom classes.
-				var classes;
-				if ( ( classes = this.removeCustomData( 'classes' ) ) ) {
-					while ( classes.length )
-						this.removeClass( classes.pop() );
+				var classes = this.removeCustomData( 'classes' );
+
+				while ( classes && classes.length ) {
+					this.removeClass( classes.pop() );
 				}
 
-				// Remove contents stylesheet from document if it's the last usage.
-				if ( !this.is( 'textarea' ) ) {
-					var doc = this.getDocument(),
-						head = doc.getHead();
-					if ( head.getCustomData( 'stylesheet' ) ) {
-						var refs = doc.getCustomData( 'stylesheet_ref' );
-						if ( !( --refs ) ) {
-							doc.removeCustomData( 'stylesheet_ref' );
-							var sheet = head.removeCustomData( 'stylesheet' );
-							sheet.remove();
-						} else {
-							doc.setCustomData( 'stylesheet_ref', refs );
-						}
-					}
+				if ( this.is( 'textarea' ) ) {
+					return;
 				}
 
-				this.editor.fire( 'contentDomUnload' );
+				var doc = this.getDocument(),
+					head = doc.getHead();
 
-				// Free up the editor reference.
-				delete this.editor;
+				if ( !head.getCustomData( 'stylesheet' ) ) {
+					return;
+				}
+
+				var refs = doc.getCustomData( 'stylesheet_ref' );
+
+				// Remove contents stylesheet from document if it's the last usage.
+				if ( !--refs ) {
+					doc.removeCustomData( 'stylesheet_ref' );
+					head.removeCustomData( 'stylesheet' ).remove();
+				} else {
+					doc.setCustomData( 'stylesheet_ref', refs );
+				}
 			}
 		}
 	} );
@@ -1282,14 +1307,18 @@
 		if ( editable && element )
 			return 0;
 
-		if ( arguments.length ) {
-			editable = this._.editable = element ? ( element instanceof CKEDITOR.editable ? element : new CKEDITOR.editable( this, element ) ) :
-			// Detach the editable from editor.
-			( editable && editable.detach(), null );
+		if ( !arguments.length ) {
+			return editable;
+		}
+
+		if ( element ) {
+			editable = element instanceof CKEDITOR.editable ? element : new CKEDITOR.editable( this, element );
+		} else {
+			editable && editable.detach();
+			editable = null;
 		}
 
-		// Just retrieve the editable.
-		return editable;
+		return this._.editable = editable;
 	};
 
 	CKEDITOR.on( 'instanceLoaded', function( evt ) {

core/editor.js

@@ -192,10 +192,8 @@
 
 		// Return the editor instance immediately to enable early stage event registrations.
 		CKEDITOR.tools.setTimeout( function() {
-			if ( this.status !== 'destroyed' ) {
+			if ( !this.isDestroyed() && !this.isDetached() ) {
 				initConfig( this, instanceConfig );
-			} else {
-				CKEDITOR.warn( 'editor-incorrect-destroy' );
 			}
 		}, 0, this );
 	}
@@ -593,6 +591,10 @@
 
 			// Load all plugin specific language files in a row.
 			CKEDITOR.scriptLoader.load( languageFiles, function() {
+				if ( editor.isDestroyed() || editor.isDetached() ) {
+					return;
+				}
+
 				// Initialize all plugins that have the "beforeInit" and "init" methods defined.
 				var methods = [ 'beforeInit', 'init', 'afterInit' ];
 				for ( var m = 0; m < methods.length; m++ ) {
@@ -604,8 +606,9 @@
 							editor.lang[ plugin.name ] = plugin.langEntries[ languageCodes[ i ] ];
 
 						// Call the plugin method (beforeInit and init).
-						if ( plugin[ methods[ m ] ] )
+						if ( plugin[ methods[ m ] ] ) {
 							plugin[ methods[ m ] ]( editor );
+						}
 					}
 				}
 
@@ -1579,6 +1582,27 @@
 		 */
 		showNotification: function( message ) {
 			alert( message ); // jshint ignore:line
+		},
+
+		/**
+		 * Provides information if editor's {@link CKEDITOR.editor#container container}
+		 * {@link CKEDITOR.dom.element#isDetached is detached}.
+		 *
+		 * @since 4.13.0
+		 * @returns {Boolean} true if editor's container is detached
+		 */
+		isDetached: function() {
+			return !!this.container && this.container.isDetached();
+		},
+
+		/**
+		 * Determins if current editor instance is destroyed.
+		 *
+		 * @since 4.13.0
+		 * @returns {Boolean} true if editor is destroyed
+		 */
+		isDestroyed: function() {
+			return this.status === 'destroyed';
 		}
 	} );
 

plugins/clipboard/plugin.js

@@ -1203,8 +1203,9 @@
 		}
 
 		function setToolbarStates() {
-			if ( editor.mode != 'wysiwyg' )
+			if ( editor.mode != 'wysiwyg' ) {
 				return;
+			}
 
 			var pasteState = stateFromNamedCommand( 'paste' );