Kaynağa Gözat

Merge branch 'master' into t/ckeditor5/1151

Aleksander Nowodzinski 6 yıl önce
ebeveyn
işleme
f591d98f80

+ 6 - 3
packages/ckeditor5-core/src/editor/editorui.js

@@ -7,12 +7,13 @@
  * @module core/editor/editorui
  */
 
+/* globals console */
+
 import ComponentFactory from '@ckeditor/ckeditor5-ui/src/componentfactory';
 import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker';
 
 import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
-import log from '@ckeditor/ckeditor5-utils/src/log';
 
 /**
  * A class providing the minimal interface that is required to successfully bootstrap any editor UI.
@@ -123,7 +124,9 @@ export default class EditorUI {
 		// It helps 3rd–party software (browser extensions, other libraries) access and recognize
 		// CKEditor 5 instances (editing roots) and use their API (there is no global editor
 		// instance registry).
-		domElement.ckeditorInstance = this.editor;
+		if ( !domElement.ckeditorInstance ) {
+			domElement.ckeditorInstance = this.editor;
+		}
 	}
 
 	/**
@@ -161,7 +164,7 @@ export default class EditorUI {
 		 * @error editor-ui-deprecated-editable-elements
 		 * @param {module:core/editor/editorui~EditorUI} editorUI Editor UI instance the deprecated property belongs to.
 		 */
-		log.warn(
+		console.warn(
 			'editor-ui-deprecated-editable-elements: ' +
 			'The EditorUI#_editableElements property has been deprecated and will be removed in the near future.',
 			{ editorUI: this } );

+ 50 - 0
packages/ckeditor5-core/src/editor/utils/securesourceelement.js

@@ -0,0 +1,50 @@
+/**
+ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
+
+/**
+ * @module core/editor/utils/securesourceelement
+ */
+
+/**
+ * Marks the source element on which the editor was initialized. This prevents other editor instances from using this element.
+ *
+ * Running multiple editor instances on the same source element causes various issues and it is
+ * crucial this helper is called as soon as the source element is known to prevent collisions.
+ *
+ * @param {module:core/editor/editor~Editor} editor Editor instance.
+ */
+export default function secureSourceElement( editor ) {
+	const sourceElement = editor.sourceElement;
+
+	// If the editor was initialized without specifying an element, we don't need to secure anything.
+	if ( !sourceElement ) {
+		return;
+	}
+
+	if ( sourceElement.ckeditorInstance ) {
+		/**
+		 * A DOM element used to create the editor (e.g.
+		 * {@link module:editor-inline/inlineeditor~InlineEditor.create `InlineEditor.create()`})
+		 * has already been used to create another editor instance. Make sure each editor is
+		 * created with an unique DOM element.
+		 *
+		 * @error editor-source-element-already-used
+		 * @param {HTMLElement} element DOM element that caused the collision.
+		 */
+		throw new CKEditorError(
+			'editor-source-element-already-used: ' +
+			'The DOM element cannot be used to create multiple editor instances.',
+			editor
+		);
+	}
+
+	sourceElement.ckeditorInstance = editor;
+
+	editor.once( 'destroy', () => {
+		delete sourceElement.ckeditorInstance;
+	} );
+}

+ 11 - 7
packages/ckeditor5-core/src/plugincollection.js

@@ -7,8 +7,9 @@
  * @module core/plugincollection
  */
 
-import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
-import log from '@ckeditor/ckeditor5-utils/src/log';
+/* globals console */
+
+import CKEditorError, { attachLinkToDocumentation } from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
@@ -189,7 +190,7 @@ export default class PluginCollection {
 			const errorMsg = 'plugincollection-plugin-not-found: Some plugins are not available and could not be loaded.';
 
 			// Log the error so it's more visible on the console. Hopefully, for better DX.
-			log.error( errorMsg, { plugins: missingPlugins } );
+			console.error( attachLinkToDocumentation( errorMsg ), { plugins: missingPlugins } );
 
 			return Promise.reject( new CKEditorError( errorMsg, this._editor, { plugins: missingPlugins } ) );
 		}
@@ -231,7 +232,9 @@ export default class PluginCollection {
 					 * @error plugincollection-load
 					 * @param {String} plugin The name of the plugin that could not be loaded.
 					 */
-					log.error( 'plugincollection-load: It was not possible to load the plugin.', { plugin: PluginConstructor } );
+					console.error( attachLinkToDocumentation(
+						'plugincollection-load: It was not possible to load the plugin.'
+					), { plugin: PluginConstructor } );
 
 					throw err;
 				} );
