From ea85f3bceb2b454792cf8ff57246304308eccc1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 30 Oct 2019 00:28:07 +0100 Subject: [PATCH 01/37] Initial Scope implementation. --- src/editor/editor.js | 63 +++++++++++++--------- src/plugincollection.js | 49 ++++++++++++----- src/scope.js | 103 +++++++++++++++++++++++++++++++++++ tests/scope.js | 115 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 293 insertions(+), 37 deletions(-) create mode 100644 src/scope.js create mode 100644 tests/scope.js diff --git a/src/editor/editor.js b/src/editor/editor.js index 52513c68..c5aaaf54 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -7,11 +7,11 @@ * @module core/editor/editor */ +import Scope from '@ckeditor/ckeditor5-core/src/scope'; import Config from '@ckeditor/ckeditor5-utils/src/config'; import EditingController from '@ckeditor/ckeditor5-engine/src/controller/editingcontroller'; import PluginCollection from '../plugincollection'; import CommandCollection from '../commandcollection'; -import Locale from '@ckeditor/ckeditor5-utils/src/locale'; import DataController from '@ckeditor/ckeditor5-engine/src/controller/datacontroller'; import Conversion from '@ckeditor/ckeditor5-engine/src/conversion/conversion'; import Model from '@ckeditor/ckeditor5-engine/src/model/model'; @@ -48,9 +48,16 @@ export default class Editor { * * Usually, not to be used directly. See the static {@link module:core/editor/editor~Editor.create `create()`} method. * - * @param {Object} [config] The editor config. + * @param {Object} [config={}] The editor config. */ - constructor( config ) { + constructor( config = {} ) { + /** + * @readonly + * @type {module:core/scope~Scope} + */ + this.scope = config.scope || new Scope( config ); + this.scope.addEditor( this ); + const availablePlugins = this.constructor.builtinPlugins; /** @@ -63,7 +70,6 @@ export default class Editor { * @member {module:utils/config~Config} */ this.config = new Config( config, this.constructor.defaultConfig ); - this.config.define( 'plugins', availablePlugins ); /** @@ -74,7 +80,7 @@ export default class Editor { * @readonly * @member {module:core/plugincollection~PluginCollection} */ - this.plugins = new PluginCollection( this, availablePlugins ); + this.plugins = new PluginCollection( this, availablePlugins, this.scope.plugins ); /** * Commands registered to the editor. @@ -92,25 +98,6 @@ export default class Editor { */ this.commands = new CommandCollection(); - const languageConfig = this.config.get( 'language' ) || {}; - - /** - * @readonly - * @member {module:utils/locale~Locale} - */ - this.locale = new Locale( { - uiLanguage: typeof languageConfig === 'string' ? languageConfig : languageConfig.ui, - contentLanguage: this.config.get( 'language.content' ) - } ); - - /** - * Shorthand for {@link module:utils/locale~Locale#t}. - * - * @see module:utils/locale~Locale#t - * @method #t - */ - this.t = this.locale.t; - /** * Indicates the editor life-cycle state. * @@ -214,6 +201,24 @@ export default class Editor { this.keystrokes.listenTo( this.editing.view.document ); } + /** + * @readonly + * @type {module:utils/locale~Locale} + */ + get locale() { + return this.scope.locale; + } + + /** + * Shorthand for {@link module:utils/locale~Locale#t}. + * + * @see module:utils/locale~Locale#t + * @method #t + */ + get t() { + return this.locale.t; + } + /** * Loads and initializes plugins specified in the config. * @@ -239,6 +244,16 @@ export default class Editor { * @returns {Promise} A promise that resolves once the editor instance is fully destroyed. */ destroy() { + // If editor scope is created by this editor + // it is enough to destroy the scope, editor will be destroyed along with it. + if ( this.scope ) { + if ( this.config.scope ) { + return this.scope.destroy(); + } + + this.scope.removeEditor( this ); + } + let readyPromise = Promise.resolve(); if ( this.state == 'initializing' ) { diff --git a/src/plugincollection.js b/src/plugincollection.js index 488a573a..f9183c04 100644 --- a/src/plugincollection.js +++ b/src/plugincollection.js @@ -23,33 +23,36 @@ export default class PluginCollection { /** * Creates an instance of the PluginCollection class. * Allows loading and initializing plugins and their dependencies. + * Allows to provide a list of already loaded plugins, these plugins won't be destroyed along with this collection. * * @param {module:core/editor/editor~Editor} editor * @param {Array.} [availablePlugins] Plugins (constructors) which the collection will be able to use * when {@link module:core/plugincollection~PluginCollection#init} is used with plugin names (strings, instead of constructors). * Usually, the editor will pass its built-in plugins to the collection so they can later be * used in `config.plugins` or `config.removePlugins` by names. + * @param {Iterable.} externalPlugins List of already initialized plugins represented by a + * `[ PluginConstructor, pluginInstance ]` pair. */ - constructor( editor, availablePlugins = [] ) { + constructor( editor, availablePlugins = [], externalPlugins = [] ) { /** * @protected - * @member {module:core/editor/editor~Editor} module:core/plugin~PluginCollection#_editor + * @type {module:core/editor/editor~Editor} */ this._editor = editor; /** - * Map of plugin constructors which can be retrieved by their names. - * * @protected - * @member {Map.} module:core/plugin~PluginCollection#_availablePlugins + * @type {Map} */ - this._availablePlugins = new Map(); + this._plugins = new Map(); /** + * Map of plugin constructors which can be retrieved by their names. + * * @protected - * @member {Map} module:core/plugin~PluginCollection#_plugins + * @type {Map.} */ - this._plugins = new Map(); + this._availablePlugins = new Map(); for ( const PluginConstructor of availablePlugins ) { this._availablePlugins.set( PluginConstructor, PluginConstructor ); @@ -58,6 +61,19 @@ export default class PluginCollection { this._availablePlugins.set( PluginConstructor.pluginName, PluginConstructor ); } } + + /** + * List of constructors of externally loaded plugins. + * + * @private + * @type {Map} + */ + this._externalPlugins = new Map(); + + for ( const [ PluginConstructor, pluginInstance ] of externalPlugins ) { + this._externalPlugins.set( PluginConstructor, pluginInstance ); + this._externalPlugins.set( pluginInstance, PluginConstructor ); + } } /** @@ -246,6 +262,10 @@ export default class PluginCollection { return promise; } + if ( that._externalPlugins.has( plugin ) ) { + return promise; + } + return promise.then( plugin[ method ].bind( plugin ) ); }, Promise.resolve() ); } @@ -278,7 +298,7 @@ export default class PluginCollection { } ); } - const plugin = new PluginConstructor( editor ); + const plugin = that._externalPlugins.get( PluginConstructor ) || new PluginConstructor( editor ); that._add( PluginConstructor, plugin ); loaded.push( plugin ); @@ -319,10 +339,13 @@ export default class PluginCollection { * @returns {Promise} */ destroy() { - const promises = Array.from( this ) - .map( ( [ , pluginInstance ] ) => pluginInstance ) - .filter( pluginInstance => typeof pluginInstance.destroy == 'function' ) - .map( pluginInstance => pluginInstance.destroy() ); + const promises = []; + + for ( const [ , pluginInstance ] of this ) { + if ( typeof pluginInstance.destroy == 'function' && !this._externalPlugins.has( pluginInstance ) ) { + promises.push( pluginInstance.destroy() ); + } + } return Promise.all( promises ); } diff --git a/src/scope.js b/src/scope.js new file mode 100644 index 00000000..fe0184eb --- /dev/null +++ b/src/scope.js @@ -0,0 +1,103 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module core/scope + */ + +import Config from '@ckeditor/ckeditor5-utils/src/config'; +import PluginCollection from './plugincollection'; +import Locale from '@ckeditor/ckeditor5-utils/src/locale'; +import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; + +export default class Scope { + constructor( config ) { + /** + * Holds all configurations specific to this scope instance. + * + * @readonly + * @type {module:utils/config~Config} + */ + this.config = new Config( config ); + + /** + * The plugins loaded and in use by this scope instance. + * + * @readonly + * @type {module:core/plugincollection~PluginCollection} + */ + this.plugins = new PluginCollection( this, this.config.get( 'plugins' ) ); + + const languageConfig = this.config.get( 'language' ) || {}; + + /** + * @readonly + * @member {module:utils/locale~Locale} + */ + this.locale = new Locale( { + uiLanguage: typeof languageConfig === 'string' ? languageConfig : languageConfig.ui, + contentLanguage: this.config.get( 'language.content' ) + } ); + + /** + * List of editors used with this scope. + * + * @private + * @type {Set} + */ + this._editors = new Set(); + } + + /** + * @param {module:core/editor/editor~Editor} editor + */ + addEditor( editor ) { + this._editors.add( editor ); + } + + /** + * @param {module:core/editor/editor~Editor} editor + */ + removeEditor( editor ) { + return this._editors.delete( editor ); + } + + initPlugins() { + const plugins = this.config.get( 'plugins' ); + + for ( const plugin of plugins ) { + if ( typeof plugin != 'function' ) { + throw new CKEditorError( 'scope-initplugins: Only constructor is allowed as a Scope plugin' ); + } + } + + return this.plugins.init( plugins ); + } + + /** + * Destroys the scope instance, releasing all resources used by it. + * + * @returns {Promise} A promise that resolves once the scope instance is fully destroyed. + */ + destroy() { + const promises = []; + + for ( const editor of Array.from( this._editors ) ) { + editor.scope = null; + promises.push( editor.destroy() ); + } + + return Promise.all( promises ) + .then( () => this.plugins.destroy() ); + } + + static create( config ) { + return new Promise( resolve => { + const scope = new this( config ); + + resolve( scope.initPlugins().then( () => scope ) ); + } ); + } +} diff --git a/tests/scope.js b/tests/scope.js new file mode 100644 index 00000000..331c2d0e --- /dev/null +++ b/tests/scope.js @@ -0,0 +1,115 @@ +/** + * @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 Scope from '../src/scope'; +import Plugin from '../src/plugin'; +import VirtualTestEditor from './_utils/virtualtesteditor'; + +describe( 'Scope', () => { + it( 'should share the same instance of plugin within editors using scope', async () => { + class ScopePluginA extends Plugin {} + class ScopePluginB extends Plugin {} + class EditorPluginA extends Plugin {} + + const scope = await Scope.create( { plugins: [ ScopePluginA, ScopePluginB ] } ); + const editorA = await VirtualTestEditor.create( { scope, plugins: [ ScopePluginA, EditorPluginA ] } ); + const editorB = await VirtualTestEditor.create( { scope, plugins: [ ScopePluginB, EditorPluginA ] } ); + + expect( editorA.plugins.get( ScopePluginA ) ).to.equal( scope.plugins.get( ScopePluginA ) ); + expect( editorA.plugins.has( ScopePluginB ) ).to.equal( false ); + expect( editorB.plugins.get( ScopePluginB ) ).to.equal( scope.plugins.get( ScopePluginB ) ); + expect( editorB.plugins.has( ScopePluginA ) ).to.equal( false ); + + expect( scope.plugins.has( EditorPluginA ) ).to.equal( false ); + expect( editorA.plugins.get( EditorPluginA ) ).to.not.equal( editorB.plugins.get( EditorPluginA ) ); + + await scope.destroy(); + } ); + + it( 'should share the same instance of plugin within editors using scope (deeps)', async () => { + class ScopePluginA extends Plugin {} + class ScopePluginB extends Plugin {} + class EditorPluginA extends Plugin { + static get requires() { + return [ ScopePluginA ]; + } + } + class EditorPluginB extends Plugin { + static get requires() { + return [ ScopePluginB ]; + } + } + + const scope = await Scope.create( { plugins: [ ScopePluginA, ScopePluginB ] } ); + const editorA = await VirtualTestEditor.create( { scope, plugins: [ EditorPluginA ] } ); + const editorB = await VirtualTestEditor.create( { scope, plugins: [ EditorPluginB ] } ); + + expect( scope.plugins.get( ScopePluginA ) ).to.equal( editorA.plugins.get( ScopePluginA ) ); + expect( scope.plugins.get( ScopePluginB ) ).to.equal( editorB.plugins.get( ScopePluginB ) ); + + await scope.destroy(); + } ); + + it( 'should not initialize twice plugin added to the scope and the editor', async () => { + const initSpy = sinon.spy(); + const afterInitSpy = sinon.spy(); + + class ScopePluginA extends Plugin { + init() { + initSpy(); + } + + afterInit() { + afterInitSpy(); + } + } + + const scope = await Scope.create( { plugins: [ ScopePluginA ] } ); + const editor = await VirtualTestEditor.create( { scope, plugins: [ ScopePluginA ] } ); + + expect( scope.plugins.get( ScopePluginA ) ).to.equal( editor.plugins.get( ScopePluginA ) ); + sinon.assert.calledOnce( initSpy ); + sinon.assert.calledOnce( afterInitSpy ); + + await scope.destroy(); + } ); + + it( 'should not destroy scope plugin when only the editor is destroyed', async () => { + const scopePluginDestroySpy = sinon.spy(); + const editorPluginDestroySpy = sinon.spy(); + + class ScopePlugin extends Plugin { + destroy() { + scopePluginDestroySpy(); + } + } + + class EditorPlugin extends Plugin { + destroy() { + editorPluginDestroySpy(); + } + } + + const scope = await Scope.create( { plugins: [ ScopePlugin ] } ); + const editor = await VirtualTestEditor.create( { scope, plugins: [ ScopePlugin, EditorPlugin ] } ); + + await editor.destroy(); + + sinon.assert.calledOnce( editorPluginDestroySpy ); + sinon.assert.notCalled( scopePluginDestroySpy ); + } ); + + it( 'should not destroy scope along with editor when scope is injected to the editor', () => { + + } ); + + it( 'should destroy all editors along with the scope when scope is injected to them', () => { + + } ); + + it( 'should destroy scope along with editor when scope is created by the editor', () => { + + } ); +} ); From 8aed6716989a4f55d7f904034c3c62a7c508cad2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 30 Oct 2019 09:30:39 +0100 Subject: [PATCH 02/37] Renamed Scope to Context. --- src/{scope.js => context.js} | 22 +++---- src/editor/editor.js | 24 ++++---- tests/context.js | 115 +++++++++++++++++++++++++++++++++++ tests/scope.js | 115 ----------------------------------- 4 files changed, 138 insertions(+), 138 deletions(-) rename src/{scope.js => context.js} (75%) create mode 100644 tests/context.js delete mode 100644 tests/scope.js diff --git a/src/scope.js b/src/context.js similarity index 75% rename from src/scope.js rename to src/context.js index fe0184eb..3049b6fe 100644 --- a/src/scope.js +++ b/src/context.js @@ -4,7 +4,7 @@ */ /** - * @module core/scope + * @module core/context */ import Config from '@ckeditor/ckeditor5-utils/src/config'; @@ -12,10 +12,10 @@ import PluginCollection from './plugincollection'; import Locale from '@ckeditor/ckeditor5-utils/src/locale'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; -export default class Scope { +export default class Context { constructor( config ) { /** - * Holds all configurations specific to this scope instance. + * Holds all configurations specific to this context instance. * * @readonly * @type {module:utils/config~Config} @@ -23,7 +23,7 @@ export default class Scope { this.config = new Config( config ); /** - * The plugins loaded and in use by this scope instance. + * The plugins loaded and in use by this context instance. * * @readonly * @type {module:core/plugincollection~PluginCollection} @@ -42,7 +42,7 @@ export default class Scope { } ); /** - * List of editors used with this scope. + * List of editors used with this context. * * @private * @type {Set} @@ -69,7 +69,7 @@ export default class Scope { for ( const plugin of plugins ) { if ( typeof plugin != 'function' ) { - throw new CKEditorError( 'scope-initplugins: Only constructor is allowed as a Scope plugin' ); + throw new CKEditorError( 'context-initplugins: Only constructor is allowed as a Context plugin' ); } } @@ -77,15 +77,15 @@ export default class Scope { } /** - * Destroys the scope instance, releasing all resources used by it. + * Destroys the context instance, releasing all resources used by it. * - * @returns {Promise} A promise that resolves once the scope instance is fully destroyed. + * @returns {Promise} A promise that resolves once the context instance is fully destroyed. */ destroy() { const promises = []; for ( const editor of Array.from( this._editors ) ) { - editor.scope = null; + editor.context = null; promises.push( editor.destroy() ); } @@ -95,9 +95,9 @@ export default class Scope { static create( config ) { return new Promise( resolve => { - const scope = new this( config ); + const context = new this( config ); - resolve( scope.initPlugins().then( () => scope ) ); + resolve( context.initPlugins().then( () => context ) ); } ); } } diff --git a/src/editor/editor.js b/src/editor/editor.js index c5aaaf54..3234154d 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -7,7 +7,7 @@ * @module core/editor/editor */ -import Scope from '@ckeditor/ckeditor5-core/src/scope'; +import Context from '@ckeditor/ckeditor5-core/src/context'; import Config from '@ckeditor/ckeditor5-utils/src/config'; import EditingController from '@ckeditor/ckeditor5-engine/src/controller/editingcontroller'; import PluginCollection from '../plugincollection'; @@ -53,10 +53,10 @@ export default class Editor { constructor( config = {} ) { /** * @readonly - * @type {module:core/scope~Scope} + * @type {module:core/context~Context} */ - this.scope = config.scope || new Scope( config ); - this.scope.addEditor( this ); + this.context = config.context || new Context( config ); + this.context.addEditor( this ); const availablePlugins = this.constructor.builtinPlugins; @@ -80,7 +80,7 @@ export default class Editor { * @readonly * @member {module:core/plugincollection~PluginCollection} */ - this.plugins = new PluginCollection( this, availablePlugins, this.scope.plugins ); + this.plugins = new PluginCollection( this, availablePlugins, this.context.plugins ); /** * Commands registered to the editor. @@ -206,7 +206,7 @@ export default class Editor { * @type {module:utils/locale~Locale} */ get locale() { - return this.scope.locale; + return this.context.locale; } /** @@ -244,14 +244,14 @@ export default class Editor { * @returns {Promise} A promise that resolves once the editor instance is fully destroyed. */ destroy() { - // If editor scope is created by this editor - // it is enough to destroy the scope, editor will be destroyed along with it. - if ( this.scope ) { - if ( this.config.scope ) { - return this.scope.destroy(); + // If editor context is created by this editor + // it is enough to destroy the context, editor will be destroyed along with it. + if ( this.context ) { + if ( this.config.context ) { + return this.context.destroy(); } - this.scope.removeEditor( this ); + this.context.removeEditor( this ); } let readyPromise = Promise.resolve(); diff --git a/tests/context.js b/tests/context.js new file mode 100644 index 00000000..1c7170e8 --- /dev/null +++ b/tests/context.js @@ -0,0 +1,115 @@ +/** + * @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 Context from '../src/context'; +import Plugin from '../src/plugin'; +import VirtualTestEditor from './_utils/virtualtesteditor'; + +describe( 'Context', () => { + it( 'should share the same instance of plugin within editors using the same context', async () => { + class ContextPluginA extends Plugin {} + class ContextPluginB extends Plugin {} + class EditorPluginA extends Plugin {} + + const context = await Context.create( { plugins: [ ContextPluginA, ContextPluginB ] } ); + const editorA = await VirtualTestEditor.create( { context, plugins: [ ContextPluginA, EditorPluginA ] } ); + const editorB = await VirtualTestEditor.create( { context, plugins: [ ContextPluginB, EditorPluginA ] } ); + + expect( editorA.plugins.get( ContextPluginA ) ).to.equal( context.plugins.get( ContextPluginA ) ); + expect( editorA.plugins.has( ContextPluginB ) ).to.equal( false ); + expect( editorB.plugins.get( ContextPluginB ) ).to.equal( context.plugins.get( ContextPluginB ) ); + expect( editorB.plugins.has( ContextPluginA ) ).to.equal( false ); + + expect( context.plugins.has( EditorPluginA ) ).to.equal( false ); + expect( editorA.plugins.get( EditorPluginA ) ).to.not.equal( editorB.plugins.get( EditorPluginA ) ); + + await context.destroy(); + } ); + + it( 'should share the same instance of plugin (dependencies) within editors using the same context', async () => { + class ContextPluginA extends Plugin {} + class ContextPluginB extends Plugin {} + class EditorPluginA extends Plugin { + static get requires() { + return [ ContextPluginA ]; + } + } + class EditorPluginB extends Plugin { + static get requires() { + return [ ContextPluginB ]; + } + } + + const context = await Context.create( { plugins: [ ContextPluginA, ContextPluginB ] } ); + const editorA = await VirtualTestEditor.create( { context, plugins: [ EditorPluginA ] } ); + const editorB = await VirtualTestEditor.create( { context, plugins: [ EditorPluginB ] } ); + + expect( context.plugins.get( ContextPluginA ) ).to.equal( editorA.plugins.get( ContextPluginA ) ); + expect( context.plugins.get( ContextPluginB ) ).to.equal( editorB.plugins.get( ContextPluginB ) ); + + await context.destroy(); + } ); + + it( 'should not initialize twice plugin added to the context and the editor', async () => { + const initSpy = sinon.spy(); + const afterInitSpy = sinon.spy(); + + class ContextPluginA extends Plugin { + init() { + initSpy(); + } + + afterInit() { + afterInitSpy(); + } + } + + const context = await Context.create( { plugins: [ ContextPluginA ] } ); + const editor = await VirtualTestEditor.create( { context, plugins: [ ContextPluginA ] } ); + + expect( context.plugins.get( ContextPluginA ) ).to.equal( editor.plugins.get( ContextPluginA ) ); + sinon.assert.calledOnce( initSpy ); + sinon.assert.calledOnce( afterInitSpy ); + + await context.destroy(); + } ); + + it( 'should not destroy context plugin when only the editor is destroyed', async () => { + const contextPluginDestroySpy = sinon.spy(); + const editorPluginDestroySpy = sinon.spy(); + + class ContextPlugin extends Plugin { + destroy() { + contextPluginDestroySpy(); + } + } + + class EditorPlugin extends Plugin { + destroy() { + editorPluginDestroySpy(); + } + } + + const context = await Context.create( { plugins: [ ContextPlugin ] } ); + const editor = await VirtualTestEditor.create( { context, plugins: [ ContextPlugin, EditorPlugin ] } ); + + await editor.destroy(); + + sinon.assert.calledOnce( editorPluginDestroySpy ); + sinon.assert.notCalled( contextPluginDestroySpy ); + } ); + + it( 'should not destroy context along with editor when context is injected to the editor', () => { + + } ); + + it( 'should destroy all editors along with the context when context is injected to them', () => { + + } ); + + it( 'should destroy context along with editor when context is created by the editor', () => { + + } ); +} ); diff --git a/tests/scope.js b/tests/scope.js deleted file mode 100644 index 331c2d0e..00000000 --- a/tests/scope.js +++ /dev/null @@ -1,115 +0,0 @@ -/** - * @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 Scope from '../src/scope'; -import Plugin from '../src/plugin'; -import VirtualTestEditor from './_utils/virtualtesteditor'; - -describe( 'Scope', () => { - it( 'should share the same instance of plugin within editors using scope', async () => { - class ScopePluginA extends Plugin {} - class ScopePluginB extends Plugin {} - class EditorPluginA extends Plugin {} - - const scope = await Scope.create( { plugins: [ ScopePluginA, ScopePluginB ] } ); - const editorA = await VirtualTestEditor.create( { scope, plugins: [ ScopePluginA, EditorPluginA ] } ); - const editorB = await VirtualTestEditor.create( { scope, plugins: [ ScopePluginB, EditorPluginA ] } ); - - expect( editorA.plugins.get( ScopePluginA ) ).to.equal( scope.plugins.get( ScopePluginA ) ); - expect( editorA.plugins.has( ScopePluginB ) ).to.equal( false ); - expect( editorB.plugins.get( ScopePluginB ) ).to.equal( scope.plugins.get( ScopePluginB ) ); - expect( editorB.plugins.has( ScopePluginA ) ).to.equal( false ); - - expect( scope.plugins.has( EditorPluginA ) ).to.equal( false ); - expect( editorA.plugins.get( EditorPluginA ) ).to.not.equal( editorB.plugins.get( EditorPluginA ) ); - - await scope.destroy(); - } ); - - it( 'should share the same instance of plugin within editors using scope (deeps)', async () => { - class ScopePluginA extends Plugin {} - class ScopePluginB extends Plugin {} - class EditorPluginA extends Plugin { - static get requires() { - return [ ScopePluginA ]; - } - } - class EditorPluginB extends Plugin { - static get requires() { - return [ ScopePluginB ]; - } - } - - const scope = await Scope.create( { plugins: [ ScopePluginA, ScopePluginB ] } ); - const editorA = await VirtualTestEditor.create( { scope, plugins: [ EditorPluginA ] } ); - const editorB = await VirtualTestEditor.create( { scope, plugins: [ EditorPluginB ] } ); - - expect( scope.plugins.get( ScopePluginA ) ).to.equal( editorA.plugins.get( ScopePluginA ) ); - expect( scope.plugins.get( ScopePluginB ) ).to.equal( editorB.plugins.get( ScopePluginB ) ); - - await scope.destroy(); - } ); - - it( 'should not initialize twice plugin added to the scope and the editor', async () => { - const initSpy = sinon.spy(); - const afterInitSpy = sinon.spy(); - - class ScopePluginA extends Plugin { - init() { - initSpy(); - } - - afterInit() { - afterInitSpy(); - } - } - - const scope = await Scope.create( { plugins: [ ScopePluginA ] } ); - const editor = await VirtualTestEditor.create( { scope, plugins: [ ScopePluginA ] } ); - - expect( scope.plugins.get( ScopePluginA ) ).to.equal( editor.plugins.get( ScopePluginA ) ); - sinon.assert.calledOnce( initSpy ); - sinon.assert.calledOnce( afterInitSpy ); - - await scope.destroy(); - } ); - - it( 'should not destroy scope plugin when only the editor is destroyed', async () => { - const scopePluginDestroySpy = sinon.spy(); - const editorPluginDestroySpy = sinon.spy(); - - class ScopePlugin extends Plugin { - destroy() { - scopePluginDestroySpy(); - } - } - - class EditorPlugin extends Plugin { - destroy() { - editorPluginDestroySpy(); - } - } - - const scope = await Scope.create( { plugins: [ ScopePlugin ] } ); - const editor = await VirtualTestEditor.create( { scope, plugins: [ ScopePlugin, EditorPlugin ] } ); - - await editor.destroy(); - - sinon.assert.calledOnce( editorPluginDestroySpy ); - sinon.assert.notCalled( scopePluginDestroySpy ); - } ); - - it( 'should not destroy scope along with editor when scope is injected to the editor', () => { - - } ); - - it( 'should destroy all editors along with the scope when scope is injected to them', () => { - - } ); - - it( 'should destroy scope along with editor when scope is created by the editor', () => { - - } ); -} ); From 1b37d8eaa5da7bc12bc5fa2e5457af9af2aa14a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 30 Oct 2019 13:03:57 +0100 Subject: [PATCH 03/37] Allow to get external plugin by name. --- src/context.js | 34 ++++++++++++++++++++++++++++++---- src/plugincollection.js | 9 ++++++--- tests/context.js | 39 ++++++++++++++++++++++++++++++++++----- 3 files changed, 70 insertions(+), 12 deletions(-) diff --git a/src/context.js b/src/context.js index 3049b6fe..b2599826 100644 --- a/src/context.js +++ b/src/context.js @@ -12,7 +12,17 @@ import PluginCollection from './plugincollection'; import Locale from '@ckeditor/ckeditor5-utils/src/locale'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; +/** + * @TODO + */ export default class Context { + /** + * Creates a context instance with a given configuration. + * + * Usually, not to be used directly. See the static {@link ~Context.create `create()`} method. + * + * @param {Object} [config={}] The context config. + */ constructor( config ) { /** * Holds all configurations specific to this context instance. @@ -28,13 +38,13 @@ export default class Context { * @readonly * @type {module:core/plugincollection~PluginCollection} */ - this.plugins = new PluginCollection( this, this.config.get( 'plugins' ) ); + this.plugins = new PluginCollection( this ); const languageConfig = this.config.get( 'language' ) || {}; /** * @readonly - * @member {module:utils/locale~Locale} + * @type {module:utils/locale~Locale} */ this.locale = new Locale( { uiLanguage: typeof languageConfig === 'string' ? languageConfig : languageConfig.ui, @@ -42,7 +52,7 @@ export default class Context { } ); /** - * List of editors used with this context. + * List of editors to which this context instance is injected. * * @private * @type {Set} @@ -51,6 +61,8 @@ export default class Context { } /** + * Adds a reference to the to which context is injected. + * * @param {module:core/editor/editor~Editor} editor */ addEditor( editor ) { @@ -58,14 +70,22 @@ export default class Context { } /** + * Removes a reference to the editor to which context was injected. + * * @param {module:core/editor/editor~Editor} editor */ removeEditor( editor ) { return this._editors.delete( editor ); } + /** + * Loads and initializes plugins specified in the config. + * + * @returns {Promise.} A promise which resolves + * once the initialization is completed providing an array of loaded plugins. + */ initPlugins() { - const plugins = this.config.get( 'plugins' ); + const plugins = this.config.get( 'plugins' ) || []; for ( const plugin of plugins ) { if ( typeof plugin != 'function' ) { @@ -93,6 +113,12 @@ export default class Context { .then( () => this.plugins.destroy() ); } + /** + * @TODO + * + * @param {Object} [config] The context config. + * @returns {Promise} A promise resolved once the context is ready. The promise resolves with the created context instance. + */ static create( config ) { return new Promise( resolve => { const context = new this( config ); diff --git a/src/plugincollection.js b/src/plugincollection.js index f9183c04..f991dd27 100644 --- a/src/plugincollection.js +++ b/src/plugincollection.js @@ -55,15 +55,13 @@ export default class PluginCollection { this._availablePlugins = new Map(); for ( const PluginConstructor of availablePlugins ) { - this._availablePlugins.set( PluginConstructor, PluginConstructor ); - if ( PluginConstructor.pluginName ) { this._availablePlugins.set( PluginConstructor.pluginName, PluginConstructor ); } } /** - * List of constructors of externally loaded plugins. + * Map of external plugins which can be retrieved by their constructors or instance. * * @private * @type {Map} @@ -73,6 +71,11 @@ export default class PluginCollection { for ( const [ PluginConstructor, pluginInstance ] of externalPlugins ) { this._externalPlugins.set( PluginConstructor, pluginInstance ); this._externalPlugins.set( pluginInstance, PluginConstructor ); + + // To make it possible to require plugin by its name. + if ( PluginConstructor.pluginName ) { + this._availablePlugins.set( PluginConstructor.pluginName, PluginConstructor ); + } } } diff --git a/tests/context.js b/tests/context.js index 1c7170e8..4a21bbfa 100644 --- a/tests/context.js +++ b/tests/context.js @@ -76,7 +76,7 @@ describe( 'Context', () => { await context.destroy(); } ); - it( 'should not destroy context plugin when only the editor is destroyed', async () => { + it( 'should not destroy context along with the editor when context is injected to the editor', async () => { const contextPluginDestroySpy = sinon.spy(); const editorPluginDestroySpy = sinon.spy(); @@ -101,15 +101,44 @@ describe( 'Context', () => { sinon.assert.notCalled( contextPluginDestroySpy ); } ); - it( 'should not destroy context along with editor when context is injected to the editor', () => { + it( 'should destroy all editors with injected context when context is destroyed', async () => { + const context = await Context.create(); + const editorA = await VirtualTestEditor.create( { context } ); + const editorB = await VirtualTestEditor.create( { context } ); + const editorC = await VirtualTestEditor.create(); - } ); + sinon.spy( editorA, 'destroy' ); + sinon.spy( editorB, 'destroy' ); + sinon.spy( editorC, 'destroy' ); - it( 'should destroy all editors along with the context when context is injected to them', () => { + await context.destroy(); + sinon.assert.calledOnce( editorA.destroy ); + sinon.assert.calledOnce( editorB.destroy ); + sinon.assert.notCalled( editorC.destroy ); } ); - it( 'should destroy context along with editor when context is created by the editor', () => { + it( 'should be able to add context plugin to the editor using pluginName property', async () => { + class ContextPluginA extends Plugin { + static get pluginName() { + return 'ContextPluginA'; + } + } + + class ContextPluginB extends Plugin { + static get pluginName() { + return 'ContextPluginB'; + } + + static get requires() { + return [ ContextPluginA ]; + } + } + + const context = await Context.create( { plugins: [ ContextPluginB ] } ); + const editor = await VirtualTestEditor.create( { context, plugins: [ 'ContextPluginA' ] } ); + expect( editor.plugins.has( ContextPluginA ) ).to.equal( true ); + expect( editor.plugins.has( ContextPluginB ) ).to.equal( false ); } ); } ); From 0ef6bfe6b7c6c6c210c3b8d98faa5ffd0d5e9012 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 30 Oct 2019 13:09:08 +0100 Subject: [PATCH 04/37] Added ContextPlugin class. --- src/contextplugin.js | 43 ++++++++++++++++++++++++++++++++++++++++++ tests/contextplugin.js | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 src/contextplugin.js create mode 100644 tests/contextplugin.js diff --git a/src/contextplugin.js b/src/contextplugin.js new file mode 100644 index 00000000..e7ac92fe --- /dev/null +++ b/src/contextplugin.js @@ -0,0 +1,43 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module core/contextplugin + */ + +import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin'; +import mix from '@ckeditor/ckeditor5-utils/src/mix'; + +/** + * The base class for {@link module:core/context~Context} plugin classes. + * + * @implements module:core/plugin~PluginInterface + * @mixes module:utils/observablemixin~ObservableMixin + */ +export default class ContextPlugin { + /** + * Creates a new plugin instance. + * + * @param {module:core/context~Context|module:core/editor/editor~Editor} context + */ + constructor( context ) { + /** + * The context instance. + * + * @readonly + * @type {module:core/context~Context|module:core/editor/editor~Editor} + */ + this.context = context; + } + + /** + * @inheritDoc + */ + destroy() { + this.stopListening(); + } +} + +mix( ContextPlugin, ObservableMixin ); diff --git a/tests/contextplugin.js b/tests/contextplugin.js new file mode 100644 index 00000000..93a93f8c --- /dev/null +++ b/tests/contextplugin.js @@ -0,0 +1,35 @@ +/** + * @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 ContextPlugin from '../src/contextplugin'; + +describe( 'ContextPlugin', () => { + const contextMock = {}; + + describe( 'constructor()', () => { + it( 'should set the `context` property', () => { + const plugin = new ContextPlugin( contextMock ); + + expect( plugin ).to.have.property( 'context' ).to.equal( contextMock ); + } ); + + describe( 'destroy()', () => { + it( 'should be defined', () => { + const plugin = new ContextPlugin( contextMock ); + + expect( plugin.destroy ).to.be.a( 'function' ); + } ); + + it( 'should stop listening', () => { + const plugin = new ContextPlugin( contextMock ); + const stopListeningSpy = sinon.spy( plugin, 'stopListening' ); + + plugin.destroy(); + + sinon.assert.calledOnce( stopListeningSpy ); + } ); + } ); + } ); +} ); From b6ab5f72d801145b016947144cad530e8d6fa107 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 30 Oct 2019 14:19:21 +0100 Subject: [PATCH 05/37] Improved usage of ContectPlugin class. --- src/context.js | 10 +++-- src/contextplugin.js | 9 +++++ tests/context.js | 95 +++++++++++++++++++++++++++++++++++++++----- 3 files changed, 101 insertions(+), 13 deletions(-) diff --git a/src/context.js b/src/context.js index b2599826..0d3c979e 100644 --- a/src/context.js +++ b/src/context.js @@ -87,9 +87,13 @@ export default class Context { initPlugins() { const plugins = this.config.get( 'plugins' ) || []; - for ( const plugin of plugins ) { - if ( typeof plugin != 'function' ) { - throw new CKEditorError( 'context-initplugins: Only constructor is allowed as a Context plugin' ); + for ( const Plugin of plugins ) { + if ( typeof Plugin != 'function' ) { + throw new CKEditorError( 'context-initplugins: Only constructor is allowed as a Context plugin.', null, { Plugin } ); + } + + if ( Plugin.isContextPlugin !== true ) { + throw new CKEditorError( 'context-initplugins: Only plugins marked as a ContextPlugin are allowed.', null, { Plugin } ); } } diff --git a/src/contextplugin.js b/src/contextplugin.js index e7ac92fe..6bcc1b11 100644 --- a/src/contextplugin.js +++ b/src/contextplugin.js @@ -38,6 +38,15 @@ export default class ContextPlugin { destroy() { this.stopListening(); } + + /** + * Static property which marks plugin as an allowed to be use directly by a {@link module:core/context~Context}. + * + * @returns {Boolean} + */ + static get isContextPlugin() { + return true; + } } mix( ContextPlugin, ObservableMixin ); diff --git a/tests/context.js b/tests/context.js index 4a21bbfa..91104062 100644 --- a/tests/context.js +++ b/tests/context.js @@ -4,13 +4,15 @@ */ import Context from '../src/context'; +import ContextPlugin from '../src/contextplugin'; import Plugin from '../src/plugin'; import VirtualTestEditor from './_utils/virtualtesteditor'; +import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; describe( 'Context', () => { it( 'should share the same instance of plugin within editors using the same context', async () => { - class ContextPluginA extends Plugin {} - class ContextPluginB extends Plugin {} + class ContextPluginA extends ContextPlugin {} + class ContextPluginB extends ContextPlugin {} class EditorPluginA extends Plugin {} const context = await Context.create( { plugins: [ ContextPluginA, ContextPluginB ] } ); @@ -29,8 +31,8 @@ describe( 'Context', () => { } ); it( 'should share the same instance of plugin (dependencies) within editors using the same context', async () => { - class ContextPluginA extends Plugin {} - class ContextPluginB extends Plugin {} + class ContextPluginA extends ContextPlugin {} + class ContextPluginB extends ContextPlugin {} class EditorPluginA extends Plugin { static get requires() { return [ ContextPluginA ]; @@ -56,7 +58,7 @@ describe( 'Context', () => { const initSpy = sinon.spy(); const afterInitSpy = sinon.spy(); - class ContextPluginA extends Plugin { + class ContextPluginA extends ContextPlugin { init() { initSpy(); } @@ -80,7 +82,7 @@ describe( 'Context', () => { const contextPluginDestroySpy = sinon.spy(); const editorPluginDestroySpy = sinon.spy(); - class ContextPlugin extends Plugin { + class ContextPluginA extends ContextPlugin { destroy() { contextPluginDestroySpy(); } @@ -92,8 +94,8 @@ describe( 'Context', () => { } } - const context = await Context.create( { plugins: [ ContextPlugin ] } ); - const editor = await VirtualTestEditor.create( { context, plugins: [ ContextPlugin, EditorPlugin ] } ); + const context = await Context.create( { plugins: [ ContextPluginA ] } ); + const editor = await VirtualTestEditor.create( { context, plugins: [ ContextPluginA, EditorPlugin ] } ); await editor.destroy(); @@ -119,13 +121,13 @@ describe( 'Context', () => { } ); it( 'should be able to add context plugin to the editor using pluginName property', async () => { - class ContextPluginA extends Plugin { + class ContextPluginA extends ContextPlugin { static get pluginName() { return 'ContextPluginA'; } } - class ContextPluginB extends Plugin { + class ContextPluginB extends ContextPlugin { static get pluginName() { return 'ContextPluginB'; } @@ -141,4 +143,77 @@ describe( 'Context', () => { expect( editor.plugins.has( ContextPluginA ) ).to.equal( true ); expect( editor.plugins.has( ContextPluginB ) ).to.equal( false ); } ); + + it( 'should throw when plugin is added to the context by name', async () => { + let caughtError; + + try { + await Context.create( { plugins: [ 'ContextPlugin' ] } ); + } catch ( error ) { + caughtError = error; + } + + expect( caughtError ).to.instanceof( CKEditorError ); + expect( caughtError.message ).match( /^context-initplugins: Only constructor is allowed as a Context plugin./ ); + } ); + + it( 'should throw when plugin added to the context is not marked as a ContextPlugin #1', async () => { + class EditorPlugin extends Plugin {} + + let caughtError; + + try { + await Context.create( { plugins: [ EditorPlugin ] } ); + } catch ( error ) { + caughtError = error; + } + + expect( caughtError ).to.instanceof( CKEditorError ); + expect( caughtError.message ).match( /^context-initplugins: Only plugins marked as a ContextPlugin are allowed./ ); + } ); + + it( 'should throw when plugin added to the context is not marked as a ContextPlugin #2', async () => { + function EditorPlugin() {} + + let caughtError; + + try { + await Context.create( { plugins: [ EditorPlugin ] } ); + } catch ( error ) { + caughtError = error; + } + + expect( caughtError ).to.instanceof( CKEditorError ); + expect( caughtError.message ).match( /^context-initplugins: Only plugins marked as a ContextPlugin are allowed./ ); + } ); + + it( 'should not throw when plugin added to the context has not a ContextPlugin proto but is marked as a ContextPlugin', async () => { + function EditorPlugin() {} + EditorPlugin.isContextPlugin = true; + + let caughtError; + + try { + await Context.create( { plugins: [ EditorPlugin ] } ); + } catch ( error ) { + caughtError = error; + } + + expect( caughtError ).to.equal( undefined ); + } ); + + it( 'should throw when plugin added to the context is not marked as a ContextPlugin #2', async () => { + function EditorPlugin() {} + + let caughtError; + + try { + await Context.create( { plugins: [ EditorPlugin ] } ); + } catch ( error ) { + caughtError = error; + } + + expect( caughtError ).to.instanceof( CKEditorError ); + expect( caughtError.message ).match( /^context-initplugins: Only plugins marked as a ContextPlugin are allowed./ ); + } ); } ); From 3e948e5c8e4720893a283dfac76f21ead271ee88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 31 Oct 2019 19:30:53 +0100 Subject: [PATCH 06/37] Test improvements. --- src/context.js | 21 +-- src/editor/editor.js | 56 +++---- src/plugincollection.js | 2 +- tests/context.js | 325 ++++++++++++++++++++------------------ tests/editor/editor.js | 69 +++++--- tests/plugincollection.js | 34 ++++ 6 files changed, 295 insertions(+), 212 deletions(-) diff --git a/src/context.js b/src/context.js index 0d3c979e..dd396f59 100644 --- a/src/context.js +++ b/src/context.js @@ -51,6 +51,14 @@ export default class Context { contentLanguage: this.config.get( 'language.content' ) } ); + /** + * Shorthand for {@link module:utils/locale~Locale#t}. + * + * @see module:utils/locale~Locale#t + * @method #t + */ + this.t = this.locale.t; + /** * List of editors to which this context instance is injected. * @@ -61,9 +69,11 @@ export default class Context { } /** - * Adds a reference to the to which context is injected. + * Adds a reference to the editor to which context is injected. * * @param {module:core/editor/editor~Editor} editor + * @param {Boolean} isHost Flag defines if context was created by this editor. It is used to decide if the context + * should be destroyed along with the editor instance. */ addEditor( editor ) { this._editors.add( editor ); @@ -106,14 +116,7 @@ export default class Context { * @returns {Promise} A promise that resolves once the context instance is fully destroyed. */ destroy() { - const promises = []; - - for ( const editor of Array.from( this._editors ) ) { - editor.context = null; - promises.push( editor.destroy() ); - } - - return Promise.all( promises ) + return Promise.all( Array.from( this._editors, editor => editor.destroy() ) ) .then( () => this.plugins.destroy() ); } diff --git a/src/editor/editor.js b/src/editor/editor.js index 3234154d..f673a5e4 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -58,6 +58,14 @@ export default class Editor { this.context = config.context || new Context( config ); this.context.addEditor( this ); + /** + * `true` when context is created by this editor or `false` when context is injected to the editor. + * + * @private + * @type {Boolean} + */ + this._isContextHost = !config.context; + const availablePlugins = this.constructor.builtinPlugins; /** @@ -82,6 +90,20 @@ export default class Editor { */ this.plugins = new PluginCollection( this, availablePlugins, this.context.plugins ); + /** + * @readonly + * @type {module:utils/locale~Locale} + */ + this.locale = this.context.locale; + + /** + * Shorthand for {@link module:utils/locale~Locale#t}. + * + * @see module:utils/locale~Locale#t + * @method #t + */ + this.t = this.locale.t; + /** * Commands registered to the editor. * @@ -201,24 +223,6 @@ export default class Editor { this.keystrokes.listenTo( this.editing.view.document ); } - /** - * @readonly - * @type {module:utils/locale~Locale} - */ - get locale() { - return this.context.locale; - } - - /** - * Shorthand for {@link module:utils/locale~Locale#t}. - * - * @see module:utils/locale~Locale#t - * @method #t - */ - get t() { - return this.locale.t; - } - /** * Loads and initializes plugins specified in the config. * @@ -244,16 +248,6 @@ export default class Editor { * @returns {Promise} A promise that resolves once the editor instance is fully destroyed. */ destroy() { - // If editor context is created by this editor - // it is enough to destroy the context, editor will be destroyed along with it. - if ( this.context ) { - if ( this.config.context ) { - return this.context.destroy(); - } - - this.context.removeEditor( this ); - } - let readyPromise = Promise.resolve(); if ( this.state == 'initializing' ) { @@ -262,6 +256,12 @@ export default class Editor { return readyPromise .then( () => { + this.context.removeEditor( this ); + + if ( this._isContextHost ) { + return this.context.destroy(); + } + } ).then( () => { this.fire( 'destroy' ); this.stopListening(); this.commands.destroy(); diff --git a/src/plugincollection.js b/src/plugincollection.js index f991dd27..cfd96c21 100644 --- a/src/plugincollection.js +++ b/src/plugincollection.js @@ -63,7 +63,7 @@ export default class PluginCollection { /** * Map of external plugins which can be retrieved by their constructors or instance. * - * @private + * @protected * @type {Map} */ this._externalPlugins = new Map(); diff --git a/tests/context.js b/tests/context.js index 91104062..ff9bbd84 100644 --- a/tests/context.js +++ b/tests/context.js @@ -6,214 +6,233 @@ import Context from '../src/context'; import ContextPlugin from '../src/contextplugin'; import Plugin from '../src/plugin'; +import Config from '@ckeditor/ckeditor5-utils/src/config'; +import Locale from '@ckeditor/ckeditor5-utils/src/locale'; import VirtualTestEditor from './_utils/virtualtesteditor'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; describe( 'Context', () => { - it( 'should share the same instance of plugin within editors using the same context', async () => { - class ContextPluginA extends ContextPlugin {} - class ContextPluginB extends ContextPlugin {} - class EditorPluginA extends Plugin {} + describe( 'config', () => { + it( 'should be created', () => { + const context = new Context(); - const context = await Context.create( { plugins: [ ContextPluginA, ContextPluginB ] } ); - const editorA = await VirtualTestEditor.create( { context, plugins: [ ContextPluginA, EditorPluginA ] } ); - const editorB = await VirtualTestEditor.create( { context, plugins: [ ContextPluginB, EditorPluginA ] } ); + expect( context.config ).to.instanceof( Config ); + } ); - expect( editorA.plugins.get( ContextPluginA ) ).to.equal( context.plugins.get( ContextPluginA ) ); - expect( editorA.plugins.has( ContextPluginB ) ).to.equal( false ); - expect( editorB.plugins.get( ContextPluginB ) ).to.equal( context.plugins.get( ContextPluginB ) ); - expect( editorB.plugins.has( ContextPluginA ) ).to.equal( false ); + it( 'should be with given configuration', () => { + const context = new Context( { foo: 'bar' } ); - expect( context.plugins.has( EditorPluginA ) ).to.equal( false ); - expect( editorA.plugins.get( EditorPluginA ) ).to.not.equal( editorB.plugins.get( EditorPluginA ) ); - - await context.destroy(); + expect( context.config.get( 'foo' ) ).to.equal( 'bar' ); + } ); } ); - it( 'should share the same instance of plugin (dependencies) within editors using the same context', async () => { - class ContextPluginA extends ContextPlugin {} - class ContextPluginB extends ContextPlugin {} - class EditorPluginA extends Plugin { - static get requires() { - return [ ContextPluginA ]; - } - } - class EditorPluginB extends Plugin { - static get requires() { - return [ ContextPluginB ]; - } - } + describe( 'locale', () => { + it( 'is instantiated and t() is exposed', () => { + const context = new Context(); + + expect( context.locale ).to.be.instanceof( Locale ); + expect( context.t ).to.equal( context.locale.t ); + } ); + + it( 'is configured with the config.language (UI and the content)', () => { + const context = new Context( { language: 'pl' } ); + + expect( context.locale.uiLanguage ).to.equal( 'pl' ); + expect( context.locale.contentLanguage ).to.equal( 'pl' ); + } ); + + it( 'is configured with the config.language (different for UI and the content)', () => { + const context = new Context( { language: { ui: 'pl', content: 'ar' } } ); - const context = await Context.create( { plugins: [ ContextPluginA, ContextPluginB ] } ); - const editorA = await VirtualTestEditor.create( { context, plugins: [ EditorPluginA ] } ); - const editorB = await VirtualTestEditor.create( { context, plugins: [ EditorPluginB ] } ); + expect( context.locale.uiLanguage ).to.equal( 'pl' ); + expect( context.locale.contentLanguage ).to.equal( 'ar' ); + } ); - expect( context.plugins.get( ContextPluginA ) ).to.equal( editorA.plugins.get( ContextPluginA ) ); - expect( context.plugins.get( ContextPluginB ) ).to.equal( editorB.plugins.get( ContextPluginB ) ); + it( 'is configured with the config.language (just the content)', () => { + const context = new Context( { language: { content: 'ar' } } ); - await context.destroy(); + expect( context.locale.uiLanguage ).to.equal( 'en' ); + expect( context.locale.contentLanguage ).to.equal( 'ar' ); + } ); } ); - it( 'should not initialize twice plugin added to the context and the editor', async () => { - const initSpy = sinon.spy(); - const afterInitSpy = sinon.spy(); + describe( 'plugins', () => { + it( 'should throw when plugin added to the context is not marked as a ContextPlugin #1', async () => { + class EditorPlugin extends Plugin {} - class ContextPluginA extends ContextPlugin { - init() { - initSpy(); - } + let caughtError; - afterInit() { - afterInitSpy(); + try { + await Context.create( { plugins: [ EditorPlugin ] } ); + } catch ( error ) { + caughtError = error; } - } - - const context = await Context.create( { plugins: [ ContextPluginA ] } ); - const editor = await VirtualTestEditor.create( { context, plugins: [ ContextPluginA ] } ); - expect( context.plugins.get( ContextPluginA ) ).to.equal( editor.plugins.get( ContextPluginA ) ); - sinon.assert.calledOnce( initSpy ); - sinon.assert.calledOnce( afterInitSpy ); + expect( caughtError ).to.instanceof( CKEditorError ); + expect( caughtError.message ).match( /^context-initplugins: Only plugins marked as a ContextPlugin are allowed./ ); + } ); - await context.destroy(); - } ); + it( 'should throw when plugin added to the context is not marked as a ContextPlugin #2', async () => { + function EditorPlugin() {} - it( 'should not destroy context along with the editor when context is injected to the editor', async () => { - const contextPluginDestroySpy = sinon.spy(); - const editorPluginDestroySpy = sinon.spy(); + let caughtError; - class ContextPluginA extends ContextPlugin { - destroy() { - contextPluginDestroySpy(); + try { + await Context.create( { plugins: [ EditorPlugin ] } ); + } catch ( error ) { + caughtError = error; } - } - class EditorPlugin extends Plugin { - destroy() { - editorPluginDestroySpy(); + expect( caughtError ).to.instanceof( CKEditorError ); + expect( caughtError.message ).match( /^context-initplugins: Only plugins marked as a ContextPlugin are allowed./ ); + } ); + + it( 'should throw when plugin is added to the context by name', async () => { + let caughtError; + + try { + await Context.create( { plugins: [ 'ContextPlugin' ] } ); + } catch ( error ) { + caughtError = error; } - } - const context = await Context.create( { plugins: [ ContextPluginA ] } ); - const editor = await VirtualTestEditor.create( { context, plugins: [ ContextPluginA, EditorPlugin ] } ); + expect( caughtError ).to.instanceof( CKEditorError ); + expect( caughtError.message ).match( /^context-initplugins: Only constructor is allowed as a Context plugin./ ); + } ); - await editor.destroy(); + it( 'should not throw when plugin as a function, marked as a ContextPlugin is added to the context', async () => { + function EditorPlugin() {} + EditorPlugin.isContextPlugin = true; - sinon.assert.calledOnce( editorPluginDestroySpy ); - sinon.assert.notCalled( contextPluginDestroySpy ); - } ); + let caughtError; + + try { + await Context.create( { plugins: [ EditorPlugin ] } ); + } catch ( error ) { + caughtError = error; + } - it( 'should destroy all editors with injected context when context is destroyed', async () => { - const context = await Context.create(); - const editorA = await VirtualTestEditor.create( { context } ); - const editorB = await VirtualTestEditor.create( { context } ); - const editorC = await VirtualTestEditor.create(); + expect( caughtError ).to.equal( undefined ); + } ); - sinon.spy( editorA, 'destroy' ); - sinon.spy( editorB, 'destroy' ); - sinon.spy( editorC, 'destroy' ); + it( 'should share the same instance of plugin within editors using the same context', async () => { + class ContextPluginA extends ContextPlugin {} + class ContextPluginB extends ContextPlugin {} + class EditorPluginA extends Plugin {} - await context.destroy(); + const context = await Context.create( { plugins: [ ContextPluginA, ContextPluginB ] } ); + const editorA = await VirtualTestEditor.create( { context, plugins: [ ContextPluginA, EditorPluginA ] } ); + const editorB = await VirtualTestEditor.create( { context, plugins: [ ContextPluginB, EditorPluginA ] } ); - sinon.assert.calledOnce( editorA.destroy ); - sinon.assert.calledOnce( editorB.destroy ); - sinon.assert.notCalled( editorC.destroy ); - } ); + expect( editorA.plugins.get( ContextPluginA ) ).to.equal( context.plugins.get( ContextPluginA ) ); + expect( editorA.plugins.has( ContextPluginB ) ).to.equal( false ); + expect( editorB.plugins.get( ContextPluginB ) ).to.equal( context.plugins.get( ContextPluginB ) ); + expect( editorB.plugins.has( ContextPluginA ) ).to.equal( false ); - it( 'should be able to add context plugin to the editor using pluginName property', async () => { - class ContextPluginA extends ContextPlugin { - static get pluginName() { - return 'ContextPluginA'; - } - } + expect( context.plugins.has( EditorPluginA ) ).to.equal( false ); + expect( editorA.plugins.get( EditorPluginA ) ).to.not.equal( editorB.plugins.get( EditorPluginA ) ); - class ContextPluginB extends ContextPlugin { - static get pluginName() { - return 'ContextPluginB'; - } + await context.destroy(); + } ); - static get requires() { - return [ ContextPluginA ]; + it( 'should share the same instance of plugin (dependencies) within editors using the same context', async () => { + class ContextPluginA extends ContextPlugin {} + class ContextPluginB extends ContextPlugin {} + class EditorPluginA extends Plugin { + static get requires() { + return [ ContextPluginA ]; + } + } + class EditorPluginB extends Plugin { + static get requires() { + return [ ContextPluginB ]; + } } - } - const context = await Context.create( { plugins: [ ContextPluginB ] } ); - const editor = await VirtualTestEditor.create( { context, plugins: [ 'ContextPluginA' ] } ); + const context = await Context.create( { plugins: [ ContextPluginA, ContextPluginB ] } ); + const editorA = await VirtualTestEditor.create( { context, plugins: [ EditorPluginA ] } ); + const editorB = await VirtualTestEditor.create( { context, plugins: [ EditorPluginB ] } ); - expect( editor.plugins.has( ContextPluginA ) ).to.equal( true ); - expect( editor.plugins.has( ContextPluginB ) ).to.equal( false ); - } ); + expect( context.plugins.get( ContextPluginA ) ).to.equal( editorA.plugins.get( ContextPluginA ) ); + expect( context.plugins.get( ContextPluginB ) ).to.equal( editorB.plugins.get( ContextPluginB ) ); - it( 'should throw when plugin is added to the context by name', async () => { - let caughtError; + await context.destroy(); + } ); - try { - await Context.create( { plugins: [ 'ContextPlugin' ] } ); - } catch ( error ) { - caughtError = error; - } + it( 'should not initialize twice plugin added to the context and the editor', async () => { + const initSpy = sinon.spy(); + const afterInitSpy = sinon.spy(); - expect( caughtError ).to.instanceof( CKEditorError ); - expect( caughtError.message ).match( /^context-initplugins: Only constructor is allowed as a Context plugin./ ); - } ); + class ContextPluginA extends ContextPlugin { + init() { + initSpy(); + } - it( 'should throw when plugin added to the context is not marked as a ContextPlugin #1', async () => { - class EditorPlugin extends Plugin {} + afterInit() { + afterInitSpy(); + } + } - let caughtError; + const context = await Context.create( { plugins: [ ContextPluginA ] } ); + const editor = await VirtualTestEditor.create( { context, plugins: [ ContextPluginA ] } ); - try { - await Context.create( { plugins: [ EditorPlugin ] } ); - } catch ( error ) { - caughtError = error; - } + expect( context.plugins.get( ContextPluginA ) ).to.equal( editor.plugins.get( ContextPluginA ) ); + sinon.assert.calledOnce( initSpy ); + sinon.assert.calledOnce( afterInitSpy ); - expect( caughtError ).to.instanceof( CKEditorError ); - expect( caughtError.message ).match( /^context-initplugins: Only plugins marked as a ContextPlugin are allowed./ ); - } ); + await context.destroy(); + } ); - it( 'should throw when plugin added to the context is not marked as a ContextPlugin #2', async () => { - function EditorPlugin() {} + it( 'should be able to add context plugin to the editor using pluginName property', async () => { + class ContextPluginA extends ContextPlugin { + static get pluginName() { + return 'ContextPluginA'; + } + } - let caughtError; + class ContextPluginB extends ContextPlugin { + static get pluginName() { + return 'ContextPluginB'; + } - try { - await Context.create( { plugins: [ EditorPlugin ] } ); - } catch ( error ) { - caughtError = error; - } + static get requires() { + return [ ContextPluginA ]; + } + } - expect( caughtError ).to.instanceof( CKEditorError ); - expect( caughtError.message ).match( /^context-initplugins: Only plugins marked as a ContextPlugin are allowed./ ); - } ); + const context = await Context.create( { plugins: [ ContextPluginB ] } ); + const editor = await VirtualTestEditor.create( { context, plugins: [ 'ContextPluginA' ] } ); - it( 'should not throw when plugin added to the context has not a ContextPlugin proto but is marked as a ContextPlugin', async () => { - function EditorPlugin() {} - EditorPlugin.isContextPlugin = true; + expect( editor.plugins.has( ContextPluginA ) ).to.equal( true ); + expect( editor.plugins.has( ContextPluginB ) ).to.equal( false ); + } ); + } ); - let caughtError; + describe( 'destroy()', () => { + it( 'should destroy plugins', async () => { + const context = await Context.create(); + const spy = sinon.spy( context.plugins, 'destroy' ); - try { - await Context.create( { plugins: [ EditorPlugin ] } ); - } catch ( error ) { - caughtError = error; - } + await context.destroy(); - expect( caughtError ).to.equal( undefined ); - } ); + sinon.assert.calledOnce( spy ); + } ); - it( 'should throw when plugin added to the context is not marked as a ContextPlugin #2', async () => { - function EditorPlugin() {} + it( 'should destroy all editors with injected context', async () => { + const context = await Context.create(); + const editorA = await VirtualTestEditor.create( { context } ); + const editorB = await VirtualTestEditor.create( { context } ); + const editorC = await VirtualTestEditor.create(); - let caughtError; + sinon.spy( editorA, 'destroy' ); + sinon.spy( editorB, 'destroy' ); + sinon.spy( editorC, 'destroy' ); - try { - await Context.create( { plugins: [ EditorPlugin ] } ); - } catch ( error ) { - caughtError = error; - } + await context.destroy(); - expect( caughtError ).to.instanceof( CKEditorError ); - expect( caughtError.message ).match( /^context-initplugins: Only plugins marked as a ContextPlugin are allowed./ ); + sinon.assert.calledOnce( editorA.destroy ); + sinon.assert.calledOnce( editorB.destroy ); + sinon.assert.notCalled( editorC.destroy ); + } ); } ); } ); diff --git a/tests/editor/editor.js b/tests/editor/editor.js index 820df969..bee5635e 100644 --- a/tests/editor/editor.js +++ b/tests/editor/editor.js @@ -6,6 +6,7 @@ /* globals window, setTimeout */ import Editor from '../../src/editor/editor'; +import Context from '../../src/context'; import Plugin from '../../src/plugin'; import Config from '@ckeditor/ckeditor5-utils/src/config'; import EditingController from '@ckeditor/ckeditor5-engine/src/controller/editingcontroller'; @@ -180,41 +181,67 @@ describe( 'Editor', () => { } ); } ); - describe( 'plugins', () => { - it( 'should be empty on new editor', () => { + describe( 'context integration', () => { + it( 'should create a new context when it is not provided through config', () => { const editor = new TestEditor(); - expect( getPlugins( editor ) ).to.be.empty; + expect( editor.context ).to.be.an.instanceof( Context ); } ); - } ); - describe( 'locale', () => { - it( 'is instantiated and t() is exposed', () => { - const editor = new TestEditor(); + it( 'should use context given through configuration when is defined', async () => { + const context = await Context.create(); + const editor = new TestEditor( { context } ); - expect( editor.locale ).to.be.instanceof( Locale ); - expect( editor.t ).to.equal( editor.locale.t ); + expect( editor.context ).to.equal( context ); } ); - it( 'is configured with the config.language (UI and the content)', () => { - const editor = new TestEditor( { language: 'pl' } ); + it( 'should destroy context created by the editor on editor destroy', async () => { + const editor = await TestEditor.create(); + const contextDestroySpy = sinon.spy( editor.context, 'destroy' ); + + await editor.destroy(); - expect( editor.locale.uiLanguage ).to.equal( 'pl' ); - expect( editor.locale.contentLanguage ).to.equal( 'pl' ); + sinon.assert.calledOnce( contextDestroySpy ); } ); - it( 'is configured with the config.language (different for UI and the content)', () => { - const editor = new TestEditor( { language: { ui: 'pl', content: 'ar' } } ); + it( 'should not destroy context along with the editor when context was injected to the editor', async () => { + const context = await Context.create(); + const editor = await TestEditor.create( { context } ); + const contextDestroySpy = sinon.spy( editor.context, 'destroy' ); - expect( editor.locale.uiLanguage ).to.equal( 'pl' ); - expect( editor.locale.contentLanguage ).to.equal( 'ar' ); + await editor.destroy(); + + sinon.assert.notCalled( contextDestroySpy ); } ); - it( 'is configured with the config.language (just the content)', () => { - const editor = new TestEditor( { language: { content: 'ar' } } ); + it( 'should add context plugins as an external editor plugins', async () => { + class ContextPlugin { + static get isContextPlugin() { + return true; + } + } + + const context = await Context.create( { plugins: [ ContextPlugin ] } ); + const editor = new TestEditor( { context } ); + + expect( editor.plugins._externalPlugins.has( ContextPlugin ) ).to.equal( true ); + } ); + } ); + + describe( 'plugins', () => { + it( 'should be empty on new editor', () => { + const editor = new TestEditor(); + + expect( getPlugins( editor ) ).to.be.empty; + } ); + } ); + + describe( 'locale', () => { + it( 'should use Context#locale and Context#t', () => { + const editor = new TestEditor(); - expect( editor.locale.uiLanguage ).to.equal( 'en' ); - expect( editor.locale.contentLanguage ).to.equal( 'ar' ); + expect( editor.locale ).to.equal( editor.context.locale ).to.instanceof( Locale ); + expect( editor.t ).to.equal( editor.context.t ); } ); } ); diff --git a/tests/plugincollection.js b/tests/plugincollection.js index 32aeb512..4c5ae954 100644 --- a/tests/plugincollection.js +++ b/tests/plugincollection.js @@ -405,6 +405,40 @@ describe( 'PluginCollection', () => { sinon.assert.calledOnce( consoleErrorStub ); } ); } ); + + it( 'should get plugin from external plugins instead of creating new instance', async () => { + const externalPlugins = new PluginCollection( editor ); + await externalPlugins.init( [ PluginA, PluginB ] ); + + const plugins = new PluginCollection( editor, [], Array.from( externalPlugins ) ); + await plugins.init( [ PluginA ] ); + + expect( getPlugins( plugins ) ).to.length( 1 ); + expect( plugins.get( PluginA ) ).to.equal( externalPlugins.get( PluginA ) ).to.instanceof( PluginA ); + } ); + + it( 'should get plugin by name from external plugins instead of creating new instance', async () => { + const externalPlugins = new PluginCollection( editor ); + await externalPlugins.init( [ PluginA, PluginB ] ); + + const plugins = new PluginCollection( editor, [], Array.from( externalPlugins ) ); + await plugins.init( [ 'A' ] ); + + expect( getPlugins( plugins ) ).to.length( 1 ); + expect( plugins.get( PluginA ) ).to.equal( externalPlugins.get( PluginA ) ).to.instanceof( PluginA ); + } ); + + it( 'should get dependency of plugin from external plugins instead of creating new instance', async () => { + const externalPlugins = new PluginCollection( editor ); + await externalPlugins.init( [ PluginA, PluginB ] ); + + const plugins = new PluginCollection( editor, [], Array.from( externalPlugins ) ); + await plugins.init( [ PluginC ] ); + + expect( getPlugins( plugins ) ).to.length( 2 ); + expect( plugins.get( PluginB ) ).to.equal( externalPlugins.get( PluginB ) ).to.instanceof( PluginB ); + expect( plugins.get( PluginC ) ).to.instanceof( PluginC ); + } ); } ); describe( 'get()', () => { From 10c3fb61d74dc1a65df453292bc1893ce71f1c6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 31 Oct 2019 19:37:00 +0100 Subject: [PATCH 07/37] Fixed invalid import path. --- src/editor/editor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/editor.js b/src/editor/editor.js index f673a5e4..2c081b20 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -7,7 +7,7 @@ * @module core/editor/editor */ -import Context from '@ckeditor/ckeditor5-core/src/context'; +import Context from '../context'; import Config from '@ckeditor/ckeditor5-utils/src/config'; import EditingController from '@ckeditor/ckeditor5-engine/src/controller/editingcontroller'; import PluginCollection from '../plugincollection'; From 12c887687506bbbd16729dda7402c55641691be2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 4 Nov 2019 12:05:14 +0100 Subject: [PATCH 08/37] Changed PendingActions to ScopePlugin. --- src/pendingactions.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pendingactions.js b/src/pendingactions.js index 8bddfb0b..ad90ead0 100644 --- a/src/pendingactions.js +++ b/src/pendingactions.js @@ -7,7 +7,7 @@ * @module core/pendingactions */ -import Plugin from './plugin'; +import ContextPlugin from './contextplugin'; import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin'; import Collection from '@ckeditor/ckeditor5-utils/src/collection'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; @@ -52,7 +52,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; * * @extends module:core/plugin~Plugin */ -export default class PendingActions extends Plugin { +export default class PendingActions extends ContextPlugin { /** * @inheritDoc */ From fffc15e370fc04a644fd50afdd2bc36129bc12c2 Mon Sep 17 00:00:00 2001 From: Piotr Jasiun Date: Thu, 7 Nov 2019 13:44:59 +0100 Subject: [PATCH 09/37] Check if context plugins requires only context plugins. --- src/plugincollection.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/plugincollection.js b/src/plugincollection.js index cfd96c21..6cade6d0 100644 --- a/src/plugincollection.js +++ b/src/plugincollection.js @@ -281,6 +281,14 @@ export default class PluginCollection { PluginConstructor.requires.forEach( RequiredPluginConstructorOrName => { const RequiredPluginConstructor = getPluginConstructor( RequiredPluginConstructorOrName ); + if ( PluginConstructor.isContextPlugin && !RequiredPluginConstructor.isContextPlugin ) { + throw new CKEditorError( + 'plugincollection-context-required: Context plugin can not require plugin which is not context plugin', + null, + { plugin: RequiredPluginConstructor.name, requiredBy: PluginConstructor.name } + ); + } + if ( removePlugins.includes( RequiredPluginConstructor ) ) { /** * Cannot load a plugin because one of its dependencies is listed in the `removePlugins` option. @@ -293,7 +301,7 @@ export default class PluginCollection { 'plugincollection-required: Cannot load a plugin because one of its dependencies is listed in' + 'the `removePlugins` option.', editor, - { plugin: RequiredPluginConstructor, requiredBy: PluginConstructor } + { plugin: RequiredPluginConstructor.name, requiredBy: PluginConstructor.name } ); } From 8c5702ec835d0e55b040da1d06d0f840892cb2c5 Mon Sep 17 00:00:00 2001 From: Piotr Jasiun Date: Wed, 20 Nov 2019 16:55:55 +0100 Subject: [PATCH 10/37] Docs: Fix incorrect link in docs. --- src/context.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/context.js b/src/context.js index dd396f59..0b445bd7 100644 --- a/src/context.js +++ b/src/context.js @@ -19,7 +19,7 @@ export default class Context { /** * Creates a context instance with a given configuration. * - * Usually, not to be used directly. See the static {@link ~Context.create `create()`} method. + * Usually, not to be used directly. See the static {@link module:core/context~Context.create `create()`} method. * * @param {Object} [config={}] The context config. */ From 120a04322ad82b36d53f17a685c042c97d00560c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 5 Dec 2019 14:50:25 +0100 Subject: [PATCH 11/37] Docs: Added missing error description for ContextPlugin requiring non-ContextPlugin. --- src/plugincollection.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/plugincollection.js b/src/plugincollection.js index 6cade6d0..9492d1c4 100644 --- a/src/plugincollection.js +++ b/src/plugincollection.js @@ -282,8 +282,20 @@ export default class PluginCollection { const RequiredPluginConstructor = getPluginConstructor( RequiredPluginConstructorOrName ); if ( PluginConstructor.isContextPlugin && !RequiredPluginConstructor.isContextPlugin ) { + /** + * If a plugin is a `ContextPlugin` all plugins it requires should also be a `ContextPlugin`, + * instead of `Plugin`. In other words, if one plugin can be used in the `Context`, + * all its requirements also should be ready to be used in the`Context`. Note that context + * provides only a part of the API provided by the editor. If one plugin needs a full + * editor API, all plugins which require it, are considered as plugins which need a full + * editor API. + * + * @error plugincollection-context-required + * @param {String} plugin The name of the required plugin. + * @param {String} requiredBy The name of the parent plugin. + */ throw new CKEditorError( - 'plugincollection-context-required: Context plugin can not require plugin which is not context plugin', + 'plugincollection-context-required: Context plugin can not require plugin which is not a context plugin', null, { plugin: RequiredPluginConstructor.name, requiredBy: PluginConstructor.name } ); @@ -294,8 +306,8 @@ export default class PluginCollection { * Cannot load a plugin because one of its dependencies is listed in the `removePlugins` option. * * @error plugincollection-required - * @param {Function} plugin The required plugin. - * @param {Function} requiredBy The parent plugin. + * @param {String} plugin The name of the required plugin. + * @param {String} requiredBy The name of the parent plugin. */ throw new CKEditorError( 'plugincollection-required: Cannot load a plugin because one of its dependencies is listed in' + From 21d714e82b41c866403bb4f370de7229cef76d57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 5 Dec 2019 14:53:18 +0100 Subject: [PATCH 12/37] Tests: Added tests for ContextPlugin dependencies. --- tests/plugincollection.js | 46 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/plugincollection.js b/tests/plugincollection.js index 4c5ae954..33488587 100644 --- a/tests/plugincollection.js +++ b/tests/plugincollection.js @@ -8,6 +8,7 @@ import Editor from '../src/editor/editor'; import PluginCollection from '../src/plugincollection'; import Plugin from '../src/plugin'; +import ContextPlugin from '../src/contextplugin'; import { expectToThrowCKEditorError, assertCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; let editor, availablePlugins; @@ -338,6 +339,51 @@ describe( 'PluginCollection', () => { } ); } ); + it( 'should throw when context plugin requires not a context plugin', async () => { + class FooContextPlugin extends ContextPlugin {} + FooContextPlugin.requires = [ PluginA ]; + + const plugins = new PluginCollection( editor, [ FooContextPlugin, PluginA ] ); + + const consoleErrorStub = sinon.stub( console, 'error' ); + let error; + + try { + await plugins.init( [ FooContextPlugin ] ); + } catch ( err ) { + error = err; + } + + assertCKEditorError( error, /^plugincollection-context-required/ ); + sinon.assert.calledOnce( consoleErrorStub ); + } ); + + it( 'should not throw when non context plugin requires context plugin', async () => { + class FooContextPlugin extends ContextPlugin {} + + class BarPlugin extends Plugin {} + BarPlugin.requires = [ FooContextPlugin ]; + + const plugins = new PluginCollection( editor, [ FooContextPlugin, BarPlugin ] ); + + await plugins.init( [ BarPlugin ] ); + + expect( getPlugins( plugins ) ).to.length( 2 ); + } ); + + it( 'should not throw when context plugin requires context plugin', async () => { + class FooContextPlugin extends ContextPlugin {} + + class BarContextPlugin extends ContextPlugin {} + BarContextPlugin.requires = [ FooContextPlugin ]; + + const plugins = new PluginCollection( editor, [ FooContextPlugin, BarContextPlugin ] ); + + await plugins.init( [ BarContextPlugin ] ); + + expect( getPlugins( plugins ) ).to.length( 2 ); + } ); + it( 'should reject when loaded plugin requires not allowed plugins', () => { const consoleErrorStub = sinon.stub( console, 'error' ); const plugins = new PluginCollection( editor, availablePlugins ); From 6cab169145a328bcce32f45af4272cd6d5ef3840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 5 Dec 2019 14:54:14 +0100 Subject: [PATCH 13/37] Used Context configuration as a default Editor configuration. --- src/context.js | 20 ++++++++++++++++++++ src/editor/editor.js | 3 ++- tests/context.js | 23 +++++++++++++++++++++++ tests/editor/editor.js | 37 ++++++++++++++++++++++++++++++++++++- 4 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/context.js b/src/context.js index 0b445bd7..ad22721a 100644 --- a/src/context.js +++ b/src/context.js @@ -110,6 +110,26 @@ export default class Context { return this.plugins.init( plugins ); } + /** + * Returns context configuration which will be copied to editors created using this context. + * + * The configuration returned by this method has removed plugins configuration - plugins are shared with all editors + * in a special way. + * + * @returns {Object} Configuration as a plain object. + */ + getConfigForEditor() { + const result = {}; + + for ( const name of this.config.names() ) { + if ( ![ 'plugins', 'removePlugins', 'extraPlugins' ].includes( name ) ) { + result[ name ] = this.config.get( name ); + } + } + + return result; + } + /** * Destroys the context instance, releasing all resources used by it. * diff --git a/src/editor/editor.js b/src/editor/editor.js index 2c081b20..a3136269 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -55,7 +55,7 @@ export default class Editor { * @readonly * @type {module:core/context~Context} */ - this.context = config.context || new Context( config ); + this.context = config.context || new Context(); this.context.addEditor( this ); /** @@ -79,6 +79,7 @@ export default class Editor { */ this.config = new Config( config, this.constructor.defaultConfig ); this.config.define( 'plugins', availablePlugins ); + this.config.define( this.context.getConfigForEditor() ); /** * The plugins loaded and in use by this editor instance. diff --git a/tests/context.js b/tests/context.js index ff9bbd84..afc2b487 100644 --- a/tests/context.js +++ b/tests/context.js @@ -26,6 +26,29 @@ describe( 'Context', () => { } ); } ); + describe( 'getConfigForEditor()', () => { + it( 'should return the configuration without plugin config', () => { + class FooPlugin extends ContextPlugin {} + class BarPlugin extends ContextPlugin {} + class BomPlugin extends ContextPlugin {} + + const context = new Context( { + language: { ui: 'pl', content: 'ar' }, + plugins: [ FooPlugin, BarPlugin ], + extraPlugins: [ BomPlugin ], + removePlugins: [ FooPlugin ], + foo: 1, + bar: 'bom' + } ); + + expect( context.getConfigForEditor() ).to.be.deep.equal( { + language: { ui: 'pl', content: 'ar' }, + foo: 1, + bar: 'bom' + } ); + } ); + } ); + describe( 'locale', () => { it( 'is instantiated and t() is exposed', () => { const context = new Context(); diff --git a/tests/editor/editor.js b/tests/editor/editor.js index bee5635e..1802bbbc 100644 --- a/tests/editor/editor.js +++ b/tests/editor/editor.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* globals window, setTimeout */ +/* globals document, window, setTimeout */ import Editor from '../../src/editor/editor'; import Context from '../../src/context'; @@ -226,6 +226,41 @@ describe( 'Editor', () => { expect( editor.plugins._externalPlugins.has( ContextPlugin ) ).to.equal( true ); } ); + + it( 'should get configuration from the context', async () => { + const context = await Context.create( { cfoo: 'bar' } ); + const editor = await TestEditor.create( { context } ); + + expect( editor.config.get( 'cfoo' ) ).to.equal( 'bar' ); + } ); + + it( 'should not overwrite the default configuration', async () => { + const context = await Context.create( { cfoo: 'bar' } ); + const editor = await TestEditor.create( { context, 'cfoo': 'bom' } ); + + expect( editor.config.get( 'cfoo' ) ).to.equal( 'bom' ); + } ); + + it( 'should not copy plugins configuration', async () => { + class ContextPlugin { + static get isContextPlugin() { + return true; + } + } + + const context = await Context.create( { plugins: [ ContextPlugin ] } ); + const editor = await TestEditor.create( { context } ); + + expect( editor.config.get( 'plugins' ) ).to.be.undefined; + } ); + + it( 'should pass DOM element using reference, not copy', async () => { + const element = document.createElement( 'div' ); + const context = await Context.create( { efoo: element } ); + const editor = await TestEditor.create( { context } ); + + expect( editor.config.get( 'efoo' ) ).to.equal( element ); + } ); } ); describe( 'plugins', () => { From 5a5793e89eb3d7294e7a172f34377784914df368 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 5 Dec 2019 14:55:57 +0100 Subject: [PATCH 14/37] Improved ids of the context errors. --- src/context.js | 14 ++++++++++---- tests/context.js | 13 ++++++++----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/context.js b/src/context.js index ad22721a..9e138bb2 100644 --- a/src/context.js +++ b/src/context.js @@ -72,8 +72,6 @@ export default class Context { * Adds a reference to the editor to which context is injected. * * @param {module:core/editor/editor~Editor} editor - * @param {Boolean} isHost Flag defines if context was created by this editor. It is used to decide if the context - * should be destroyed along with the editor instance. */ addEditor( editor ) { this._editors.add( editor ); @@ -99,11 +97,19 @@ export default class Context { for ( const Plugin of plugins ) { if ( typeof Plugin != 'function' ) { - throw new CKEditorError( 'context-initplugins: Only constructor is allowed as a Context plugin.', null, { Plugin } ); + throw new CKEditorError( + 'context-initplugins-constructor-only: Only constructor is allowed as a Context plugin.', + null, + { Plugin } + ); } if ( Plugin.isContextPlugin !== true ) { - throw new CKEditorError( 'context-initplugins: Only plugins marked as a ContextPlugin are allowed.', null, { Plugin } ); + throw new CKEditorError( + 'context-initplugins-invalid-plugin: Only plugin marked as a ContextPlugin is allowed.', + null, + { Plugin } + ); } } diff --git a/tests/context.js b/tests/context.js index afc2b487..8d8c17c8 100644 --- a/tests/context.js +++ b/tests/context.js @@ -80,7 +80,7 @@ describe( 'Context', () => { } ); describe( 'plugins', () => { - it( 'should throw when plugin added to the context is not marked as a ContextPlugin #1', async () => { + it( 'should throw when plugin added to the context is not marked as a ContextPlugin (Plugin)', async () => { class EditorPlugin extends Plugin {} let caughtError; @@ -92,10 +92,11 @@ describe( 'Context', () => { } expect( caughtError ).to.instanceof( CKEditorError ); - expect( caughtError.message ).match( /^context-initplugins: Only plugins marked as a ContextPlugin are allowed./ ); + expect( caughtError.message ) + .match( /^context-initplugins-invalid-plugin: Only plugin marked as a ContextPlugin is allowed./ ); } ); - it( 'should throw when plugin added to the context is not marked as a ContextPlugin #2', async () => { + it( 'should throw when plugin added to the context is not marked as a ContextPlugin (Function)', async () => { function EditorPlugin() {} let caughtError; @@ -107,7 +108,8 @@ describe( 'Context', () => { } expect( caughtError ).to.instanceof( CKEditorError ); - expect( caughtError.message ).match( /^context-initplugins: Only plugins marked as a ContextPlugin are allowed./ ); + expect( caughtError.message ) + .match( /^context-initplugins-invalid-plugin: Only plugin marked as a ContextPlugin is allowed./ ); } ); it( 'should throw when plugin is added to the context by name', async () => { @@ -120,7 +122,8 @@ describe( 'Context', () => { } expect( caughtError ).to.instanceof( CKEditorError ); - expect( caughtError.message ).match( /^context-initplugins: Only constructor is allowed as a Context plugin./ ); + expect( caughtError.message ) + .match( /^context-initplugins-constructor-only: Only constructor is allowed as a Context plugin./ ); } ); it( 'should not throw when plugin as a function, marked as a ContextPlugin is added to the context', async () => { From 41ce83cb6d2e26ea044b553d8cb1d1df4cee4dee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 17 Dec 2019 13:14:34 +0100 Subject: [PATCH 15/37] Added language configuration to the context when it is created by the editor. --- src/editor/editor.js | 2 +- tests/editor/editor.js | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/editor/editor.js b/src/editor/editor.js index a3136269..14a5925f 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -55,7 +55,7 @@ export default class Editor { * @readonly * @type {module:core/context~Context} */ - this.context = config.context || new Context(); + this.context = config.context || new Context( { language: config.language } ); this.context.addEditor( this ); /** diff --git a/tests/editor/editor.js b/tests/editor/editor.js index 1802bbbc..59b66e71 100644 --- a/tests/editor/editor.js +++ b/tests/editor/editor.js @@ -278,6 +278,15 @@ describe( 'Editor', () => { expect( editor.locale ).to.equal( editor.context.locale ).to.instanceof( Locale ); expect( editor.t ).to.equal( editor.context.t ); } ); + + it( 'should use locale instance with a proper configuration', () => { + const editor = new TestEditor( { + language: 'pl' + } ); + + expect( editor.locale ).to.have.property( 'uiLanguage', 'pl' ); + expect( editor.locale ).to.have.property( 'contentLanguage', 'pl' ); + } ); } ); describe( 'state', () => { From e3bbf8afba36425c422535788aaced566d82b034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 17 Dec 2019 13:57:23 +0100 Subject: [PATCH 16/37] Changed the way of marking context as created by the editor. --- src/context.js | 35 ++++++++++++++++++++++++++++++++--- src/editor/editor.js | 19 +++++++++---------- tests/editor/editor.js | 11 +++++++++++ 3 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/context.js b/src/context.js index 9e138bb2..1cf0f007 100644 --- a/src/context.js +++ b/src/context.js @@ -59,6 +59,15 @@ export default class Context { */ this.t = this.locale.t; + /** + * When the context is created by an editor then the editor instance is + * stored as an owner of this context. + * + * @readonly + * @type {Boolean} + */ + this.isCreatedByEditor = false; + /** * List of editors to which this context instance is injected. * @@ -69,16 +78,36 @@ export default class Context { } /** - * Adds a reference to the editor to which context is injected. + * Adds a reference to the editor which is used with this context. + * + * When the context is created by the editor it is additionally + * marked as a {@link ~Context#isCreatedByEditor} what is used + * in the destroy chain. * * @param {module:core/editor/editor~Editor} editor + * @param {Boolean} isContextOwner */ - addEditor( editor ) { + addEditor( editor, isContextOwner ) { + if ( this.isCreatedByEditor ) { + /** + * Cannot add multiple editors to the context which is created by the editor. + * + * @error context-addEditor-to-private-context + */ + throw new CKEditorError( + 'context-addEditor-to-private-context: Cannot add multiple editors to the context which is created by the editor.' + ); + } + this._editors.add( editor ); + + if ( isContextOwner ) { + this.isCreatedByEditor = true; + } } /** - * Removes a reference to the editor to which context was injected. + * Removes a reference to the editor which was used with this context. * * @param {module:core/editor/editor~Editor} editor */ diff --git a/src/editor/editor.js b/src/editor/editor.js index 14a5925f..11822a9f 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -52,19 +52,14 @@ export default class Editor { */ constructor( config = {} ) { /** + * The editor context. + * When it is not provided through the configuration then the editor creates it. + * * @readonly * @type {module:core/context~Context} */ this.context = config.context || new Context( { language: config.language } ); - this.context.addEditor( this ); - - /** - * `true` when context is created by this editor or `false` when context is injected to the editor. - * - * @private - * @type {Boolean} - */ - this._isContextHost = !config.context; + this.context.addEditor( this, !config.context ); const availablePlugins = this.constructor.builtinPlugins; @@ -257,9 +252,13 @@ export default class Editor { return readyPromise .then( () => { + // Remove the editor from the context to avoid destroying it + // one more time when context will be destroyed. this.context.removeEditor( this ); - if ( this._isContextHost ) { + // When the editor was an owner of the context then + // the context should be destroyed along with the editor. + if ( this.context.isCreatedByEditor ) { return this.context.destroy(); } } ).then( () => { diff --git a/tests/editor/editor.js b/tests/editor/editor.js index 59b66e71..b284175e 100644 --- a/tests/editor/editor.js +++ b/tests/editor/editor.js @@ -186,6 +186,7 @@ describe( 'Editor', () => { const editor = new TestEditor(); expect( editor.context ).to.be.an.instanceof( Context ); + expect( editor.context.isCreatedByEditor ).to.true; } ); it( 'should use context given through configuration when is defined', async () => { @@ -193,6 +194,16 @@ describe( 'Editor', () => { const editor = new TestEditor( { context } ); expect( editor.context ).to.equal( context ); + expect( editor.context.isCreatedByEditor ).to.false; + } ); + + it( 'should throw when try to use context created by one editor with the other editor', () => { + const editor = new TestEditor(); + + expectToThrowCKEditorError( () => { + // eslint-disable-next-line no-new + new TestEditor( { context: editor.context } ); + }, /^context-addEditor-to-private-context/ ); } ); it( 'should destroy context created by the editor on editor destroy', async () => { From 837e6b8c2d233d6a8cd32bb92c783eb0626dbc64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 17 Dec 2019 14:20:20 +0100 Subject: [PATCH 17/37] Improved docs. --- src/context.js | 12 +++++++++++- src/plugincollection.js | 18 +++++++++--------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/context.js b/src/context.js index 1cf0f007..05591e40 100644 --- a/src/context.js +++ b/src/context.js @@ -126,6 +126,11 @@ export default class Context { for ( const Plugin of plugins ) { if ( typeof Plugin != 'function' ) { + /** + * Only constructor is allowed as a Context plugin. + * + * @error context-initplugins-constructor-only + */ throw new CKEditorError( 'context-initplugins-constructor-only: Only constructor is allowed as a Context plugin.', null, @@ -134,6 +139,11 @@ export default class Context { } if ( Plugin.isContextPlugin !== true ) { + /** + * Only plugin marked as a ContextPlugin is allowed to be used with a context. + * + * @error context-initplugins-invalid-plugin + */ throw new CKEditorError( 'context-initplugins-invalid-plugin: Only plugin marked as a ContextPlugin is allowed.', null, @@ -149,7 +159,7 @@ export default class Context { * Returns context configuration which will be copied to editors created using this context. * * The configuration returned by this method has removed plugins configuration - plugins are shared with all editors - * in a special way. + * through another mechanism. * * @returns {Object} Configuration as a plain object. */ diff --git a/src/plugincollection.js b/src/plugincollection.js index 9492d1c4..7eccd5c8 100644 --- a/src/plugincollection.js +++ b/src/plugincollection.js @@ -25,7 +25,7 @@ export default class PluginCollection { * Allows loading and initializing plugins and their dependencies. * Allows to provide a list of already loaded plugins, these plugins won't be destroyed along with this collection. * - * @param {module:core/editor/editor~Editor} editor + * @param {module:core/editor/editor~Editor|module:core/context~Context} context * @param {Array.} [availablePlugins] Plugins (constructors) which the collection will be able to use * when {@link module:core/plugincollection~PluginCollection#init} is used with plugin names (strings, instead of constructors). * Usually, the editor will pass its built-in plugins to the collection so they can later be @@ -33,12 +33,12 @@ export default class PluginCollection { * @param {Iterable.} externalPlugins List of already initialized plugins represented by a * `[ PluginConstructor, pluginInstance ]` pair. */ - constructor( editor, availablePlugins = [], externalPlugins = [] ) { + constructor( context, availablePlugins = [], externalPlugins = [] ) { /** * @protected - * @type {module:core/editor/editor~Editor} + * @type {module:core/editor/editor~Editor|module:core/context~Context} */ - this._editor = editor; + this._context = context; /** * @protected @@ -139,7 +139,7 @@ export default class PluginCollection { pluginName = key.pluginName || key.name; } - throw new CKEditorError( errorMsg, this._editor, { plugin: pluginName } ); + throw new CKEditorError( errorMsg, this._context, { plugin: pluginName } ); } return plugin; @@ -176,7 +176,7 @@ export default class PluginCollection { */ init( plugins, removePlugins = [] ) { const that = this; - const editor = this._editor; + const context = this._context; const loading = new Set(); const loaded = []; @@ -211,7 +211,7 @@ export default class PluginCollection { // Log the error so it's more visible on the console. Hopefully, for better DX. console.error( attachLinkToDocumentation( errorMsg ), { plugins: missingPlugins } ); - return Promise.reject( new CKEditorError( errorMsg, this._editor, { plugins: missingPlugins } ) ); + return Promise.reject( new CKEditorError( errorMsg, context, { plugins: missingPlugins } ) ); } return Promise.all( pluginConstructors.map( loadPlugin ) ) @@ -312,7 +312,7 @@ export default class PluginCollection { throw new CKEditorError( 'plugincollection-required: Cannot load a plugin because one of its dependencies is listed in' + 'the `removePlugins` option.', - editor, + context, { plugin: RequiredPluginConstructor.name, requiredBy: PluginConstructor.name } ); } @@ -321,7 +321,7 @@ export default class PluginCollection { } ); } - const plugin = that._externalPlugins.get( PluginConstructor ) || new PluginConstructor( editor ); + const plugin = that._externalPlugins.get( PluginConstructor ) || new PluginConstructor( context ); that._add( PluginConstructor, plugin ); loaded.push( plugin ); From 76d3201d0acb2227c15923d7c04f735871b70f93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 17 Dec 2019 14:28:22 +0100 Subject: [PATCH 18/37] Changed Editor#context property from readonly to protected. --- src/editor/editor.js | 18 +++++++++--------- tests/editor/editor.js | 18 +++++++++--------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/editor/editor.js b/src/editor/editor.js index 11822a9f..c26ca950 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -55,11 +55,11 @@ export default class Editor { * The editor context. * When it is not provided through the configuration then the editor creates it. * - * @readonly + * @protected * @type {module:core/context~Context} */ - this.context = config.context || new Context( { language: config.language } ); - this.context.addEditor( this, !config.context ); + this._context = config.context || new Context( { language: config.language } ); + this._context.addEditor( this, !config.context ); const availablePlugins = this.constructor.builtinPlugins; @@ -74,7 +74,7 @@ export default class Editor { */ this.config = new Config( config, this.constructor.defaultConfig ); this.config.define( 'plugins', availablePlugins ); - this.config.define( this.context.getConfigForEditor() ); + this.config.define( this._context.getConfigForEditor() ); /** * The plugins loaded and in use by this editor instance. @@ -84,13 +84,13 @@ export default class Editor { * @readonly * @member {module:core/plugincollection~PluginCollection} */ - this.plugins = new PluginCollection( this, availablePlugins, this.context.plugins ); + this.plugins = new PluginCollection( this, availablePlugins, this._context.plugins ); /** * @readonly * @type {module:utils/locale~Locale} */ - this.locale = this.context.locale; + this.locale = this._context.locale; /** * Shorthand for {@link module:utils/locale~Locale#t}. @@ -254,12 +254,12 @@ export default class Editor { .then( () => { // Remove the editor from the context to avoid destroying it // one more time when context will be destroyed. - this.context.removeEditor( this ); + this._context.removeEditor( this ); // When the editor was an owner of the context then // the context should be destroyed along with the editor. - if ( this.context.isCreatedByEditor ) { - return this.context.destroy(); + if ( this._context.isCreatedByEditor ) { + return this._context.destroy(); } } ).then( () => { this.fire( 'destroy' ); diff --git a/tests/editor/editor.js b/tests/editor/editor.js index b284175e..70d91e5e 100644 --- a/tests/editor/editor.js +++ b/tests/editor/editor.js @@ -185,16 +185,16 @@ describe( 'Editor', () => { it( 'should create a new context when it is not provided through config', () => { const editor = new TestEditor(); - expect( editor.context ).to.be.an.instanceof( Context ); - expect( editor.context.isCreatedByEditor ).to.true; + expect( editor._context ).to.be.an.instanceof( Context ); + expect( editor._context.isCreatedByEditor ).to.true; } ); it( 'should use context given through configuration when is defined', async () => { const context = await Context.create(); const editor = new TestEditor( { context } ); - expect( editor.context ).to.equal( context ); - expect( editor.context.isCreatedByEditor ).to.false; + expect( editor._context ).to.equal( context ); + expect( editor._context.isCreatedByEditor ).to.false; } ); it( 'should throw when try to use context created by one editor with the other editor', () => { @@ -202,13 +202,13 @@ describe( 'Editor', () => { expectToThrowCKEditorError( () => { // eslint-disable-next-line no-new - new TestEditor( { context: editor.context } ); + new TestEditor( { context: editor._context } ); }, /^context-addEditor-to-private-context/ ); } ); it( 'should destroy context created by the editor on editor destroy', async () => { const editor = await TestEditor.create(); - const contextDestroySpy = sinon.spy( editor.context, 'destroy' ); + const contextDestroySpy = sinon.spy( editor._context, 'destroy' ); await editor.destroy(); @@ -218,7 +218,7 @@ describe( 'Editor', () => { it( 'should not destroy context along with the editor when context was injected to the editor', async () => { const context = await Context.create(); const editor = await TestEditor.create( { context } ); - const contextDestroySpy = sinon.spy( editor.context, 'destroy' ); + const contextDestroySpy = sinon.spy( editor._context, 'destroy' ); await editor.destroy(); @@ -286,8 +286,8 @@ describe( 'Editor', () => { it( 'should use Context#locale and Context#t', () => { const editor = new TestEditor(); - expect( editor.locale ).to.equal( editor.context.locale ).to.instanceof( Locale ); - expect( editor.t ).to.equal( editor.context.t ); + expect( editor.locale ).to.equal( editor._context.locale ).to.instanceof( Locale ); + expect( editor.t ).to.equal( editor._context.t ); } ); it( 'should use locale instance with a proper configuration', () => { From bfc3626ebca5999e36f8e60e349ed8bdba6aa004 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 18 Dec 2019 13:09:03 +0100 Subject: [PATCH 19/37] Added isContextPlugin property to the PluginInterface. --- src/contextplugin.js | 4 +--- src/plugin.js | 15 +++++++++++++++ tests/contextplugin.js | 26 +++++++++++++++----------- tests/plugin.js | 26 +++++++++++++++----------- 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/src/contextplugin.js b/src/contextplugin.js index 6bcc1b11..e24bd74f 100644 --- a/src/contextplugin.js +++ b/src/contextplugin.js @@ -40,9 +40,7 @@ export default class ContextPlugin { } /** - * Static property which marks plugin as an allowed to be use directly by a {@link module:core/context~Context}. - * - * @returns {Boolean} + * @inheritDoc */ static get isContextPlugin() { return true; diff --git a/src/plugin.js b/src/plugin.js index 506387ee..0aefd8ac 100644 --- a/src/plugin.js +++ b/src/plugin.js @@ -46,6 +46,13 @@ export default class Plugin { destroy() { this.stopListening(); } + + /** + * @inheritDoc + */ + static get isContextPlugin() { + return false; + } } mix( Plugin, ObservableMixin ); @@ -180,6 +187,14 @@ mix( Plugin, ObservableMixin ); * @returns {null|Promise} */ +/** + * A flag which defines if plugin is allowed or not allowed to be use directly by a {@link module:core/context~Context}. + * + * @static + * @readonly + * @member {Boolean} module:core/plugin~PluginInterface.isContextPlugin + */ + /** * Array of loaded plugins. * diff --git a/tests/contextplugin.js b/tests/contextplugin.js index 93a93f8c..4eddf753 100644 --- a/tests/contextplugin.js +++ b/tests/contextplugin.js @@ -8,28 +8,32 @@ import ContextPlugin from '../src/contextplugin'; describe( 'ContextPlugin', () => { const contextMock = {}; + it( 'should be marked as a context plugin', () => { + expect( ContextPlugin.isContextPlugin ).to.true; + } ); + describe( 'constructor()', () => { it( 'should set the `context` property', () => { const plugin = new ContextPlugin( contextMock ); expect( plugin ).to.have.property( 'context' ).to.equal( contextMock ); } ); + } ); - describe( 'destroy()', () => { - it( 'should be defined', () => { - const plugin = new ContextPlugin( contextMock ); + describe( 'destroy()', () => { + it( 'should be defined', () => { + const plugin = new ContextPlugin( contextMock ); - expect( plugin.destroy ).to.be.a( 'function' ); - } ); + expect( plugin.destroy ).to.be.a( 'function' ); + } ); - it( 'should stop listening', () => { - const plugin = new ContextPlugin( contextMock ); - const stopListeningSpy = sinon.spy( plugin, 'stopListening' ); + it( 'should stop listening', () => { + const plugin = new ContextPlugin( contextMock ); + const stopListeningSpy = sinon.spy( plugin, 'stopListening' ); - plugin.destroy(); + plugin.destroy(); - sinon.assert.calledOnce( stopListeningSpy ); - } ); + sinon.assert.calledOnce( stopListeningSpy ); } ); } ); } ); diff --git a/tests/plugin.js b/tests/plugin.js index 7ca1fb16..df05928d 100644 --- a/tests/plugin.js +++ b/tests/plugin.js @@ -13,28 +13,32 @@ describe( 'Plugin', () => { editor = new Editor(); } ); + it( 'should not be marked as a context plugin', () => { + expect( Plugin.isContextPlugin ).to.false; + } ); + describe( 'constructor()', () => { it( 'should set the `editor` property', () => { const plugin = new Plugin( editor ); expect( plugin ).to.have.property( 'editor' ).to.equal( editor ); } ); + } ); - describe( 'destroy()', () => { - it( 'should be defined', () => { - const plugin = new Plugin( editor ); + describe( 'destroy()', () => { + it( 'should be defined', () => { + const plugin = new Plugin( editor ); - expect( plugin.destroy ).to.be.a( 'function' ); - } ); + expect( plugin.destroy ).to.be.a( 'function' ); + } ); - it( 'should stop listening', () => { - const plugin = new Plugin( editor ); - const stopListeningSpy = sinon.spy( plugin, 'stopListening' ); + it( 'should stop listening', () => { + const plugin = new Plugin( editor ); + const stopListeningSpy = sinon.spy( plugin, 'stopListening' ); - plugin.destroy(); + plugin.destroy(); - expect( stopListeningSpy.calledOnce ).to.equal( true ); - } ); + expect( stopListeningSpy.calledOnce ).to.equal( true ); } ); } ); } ); From 16bfc8fdbc9a83b9f2badb1fbd8b9aa48971475a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 18 Dec 2019 13:14:45 +0100 Subject: [PATCH 20/37] Renamed isCreatedByEditor to wasCreatedByEditor. --- src/context.js | 11 +++++------ src/editor/editor.js | 2 +- tests/editor/editor.js | 4 ++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/context.js b/src/context.js index 05591e40..2e38d61e 100644 --- a/src/context.js +++ b/src/context.js @@ -60,13 +60,12 @@ export default class Context { this.t = this.locale.t; /** - * When the context is created by an editor then the editor instance is - * stored as an owner of this context. + * `true` when the context is created by an editor `false` otherwise. * * @readonly * @type {Boolean} */ - this.isCreatedByEditor = false; + this.wasCreatedByEditor = false; /** * List of editors to which this context instance is injected. @@ -81,14 +80,14 @@ export default class Context { * Adds a reference to the editor which is used with this context. * * When the context is created by the editor it is additionally - * marked as a {@link ~Context#isCreatedByEditor} what is used + * marked as a {@link ~Context#wasCreatedByEditor} what is used * in the destroy chain. * * @param {module:core/editor/editor~Editor} editor * @param {Boolean} isContextOwner */ addEditor( editor, isContextOwner ) { - if ( this.isCreatedByEditor ) { + if ( this.wasCreatedByEditor ) { /** * Cannot add multiple editors to the context which is created by the editor. * @@ -102,7 +101,7 @@ export default class Context { this._editors.add( editor ); if ( isContextOwner ) { - this.isCreatedByEditor = true; + this.wasCreatedByEditor = true; } } diff --git a/src/editor/editor.js b/src/editor/editor.js index c26ca950..8990caf0 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -258,7 +258,7 @@ export default class Editor { // When the editor was an owner of the context then // the context should be destroyed along with the editor. - if ( this._context.isCreatedByEditor ) { + if ( this._context.wasCreatedByEditor ) { return this._context.destroy(); } } ).then( () => { diff --git a/tests/editor/editor.js b/tests/editor/editor.js index 70d91e5e..4597ca88 100644 --- a/tests/editor/editor.js +++ b/tests/editor/editor.js @@ -186,7 +186,7 @@ describe( 'Editor', () => { const editor = new TestEditor(); expect( editor._context ).to.be.an.instanceof( Context ); - expect( editor._context.isCreatedByEditor ).to.true; + expect( editor._context.wasCreatedByEditor ).to.true; } ); it( 'should use context given through configuration when is defined', async () => { @@ -194,7 +194,7 @@ describe( 'Editor', () => { const editor = new TestEditor( { context } ); expect( editor._context ).to.equal( context ); - expect( editor._context.isCreatedByEditor ).to.false; + expect( editor._context.wasCreatedByEditor ).to.false; } ); it( 'should throw when try to use context created by one editor with the other editor', () => { From 092d2a818e6b4dcdc277e9ed022a365be663d8c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 18 Dec 2019 14:11:20 +0100 Subject: [PATCH 21/37] Minor docs improvement. --- src/context.js | 4 ++-- tests/editor/editor.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/context.js b/src/context.js index 2e38d61e..14c89206 100644 --- a/src/context.js +++ b/src/context.js @@ -91,10 +91,10 @@ export default class Context { /** * Cannot add multiple editors to the context which is created by the editor. * - * @error context-addEditor-to-private-context + * @error context-addEditor-private-context */ throw new CKEditorError( - 'context-addEditor-to-private-context: Cannot add multiple editors to the context which is created by the editor.' + 'context-addEditor-private-context: Cannot add multiple editors to the context which is created by the editor.' ); } diff --git a/tests/editor/editor.js b/tests/editor/editor.js index 4597ca88..98210449 100644 --- a/tests/editor/editor.js +++ b/tests/editor/editor.js @@ -203,7 +203,7 @@ describe( 'Editor', () => { expectToThrowCKEditorError( () => { // eslint-disable-next-line no-new new TestEditor( { context: editor._context } ); - }, /^context-addEditor-to-private-context/ ); + }, /^context-addEditor-private-context/ ); } ); it( 'should destroy context created by the editor on editor destroy', async () => { From 3fedf1c77ab6fe7af4c826c592746d5f6ca5293a Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Wed, 18 Dec 2019 14:35:59 +0100 Subject: [PATCH 22/37] Docs: Provided documentation for `Context` class and `Context.create()`. --- src/context.js | 65 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/src/context.js b/src/context.js index 14c89206..7ef4b86c 100644 --- a/src/context.js +++ b/src/context.js @@ -13,7 +13,31 @@ import Locale from '@ckeditor/ckeditor5-utils/src/locale'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; /** - * @TODO + * Provides a common, higher level environment for solutions which use multiple {@link module:core/editor/editor~Editor editors} + * or/and plugins that work outside of an editor. Use it instead of {@link module:core/editor/editor~Editor.create `Editor.create()`} + * in advanced application integrations. + * + * All configuration options passed to a `Context` will be used as default options for editor instances initialized in that context. + * + * {@link module:core/contextplugin~ContextPlugin `ContextPlugin`s} passed to a `Context` instance will be shared among all + * editor instances initialized in that context. These will be the same plugin instances for all the editors. + * + * **Note:** `Context` can only be initialized with {@link module:core/contextplugin~ContextPlugin `ContextPlugin`s} + * (e.g. {@glink features/collaboration/comments/comments comments}). Regular {@link module:core/plugin~Plugin `Plugin`s} require an + * editor instance to work and cannot be added to a `Context`. + * + * **Note:** You can add `ContextPlugin` to an editor instance, though. + * + * If you are using multiple editor instances on one page and use any `ContextPlugin`s, create `Context` to share configuration and plugins + * among those editors. Some plugins will use the information about all existing editors to better integrate between them. + * + * If you are using plugins that do not require an editor to work (e.g. {@glink features/collaboration/comments/comments comments}) + * enable and configure them using `Context`. + * + * If you are using only a single editor on each page use {@link module:core/editor/editor~Editor.create `Editor.create()`} instead. + * In such case, `Context` instance will be created by the editor instance in a transparent way. + * + * See {@link module:core/context~Context.create `Context.create()`} for usage examples. */ export default class Context { /** @@ -185,7 +209,44 @@ export default class Context { } /** - * @TODO + * Creates and initializes a new context instance. + * + * const commonConfig = { ... }; // Configuration for all the plugins and editors. + * const editorPlugins = [ ... ]; // Regular `Plugin`s here. + * + * const context = await Context.create( { + * // Only `ContextPlugin`s here. + * plugins: [ ... ], + * + * // Configure language for all the editors (it cannot be overwritten). + * language: { ... }, + * + * // Configuration for context plugins. + * comments: { ... }, + * ... + * + * // Default configuration for editor plugins. + * toolbar: { ... }, + * image: { ... }, + * ... + * } ); + * + * const editor1 = await ClassicEditor.create( + * document.getElementById( 'editor1' ), + * { + * editorPlugins, + * context + * } + * ); + * + * const editor2 = await ClassicEditor.create( + * document.getElementById( 'editor2' ), + * { + * editorPlugins, + * context, + * toolbar: { ... } // You can overwrite context's configuration. + * } + * ); * * @param {Object} [config] The context config. * @returns {Promise} A promise resolved once the context is ready. The promise resolves with the created context instance. From 8251c6784d1644398fd7e941117aa216a57e7f46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 19 Dec 2019 11:41:04 +0100 Subject: [PATCH 23/37] Improved how the context is destroyed by the editor when the editor has created the context. --- src/context.js | 131 +++++++++++++++++++++++------------------ src/editor/editor.js | 16 ++--- tests/context.js | 4 +- tests/editor/editor.js | 2 - 4 files changed, 82 insertions(+), 71 deletions(-) diff --git a/src/context.js b/src/context.js index 7ef4b86c..337f90d3 100644 --- a/src/context.js +++ b/src/context.js @@ -83,14 +83,6 @@ export default class Context { */ this.t = this.locale.t; - /** - * `true` when the context is created by an editor `false` otherwise. - * - * @readonly - * @type {Boolean} - */ - this.wasCreatedByEditor = false; - /** * List of editors to which this context instance is injected. * @@ -98,44 +90,17 @@ export default class Context { * @type {Set} */ this._editors = new Set(); - } - - /** - * Adds a reference to the editor which is used with this context. - * - * When the context is created by the editor it is additionally - * marked as a {@link ~Context#wasCreatedByEditor} what is used - * in the destroy chain. - * - * @param {module:core/editor/editor~Editor} editor - * @param {Boolean} isContextOwner - */ - addEditor( editor, isContextOwner ) { - if ( this.wasCreatedByEditor ) { - /** - * Cannot add multiple editors to the context which is created by the editor. - * - * @error context-addEditor-private-context - */ - throw new CKEditorError( - 'context-addEditor-private-context: Cannot add multiple editors to the context which is created by the editor.' - ); - } - - this._editors.add( editor ); - - if ( isContextOwner ) { - this.wasCreatedByEditor = true; - } - } - /** - * Removes a reference to the editor which was used with this context. - * - * @param {module:core/editor/editor~Editor} editor - */ - removeEditor( editor ) { - return this._editors.delete( editor ); + /** + * Reference to the editor which created the context. + * Null when the context was created outside of the editor. + * + * It is used to destroy the context when removing the editor that created the context. + * + * @private + * @type {module:core/editor/editor~Editor|null} + */ + this._contextOwner = null; } /** @@ -178,15 +143,79 @@ export default class Context { return this.plugins.init( plugins ); } + /** + * Destroys the context instance, releasing all resources used by it. + * + * @returns {Promise} A promise that resolves once the context instance is fully destroyed. + */ + destroy() { + return Promise.all( Array.from( this._editors, editor => editor.destroy() ) ) + .then( () => this.plugins.destroy() ); + } + + /** + * Adds a reference to the editor which is used with this context. + * + * When the given editor has created the context then the reference to this editor will be stored + * as a {@link ~Context#_contextOwner}. + * + * This method should be used only by the editor. + * + * @protected + * @param {module:core/editor/editor~Editor} editor + * @param {Boolean} isContextOwner + */ + _addEditor( editor, isContextOwner ) { + if ( this._contextOwner ) { + /** + * Cannot add multiple editors to the context which is created by the editor. + * + * @error context-addEditor-private-context + */ + throw new CKEditorError( + 'context-addEditor-private-context: Cannot add multiple editors to the context which is created by the editor.' + ); + } + + this._editors.add( editor ); + + if ( isContextOwner ) { + this._contextOwner = editor; + } + } + + /** + * Removes a reference to the editor which was used with this context. + * When the context was created by the given editor then the context will be destroyed. + * + * This method should be used only by the editor. + * + * @protected + * @param {module:core/editor/editor~Editor} editor + * @return {Promise} A promise that resolves once the editor is removed from the context or when the context has been destroyed. + */ + _removeEditor( editor ) { + this._editors.delete( editor ); + + if ( this._contextOwner === editor ) { + return this.destroy(); + } + + return Promise.resolve(); + } + /** * Returns context configuration which will be copied to editors created using this context. * * The configuration returned by this method has removed plugins configuration - plugins are shared with all editors * through another mechanism. * + * This method should be used only by the editor. + * + * @protected * @returns {Object} Configuration as a plain object. */ - getConfigForEditor() { + _getConfigForEditor() { const result = {}; for ( const name of this.config.names() ) { @@ -198,16 +227,6 @@ export default class Context { return result; } - /** - * Destroys the context instance, releasing all resources used by it. - * - * @returns {Promise} A promise that resolves once the context instance is fully destroyed. - */ - destroy() { - return Promise.all( Array.from( this._editors, editor => editor.destroy() ) ) - .then( () => this.plugins.destroy() ); - } - /** * Creates and initializes a new context instance. * diff --git a/src/editor/editor.js b/src/editor/editor.js index 8990caf0..975d4719 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -59,7 +59,7 @@ export default class Editor { * @type {module:core/context~Context} */ this._context = config.context || new Context( { language: config.language } ); - this._context.addEditor( this, !config.context ); + this._context._addEditor( this, !config.context ); const availablePlugins = this.constructor.builtinPlugins; @@ -74,7 +74,7 @@ export default class Editor { */ this.config = new Config( config, this.constructor.defaultConfig ); this.config.define( 'plugins', availablePlugins ); - this.config.define( this._context.getConfigForEditor() ); + this.config.define( this._context._getConfigForEditor() ); /** * The plugins loaded and in use by this editor instance. @@ -252,15 +252,9 @@ export default class Editor { return readyPromise .then( () => { - // Remove the editor from the context to avoid destroying it - // one more time when context will be destroyed. - this._context.removeEditor( this ); - - // When the editor was an owner of the context then - // the context should be destroyed along with the editor. - if ( this._context.wasCreatedByEditor ) { - return this._context.destroy(); - } + // Remove the editor from the context. + // When the context was created by this editor then then the context will be destroyed. + return this._context._removeEditor( this ); } ).then( () => { this.fire( 'destroy' ); this.stopListening(); diff --git a/tests/context.js b/tests/context.js index 8d8c17c8..94214120 100644 --- a/tests/context.js +++ b/tests/context.js @@ -26,7 +26,7 @@ describe( 'Context', () => { } ); } ); - describe( 'getConfigForEditor()', () => { + describe( '_getConfigForEditor()', () => { it( 'should return the configuration without plugin config', () => { class FooPlugin extends ContextPlugin {} class BarPlugin extends ContextPlugin {} @@ -41,7 +41,7 @@ describe( 'Context', () => { bar: 'bom' } ); - expect( context.getConfigForEditor() ).to.be.deep.equal( { + expect( context._getConfigForEditor() ).to.be.deep.equal( { language: { ui: 'pl', content: 'ar' }, foo: 1, bar: 'bom' diff --git a/tests/editor/editor.js b/tests/editor/editor.js index 98210449..e490e229 100644 --- a/tests/editor/editor.js +++ b/tests/editor/editor.js @@ -186,7 +186,6 @@ describe( 'Editor', () => { const editor = new TestEditor(); expect( editor._context ).to.be.an.instanceof( Context ); - expect( editor._context.wasCreatedByEditor ).to.true; } ); it( 'should use context given through configuration when is defined', async () => { @@ -194,7 +193,6 @@ describe( 'Editor', () => { const editor = new TestEditor( { context } ); expect( editor._context ).to.equal( context ); - expect( editor._context.wasCreatedByEditor ).to.false; } ); it( 'should throw when try to use context created by one editor with the other editor', () => { From 6ca797a8d7b27dc1542d9afe81d6264ae2ab8469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 19 Dec 2019 12:42:01 +0100 Subject: [PATCH 24/37] Moved context destroy at the end of the editor destroy chain. --- src/editor/editor.js | 9 ++++----- tests/editor/editor.js | 6 ++++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/editor/editor.js b/src/editor/editor.js index 975d4719..74fa9edd 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -252,10 +252,6 @@ export default class Editor { return readyPromise .then( () => { - // Remove the editor from the context. - // When the context was created by this editor then then the context will be destroyed. - return this._context._removeEditor( this ); - } ).then( () => { this.fire( 'destroy' ); this.stopListening(); this.commands.destroy(); @@ -266,7 +262,10 @@ export default class Editor { this.data.destroy(); this.editing.destroy(); this.keystrokes.destroy(); - } ); + } ) + // Remove the editor from the context. + // When the context was created by this editor then then the context will be destroyed. + .then( () => this._context._removeEditor( this ) ); } /** diff --git a/tests/editor/editor.js b/tests/editor/editor.js index e490e229..b301be28 100644 --- a/tests/editor/editor.js +++ b/tests/editor/editor.js @@ -188,7 +188,7 @@ describe( 'Editor', () => { expect( editor._context ).to.be.an.instanceof( Context ); } ); - it( 'should use context given through configuration when is defined', async () => { + it( 'should use context given through config', async () => { const context = await Context.create(); const editor = new TestEditor( { context } ); @@ -204,13 +204,15 @@ describe( 'Editor', () => { }, /^context-addEditor-private-context/ ); } ); - it( 'should destroy context created by the editor on editor destroy', async () => { + it( 'should destroy context created by the editor at the end of the editor destroy chain', async () => { const editor = await TestEditor.create(); + const editorPluginsDestroySpy = sinon.spy( editor.plugins, 'destroy' ); const contextDestroySpy = sinon.spy( editor._context, 'destroy' ); await editor.destroy(); sinon.assert.calledOnce( contextDestroySpy ); + expect( editorPluginsDestroySpy.calledBefore( contextDestroySpy ) ).to.true; } ); it( 'should not destroy context along with the editor when context was injected to the editor', async () => { From 78e2c715909f40f54d9012a2432fd39d7cf8b09f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 19 Dec 2019 12:50:28 +0100 Subject: [PATCH 25/37] Fixed invalid links. --- src/context.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/context.js b/src/context.js index 337f90d3..392eb353 100644 --- a/src/context.js +++ b/src/context.js @@ -23,7 +23,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; * editor instances initialized in that context. These will be the same plugin instances for all the editors. * * **Note:** `Context` can only be initialized with {@link module:core/contextplugin~ContextPlugin `ContextPlugin`s} - * (e.g. {@glink features/collaboration/comments/comments comments}). Regular {@link module:core/plugin~Plugin `Plugin`s} require an + * (e.g. [comments](https://ckeditor.com/collaboration/comments/)). Regular {@link module:core/plugin~Plugin `Plugin`s} require an * editor instance to work and cannot be added to a `Context`. * * **Note:** You can add `ContextPlugin` to an editor instance, though. @@ -31,7 +31,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; * If you are using multiple editor instances on one page and use any `ContextPlugin`s, create `Context` to share configuration and plugins * among those editors. Some plugins will use the information about all existing editors to better integrate between them. * - * If you are using plugins that do not require an editor to work (e.g. {@glink features/collaboration/comments/comments comments}) + * If you are using plugins that do not require an editor to work (e.g. [comments](https://ckeditor.com/collaboration/comments/)) * enable and configure them using `Context`. * * If you are using only a single editor on each page use {@link module:core/editor/editor~Editor.create `Editor.create()`} instead. From 396212051bf50c70bc924c7a05a600b7e8afffbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 19 Dec 2019 12:59:54 +0100 Subject: [PATCH 26/37] Improved docs for the Context class. --- src/context.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/context.js b/src/context.js index 392eb353..a9a8bacb 100644 --- a/src/context.js +++ b/src/context.js @@ -87,7 +87,7 @@ export default class Context { * List of editors to which this context instance is injected. * * @private - * @type {Set} + * @type {Set.} */ this._editors = new Set(); @@ -95,7 +95,7 @@ export default class Context { * Reference to the editor which created the context. * Null when the context was created outside of the editor. * - * It is used to destroy the context when removing the editor that created the context. + * It is used to destroy the context when removing the editor that has created the context. * * @private * @type {module:core/editor/editor~Editor|null} @@ -115,7 +115,7 @@ export default class Context { for ( const Plugin of plugins ) { if ( typeof Plugin != 'function' ) { /** - * Only constructor is allowed as a Context plugin. + * Only constructor is allowed as a {@link module:core/contextplugin~ContextPlugin}. * * @error context-initplugins-constructor-only */ @@ -128,7 +128,7 @@ export default class Context { if ( Plugin.isContextPlugin !== true ) { /** - * Only plugin marked as a ContextPlugin is allowed to be used with a context. + * Only plugin marked as a {@link module:core/contextplugin~ContextPlugin} is allowed to be used with a context. * * @error context-initplugins-invalid-plugin */ @@ -144,7 +144,8 @@ export default class Context { } /** - * Destroys the context instance, releasing all resources used by it. + * Destroys the context instance, and all editors used with the context. + * Releasing all resources used by the context. * * @returns {Promise} A promise that resolves once the context instance is fully destroyed. */ @@ -163,7 +164,7 @@ export default class Context { * * @protected * @param {module:core/editor/editor~Editor} editor - * @param {Boolean} isContextOwner + * @param {Boolean} isContextOwner Stores the given editor as a context owner. */ _addEditor( editor, isContextOwner ) { if ( this._contextOwner ) { From fd5cc62b0096038093221037dc294ddf1deebfba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 19 Dec 2019 13:04:45 +0100 Subject: [PATCH 27/37] Changed usage of async/await to promises in code sample. --- src/context.js | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/context.js b/src/context.js index a9a8bacb..50042ffe 100644 --- a/src/context.js +++ b/src/context.js @@ -234,7 +234,7 @@ export default class Context { * const commonConfig = { ... }; // Configuration for all the plugins and editors. * const editorPlugins = [ ... ]; // Regular `Plugin`s here. * - * const context = await Context.create( { + * Context.create( { * // Only `ContextPlugin`s here. * plugins: [ ... ], * @@ -249,24 +249,28 @@ export default class Context { * toolbar: { ... }, * image: { ... }, * ... - * } ); + * } ).then( context => { + * const promises = []; + * + * promises.push( ClassicEditor.create( + * document.getElementById( 'editor1' ), + * { + * editorPlugins, + * context + * } + * ) ); * - * const editor1 = await ClassicEditor.create( - * document.getElementById( 'editor1' ), - * { - * editorPlugins, - * context - * } - * ); + * promises.push( ClassicEditor.create( + * document.getElementById( 'editor2' ), + * { + * editorPlugins, + * context, + * toolbar: { ... } // You can overwrite context's configuration. + * } + * ) ); * - * const editor2 = await ClassicEditor.create( - * document.getElementById( 'editor2' ), - * { - * editorPlugins, - * context, - * toolbar: { ... } // You can overwrite context's configuration. - * } - * ); + * return Promise.all( promises ); + * } ); * * @param {Object} [config] The context config. * @returns {Promise} A promise resolved once the context is ready. The promise resolves with the created context instance. From dd9b62632f9599a03265297337042a8102d27a51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 19 Dec 2019 13:52:20 +0100 Subject: [PATCH 28/37] Improved docs for the ContextPlugin. --- src/contextplugin.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/contextplugin.js b/src/contextplugin.js index e24bd74f..6744096c 100644 --- a/src/contextplugin.js +++ b/src/contextplugin.js @@ -13,6 +13,15 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix'; /** * The base class for {@link module:core/context~Context} plugin classes. * + * A context plugin can either be initialized for an editor or for a context. In other words, it can either + * work within one editor instance or with one or more editor instances that use a single context. + * It's the context plugin's role to implement handling for both modes. + * + * A list of rules for the context plugin: + * * it should be possible, for the context plugin to requires another context plugin, + * * it should be possible, for the {@link module:core/plugin~Plugin editor plugin} to requires context plugin, + * * it should NOT be possible, for the context plugin to require the {@link module:core/plugin~Plugin editor plugin}. + * * @implements module:core/plugin~PluginInterface * @mixes module:utils/observablemixin~ObservableMixin */ From ce97ba4a78ba995b15cbb89e9576ebe1ca5a2834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 19 Dec 2019 14:06:05 +0100 Subject: [PATCH 29/37] Renamed _getConfigForEditor method to _getEditorConfig. --- src/context.js | 2 +- src/editor/editor.js | 2 +- tests/context.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/context.js b/src/context.js index 50042ffe..d61f6093 100644 --- a/src/context.js +++ b/src/context.js @@ -216,7 +216,7 @@ export default class Context { * @protected * @returns {Object} Configuration as a plain object. */ - _getConfigForEditor() { + _getEditorConfig() { const result = {}; for ( const name of this.config.names() ) { diff --git a/src/editor/editor.js b/src/editor/editor.js index 74fa9edd..eb254413 100644 --- a/src/editor/editor.js +++ b/src/editor/editor.js @@ -74,7 +74,7 @@ export default class Editor { */ this.config = new Config( config, this.constructor.defaultConfig ); this.config.define( 'plugins', availablePlugins ); - this.config.define( this._context._getConfigForEditor() ); + this.config.define( this._context._getEditorConfig() ); /** * The plugins loaded and in use by this editor instance. diff --git a/tests/context.js b/tests/context.js index 94214120..cd47183a 100644 --- a/tests/context.js +++ b/tests/context.js @@ -26,7 +26,7 @@ describe( 'Context', () => { } ); } ); - describe( '_getConfigForEditor()', () => { + describe( '_getEditorConfig()', () => { it( 'should return the configuration without plugin config', () => { class FooPlugin extends ContextPlugin {} class BarPlugin extends ContextPlugin {} @@ -41,7 +41,7 @@ describe( 'Context', () => { bar: 'bom' } ); - expect( context._getConfigForEditor() ).to.be.deep.equal( { + expect( context._getEditorConfig() ).to.be.deep.equal( { language: { ui: 'pl', content: 'ar' }, foo: 1, bar: 'bom' From 9637a494400cb464fea6e4750b46bb1cd0f221be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 19 Dec 2019 14:11:32 +0100 Subject: [PATCH 30/37] Changed invalid PendingActions docs. Added test checking if PendingActions is a context plugin. --- src/pendingactions.js | 2 +- tests/pendingactions.js | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/pendingactions.js b/src/pendingactions.js index ad90ead0..ddb8cc98 100644 --- a/src/pendingactions.js +++ b/src/pendingactions.js @@ -50,7 +50,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; * and by features like {@link module:autosave/autosave~Autosave} to detect whether there are any ongoing actions. * Read more about saving the data in the {@glink builds/guides/integration/saving-data Saving and getting data} guide. * - * @extends module:core/plugin~Plugin + * @extends module:core/contextplugin~ContextPlugin */ export default class PendingActions extends ContextPlugin { /** diff --git a/tests/pendingactions.js b/tests/pendingactions.js index 166432aa..23621955 100644 --- a/tests/pendingactions.js +++ b/tests/pendingactions.js @@ -27,6 +27,10 @@ describe( 'PendingActions', () => { expect( PendingActions ).to.have.property( 'pluginName', 'PendingActions' ); } ); + it( 'should be marked as a context plugin', () => { + expect( PendingActions.isContextPlugin ).to.true; + } ); + describe( 'init()', () => { it( 'should have hasAny observable', () => { const spy = sinon.spy(); From 99b98b869a14829976b7addcbf00dc4ba809717a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 19 Dec 2019 14:16:04 +0100 Subject: [PATCH 31/37] Renamed externalPlugins to contextPlugins. --- src/plugincollection.js | 20 ++++++++++---------- tests/editor/editor.js | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/plugincollection.js b/src/plugincollection.js index 7eccd5c8..fc770c50 100644 --- a/src/plugincollection.js +++ b/src/plugincollection.js @@ -30,10 +30,10 @@ export default class PluginCollection { * when {@link module:core/plugincollection~PluginCollection#init} is used with plugin names (strings, instead of constructors). * Usually, the editor will pass its built-in plugins to the collection so they can later be * used in `config.plugins` or `config.removePlugins` by names. - * @param {Iterable.} externalPlugins List of already initialized plugins represented by a + * @param {Iterable.} contextPlugins List of already initialized plugins represented by a * `[ PluginConstructor, pluginInstance ]` pair. */ - constructor( context, availablePlugins = [], externalPlugins = [] ) { + constructor( context, availablePlugins = [], contextPlugins = [] ) { /** * @protected * @type {module:core/editor/editor~Editor|module:core/context~Context} @@ -61,16 +61,16 @@ export default class PluginCollection { } /** - * Map of external plugins which can be retrieved by their constructors or instance. + * Map of {@link module:core/contextplugin~ContextPlugin context plugins} which can be retrieved by their constructors or instances. * * @protected * @type {Map} */ - this._externalPlugins = new Map(); + this._contextPlugins = new Map(); - for ( const [ PluginConstructor, pluginInstance ] of externalPlugins ) { - this._externalPlugins.set( PluginConstructor, pluginInstance ); - this._externalPlugins.set( pluginInstance, PluginConstructor ); + for ( const [ PluginConstructor, pluginInstance ] of contextPlugins ) { + this._contextPlugins.set( PluginConstructor, pluginInstance ); + this._contextPlugins.set( pluginInstance, PluginConstructor ); // To make it possible to require plugin by its name. if ( PluginConstructor.pluginName ) { @@ -265,7 +265,7 @@ export default class PluginCollection { return promise; } - if ( that._externalPlugins.has( plugin ) ) { + if ( that._contextPlugins.has( plugin ) ) { return promise; } @@ -321,7 +321,7 @@ export default class PluginCollection { } ); } - const plugin = that._externalPlugins.get( PluginConstructor ) || new PluginConstructor( context ); + const plugin = that._contextPlugins.get( PluginConstructor ) || new PluginConstructor( context ); that._add( PluginConstructor, plugin ); loaded.push( plugin ); @@ -365,7 +365,7 @@ export default class PluginCollection { const promises = []; for ( const [ , pluginInstance ] of this ) { - if ( typeof pluginInstance.destroy == 'function' && !this._externalPlugins.has( pluginInstance ) ) { + if ( typeof pluginInstance.destroy == 'function' && !this._contextPlugins.has( pluginInstance ) ) { promises.push( pluginInstance.destroy() ); } } diff --git a/tests/editor/editor.js b/tests/editor/editor.js index b301be28..c4c8c566 100644 --- a/tests/editor/editor.js +++ b/tests/editor/editor.js @@ -225,7 +225,7 @@ describe( 'Editor', () => { sinon.assert.notCalled( contextDestroySpy ); } ); - it( 'should add context plugins as an external editor plugins', async () => { + it( 'should add context plugins to the editor plugins', async () => { class ContextPlugin { static get isContextPlugin() { return true; @@ -235,7 +235,7 @@ describe( 'Editor', () => { const context = await Context.create( { plugins: [ ContextPlugin ] } ); const editor = new TestEditor( { context } ); - expect( editor.plugins._externalPlugins.has( ContextPlugin ) ).to.equal( true ); + expect( editor.plugins._contextPlugins.has( ContextPlugin ) ).to.equal( true ); } ); it( 'should get configuration from the context', async () => { From e96aeed8d8924b10c7009e6957ad1f3f289d7f43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 16 Jan 2020 10:30:02 +0100 Subject: [PATCH 32/37] Minor API docs improvements. --- src/context.js | 64 +++++++++++++++++++++++--------------------- src/contextplugin.js | 14 +++++----- 2 files changed, 41 insertions(+), 37 deletions(-) diff --git a/src/context.js b/src/context.js index d61f6093..155a4321 100644 --- a/src/context.js +++ b/src/context.js @@ -234,43 +234,45 @@ export default class Context { * const commonConfig = { ... }; // Configuration for all the plugins and editors. * const editorPlugins = [ ... ]; // Regular `Plugin`s here. * - * Context.create( { - * // Only `ContextPlugin`s here. - * plugins: [ ... ], + * Context + * .create( { + * // Only `ContextPlugin`s here. + * plugins: [ ... ], * - * // Configure language for all the editors (it cannot be overwritten). - * language: { ... }, + * // Configure language for all the editors (it cannot be overwritten). + * language: { ... }, * - * // Configuration for context plugins. - * comments: { ... }, - * ... + * // Configuration for context plugins. + * comments: { ... }, + * ... * - * // Default configuration for editor plugins. - * toolbar: { ... }, - * image: { ... }, - * ... - * } ).then( context => { - * const promises = []; + * // Default configuration for editor plugins. + * toolbar: { ... }, + * image: { ... }, + * ... + * } ) + * .then( context => { + * const promises = []; * - * promises.push( ClassicEditor.create( - * document.getElementById( 'editor1' ), - * { - * editorPlugins, - * context - * } - * ) ); + * promises.push( ClassicEditor.create( + * document.getElementById( 'editor1' ), + * { + * editorPlugins, + * context + * } + * ) ); * - * promises.push( ClassicEditor.create( - * document.getElementById( 'editor2' ), - * { - * editorPlugins, - * context, - * toolbar: { ... } // You can overwrite context's configuration. - * } - * ) ); + * promises.push( ClassicEditor.create( + * document.getElementById( 'editor2' ), + * { + * editorPlugins, + * context, + * toolbar: { ... } // You can overwrite context's configuration. + * } + * ) ); * - * return Promise.all( promises ); - * } ); + * return Promise.all( promises ); + * } ); * * @param {Object} [config] The context config. * @returns {Promise} A promise resolved once the context is ready. The promise resolves with the created context instance. diff --git a/src/contextplugin.js b/src/contextplugin.js index 6744096c..9d660239 100644 --- a/src/contextplugin.js +++ b/src/contextplugin.js @@ -13,14 +13,16 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix'; /** * The base class for {@link module:core/context~Context} plugin classes. * - * A context plugin can either be initialized for an editor or for a context. In other words, it can either + * A context plugin can either be initialized for an {@link module:core/editor/editor~Editor editor} or for + * a {@link module:core/context~Context context}. In other words, it can either * work within one editor instance or with one or more editor instances that use a single context. - * It's the context plugin's role to implement handling for both modes. + * It is the context plugin's role to implement handling for both modes. * - * A list of rules for the context plugin: - * * it should be possible, for the context plugin to requires another context plugin, - * * it should be possible, for the {@link module:core/plugin~Plugin editor plugin} to requires context plugin, - * * it should NOT be possible, for the context plugin to require the {@link module:core/plugin~Plugin editor plugin}. + * A couple of rules for interaction between editor plugins and context plugins: + * + * * a context plugin can require another context plugin, + * * an {@link module:core/plugin~Plugin editor plugin} can require a context plugin, + * * a context plugin MUST NOT require an {@link module:core/plugin~Plugin editor plugin}. * * @implements module:core/plugin~PluginInterface * @mixes module:utils/observablemixin~ObservableMixin From 46f1e295f2834c3971d0338656b9fb1319f3cff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 16 Jan 2020 10:47:43 +0100 Subject: [PATCH 33/37] Docs: Wording. [skip ci] --- src/context.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/context.js b/src/context.js index 155a4321..b6d3425b 100644 --- a/src/context.js +++ b/src/context.js @@ -115,12 +115,12 @@ export default class Context { for ( const Plugin of plugins ) { if ( typeof Plugin != 'function' ) { /** - * Only constructor is allowed as a {@link module:core/contextplugin~ContextPlugin}. + * Only a constructor function is allowed as a {@link module:core/contextplugin~ContextPlugin context plugin}. * * @error context-initplugins-constructor-only */ throw new CKEditorError( - 'context-initplugins-constructor-only: Only constructor is allowed as a Context plugin.', + 'context-initplugins-constructor-only: Only a constructor function is allowed as a context plugin.', null, { Plugin } ); @@ -128,12 +128,13 @@ export default class Context { if ( Plugin.isContextPlugin !== true ) { /** - * Only plugin marked as a {@link module:core/contextplugin~ContextPlugin} is allowed to be used with a context. + * Only a plugin marked as a {@link module:core/contextplugin~ContextPlugin.isContextPlugin context plugin} + * is allowed to be used with a context. * * @error context-initplugins-invalid-plugin */ throw new CKEditorError( - 'context-initplugins-invalid-plugin: Only plugin marked as a ContextPlugin is allowed.', + 'context-initplugins-invalid-plugin: Only a plugin marked as a ContextPlugin is allowed to be used with a context.', null, { Plugin } ); From 4e5fc9012bb56b83d58d09a510d43ec7a1fceec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 16 Jan 2020 11:46:12 +0100 Subject: [PATCH 34/37] Tests: Updated conditions too (after changing error names). --- tests/_utils-tests/articlepluginset.js | 2 +- tests/context.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/_utils-tests/articlepluginset.js b/tests/_utils-tests/articlepluginset.js index 882337b9..dce2a2e7 100644 --- a/tests/_utils-tests/articlepluginset.js +++ b/tests/_utils-tests/articlepluginset.js @@ -45,7 +45,7 @@ describe( 'ArticlePluginSet', () => { editorElement.remove(); } ); - it( 'should be loaded', () => { + it.only( 'should be loaded', () => { expect( editor.plugins.get( ArticlePluginSet ) ).to.be.instanceOf( ArticlePluginSet ); } ); diff --git a/tests/context.js b/tests/context.js index cd47183a..dd921d67 100644 --- a/tests/context.js +++ b/tests/context.js @@ -93,7 +93,7 @@ describe( 'Context', () => { expect( caughtError ).to.instanceof( CKEditorError ); expect( caughtError.message ) - .match( /^context-initplugins-invalid-plugin: Only plugin marked as a ContextPlugin is allowed./ ); + .match( /^context-initplugins-invalid-plugin:/ ); } ); it( 'should throw when plugin added to the context is not marked as a ContextPlugin (Function)', async () => { @@ -109,7 +109,7 @@ describe( 'Context', () => { expect( caughtError ).to.instanceof( CKEditorError ); expect( caughtError.message ) - .match( /^context-initplugins-invalid-plugin: Only plugin marked as a ContextPlugin is allowed./ ); + .match( /^context-initplugins-invalid-plugin:/ ); } ); it( 'should throw when plugin is added to the context by name', async () => { @@ -123,7 +123,7 @@ describe( 'Context', () => { expect( caughtError ).to.instanceof( CKEditorError ); expect( caughtError.message ) - .match( /^context-initplugins-constructor-only: Only constructor is allowed as a Context plugin./ ); + .match( /^context-initplugins-constructor-only:/ ); } ); it( 'should not throw when plugin as a function, marked as a ContextPlugin is added to the context', async () => { From 5e3e80392dedee54d8956e527cdcce80743c59c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 16 Jan 2020 12:06:34 +0100 Subject: [PATCH 35/37] Tests: Fixed the cleanup util test leaking itself by not using it(). Also: reverted stupid it.only() change. --- tests/_utils-tests/articlepluginset.js | 2 +- tests/_utils-tests/cleanup.js | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/_utils-tests/articlepluginset.js b/tests/_utils-tests/articlepluginset.js index dce2a2e7..882337b9 100644 --- a/tests/_utils-tests/articlepluginset.js +++ b/tests/_utils-tests/articlepluginset.js @@ -45,7 +45,7 @@ describe( 'ArticlePluginSet', () => { editorElement.remove(); } ); - it.only( 'should be loaded', () => { + it( 'should be loaded', () => { expect( editor.plugins.get( ArticlePluginSet ) ).to.be.instanceOf( ArticlePluginSet ); } ); diff --git a/tests/_utils-tests/cleanup.js b/tests/_utils-tests/cleanup.js index 0f9a9302..c01f2c8d 100644 --- a/tests/_utils-tests/cleanup.js +++ b/tests/_utils-tests/cleanup.js @@ -11,17 +11,19 @@ import { removeEditorBodyOrphans } from '../_utils/cleanup'; describe( 'cleanup util', () => { describe( 'removeEditorBodyOrphans()', () => { - const locale = new Locale(); - const uiViews = [ new EditorUIView( locale ), new EditorUIView( locale ) ]; + it( 'cleans up all editor elements', () => { + const locale = new Locale(); + const uiViews = [ new EditorUIView( locale ), new EditorUIView( locale ) ]; - for ( const view of uiViews ) { - view.render(); - } + for ( const view of uiViews ) { + view.render(); + } - expect( document.querySelectorAll( '.ck-body' ) ).to.have.length( 2 ); + expect( document.querySelectorAll( '.ck-body' ) ).to.have.length( 2 ); - removeEditorBodyOrphans(); + removeEditorBodyOrphans(); - expect( document.querySelectorAll( '.ck-body' ) ).to.have.length( 0 ); + expect( document.querySelectorAll( '.ck-body' ) ).to.have.length( 0 ); + } ); } ); } ); From 9748183afac67290070c6fd426a499194397d3b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 16 Jan 2020 12:19:45 +0100 Subject: [PATCH 36/37] Tests: Updated orphans cleanup after introduction of the new body collection wrapper. --- tests/_utils-tests/cleanup.js | 20 ++++++++++++++++++-- tests/_utils/cleanup.js | 2 +- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/tests/_utils-tests/cleanup.js b/tests/_utils-tests/cleanup.js index c01f2c8d..6db55c1c 100644 --- a/tests/_utils-tests/cleanup.js +++ b/tests/_utils-tests/cleanup.js @@ -11,7 +11,7 @@ import { removeEditorBodyOrphans } from '../_utils/cleanup'; describe( 'cleanup util', () => { describe( 'removeEditorBodyOrphans()', () => { - it( 'cleans up all editor elements', () => { + it( 'removes the body collection wrapper', () => { const locale = new Locale(); const uiViews = [ new EditorUIView( locale ), new EditorUIView( locale ) ]; @@ -19,11 +19,27 @@ describe( 'cleanup util', () => { view.render(); } - expect( document.querySelectorAll( '.ck-body' ) ).to.have.length( 2 ); + // Body collection reuses its wrapper, hence 1. + expect( document.querySelectorAll( '.ck-body-wrapper' ) ).to.have.length( 1 ); removeEditorBodyOrphans(); + expect( document.querySelectorAll( '.ck-body-wrapper' ) ).to.have.length( 0 ); expect( document.querySelectorAll( '.ck-body' ) ).to.have.length( 0 ); } ); + + // Right now, body collection should reuse its wrapper, but it doesn't cost us much to + // ensure that we remove all. + it( 'removes all body collection wrappers', () => { + const wrapper = document.createElement( 'div' ); + wrapper.classList.add( 'ck-body-wrapper' ); + + document.body.appendChild( wrapper ); + document.body.appendChild( wrapper.cloneNode() ); + + removeEditorBodyOrphans(); + + expect( document.querySelectorAll( '.ck-body-wrapper' ) ).to.have.length( 0 ); + } ); } ); } ); diff --git a/tests/_utils/cleanup.js b/tests/_utils/cleanup.js index 77363f33..8f70ce61 100644 --- a/tests/_utils/cleanup.js +++ b/tests/_utils/cleanup.js @@ -13,7 +13,7 @@ * See https://github.com/ckeditor/ckeditor5/issues/6018 for more details. */ export function removeEditorBodyOrphans() { - for ( const bodyOrphan of document.querySelectorAll( '.ck-body' ) ) { + for ( const bodyOrphan of document.querySelectorAll( '.ck-body-wrapper' ) ) { bodyOrphan.remove(); } } From 0b92d967525303324f2b8aa0233f255a89e76191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 16 Jan 2020 12:27:05 +0100 Subject: [PATCH 37/37] Internal: Updated new license headers. --- src/context.js | 2 +- src/contextplugin.js | 2 +- tests/_utils-tests/cleanup.js | 2 +- tests/context.js | 2 +- tests/contextplugin.js | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/context.js b/src/context.js index b6d3425b..7e85e190 100644 --- a/src/context.js +++ b/src/context.js @@ -1,5 +1,5 @@ /** - * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ diff --git a/src/contextplugin.js b/src/contextplugin.js index 9d660239..d1d40773 100644 --- a/src/contextplugin.js +++ b/src/contextplugin.js @@ -1,5 +1,5 @@ /** - * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ diff --git a/tests/_utils-tests/cleanup.js b/tests/_utils-tests/cleanup.js index 6db55c1c..b0f7d381 100644 --- a/tests/_utils-tests/cleanup.js +++ b/tests/_utils-tests/cleanup.js @@ -1,5 +1,5 @@ /** - * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ diff --git a/tests/context.js b/tests/context.js index dd921d67..333abafc 100644 --- a/tests/context.js +++ b/tests/context.js @@ -1,5 +1,5 @@ /** - * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ diff --git a/tests/contextplugin.js b/tests/contextplugin.js index 4eddf753..a932fab0 100644 --- a/tests/contextplugin.js +++ b/tests/contextplugin.js @@ -1,5 +1,5 @@ /** - * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */