From f28cec15e13052ee1816abfd872838e0d26c060d Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Tue, 4 Jun 2019 15:30:39 +0200 Subject: [PATCH 1/9] Introduced the "secureSourceElement()" util which checks whether specified sourceElement was used more than once. --- src/editor/utils/securesourceelement.js | 47 +++++++++++++++ tests/editor/utils/securesourceelement.js | 69 +++++++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 src/editor/utils/securesourceelement.js create mode 100644 tests/editor/utils/securesourceelement.js diff --git a/src/editor/utils/securesourceelement.js b/src/editor/utils/securesourceelement.js new file mode 100644 index 00000000..e79492d7 --- /dev/null +++ b/src/editor/utils/securesourceelement.js @@ -0,0 +1,47 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; + +/** + * @module core/editor/utils/securesourceelement + */ + +const SECURE_ATTRIBUTE = 'data-ckeditor5'; + +/** + * Checks if the editor was initialized using a source element. If yes, it prevents to creating another editor + * using the same source element. In other words, you cannot use the same source element more than once. + * + * @param {module:core/editor/editor~Editor} editor Editor instance. + */ +export default function secureSourceElement( editor ) { + const sourceElement = editor.sourceElement; + + // If the editor was initialized without specifying an element, we don't need to secure anything. + if ( !sourceElement ) { + return; + } + + if ( sourceElement.hasAttribute( SECURE_ATTRIBUTE ) ) { + /** + * An element passed to the editor creator has been passed more than once. The element can be used only once. + * + * @error securesourceelement-source-element-used-more-than-once + */ + throw new CKEditorError( + 'securesourceelement-source-element-used-more-than-once: ' + + 'The editor cannot be initialized using the same source element more than once.' + ); + } + + // Mark the source element. + sourceElement.setAttribute( SECURE_ATTRIBUTE, 'true' ); + + // Remove the attribute when the editor is being destroyed. + editor.once( 'destroy', () => { + sourceElement.removeAttribute( SECURE_ATTRIBUTE ); + } ); +} diff --git a/tests/editor/utils/securesourceelement.js b/tests/editor/utils/securesourceelement.js new file mode 100644 index 00000000..1ef671ff --- /dev/null +++ b/tests/editor/utils/securesourceelement.js @@ -0,0 +1,69 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* global document */ + +import secureSourceElement from '../../../src/editor/utils/securesourceelement'; +import Editor from '../../../src/editor/editor'; +import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; + +describe( 'secureSourceElement()', () => { + let editor, sourceElement; + + beforeEach( () => { + class CustomEditor extends Editor {} + + sourceElement = document.createElement( 'div' ); + editor = new CustomEditor(); + + editor.sourceElement = sourceElement; + editor.state = 'ready'; + } ); + + afterEach( () => { + if ( editor ) { + return editor.destroy(); + } + } ); + + it( 'does nothing if the editor was not initialized using the source element', () => { + delete editor.sourceElement; + + expect( () => { + secureSourceElement( editor ); + } ).to.not.throw(); + } ); + + it( 'does nothing if the editor was initialized using the element for the first time', () => { + expect( () => { + secureSourceElement( editor ); + } ).to.not.throw(); + } ); + + it( 'sets the data attribute after initializing the editor', () => { + secureSourceElement( editor ); + + expect( sourceElement.hasAttribute( 'data-ckeditor5' ) ).to.equal( true ); + } ); + + it( 'removes the data attribute after destroying the editor', () => { + secureSourceElement( editor ); + + return editor.destroy() + .then( () => { + editor = null; + + expect( sourceElement.hasAttribute( 'data-ckeditor5' ) ).to.equal( false ); + } ); + } ); + + it( 'throws an error if the same element was used twice', () => { + sourceElement.setAttribute( 'data-ckeditor5', 'true' ); + + expect( () => { + secureSourceElement( editor ); + } ).to.throw( CKEditorError, /^securesourceelement-source-element-used-more-than-once/ ); + } ); +} ); From 39bd02106d006df29865831b865dd93e5b503d6d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 19 Jul 2019 16:56:59 +0200 Subject: [PATCH 2/9] Moved secureSourceElement() to ElementApiMixin. --- src/editor/utils/elementapimixin.js | 39 +++++++++++++ src/editor/utils/securesourceelement.js | 47 --------------- tests/editor/utils/elementapimixin.js | 48 ++++++++++++++++ tests/editor/utils/securesourceelement.js | 69 ----------------------- 4 files changed, 87 insertions(+), 116 deletions(-) delete mode 100644 src/editor/utils/securesourceelement.js delete mode 100644 tests/editor/utils/securesourceelement.js diff --git a/src/editor/utils/elementapimixin.js b/src/editor/utils/elementapimixin.js index 765fcac7..7906e1df 100644 --- a/src/editor/utils/elementapimixin.js +++ b/src/editor/utils/elementapimixin.js @@ -10,6 +10,8 @@ import setDataInElement from '@ckeditor/ckeditor5-utils/src/dom/setdatainelement * @module core/editor/utils/elementapimixin */ +const SECURE_SOURCE_ELEMENT_ATTRIBUTE = 'data-ckeditor5-element'; + /** * Implementation of the {@link module:core/editor/utils/elementapimixin~ElementApi}. * @@ -37,6 +39,43 @@ const ElementApiMixin = { } setDataInElement( this.sourceElement, this.data.get() ); + }, + + /** + * Marks the source element the editor was initialized on preventing other editor instances from + * using this element. + * + * Running multiple editor instances on the same source element causes various issues and it is + * crucial this helper is called as soon as the source element is known to prevent collisions. + */ + secureSourceElement() { + const sourceElement = this.sourceElement; + + // If the editor was initialized without specifying an element, we don't need to secure anything. + if ( !sourceElement ) { + return; + } + + if ( sourceElement.hasAttribute( SECURE_SOURCE_ELEMENT_ATTRIBUTE ) ) { + /** + * A DOM element used to create the editor (e.g. + * {@link module:editor-classic/classiceditor~ClassicEditor.create `ClassicEditor.create()`}) + * has already been used to create another editor instance. Make sure each editor is + * created with an unique DOM element. + * + * @error editor-source-element-used-more-than-once + */ + throw new CKEditorError( + 'editor-source-element-used-more-than-once: ' + + 'The DOM source element cannot be used to create an editor more than once.' + ); + } + + sourceElement.setAttribute( SECURE_SOURCE_ELEMENT_ATTRIBUTE, 'true' ); + + this.once( 'destroy', () => { + sourceElement.removeAttribute( SECURE_SOURCE_ELEMENT_ATTRIBUTE ); + } ); } }; diff --git a/src/editor/utils/securesourceelement.js b/src/editor/utils/securesourceelement.js deleted file mode 100644 index e79492d7..00000000 --- a/src/editor/utils/securesourceelement.js +++ /dev/null @@ -1,47 +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 CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; - -/** - * @module core/editor/utils/securesourceelement - */ - -const SECURE_ATTRIBUTE = 'data-ckeditor5'; - -/** - * Checks if the editor was initialized using a source element. If yes, it prevents to creating another editor - * using the same source element. In other words, you cannot use the same source element more than once. - * - * @param {module:core/editor/editor~Editor} editor Editor instance. - */ -export default function secureSourceElement( editor ) { - const sourceElement = editor.sourceElement; - - // If the editor was initialized without specifying an element, we don't need to secure anything. - if ( !sourceElement ) { - return; - } - - if ( sourceElement.hasAttribute( SECURE_ATTRIBUTE ) ) { - /** - * An element passed to the editor creator has been passed more than once. The element can be used only once. - * - * @error securesourceelement-source-element-used-more-than-once - */ - throw new CKEditorError( - 'securesourceelement-source-element-used-more-than-once: ' + - 'The editor cannot be initialized using the same source element more than once.' - ); - } - - // Mark the source element. - sourceElement.setAttribute( SECURE_ATTRIBUTE, 'true' ); - - // Remove the attribute when the editor is being destroyed. - editor.once( 'destroy', () => { - sourceElement.removeAttribute( SECURE_ATTRIBUTE ); - } ); -} diff --git a/tests/editor/utils/elementapimixin.js b/tests/editor/utils/elementapimixin.js index 54587a19..46745357 100644 --- a/tests/editor/utils/elementapimixin.js +++ b/tests/editor/utils/elementapimixin.js @@ -53,4 +53,52 @@ describe( 'ElementApiMixin', () => { ); } ); } ); + + describe( 'secureSourceElement()', () => { + let sourceElement; + + beforeEach( () => { + sourceElement = document.createElement( 'div' ); + + editor.sourceElement = sourceElement; + editor.state = 'ready'; + } ); + + it( 'does not throw if the editor was not initialized using the source element', () => { + delete editor.sourceElement; + + expect( () => { + editor.secureSourceElement(); + } ).to.not.throw(); + } ); + + it( 'does not throw if the editor was initialized using the element for the first time', () => { + expect( () => { + editor.secureSourceElement(); + } ).to.not.throw(); + } ); + + it( 'sets the data attribute after initializing the editor', () => { + editor.secureSourceElement(); + + expect( sourceElement.hasAttribute( 'data-ckeditor5-element' ) ).to.equal( true ); + } ); + + it( 'removes the data attribute after destroying the editor', () => { + editor.secureSourceElement(); + + return editor.destroy() + .then( () => { + expect( sourceElement.hasAttribute( 'data-ckeditor5-element' ) ).to.equal( false ); + } ); + } ); + + it( 'throws an error if the same element was used twice', () => { + sourceElement.setAttribute( 'data-ckeditor5-element', 'true' ); + + expectToThrowCKEditorError( + () => editor.secureSourceElement(), + /^editor-source-element-used-more-than-once/ ); + } ); + } ); } ); diff --git a/tests/editor/utils/securesourceelement.js b/tests/editor/utils/securesourceelement.js deleted file mode 100644 index 1ef671ff..00000000 --- a/tests/editor/utils/securesourceelement.js +++ /dev/null @@ -1,69 +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 - */ - -/* global document */ - -import secureSourceElement from '../../../src/editor/utils/securesourceelement'; -import Editor from '../../../src/editor/editor'; -import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; - -describe( 'secureSourceElement()', () => { - let editor, sourceElement; - - beforeEach( () => { - class CustomEditor extends Editor {} - - sourceElement = document.createElement( 'div' ); - editor = new CustomEditor(); - - editor.sourceElement = sourceElement; - editor.state = 'ready'; - } ); - - afterEach( () => { - if ( editor ) { - return editor.destroy(); - } - } ); - - it( 'does nothing if the editor was not initialized using the source element', () => { - delete editor.sourceElement; - - expect( () => { - secureSourceElement( editor ); - } ).to.not.throw(); - } ); - - it( 'does nothing if the editor was initialized using the element for the first time', () => { - expect( () => { - secureSourceElement( editor ); - } ).to.not.throw(); - } ); - - it( 'sets the data attribute after initializing the editor', () => { - secureSourceElement( editor ); - - expect( sourceElement.hasAttribute( 'data-ckeditor5' ) ).to.equal( true ); - } ); - - it( 'removes the data attribute after destroying the editor', () => { - secureSourceElement( editor ); - - return editor.destroy() - .then( () => { - editor = null; - - expect( sourceElement.hasAttribute( 'data-ckeditor5' ) ).to.equal( false ); - } ); - } ); - - it( 'throws an error if the same element was used twice', () => { - sourceElement.setAttribute( 'data-ckeditor5', 'true' ); - - expect( () => { - secureSourceElement( editor ); - } ).to.throw( CKEditorError, /^securesourceelement-source-element-used-more-than-once/ ); - } ); -} ); From 097805c6b283bc098e73004e9b84b4c748409148 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 19 Jul 2019 17:02:35 +0200 Subject: [PATCH 3/9] Used the dataset API in the ElementApiMixin#secureSourceElement(). --- src/editor/utils/elementapimixin.js | 8 +++----- tests/editor/utils/elementapimixin.js | 6 +++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/editor/utils/elementapimixin.js b/src/editor/utils/elementapimixin.js index 7906e1df..656fe615 100644 --- a/src/editor/utils/elementapimixin.js +++ b/src/editor/utils/elementapimixin.js @@ -10,8 +10,6 @@ import setDataInElement from '@ckeditor/ckeditor5-utils/src/dom/setdatainelement * @module core/editor/utils/elementapimixin */ -const SECURE_SOURCE_ELEMENT_ATTRIBUTE = 'data-ckeditor5-element'; - /** * Implementation of the {@link module:core/editor/utils/elementapimixin~ElementApi}. * @@ -56,7 +54,7 @@ const ElementApiMixin = { return; } - if ( sourceElement.hasAttribute( SECURE_SOURCE_ELEMENT_ATTRIBUTE ) ) { + if ( sourceElement.dataset.ckeditorSecuredElement ) { /** * A DOM element used to create the editor (e.g. * {@link module:editor-classic/classiceditor~ClassicEditor.create `ClassicEditor.create()`}) @@ -71,10 +69,10 @@ const ElementApiMixin = { ); } - sourceElement.setAttribute( SECURE_SOURCE_ELEMENT_ATTRIBUTE, 'true' ); + sourceElement.dataset.ckeditorSecuredElement = true; this.once( 'destroy', () => { - sourceElement.removeAttribute( SECURE_SOURCE_ELEMENT_ATTRIBUTE ); + delete sourceElement.dataset.ckeditorSecuredElement; } ); } }; diff --git a/tests/editor/utils/elementapimixin.js b/tests/editor/utils/elementapimixin.js index 46745357..2cb0edcc 100644 --- a/tests/editor/utils/elementapimixin.js +++ b/tests/editor/utils/elementapimixin.js @@ -81,7 +81,7 @@ describe( 'ElementApiMixin', () => { it( 'sets the data attribute after initializing the editor', () => { editor.secureSourceElement(); - expect( sourceElement.hasAttribute( 'data-ckeditor5-element' ) ).to.equal( true ); + expect( sourceElement.dataset.ckeditorSecuredElement ).to.equal( 'true' ); } ); it( 'removes the data attribute after destroying the editor', () => { @@ -89,12 +89,12 @@ describe( 'ElementApiMixin', () => { return editor.destroy() .then( () => { - expect( sourceElement.hasAttribute( 'data-ckeditor5-element' ) ).to.equal( false ); + expect( sourceElement.dataset.ckeditorSecuredElement ).to.be.undefined; } ); } ); it( 'throws an error if the same element was used twice', () => { - sourceElement.setAttribute( 'data-ckeditor5-element', 'true' ); + sourceElement.dataset.ckeditorSecuredElement = true; expectToThrowCKEditorError( () => editor.secureSourceElement(), From eb92a65988665243ec8373e26880a21580553a27 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 22 Jul 2019 10:43:46 +0200 Subject: [PATCH 4/9] Moved ElementApiMixin#secureSourceElement() back to the helper because it does not make sense in the mixin because the mixin is used wrongly in the first place :P --- src/editor/utils/elementapimixin.js | 37 ------------- src/editor/utils/securesourceelement.js | 49 +++++++++++++++++ tests/editor/utils/elementapimixin.js | 48 ---------------- tests/editor/utils/securesourceelement.js | 67 +++++++++++++++++++++++ 4 files changed, 116 insertions(+), 85 deletions(-) create mode 100644 src/editor/utils/securesourceelement.js create mode 100644 tests/editor/utils/securesourceelement.js diff --git a/src/editor/utils/elementapimixin.js b/src/editor/utils/elementapimixin.js index 656fe615..765fcac7 100644 --- a/src/editor/utils/elementapimixin.js +++ b/src/editor/utils/elementapimixin.js @@ -37,43 +37,6 @@ const ElementApiMixin = { } setDataInElement( this.sourceElement, this.data.get() ); - }, - - /** - * Marks the source element the editor was initialized on preventing other editor instances from - * using this element. - * - * Running multiple editor instances on the same source element causes various issues and it is - * crucial this helper is called as soon as the source element is known to prevent collisions. - */ - secureSourceElement() { - const sourceElement = this.sourceElement; - - // If the editor was initialized without specifying an element, we don't need to secure anything. - if ( !sourceElement ) { - return; - } - - if ( sourceElement.dataset.ckeditorSecuredElement ) { - /** - * A DOM element used to create the editor (e.g. - * {@link module:editor-classic/classiceditor~ClassicEditor.create `ClassicEditor.create()`}) - * has already been used to create another editor instance. Make sure each editor is - * created with an unique DOM element. - * - * @error editor-source-element-used-more-than-once - */ - throw new CKEditorError( - 'editor-source-element-used-more-than-once: ' + - 'The DOM source element cannot be used to create an editor more than once.' - ); - } - - sourceElement.dataset.ckeditorSecuredElement = true; - - this.once( 'destroy', () => { - delete sourceElement.dataset.ckeditorSecuredElement; - } ); } }; diff --git a/src/editor/utils/securesourceelement.js b/src/editor/utils/securesourceelement.js new file mode 100644 index 00000000..aa9a9e04 --- /dev/null +++ b/src/editor/utils/securesourceelement.js @@ -0,0 +1,49 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; + +/** + * @module core/editor/utils/securesourceelement + */ + +/** + * Marks the source element the editor was initialized on preventing other editor instances from + * using this element. + * + * Running multiple editor instances on the same source element causes various issues and it is + * crucial this helper is called as soon as the source element is known to prevent collisions. + * + * @param {module:core/editor/editor~Editor} editor Editor instance. + */ +export default function secureSourceElement( editor ) { + const sourceElement = editor.sourceElement; + + // If the editor was initialized without specifying an element, we don't need to secure anything. + if ( !sourceElement ) { + return; + } + + if ( sourceElement.dataset.ckeditorSecuredElement ) { + /** + * A DOM element used to create the editor (e.g. + * {@link module:editor-classic/classiceditor~ClassicEditor.create `ClassicEditor.create()`}) + * has already been used to create another editor instance. Make sure each editor is + * created with an unique DOM element. + * + * @error editor-source-element-used-more-than-once + */ + throw new CKEditorError( + 'editor-source-element-used-more-than-once: ' + + 'The DOM source element cannot be used to create an editor more than once.' + ); + } + + sourceElement.dataset.ckeditorSecuredElement = true; + + editor.once( 'destroy', () => { + delete sourceElement.dataset.ckeditorSecuredElement; + } ); +} diff --git a/tests/editor/utils/elementapimixin.js b/tests/editor/utils/elementapimixin.js index 2cb0edcc..54587a19 100644 --- a/tests/editor/utils/elementapimixin.js +++ b/tests/editor/utils/elementapimixin.js @@ -53,52 +53,4 @@ describe( 'ElementApiMixin', () => { ); } ); } ); - - describe( 'secureSourceElement()', () => { - let sourceElement; - - beforeEach( () => { - sourceElement = document.createElement( 'div' ); - - editor.sourceElement = sourceElement; - editor.state = 'ready'; - } ); - - it( 'does not throw if the editor was not initialized using the source element', () => { - delete editor.sourceElement; - - expect( () => { - editor.secureSourceElement(); - } ).to.not.throw(); - } ); - - it( 'does not throw if the editor was initialized using the element for the first time', () => { - expect( () => { - editor.secureSourceElement(); - } ).to.not.throw(); - } ); - - it( 'sets the data attribute after initializing the editor', () => { - editor.secureSourceElement(); - - expect( sourceElement.dataset.ckeditorSecuredElement ).to.equal( 'true' ); - } ); - - it( 'removes the data attribute after destroying the editor', () => { - editor.secureSourceElement(); - - return editor.destroy() - .then( () => { - expect( sourceElement.dataset.ckeditorSecuredElement ).to.be.undefined; - } ); - } ); - - it( 'throws an error if the same element was used twice', () => { - sourceElement.dataset.ckeditorSecuredElement = true; - - expectToThrowCKEditorError( - () => editor.secureSourceElement(), - /^editor-source-element-used-more-than-once/ ); - } ); - } ); } ); diff --git a/tests/editor/utils/securesourceelement.js b/tests/editor/utils/securesourceelement.js new file mode 100644 index 00000000..567ceb47 --- /dev/null +++ b/tests/editor/utils/securesourceelement.js @@ -0,0 +1,67 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* global document */ + +import secureSourceElement from '../../../src/editor/utils/securesourceelement'; +import Editor from '../../../src/editor/editor'; +import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; + +describe( 'secureSourceElement()', () => { + let editor, sourceElement; + + beforeEach( () => { + class CustomEditor extends Editor {} + + sourceElement = document.createElement( 'div' ); + editor = new CustomEditor(); + + editor.sourceElement = sourceElement; + editor.state = 'ready'; + } ); + + afterEach( () => { + if ( editor ) { + return editor.destroy(); + } + } ); + + it( 'does not throw if the editor was not initialized using the source element', () => { + delete editor.sourceElement; + + expect( () => { + secureSourceElement( editor ); + } ).to.not.throw(); + } ); + + it( 'does not throw if the editor was initialized using the element for the first time', () => { + expect( () => { + secureSourceElement( editor ); + } ).to.not.throw(); + } ); + + it( 'sets the data attribute after initializing the editor', () => { + secureSourceElement( editor ); + + expect( sourceElement.dataset.ckeditorSecuredElement ).to.equal( 'true' ); + } ); + + it( 'removes the data attribute after destroying the editor', () => { + secureSourceElement( editor ); + + return editor.destroy() + .then( () => { + expect( sourceElement.dataset.ckeditorSecuredElement ).to.be.undefined; + } ); + } ); + + it( 'throws an error if the same element was used twice', () => { + sourceElement.dataset.ckeditorSecuredElement = true; + + expectToThrowCKEditorError( + () => secureSourceElement( editor ), + /^editor-source-element-used-more-than-once/ ); + } ); +} ); From 810e4a4a0c1b5242f694ba6a7b20d6dd1471473b Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 22 Jul 2019 11:36:27 +0200 Subject: [PATCH 5/9] Code refactoring. --- src/editor/utils/securesourceelement.js | 11 +++++++---- tests/editor/utils/securesourceelement.js | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/editor/utils/securesourceelement.js b/src/editor/utils/securesourceelement.js index aa9a9e04..71ddf290 100644 --- a/src/editor/utils/securesourceelement.js +++ b/src/editor/utils/securesourceelement.js @@ -29,15 +29,18 @@ export default function secureSourceElement( editor ) { if ( sourceElement.dataset.ckeditorSecuredElement ) { /** * A DOM element used to create the editor (e.g. - * {@link module:editor-classic/classiceditor~ClassicEditor.create `ClassicEditor.create()`}) + * {@link module:editor-inline/inlineeditor~InlineEditor.create `InlineEditor.create()`}) * has already been used to create another editor instance. Make sure each editor is * created with an unique DOM element. * - * @error editor-source-element-used-more-than-once + * @error securesourceelement-element-used-more-than-once + * @param {HTMLElement} element DOM element that caused the collision. */ throw new CKEditorError( - 'editor-source-element-used-more-than-once: ' + - 'The DOM source element cannot be used to create an editor more than once.' + 'securesourceelement-element-used-more-than-once: ' + + 'The DOM element cannot be used to create multiple editor instances.', + editor, + { element: sourceElement } ); } diff --git a/tests/editor/utils/securesourceelement.js b/tests/editor/utils/securesourceelement.js index 567ceb47..5f2e6f78 100644 --- a/tests/editor/utils/securesourceelement.js +++ b/tests/editor/utils/securesourceelement.js @@ -60,8 +60,8 @@ describe( 'secureSourceElement()', () => { it( 'throws an error if the same element was used twice', () => { sourceElement.dataset.ckeditorSecuredElement = true; - expectToThrowCKEditorError( - () => secureSourceElement( editor ), - /^editor-source-element-used-more-than-once/ ); + expectToThrowCKEditorError( () => { + secureSourceElement( editor ); + }, /^securesourceelement-element-used-more-than-once/, editor, { element: sourceElement } ); } ); } ); From db15727da164d5c61811802614a5191572f8f057 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 22 Jul 2019 14:40:33 +0200 Subject: [PATCH 6/9] Used the editor.sourceElement#ckeditorInstance property to secure the source element. --- src/editor/utils/securesourceelement.js | 9 ++++----- tests/editor/utils/securesourceelement.js | 12 ++++++------ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/editor/utils/securesourceelement.js b/src/editor/utils/securesourceelement.js index 71ddf290..8662266d 100644 --- a/src/editor/utils/securesourceelement.js +++ b/src/editor/utils/securesourceelement.js @@ -26,7 +26,7 @@ export default function secureSourceElement( editor ) { return; } - if ( sourceElement.dataset.ckeditorSecuredElement ) { + if ( sourceElement.ckeditorInstance ) { /** * A DOM element used to create the editor (e.g. * {@link module:editor-inline/inlineeditor~InlineEditor.create `InlineEditor.create()`}) @@ -39,14 +39,13 @@ export default function secureSourceElement( editor ) { throw new CKEditorError( 'securesourceelement-element-used-more-than-once: ' + 'The DOM element cannot be used to create multiple editor instances.', - editor, - { element: sourceElement } + editor ); } - sourceElement.dataset.ckeditorSecuredElement = true; + sourceElement.ckeditorInstance = editor; editor.once( 'destroy', () => { - delete sourceElement.dataset.ckeditorSecuredElement; + delete sourceElement.ckeditorInstance; } ); } diff --git a/tests/editor/utils/securesourceelement.js b/tests/editor/utils/securesourceelement.js index 5f2e6f78..f6303419 100644 --- a/tests/editor/utils/securesourceelement.js +++ b/tests/editor/utils/securesourceelement.js @@ -42,26 +42,26 @@ describe( 'secureSourceElement()', () => { } ).to.not.throw(); } ); - it( 'sets the data attribute after initializing the editor', () => { + it( 'sets the property after initializing the editor', () => { secureSourceElement( editor ); - expect( sourceElement.dataset.ckeditorSecuredElement ).to.equal( 'true' ); + expect( sourceElement.ckeditorInstance ).to.equal( editor ); } ); - it( 'removes the data attribute after destroying the editor', () => { + it( 'removes the property after destroying the editor', () => { secureSourceElement( editor ); return editor.destroy() .then( () => { - expect( sourceElement.dataset.ckeditorSecuredElement ).to.be.undefined; + expect( sourceElement.ckeditorInstance ).to.be.undefined; } ); } ); it( 'throws an error if the same element was used twice', () => { - sourceElement.dataset.ckeditorSecuredElement = true; + sourceElement.ckeditorInstance = 'foo'; expectToThrowCKEditorError( () => { secureSourceElement( editor ); - }, /^securesourceelement-element-used-more-than-once/, editor, { element: sourceElement } ); + }, /^securesourceelement-element-used-more-than-once/, editor ); } ); } ); From 97882c718b1ef7ceeccff16f2b79dc02ac1aec01 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 22 Jul 2019 14:40:59 +0200 Subject: [PATCH 7/9] Do not override element#ckeditorInstance in EditorUI (just to stay on the safe side). --- src/editor/editorui.js | 4 +++- tests/editor/editorui.js | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/editor/editorui.js b/src/editor/editorui.js index 7047d861..0c7f6cc2 100644 --- a/src/editor/editorui.js +++ b/src/editor/editorui.js @@ -124,7 +124,9 @@ export default class EditorUI { // It helps 3rd–party software (browser extensions, other libraries) access and recognize // CKEditor 5 instances (editing roots) and use their API (there is no global editor // instance registry). - domElement.ckeditorInstance = this.editor; + if ( !domElement.ckeditorInstance ) { + domElement.ckeditorInstance = this.editor; + } } /** diff --git a/tests/editor/editorui.js b/tests/editor/editorui.js index c7e4b496..8ebb2ab3 100644 --- a/tests/editor/editorui.js +++ b/tests/editor/editorui.js @@ -130,6 +130,17 @@ describe( 'EditorUI', () => { expect( element.ckeditorInstance ).to.equal( editor ); } ); + + it( 'does not override a reference to the editor instance in domElement#ckeditorInstance', () => { + const ui = new EditorUI( editor ); + const element = document.createElement( 'div' ); + + element.ckeditorInstance = 'foo'; + + ui.setEditableElement( 'main', element ); + + expect( element.ckeditorInstance ).to.equal( 'foo' ); + } ); } ); describe( 'getEditableElement()', () => { From e7dd2d8bb3f8c155fe9b210d777116fb190afcfa Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 23 Jul 2019 08:46:57 +0200 Subject: [PATCH 8/9] Code refactoring: Renamed the error thrown by the secureSourceElement() helper. --- src/editor/utils/securesourceelement.js | 4 ++-- tests/editor/utils/securesourceelement.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/editor/utils/securesourceelement.js b/src/editor/utils/securesourceelement.js index 8662266d..184255e8 100644 --- a/src/editor/utils/securesourceelement.js +++ b/src/editor/utils/securesourceelement.js @@ -33,11 +33,11 @@ export default function secureSourceElement( editor ) { * has already been used to create another editor instance. Make sure each editor is * created with an unique DOM element. * - * @error securesourceelement-element-used-more-than-once + * @error editor-source-element-already-used * @param {HTMLElement} element DOM element that caused the collision. */ throw new CKEditorError( - 'securesourceelement-element-used-more-than-once: ' + + 'editor-source-element-already-used: ' + 'The DOM element cannot be used to create multiple editor instances.', editor ); diff --git a/tests/editor/utils/securesourceelement.js b/tests/editor/utils/securesourceelement.js index f6303419..8388e14f 100644 --- a/tests/editor/utils/securesourceelement.js +++ b/tests/editor/utils/securesourceelement.js @@ -62,6 +62,6 @@ describe( 'secureSourceElement()', () => { expectToThrowCKEditorError( () => { secureSourceElement( editor ); - }, /^securesourceelement-element-used-more-than-once/, editor ); + }, /^editor-source-element-already-used/, editor ); } ); } ); From b8656e412c2ead30dab8f59ee751c114e5a90a68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 23 Jul 2019 12:18:15 +0200 Subject: [PATCH 9/9] Rewrite secureSourceElement() description. --- src/editor/utils/securesourceelement.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/editor/utils/securesourceelement.js b/src/editor/utils/securesourceelement.js index 184255e8..f3b62df2 100644 --- a/src/editor/utils/securesourceelement.js +++ b/src/editor/utils/securesourceelement.js @@ -10,8 +10,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; */ /** - * Marks the source element the editor was initialized on preventing other editor instances from - * using this element. + * Marks the source element on which the editor was initialized. This prevents other editor instances from using this element. * * Running multiple editor instances on the same source element causes various issues and it is * crucial this helper is called as soon as the source element is known to prevent collisions.