@@ -371,13 +374,14 @@ export default class PluginCollection {
 			 * @param {Function} plugin1 The first plugin constructor.
 			 * @param {Function} plugin2 The second plugin constructor.
 			 */
-			log.warn(
+			throw new CKEditorError(
 				'plugincollection-plugin-name-conflict: Two plugins with the same name were loaded.',
+				null,
 				{ pluginName, plugin1: this._plugins.get( pluginName ).constructor, plugin2: PluginConstructor }
 			);
-		} else {
-			this._plugins.set( pluginName, plugin );
 		}
+
+		this._plugins.set( pluginName, plugin );
 	}
 }
 

+ 13 - 3
packages/ckeditor5-core/tests/editor/editorui.js

@@ -10,9 +10,8 @@ import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker';
 import ComponentFactory from '@ckeditor/ckeditor5-ui/src/componentfactory';
 
 import testUtils from '../_utils/utils';
-import log from '@ckeditor/ckeditor5-utils/src/log';
 
-/* global document */
+/* global document, console */
 
 describe( 'EditorUI', () => {
 	let editor, ui;
@@ -131,6 +130,17 @@ describe( 'EditorUI', () => {
 
 			expect( element.ckeditorInstance ).to.equal( editor );
 		} );
+
+		it( 'does not override a reference to the editor instance in domElement#ckeditorInstance', () => {
+			const ui = new EditorUI( editor );
+			const element = document.createElement( 'div' );
+
+			element.ckeditorInstance = 'foo';
+
+			ui.setEditableElement( 'main', element );
+
+			expect( element.ckeditorInstance ).to.equal( 'foo' );
+		} );
 	} );
 
 	describe( 'getEditableElement()', () => {
@@ -186,7 +196,7 @@ describe( 'EditorUI', () => {
 	describe( '_editableElements()', () => {
 		it( 'should warn about deprecation', () => {
 			const ui = new EditorUI( editor );
-			const stub = testUtils.sinon.stub( log, 'warn' );
+			const stub = testUtils.sinon.stub( console, 'warn' );
 
 			expect( ui._editableElements ).to.be.instanceOf( Map );
 			sinon.assert.calledWithMatch( stub, 'editor-ui-deprecated-editable-elements' );

+ 67 - 0
packages/ckeditor5-core/tests/editor/utils/securesourceelement.js

@@ -0,0 +1,67 @@
+/**
+ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+/* global document */
+
+import secureSourceElement from '../../../src/editor/utils/securesourceelement';
+import Editor from '../../../src/editor/editor';
+import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
+
+describe( 'secureSourceElement()', () => {
+	let editor, sourceElement;
+
+	beforeEach( () => {
+		class CustomEditor extends Editor {}
+
+		sourceElement = document.createElement( 'div' );
+		editor = new CustomEditor();
+
+		editor.sourceElement = sourceElement;
+		editor.state = 'ready';
+	} );
+
+	afterEach( () => {
+		if ( editor ) {
+			return editor.destroy();
+		}
+	} );
+
+	it( 'does not throw if the editor was not initialized using the source element', () => {
+		delete editor.sourceElement;
+
+		expect( () => {
+			secureSourceElement( editor );
+		} ).to.not.throw();
+	} );
+
+	it( 'does not throw if the editor was initialized using the element for the first time', () => {
+		expect( () => {
+			secureSourceElement( editor );
+		} ).to.not.throw();
+	} );
+
+	it( 'sets the property after initializing the editor', () => {
+		secureSourceElement( editor );
+
+		expect( sourceElement.ckeditorInstance ).to.equal( editor );
+	} );
+
+	it( 'removes the property after destroying the editor', () => {
+		secureSourceElement( editor );
+
+		return editor.destroy()
+			.then( () => {
+				expect( sourceElement.ckeditorInstance ).to.be.undefined;
+			} );
+	} );
+
+	it( 'throws an error if the same element was used twice', () => {
+		sourceElement.ckeditorInstance = 'foo';
+
+		expectToThrowCKEditorError( () => {
+			secureSourceElement( editor );
+		}, /^editor-source-element-already-used/, editor );
+	} );
+} );

+ 46 - 57
packages/ckeditor5-core/tests/plugincollection.js

@@ -3,13 +3,11 @@
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
 
-/* globals setTimeout */
+/* globals setTimeout, console */
 
-import testUtils from '../tests/_utils/utils';
 import Editor from '../src/editor/editor';
 import PluginCollection from '../src/plugincollection';
 import Plugin from '../src/plugin';
-import log from '@ckeditor/ckeditor5-utils/src/log';
 import { expectToThrowCKEditorError, assertCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
 
 let editor, availablePlugins;
@@ -19,8 +17,6 @@ class ChildPlugin extends Plugin {}
 class GrandPlugin extends ChildPlugin {}
 
 describe( 'PluginCollection', () => {
-	testUtils.createSinonSandbox();
-
 	before( () => {
 		PluginA = createPlugin( 'A' );
 		PluginB = createPlugin( 'B' );
@@ -73,6 +69,10 @@ describe( 'PluginCollection', () => {
 		PluginFoo.requires = [];
 	} );
 
+	afterEach( () => {
+		sinon.restore();
+	} );
+
 	describe( 'load()', () => {
 		it( 'should not fail when trying to load 0 plugins (empty array)', () => {
 			const plugins = new PluginCollection( editor, availablePlugins );
@@ -194,7 +194,7 @@ describe( 'PluginCollection', () => {
 		} );
 
 		it( 'should load plugin which does not extend the base Plugin class', () => {
-			class Y { }
+			class Y {}
 
 			const plugins = new PluginCollection( editor, availablePlugins );
 
@@ -240,7 +240,7 @@ describe( 'PluginCollection', () => {
 		} );
 
 		it( 'should reject on broken plugins (forward the error thrown in a plugin)', () => {
-			const logSpy = testUtils.sinon.stub( log, 'error' );
+			const consoleErrorStub = sinon.stub( console, 'error' );
 
 			const plugins = new PluginCollection( editor, availablePlugins );
 
@@ -253,13 +253,13 @@ describe( 'PluginCollection', () => {
 					expect( err ).to.be.an.instanceof( TestError );
 					expect( err ).to.have.property( 'message', 'Some error inside a plugin' );
 
-					sinon.assert.calledOnce( logSpy );
-					expect( logSpy.args[ 0 ][ 0 ] ).to.match( /^plugincollection-load:/ );
+					sinon.assert.calledOnce( consoleErrorStub );
+					expect( consoleErrorStub.args[ 0 ][ 0 ] ).to.match( /^plugincollection-load:/ );
 				} );
 		} );
 
 		it( 'should reject when loading non-existent plugin', () => {
-			const logSpy = testUtils.sinon.stub( log, 'error' );
+			const consoleErrorStub = sinon.stub( console, 'error' );
 
 			const plugins = new PluginCollection( editor, availablePlugins );
 
@@ -271,8 +271,8 @@ describe( 'PluginCollection', () => {
 				.catch( err => {
 					assertCKEditorError( err, /^plugincollection-plugin-not-found/, editor );
 
-					sinon.assert.calledOnce( logSpy );
-					expect( logSpy.args[ 0 ][ 0 ] ).to.match( /^plugincollection-plugin-not-found:/ );
+					sinon.assert.calledOnce( consoleErrorStub );
+					expect( consoleErrorStub.args[ 0 ][ 0 ] ).to.match( /^plugincollection-plugin-not-found:/ );
 				} );
 		} );
 
@@ -339,7 +339,7 @@ describe( 'PluginCollection', () => {
 		} );
 
 		it( 'should reject when loaded plugin requires not allowed plugins', () => {
-			const logSpy = testUtils.sinon.stub( log, 'error' );
+			const consoleErrorStub = sinon.stub( console, 'error' );
 			const plugins = new PluginCollection( editor, availablePlugins );
 
 			return plugins.init( [ PluginA, PluginB, PluginC, PluginD ], [ PluginA, PluginB ] )
@@ -350,72 +350,61 @@ describe( 'PluginCollection', () => {
 				.catch( err => {
 					assertCKEditorError( err, /^plugincollection-required/, editor );
 
-					expect( logSpy.calledTwice ).to.equal( true );
+					sinon.assert.calledTwice( consoleErrorStub );
 				} );
 		} );
 
-		it( 'logs if tries to load more than one plugin with the same name', () => {
-			const logSpy = testUtils.sinon.stub( log, 'warn' );
+		it( 'should reject when loading more than one plugin with the same name', () => {
 			const plugins = new PluginCollection( editor );
+			const consoleErrorStub = sinon.stub( console, 'error' );
 
 			return plugins.init( [ PluginFoo, AnotherPluginFoo ] )
 				.then( () => {
-					expect( getPlugins( plugins ).length ).to.equal( 2 );
-
-					expect( plugins.get( 'Foo' ) ).to.be.an.instanceof( PluginFoo );
-					expect( plugins.get( PluginFoo ) ).to.be.an.instanceof( PluginFoo );
-					expect( plugins.get( AnotherPluginFoo ) ).to.be.an.instanceof( AnotherPluginFoo );
-
-					expect( logSpy.calledOnce ).to.equal( true );
-					expect( logSpy.firstCall.args[ 0 ] ).to.match( /^plugincollection-plugin-name-conflict:/ );
-
-					const warnData = logSpy.firstCall.args[ 1 ];
-					expect( warnData.pluginName ).to.equal( 'Foo' );
-					expect( warnData.plugin1 ).to.equal( PluginFoo );
-					expect( warnData.plugin2 ).to.equal( AnotherPluginFoo );
+					throw new Error( 'The `init()` method should fail.' );
+				} )
+				.catch( err => {
+					assertCKEditorError( err, /^plugincollection-plugin-name-conflict:/, null, {
+						pluginName: 'Foo',
+						plugin1: PluginFoo,
+						plugin2: AnotherPluginFoo
+					} );
+					sinon.assert.calledOnce( consoleErrorStub );
 				} );
 		} );
 
-		it( 'logs if tries to load more than one plugin with the same name (plugin requires plugin with the same name)', () => {
+		it( 'should reject when loading more than one plugin with the same name (plugin requires plugin with the same name)', () => {
 			PluginFoo.requires = [ AnotherPluginFoo ];
 
-			const logSpy = testUtils.sinon.stub( log, 'warn' );
 			const plugins = new PluginCollection( editor );
+			const consoleErrorStub = sinon.stub( console, 'error' );
 
 			return plugins.init( [ PluginFoo ] )
 				.then( () => {
-					expect( getPlugins( plugins ).length ).to.equal( 2 );
-
-					expect( plugins.get( 'Foo' ) ).to.be.an.instanceof( AnotherPluginFoo );
-					expect( plugins.get( AnotherPluginFoo ) ).to.be.an.instanceof( AnotherPluginFoo );
-					expect( plugins.get( PluginFoo ) ).to.be.an.instanceof( PluginFoo );
+					throw new Error( 'The `init()` method should fail.' );
+				} )
+				.catch( err => {
+					assertCKEditorError( err, /^plugincollection-plugin-name-conflict:/, null );
 
-					expect( logSpy.calledOnce ).to.equal( true );
-					expect( logSpy.firstCall.args[ 0 ] ).to.match( /^plugincollection-plugin-name-conflict:/ );
+					sinon.assert.calledOnce( consoleErrorStub );
 				} );
 		} );
 
-		it(
-			'logs if tries to load more than one plugin with the same name (plugin with the same name is built-in the PluginCollection)',
-			() => {
-				availablePlugins = [ PluginFoo ];
-
-				const logSpy = testUtils.sinon.stub( log, 'warn' );
-				const plugins = new PluginCollection( editor, availablePlugins );
+		it( 'should reject when loading more than one plugin with the same name' +
+			'(plugin with the same name is built-in the PluginCollection)', () => {
+			availablePlugins = [ PluginFoo ];
 
-				return plugins.init( [ 'Foo', AnotherPluginFoo ] )
-					.then( () => {
-						expect( getPlugins( plugins ).length ).to.equal( 2 );
-
-						expect( plugins.get( 'Foo' ) ).to.be.an.instanceof( PluginFoo );
-						expect( plugins.get( PluginFoo ) ).to.be.an.instanceof( PluginFoo );
-						expect( plugins.get( AnotherPluginFoo ) ).to.be.an.instanceof( AnotherPluginFoo );
+			const plugins = new PluginCollection( editor, availablePlugins );
+			const consoleErrorStub = sinon.stub( console, 'error' );
 
-						expect( logSpy.calledOnce ).to.equal( true );
-						expect( logSpy.firstCall.args[ 0 ] ).to.match( /^plugincollection-plugin-name-conflict:/ );
-					} );
-			}
-		);
+			return plugins.init( [ 'Foo', AnotherPluginFoo ] )
+				.then( () => {
+					throw new Error( 'The `init()` method should fail.' );
+				} )
+				.catch( err => {
+					assertCKEditorError( err, /^plugincollection-plugin-name-conflict:/, null );
+					sinon.assert.calledOnce( consoleErrorStub );
+				} );
+		} );
 	} );
 
 	describe( 'get()', () => {