From f6d139dc7214b4c59c4b48d5e6622ef1f05862c5 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 10:20:20 +0200 Subject: [PATCH 01/28] Created a RawElement class. --- .../ckeditor5-engine/src/view/rawelement.js | 163 ++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 packages/ckeditor5-engine/src/view/rawelement.js diff --git a/packages/ckeditor5-engine/src/view/rawelement.js b/packages/ckeditor5-engine/src/view/rawelement.js new file mode 100644 index 00000000000..40158c55b24 --- /dev/null +++ b/packages/ckeditor5-engine/src/view/rawelement.js @@ -0,0 +1,163 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module engine/view/rawelement + */ + +import Element from './element'; +import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; +import Node from './node'; + +/** + * The raw element class. + * + * It is used to represent elements that TODO. + * + * To create a new raw element use the + * {@link module:engine/view/downcastwriter~DowncastWriter#createRawElement `downcastWriter#createRawElement()`} method. + * + * @extends module:engine/view/element~Element + */ +export default class RawElement extends Element { + /** + * Creates new instance of RawElement. + * + * Throws {@link module:utils/ckeditorerror~CKEditorError CKEditorError} `view-rawelement-cannot-add` when `children` parameter + * is passed, to inform that usage of `RawElement` is incorrect (adding child nodes to `RawElement` is forbidden). + * + * @see module:engine/view/downcastwriter~DowncastWriter#createRawElement + * @protected + * @param {module:engine/view/document~Document} document The document instance to which this element belongs. + * @param {String} name Node name. + * @param {Object|Iterable} [attrs] Collection of attributes. + * @param {module:engine/view/node~Node|Iterable.} [children] + * A list of nodes to be inserted into created element. + */ + constructor( document, name, attrs, children ) { + super( document, name, attrs, children ); + + /** + * Returns `null` because filler is not needed for RawElements. + * + * @method #getFillerOffset + * @returns {null} Always returns null. + */ + this.getFillerOffset = getFillerOffset; + } + + /** + * Checks whether this object is of the given type or name. + * + * rawElement.is( 'rawElement' ); // -> true + * rawElement.is( 'element' ); // -> true + * rawElement.is( 'node' ); // -> true + * rawElement.is( 'view:rawElement' ); // -> true + * rawElement.is( 'view:element' ); // -> true + * rawElement.is( 'view:node' ); // -> true + * + * rawElement.is( 'model:element' ); // -> false + * rawElement.is( 'documentFragment' ); // -> false + * + * Assuming that the object being checked is a raw element, you can also check its + * {@link module:engine/view/rawelement~RawElement#name name}: + * + * rawElement.is( 'img' ); // -> true if this is a img element + * rawElement.is( 'rawElement', 'img' ); // -> same as above + * text.is( 'img' ); -> false + * + * {@link module:engine/view/node~Node#is Check the entire list of view objects} which implement the `is()` method. + * + * @param {String} type Type to check when `name` parameter is present. + * Otherwise, it acts like the `name` parameter. + * @param {String} [name] Element name. + * @returns {Boolean} + */ + is( type, name = null ) { + if ( !name ) { + return type === 'rawElement' || type === 'view:rawElement' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === this.name || type === 'view:' + this.name || + type === 'element' || type === 'view:element' || + type === 'node' || type === 'view:node'; + } else { + return name === this.name && ( + type === 'rawElement' || type === 'view:rawElement' || + type === 'element' || type === 'view:element' + ); + } + } + + /** + * Overrides {@link module:engine/view/element~Element#_insertChild} method. + * Throws {@link module:utils/ckeditorerror~CKEditorError CKEditorError} `view-rawelement-cannot-add` to prevent + * adding any child nodes to a `RawElement`. + * + * @protected + */ + _insertChild( index, nodes ) { + if ( nodes && ( nodes instanceof Node || Array.from( nodes ).length > 0 ) ) { + /** + * Cannot add children to a {@link module:engine/view/rawelement~RawElement}. + * + * @error view-rawelement-cannot-add + */ + throw new CKEditorError( + 'view-rawelement-cannot-add: Cannot add child nodes to RawElement instance.', + [ this, nodes ] + ); + } + } + + /** + * Renders this {@link module:engine/view/rawelement~RawElement} to DOM. This method is called by + * {@link module:engine/view/domconverter~DomConverter}. + * Do not use inheritance to create custom rendering method, replace `render()` method instead: + * + * const myRawElement = downcastWriter.createRawElement( 'span' ); + * + * myRawElement.render = function( domDocument ) { + * const domElement = this.toDomElement( domDocument ); + * domElement.innerHTML = 'this is ui element'; + * + * return domElement; + * }; + * + * If changes in your raw element should trigger some editor UI update you should call + * the {@link module:core/editor/editorui~EditorUI#update `editor.ui.update()`} method + * after rendering your UI element. + * + * @param {Document} domDocument + * @returns {HTMLElement} + */ + render( domDocument ) { + return this.toDomElement( domDocument ); + } + + /** + * Creates DOM element based on this view `RawElement`. + * + * **Note**: Each time this method is called new DOM element is created. + * + * @param {Document} domDocument + * @returns {HTMLElement} + */ + toDomElement( domDocument ) { + const domElement = domDocument.createElement( this.name ); + + for ( const key of this.getAttributeKeys() ) { + domElement.setAttribute( key, this.getAttribute( key ) ); + } + + return domElement; + } +} + +// Returns `null` because block filler is not needed for RawElements. +// +// @returns {null} +function getFillerOffset() { + return null; +} From 2deb00b7210d7f15dba6f232fbed346228c75f50 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 10:21:24 +0200 Subject: [PATCH 02/28] Added support for RawElement in getData(), stringify(), and parse() view utils. --- .../ckeditor5-engine/src/dev-utils/view.js | 20 +++++-- .../ckeditor5-engine/tests/dev-utils/view.js | 58 ++++++++++++++++++- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/packages/ckeditor5-engine/src/dev-utils/view.js b/packages/ckeditor5-engine/src/dev-utils/view.js index 38629c7a062..0be96687ebd 100644 --- a/packages/ckeditor5-engine/src/dev-utils/view.js +++ b/packages/ckeditor5-engine/src/dev-utils/view.js @@ -25,6 +25,7 @@ import AttributeElement from '../view/attributeelement'; import ContainerElement from '../view/containerelement'; import EmptyElement from '../view/emptyelement'; import UIElement from '../view/uielement'; +import RawElement from '../view/rawelement'; import { StylesProcessor } from '../view/stylesmap'; const ELEMENT_RANGE_START_TOKEN = '['; @@ -35,7 +36,8 @@ const allowedTypes = { 'container': ContainerElement, 'attribute': AttributeElement, 'empty': EmptyElement, - 'ui': UIElement + 'ui': UIElement, + 'raw': RawElement }; /** @@ -55,6 +57,8 @@ const allowedTypes = { * (``). * @param {Boolean} [options.renderUIElements=false] When set to `true`, the inner content of each * {@link module:engine/view/uielement~UIElement} will be printed. + * @param {Boolean} [options.renderRawElements=false] When set to `true`, the inner content of each + * {@link module:engine/view/rawelement~RawElement} will be printed. * @returns {String} The stringified data. */ export function getData( view, options = {} ) { @@ -70,6 +74,7 @@ export function getData( view, options = {} ) { showType: options.showType, showPriority: options.showPriority, renderUIElements: options.renderUIElements, + renderRawElements: options.renderRawElements, ignoreRoot: true }; @@ -234,6 +239,8 @@ setData._parse = parse; * `{` and `}` and the selection outside the text as `[` and `]`. When set to `false`, both will be marked as `[` and `]` only. * @param {Boolean} [options.renderUIElements=false] When set to `true`, the inner content of each * {@link module:engine/view/uielement~UIElement} will be printed. + * @param {Boolean} [options.renderRawElements=false] When set to `true`, the inner content of each + * {@link module:engine/view/rawelement~RawElement} will be printed. * @returns {String} An HTML-like string representing the view. */ export function stringify( node, selectionOrPositionOrRange = null, options = {} ) { @@ -622,6 +629,8 @@ class ViewStringify { * `{` and `}` and the selection outside the text as `[` and `]`. When set to `false`, both are marked as `[` and `]`. * @param {Boolean} [options.renderUIElements=false] When set to `true`, the inner content of each * {@link module:engine/view/uielement~UIElement} will be printed. + * @param {Boolean} [options.renderRawElements=false] When set to `true`, the inner content of each + * {@link module:engine/view/rawelement~RawElement} will be printed. */ constructor( root, selection, options ) { this.root = root; @@ -638,6 +647,7 @@ class ViewStringify { this.ignoreRoot = !!options.ignoreRoot; this.sameSelectionCharacters = !!options.sameSelectionCharacters; this.renderUIElements = !!options.renderUIElements; + this.renderRawElements = !!options.renderRawElements; } /** @@ -670,7 +680,7 @@ class ViewStringify { callback( this._stringifyElementOpen( root ) ); } - if ( this.renderUIElements && root.is( 'uiElement' ) ) { + if ( ( this.renderUIElements && root.is( 'uiElement' ) ) || ( this.renderRawElements && root.is( 'rawElement' ) ) ) { callback( root.render( document ).innerHTML ); } else { let offset = 0; @@ -943,10 +953,10 @@ function _convertViewElements( rootNode ) { for ( const child of [ ...rootNode.getChildren() ] ) { if ( convertedElement.is( 'emptyElement' ) ) { throw new Error( 'Parse error - cannot parse inside EmptyElement.' ); - } - - if ( convertedElement.is( 'uiElement' ) ) { + } else if ( convertedElement.is( 'uiElement' ) ) { throw new Error( 'Parse error - cannot parse inside UIElement.' ); + } else if ( convertedElement.is( 'rawElement' ) ) { + throw new Error( 'Parse error - cannot parse inside RawElement.' ); } convertedElement._appendChild( _convertViewElements( child ) ); diff --git a/packages/ckeditor5-engine/tests/dev-utils/view.js b/packages/ckeditor5-engine/tests/dev-utils/view.js index 27ee963201c..e983535b30e 100644 --- a/packages/ckeditor5-engine/tests/dev-utils/view.js +++ b/packages/ckeditor5-engine/tests/dev-utils/view.js @@ -14,6 +14,7 @@ import AttributeElement from '../../src/view/attributeelement'; import ContainerElement from '../../src/view/containerelement'; import EmptyElement from '../../src/view/emptyelement'; import UIElement from '../../src/view/uielement'; +import RawElement from '../../src/view/rawelement'; import Text from '../../src/view/text'; import DocumentSelection from '../../src/view/documentselection'; import Range from '../../src/view/range'; @@ -404,6 +405,45 @@ describe( 'view test utils', () => { .to.equal( 'foo' ); } ); + it( 'should stringify a RawElement', () => { + const span = new RawElement( viewDocument, 'span' ); + const p = new ContainerElement( viewDocument, 'p', null, span ); + expect( stringify( p, null, { showType: true } ) ) + .to.equal( '' ); + } ); + + it( 'should not stringify the inner RawElement content (renderRawElements=false)', () => { + const span = new RawElement( viewDocument, 'span' ); + + span.render = function( domDocument ) { + const domElement = this.toDomElement( domDocument ); + + domElement.innerHTML = 'foo'; + + return domElement; + }; + + const p = new ContainerElement( viewDocument, 'p', null, span ); + expect( stringify( p, null, { showType: true } ) ) + .to.equal( '' ); + } ); + + it( 'should stringify a RawElement, (renderRawElements=true)', () => { + const span = new RawElement( viewDocument, 'span' ); + + span.render = function( domDocument ) { + const domElement = this.toDomElement( domDocument ); + + domElement.innerHTML = 'foo'; + + return domElement; + }; + + const p = new ContainerElement( viewDocument, 'p', null, span ); + expect( stringify( p, null, { showType: true, renderRawElements: true } ) ) + .to.equal( 'foo' ); + } ); + it( 'should sort classes in specified element', () => { const text = new Text( viewDocument, 'foobar' ); const b = new Element( viewDocument, 'b', { @@ -714,29 +754,41 @@ describe( 'view test utils', () => { expect( stringify( data ) ).to.equal( '

texttest

' ); } ); - it( 'should parse EmptyElement', () => { + it( 'should parse an EmptyElement', () => { const parsed = parse( '' ); expect( parsed ).to.be.instanceof( EmptyElement ); } ); - it( 'should parse UIElement', () => { + it( 'should parse a UIElement', () => { const parsed = parse( '' ); expect( parsed ).to.be.instanceof( UIElement ); } ); + it( 'should parse a RawElement', () => { + const parsed = parse( '' ); + + expect( parsed ).to.be.instanceof( RawElement ); + } ); + it( 'should throw an error if EmptyElement is not empty', () => { expect( () => { parse( 'foo bar' ); } ).to.throw( Error, 'Parse error - cannot parse inside EmptyElement.' ); } ); - it( 'should throw an error if UIElement is not empty', () => { + it( 'should throw an error if a UIElement is not empty', () => { expect( () => { parse( 'foo bar' ); } ).to.throw( Error, 'Parse error - cannot parse inside UIElement.' ); } ); + + it( 'should throw an error if a RawElement is not empty', () => { + expect( () => { + parse( 'foo bar' ); + } ).to.throw( Error, 'Parse error - cannot parse inside RawElement.' ); + } ); } ); } ); From e694001462b960aee8bb62cc7e4753c84f6cd5bc Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 11:46:50 +0200 Subject: [PATCH 03/28] Added RawElement support in the MutationObserver. --- .../src/view/observer/mutationobserver.js | 8 +-- .../tests/view/observer/mutationobserver.js | 60 +++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/observer/mutationobserver.js b/packages/ckeditor5-engine/src/view/observer/mutationobserver.js index 44cd39af137..ca983ce13ec 100644 --- a/packages/ckeditor5-engine/src/view/observer/mutationobserver.js +++ b/packages/ckeditor5-engine/src/view/observer/mutationobserver.js @@ -150,8 +150,8 @@ export default class MutationObserver extends Observer { if ( mutation.type === 'childList' ) { const element = domConverter.mapDomToView( mutation.target ); - // Do not collect mutations from UIElements. - if ( element && element.is( 'uiElement' ) ) { + // Do not collect mutations from UIElements and RawElements. + if ( element && ( element.is( 'uiElement' ) || element.is( 'rawElement' ) ) ) { continue; } @@ -165,8 +165,8 @@ export default class MutationObserver extends Observer { for ( const mutation of domMutations ) { const element = domConverter.mapDomToView( mutation.target ); - // Do not collect mutations from UIElements. - if ( element && element.is( 'uiElement' ) ) { + // Do not collect mutations from UIElements and RawElements. + if ( element && ( element.is( 'uiElement' ) || element.is( 'rawElement' ) ) ) { continue; } diff --git a/packages/ckeditor5-engine/tests/view/observer/mutationobserver.js b/packages/ckeditor5-engine/tests/view/observer/mutationobserver.js index 9e563755b16..dd17d87e590 100644 --- a/packages/ckeditor5-engine/tests/view/observer/mutationobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/mutationobserver.js @@ -8,6 +8,7 @@ import View from '../../../src/view/view'; import MutationObserver from '../../../src/view/observer/mutationobserver'; import UIElement from '../../../src/view/uielement'; +import RawElement from '../../../src/view/rawelement'; import createViewRoot from '../_utils/createroot'; import { parse } from '../../../src/dev-utils/view'; import { StylesProcessor } from '../../../src/view/stylesmap'; @@ -591,6 +592,65 @@ describe( 'MutationObserver', () => { } ); } ); + describe( 'RawElement integration', () => { + const renderStub = sinon.stub(); + function createRawElement( name ) { + const element = new RawElement( name ); + + element.render = function( domDocument ) { + const root = this.toDomElement( domDocument ); + root.innerHTML = 'foo bar'; + + return root; + }; + + return element; + } + + beforeEach( () => { + const rawElement = createRawElement( 'div' ); + viewRoot._appendChild( rawElement ); + + view.forceRender(); + renderStub.reset(); + view.on( 'render', renderStub ); + } ); + + it( 'should not collect text mutations from RawElement', () => { + domEditor.childNodes[ 2 ].childNodes[ 0 ].data = 'foom'; + + mutationObserver.flush(); + + expect( lastMutations ).to.be.null; + } ); + + it( 'should not cause a render from RawElement', () => { + domEditor.childNodes[ 2 ].childNodes[ 0 ].data = 'foom'; + + mutationObserver.flush(); + + expect( renderStub.callCount ).to.equal( 0 ); + } ); + + it( 'should not collect child mutations from RawElement', () => { + const span = document.createElement( 'span' ); + domEditor.childNodes[ 2 ].appendChild( span ); + + mutationObserver.flush(); + + expect( lastMutations ).to.be.null; + } ); + + it( 'should not cause a render when RawElement gets a child', () => { + const span = document.createElement( 'span' ); + domEditor.childNodes[ 2 ].appendChild( span ); + + mutationObserver.flush(); + + expect( renderStub.callCount ).to.equal( 0 ); + } ); + } ); + function expectDomEditorNotToChange() { expect( domEditor.childNodes.length ).to.equal( 2 ); expect( domEditor.childNodes[ 0 ].tagName ).to.equal( 'P' ); From eec1c98e83f21a2a2ec727d05070d255509619fc Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 14:51:29 +0200 Subject: [PATCH 04/28] Made it possible to use the RawElement in the DomConverter. --- .../ckeditor5-engine/src/view/domconverter.js | 69 ++++---- .../tests/view/domconverter/rawelement.js | 152 ++++++++++++++++++ .../tests/view/domconverter/uielement.js | 8 +- 3 files changed, 196 insertions(+), 33 deletions(-) create mode 100644 packages/ckeditor5-engine/tests/view/domconverter/rawelement.js diff --git a/packages/ckeditor5-engine/src/view/domconverter.js b/packages/ckeditor5-engine/src/view/domconverter.js index 8abeb1d6cf6..b2fb7706e13 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.js +++ b/packages/ckeditor5-engine/src/view/domconverter.js @@ -29,16 +29,16 @@ import { isElement } from 'lodash-es'; const BR_FILLER_REF = BR_FILLER( document ); /** - * DomConverter is a set of tools to do transformations between DOM nodes and view nodes. It also handles - * {@link module:engine/view/domconverter~DomConverter#bindElements binding} these nodes. + * `DomConverter` is a set of tools to do transformations between DOM nodes and view nodes. It also handles + * {@link module:engine/view/domconverter~DomConverter#bindElements bindings} between these nodes. * - * The instance of DOMConverter is available in {@link module:engine/view/view~View#domConverter `editor.editing.view.domConverter`}. + * The instance of `DOMConverter` is available under {@link module:engine/view/view~View#domConverter `editor.editing.view.domConverter`}. * - * DomConverter does not check which nodes should be rendered (use {@link module:engine/view/renderer~Renderer}), does not keep a + * `DomConverter` does not check which nodes should be rendered (use {@link module:engine/view/renderer~Renderer}), does not keep a * state of a tree nor keeps synchronization between tree view and DOM tree (use {@link module:engine/view/document~Document}). * - * DomConverter keeps DOM elements to View element bindings, so when the converter will be destroyed, the binding will - * be lost. Two converters will keep separate binding maps, so one tree view can be bound with two DOM trees. + * `DomConverter` keeps DOM elements to View element bindings, so when the converter gets destroyed, the bindings are lost. + * Two converters will keep separate binding maps, so one tree view can be bound with two DOM trees. */ export default class DomConverter { /** @@ -229,8 +229,12 @@ export default class DomConverter { return domElement; } else { + // RawElement has its own render() method (see https://github.com/ckeditor/ckeditor5/issues/4469). + if ( viewNode.is( 'rawElement' ) ) { + domElement = viewNode.render( domDocument ); + } // Create DOM element. - if ( viewNode.hasAttribute( 'xmlns' ) ) { + else if ( viewNode.hasAttribute( 'xmlns' ) ) { domElement = domDocument.createElementNS( viewNode.getAttribute( 'xmlns' ), viewNode.name ); } else { domElement = domDocument.createElement( viewNode.name ); @@ -392,11 +396,11 @@ export default class DomConverter { return null; } - // When node is inside UIElement return that UIElement as it's view representation. - const uiElement = this.getParentUIElement( domNode, this._domToViewMapping ); + // When node is inside a UIElement or a RawElement return that parent as it's view representation. + const hostElement = this.getHostViewElement( domNode, this._domToViewMapping ); - if ( uiElement ) { - return uiElement; + if ( hostElement ) { + return hostElement; } if ( isText( domNode ) ) { @@ -550,10 +554,10 @@ export default class DomConverter { return this.domPositionToView( domParent.parentNode, indexOf( domParent ) ); } - // If position is somewhere inside UIElement - return position before that element. + // If position is somewhere inside UIElement or a RawElement - return position before that element. const viewElement = this.mapDomToView( domParent ); - if ( viewElement && viewElement.is( 'uiElement' ) ) { + if ( viewElement && ( viewElement.is( 'uiElement' ) || viewElement.is( 'rawElement' ) ) ) { return ViewPosition._createBefore( viewElement ); } @@ -605,14 +609,18 @@ export default class DomConverter { * {@link module:engine/view/documentfragment~DocumentFragment} for provided DOM element or * document fragment. If there is no view item {@link module:engine/view/domconverter~DomConverter#bindElements bound} * to the given DOM - `undefined` is returned. - * For all DOM elements rendered by {@link module:engine/view/uielement~UIElement} that UIElement will be returned. + * + * For all DOM elements rendered by a {@link module:engine/view/uielement~UIElement} or + * a {@link module:engine/view/rawelement~RawElement}, the parent `UIElement` or `RawElement` will be returned. * * @param {DocumentFragment|Element} domElementOrDocumentFragment DOM element or document fragment. * @returns {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment|undefined} * Corresponding view element, document fragment or `undefined` if no element was bound. */ mapDomToView( domElementOrDocumentFragment ) { - return this.getParentUIElement( domElementOrDocumentFragment ) || this._domToViewMapping.get( domElementOrDocumentFragment ); + const hostElement = this.getHostViewElement( domElementOrDocumentFragment ); + + return hostElement || this._domToViewMapping.get( domElementOrDocumentFragment ); } /** @@ -625,7 +633,8 @@ export default class DomConverter { * If this is a first child in the parent and the parent is a {@link module:engine/view/domconverter~DomConverter#bindElements bound} * element, it is used to find the corresponding text node. * - * For all text nodes rendered by {@link module:engine/view/uielement~UIElement} that UIElement will be returned. + * For all text nodes rendered by a {@link module:engine/view/uielement~UIElement} or + * a {@link module:engine/view/rawelement~RawElement}, the parent `UIElement` or `RawElement` will be returned. * * Otherwise `null` is returned. * @@ -640,11 +649,11 @@ export default class DomConverter { return null; } - // If DOM text was rendered by UIElement - return that element. - const uiElement = this.getParentUIElement( domText ); + // If DOM text was rendered by a UIElement or a RawElement - return this parent element. + const hostElement = this.getHostViewElement( domText ); - if ( uiElement ) { - return uiElement; + if ( hostElement ) { + return hostElement; } const previousSibling = domText.previousSibling; @@ -858,13 +867,13 @@ export default class DomConverter { } /** - * Returns parent {@link module:engine/view/uielement~UIElement} for provided DOM node. Returns `null` if there is no - * parent UIElement. + * Returns a parent {@link module:engine/view/uielement~UIElement} or {@link module:engine/view/rawelement~RawElement} + * that hosts the provided DOM node. Returns `null` if there is no such parent. * * @param {Node} domNode - * @returns {module:engine/view/uielement~UIElement|null} + * @returns {module:engine/view/uielement~UIElement|module:engine/view/rawelement~RawElement|null} */ - getParentUIElement( domNode ) { + getHostViewElement( domNode ) { const ancestors = getAncestors( domNode ); // Remove domNode from the list. @@ -874,7 +883,7 @@ export default class DomConverter { const domNode = ancestors.pop(); const viewNode = this._domToViewMapping.get( domNode ); - if ( viewNode && viewNode.is( 'uiElement' ) ) { + if ( viewNode && ( viewNode.is( 'uiElement' ) || viewNode.is( 'rawElement' ) ) ) { return viewNode; } } @@ -887,7 +896,8 @@ export default class DomConverter { * * The following places are considered as incorrect for selection boundaries: * * before or in the middle of the inline filler sequence, - * * inside the DOM element which represents {@link module:engine/view/uielement~UIElement a view ui element}. + * * inside the DOM element which represents {@link module:engine/view/uielement~UIElement a view UI element}. + * * inside the DOM element which represents {@link module:engine/view/rawelement~RawElement a view raw element}. * * @param {Selection} domSelection DOM Selection object to be checked. * @returns {Boolean} `true` if the given selection is at a correct place, `false` otherwise. @@ -919,9 +929,10 @@ export default class DomConverter { const viewParent = this.mapDomToView( domParent ); - // If selection is in `view.UIElement`, it is incorrect. Note that `mapDomToView()` returns `view.UIElement` - // also for any dom element that is inside the view ui element (so we don't need to perform any additional checks). - if ( viewParent && viewParent.is( 'uiElement' ) ) { + // The position is incorrect when anchored inside a UIElement or a RawElement. + // Note: In case of UIElement and RawElement, mapDomToView() returns a parent element for any DOM child + // so there's no need to perform any additional checks. + if ( viewParent && ( viewParent.is( 'uiElement' ) || viewParent.is( 'rawElement' ) ) ) { return false; } diff --git a/packages/ckeditor5-engine/tests/view/domconverter/rawelement.js b/packages/ckeditor5-engine/tests/view/domconverter/rawelement.js new file mode 100644 index 00000000000..2b6fa9c7163 --- /dev/null +++ b/packages/ckeditor5-engine/tests/view/domconverter/rawelement.js @@ -0,0 +1,152 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* globals document, HTMLElement */ + +import ViewRawElement from '../../../src/view/rawelement'; +import ViewContainer from '../../../src/view/containerelement'; +import DomConverter from '../../../src/view/domconverter'; +import ViewDocument from '../../../src/view/document'; +import { StylesProcessor } from '../../../src/view/stylesmap'; + +describe( 'DOMConverter RawElement integration', () => { + let converter, viewDocument; + + function createRawElement( name ) { + const element = new ViewRawElement( viewDocument, name ); + + element.render = function( domDocument ) { + const root = this.toDomElement( domDocument ); + root.innerHTML = '

foo bar

'; + + return root; + }; + + return element; + } + + beforeEach( () => { + viewDocument = new ViewDocument( new StylesProcessor() ); + converter = new DomConverter( viewDocument ); + } ); + + describe( 'viewToDom()', () => { + it( 'should create a DOM element from a RawElement', () => { + const rawElement = new ViewRawElement( viewDocument, 'div' ); + const domElement = converter.viewToDom( rawElement, document ); + + expect( domElement ).to.be.instanceOf( HTMLElement ); + } ); + + it( 'should create a DOM structure from a RawElement', () => { + const myElement = createRawElement( 'div' ); + const domElement = converter.viewToDom( myElement, document ); + + expect( domElement ).to.be.instanceOf( HTMLElement ); + expect( domElement.innerHTML ).to.equal( '

foo bar

' ); + } ); + + it( 'should create a DOM structure entirely mapped to a single RawElement', () => { + const myElement = createRawElement( 'div' ); + const domElement = converter.viewToDom( myElement, document, { bind: true } ); + const domParagraph = domElement.childNodes[ 0 ]; + + expect( converter.mapDomToView( domElement ) ).to.equal( myElement ); + expect( converter.mapDomToView( domParagraph ) ).to.equal( myElement ); + expect( converter.mapDomToView( domParagraph.childNodes[ 0 ] ) ).to.equal( myElement ); + } ); + } ); + + describe( 'domToView()', () => { + it( 'should return a RawElement', () => { + const rawElement = createRawElement( 'div' ); + const domElement = converter.viewToDom( rawElement, document, { bind: true } ); + + expect( converter.domToView( domElement ) ).to.equal( rawElement ); + } ); + + it( 'should return a RawElement for all nodes inside of it', () => { + const rawElement = createRawElement( 'div' ); + const domElement = converter.viewToDom( rawElement, document, { bind: true } ); + + const domParagraph = domElement.childNodes[ 0 ]; + const domSpan = domParagraph.childNodes[ 0 ]; + + expect( converter.domToView( domParagraph ) ).to.equal( rawElement ); + expect( converter.domToView( domSpan ) ).to.equal( rawElement ); + expect( converter.domToView( domParagraph.childNodes[ 0 ] ) ).equal( rawElement ); + expect( converter.domToView( domSpan.childNodes[ 0 ] ) ).equal( rawElement ); + } ); + } ); + + describe( 'domPositionToView()', () => { + it( 'should convert a position inside a RawElement to a position before it', () => { + const rawElement = createRawElement( 'h1' ); + const container = new ViewContainer( viewDocument, 'div', null, [ new ViewContainer( viewDocument, 'div' ), rawElement ] ); + const domContainer = converter.viewToDom( container, document, { bind: true } ); + + const viewPosition = converter.domPositionToView( domContainer.childNodes[ 1 ], 0 ); + + expect( viewPosition.parent ).to.equal( container ); + expect( viewPosition.offset ).to.equal( 1 ); + } ); + + it( 'should convert a position inside RawElement children to a position before it', () => { + const rawElement = createRawElement( 'h1' ); + const container = new ViewContainer( viewDocument, 'div', null, [ new ViewContainer( viewDocument, 'div' ), rawElement ] ); + const domContainer = converter.viewToDom( container, document, { bind: true } ); + + const viewPosition = converter.domPositionToView( domContainer.childNodes[ 1 ].childNodes[ 0 ], 1 ); + + expect( viewPosition.parent ).to.equal( container ); + expect( viewPosition.offset ).to.equal( 1 ); + } ); + } ); + + describe( 'mapDomToView()', () => { + it( 'should return a RawElement for all DOM elements inside of it', () => { + const myElement = createRawElement( 'div' ); + const domElement = converter.viewToDom( myElement, document, { bind: true } ); + + expect( converter.mapDomToView( domElement ) ).to.equal( myElement ); + + const domParagraph = domElement.childNodes[ 0 ]; + expect( converter.mapDomToView( domParagraph ) ).to.equal( myElement ); + + const domSpan = domParagraph.childNodes[ 0 ]; + expect( converter.mapDomToView( domSpan ) ).to.equal( myElement ); + } ); + } ); + + describe( 'findCorrespondingViewText()', () => { + it( 'should return a RawElement for all DOM text nodes inside of it', () => { + const myElement = createRawElement( 'div' ); + const domElement = converter.viewToDom( myElement, document, { bind: true } ); + + const domText = domElement.querySelector( 'span' ).childNodes[ 0 ]; + expect( converter.findCorrespondingViewText( domText ) ).to.equal( myElement ); + } ); + } ); + + describe( 'getHostViewElement()', () => { + it( 'should return a RawElement for all DOM children', () => { + const rawElement = createRawElement( 'div' ); + const domElement = converter.viewToDom( rawElement, document, { bind: true } ); + + const domParagraph = domElement.childNodes[ 0 ]; + const domSpan = domParagraph.childNodes[ 0 ]; + + expect( converter.getHostViewElement( domParagraph ) ).to.equal( rawElement ); + expect( converter.getHostViewElement( domSpan ) ).to.equal( rawElement ); + } ); + + it( 'should return "null" for the parent itself', () => { + const rawElement = createRawElement( 'div' ); + const domElement = converter.viewToDom( rawElement, document, { bind: true } ); + + expect( converter.getHostViewElement( domElement ) ).to.be.null; + } ); + } ); +} ); diff --git a/packages/ckeditor5-engine/tests/view/domconverter/uielement.js b/packages/ckeditor5-engine/tests/view/domconverter/uielement.js index 5ea8b14c042..7e77e092aff 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/uielement.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/uielement.js @@ -130,7 +130,7 @@ describe( 'DOMConverter UIElement integration', () => { } ); } ); - describe( 'getParentUIElement()', () => { + describe( 'getHostViewElement()', () => { it( 'should return UIElement for DOM children', () => { const uiElement = createUIElement( 'div' ); const domElement = converter.viewToDom( uiElement, document, { bind: true } ); @@ -138,15 +138,15 @@ describe( 'DOMConverter UIElement integration', () => { const domParagraph = domElement.childNodes[ 0 ]; const domSpan = domParagraph.childNodes[ 0 ]; - expect( converter.getParentUIElement( domParagraph ) ).to.equal( uiElement ); - expect( converter.getParentUIElement( domSpan ) ).to.equal( uiElement ); + expect( converter.getHostViewElement( domParagraph ) ).to.equal( uiElement ); + expect( converter.getHostViewElement( domSpan ) ).to.equal( uiElement ); } ); it( 'should return null for element itself', () => { const uiElement = createUIElement( 'div' ); const domElement = converter.viewToDom( uiElement, document, { bind: true } ); - expect( converter.getParentUIElement( domElement ) ).to.be.null; + expect( converter.getHostViewElement( domElement ) ).to.be.null; } ); } ); } ); From c1e0eba0953de9b73d130924094973f119d174ca Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 15:36:50 +0200 Subject: [PATCH 05/28] Integrated the Renderer class with RawElement. --- .../ckeditor5-engine/src/view/renderer.js | 8 ++-- .../ckeditor5-engine/tests/view/renderer.js | 43 ++++++++++++++++++- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/renderer.js b/packages/ckeditor5-engine/src/view/renderer.js index e47fcb2b9f0..9fdc0c772a6 100644 --- a/packages/ckeditor5-engine/src/view/renderer.js +++ b/packages/ckeditor5-engine/src/view/renderer.js @@ -273,10 +273,10 @@ export default class Renderer { const deleteIndex = counter.equal + counter.delete; const viewChild = viewElement.getChild( insertIndex ); - // The 'uiElement' is a special one and its children are not stored in a view (#799), - // so we cannot use it with replacing flow (since it uses view children during rendering - // which will always result in rendering empty element). - if ( viewChild && !viewChild.is( 'uiElement' ) ) { + // UIElement and RawElement are special cases. Their children are not stored in a view (#799) + // so we cannot use them with replacing flow (since they use view children during rendering + // which will always result in rendering empty elements). + if ( viewChild && !( viewChild.is( 'uiElement' ) || viewChild.is( 'rawElement' ) ) ) { this._updateElementMappings( viewChild, actualDomChildren[ deleteIndex ] ); } diff --git a/packages/ckeditor5-engine/tests/view/renderer.js b/packages/ckeditor5-engine/tests/view/renderer.js index a39598d86e9..cc4e86ac623 100644 --- a/packages/ckeditor5-engine/tests/view/renderer.js +++ b/packages/ckeditor5-engine/tests/view/renderer.js @@ -10,10 +10,11 @@ import ViewElement from '../../src/view/element'; import ViewEditableElement from '../../src/view/editableelement'; import ViewContainerElement from '../../src/view/containerelement'; import ViewAttributeElement from '../../src/view/attributeelement'; +import ViewRawElement from '../../src/view/rawelement'; +import ViewUIElement from '../../src/view/uielement'; import ViewText from '../../src/view/text'; import ViewRange from '../../src/view/range'; import ViewPosition from '../../src/view/position'; -import UIElement from '../../src/view/uielement'; import DocumentSelection from '../../src/view/documentselection'; import DomConverter from '../../src/view/domconverter'; import Renderer from '../../src/view/renderer'; @@ -3264,7 +3265,7 @@ describe( 'Renderer', () => { it( 'should handle uiElement rendering', () => { function createUIElement( id, text ) { - const element = new UIElement( viewDocument, 'span' ); + const element = new ViewUIElement( viewDocument, 'span' ); element.render = function( domDocument ) { const domElement = this.toDomElement( domDocument ); domElement.innerText = `${ text }`; @@ -3300,6 +3301,44 @@ describe( 'Renderer', () => { '

FooUI2 Bar

' ) ); } ); + it( 'should handle rawElement rendering', () => { + function createRawElement( id, text ) { + const element = new ViewRawElement( viewDocument, 'span' ); + element.render = function( domDocument ) { + const domElement = this.toDomElement( domDocument ); + domElement.innerText = `${ text }`; + return domElement; + }; + + return element; + } + + const raw1 = createRawElement( 'id1', 'RAW1' ); + const raw2 = createRawElement( 'id2', 'RAW2' ); + const viewP = new ViewContainerElement( viewDocument, 'p', null, [ + new ViewText( viewDocument, 'Foo ' ), + raw1, + new ViewText( viewDocument, 'Bar' ) + ] ); + viewRoot._appendChild( viewP ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + expect( normalizeHtml( domRoot.innerHTML ) ).to.equal( normalizeHtml( + '

Foo RAW1Bar

' ) ); + + viewP._removeChildren( 0, viewP.childCount ); + viewP._insertChild( 0, [ new ViewText( viewDocument, 'Foo' ), raw2, new ViewText( viewDocument, ' Bar' ) ] ); + + renderer.markToSync( 'children', viewRoot ); + renderer.markToSync( 'children', viewP ); + renderer.render(); + + expect( normalizeHtml( domRoot.innerHTML ) ).to.equal( normalizeHtml( + '

FooRAW2 Bar

' ) ); + } ); + it( 'should handle linking entire content', () => { viewRoot._appendChild( parse( 'FooBar' ) ); From 3ddcc77228699005725e656d24c29fe795fbfb6d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 15:49:01 +0200 Subject: [PATCH 06/28] Integrated the DowncastWriter with the RawElement. --- .../src/view/downcastwriter.js | 49 +++++++++++++++++-- .../view/downcastwriter/breakattributes.js | 22 +++++++++ .../tests/view/downcastwriter/clear.js | 11 +++++ .../tests/view/downcastwriter/insert.js | 12 +++++ .../view/downcastwriter/mergeattributes.js | 7 +++ .../tests/view/downcastwriter/move.js | 24 +++++++++ .../tests/view/downcastwriter/remove.js | 29 +++++++++++ .../tests/view/downcastwriter/unwrap.js | 20 ++++++++ .../tests/view/downcastwriter/wrap.js | 19 +++++++ .../tests/view/downcastwriter/writer.js | 20 ++++++++ 10 files changed, 209 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/downcastwriter.js b/packages/ckeditor5-engine/src/view/downcastwriter.js index e2ad544800a..3f41eb94d65 100644 --- a/packages/ckeditor5-engine/src/view/downcastwriter.js +++ b/packages/ckeditor5-engine/src/view/downcastwriter.js @@ -14,6 +14,7 @@ import ContainerElement from './containerelement'; import AttributeElement from './attributeelement'; import EmptyElement from './emptyelement'; import UIElement from './uielement'; +import RawElement from './rawelement'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import DocumentFragment from './documentfragment'; import isIterable from '@ckeditor/ckeditor5-utils/src/isiterable'; @@ -272,6 +273,36 @@ export default class DowncastWriter { return uiElement; } + /** + * Creates a new {@link module:engine/view/rawelement~RawElement}. + * + * writer.createRawElement( 'span' ); + * writer.createRawElement( 'span', { id: 'foo-1234' } ); + * + * Custom render function can be provided as third parameter: + * + * writer.createRawElement( 'span', null, function( domDocument ) { + * const domElement = this.toDomElement( domDocument ); + * domElement.innerHTML = 'this is ui element'; + * + * return domElement; + * } ); + * + * @param {String} name Name of the element. + * @param {Object} [attributes] Elements attributes. + * @param {Function} [renderFunction] Custom render function. + * @returns {module:engine/view/rawelement~RawElement} Created element. + */ + createRawElement( name, attributes, renderFunction ) { + const rawElement = new RawElement( this.document, name, attributes ); + + if ( renderFunction ) { + rawElement.render = renderFunction; + } + + return rawElement; + } + /** * Adds or overwrite element's attribute with a specified key and value. * @@ -831,7 +862,7 @@ export default class DowncastWriter { * @param {module:engine/view/range~Range} range Range to wrap. * @param {module:engine/view/attributeelement~AttributeElement} attribute Attribute element to use as wrapper. * @returns {module:engine/view/range~Range} range Range after wrapping, spanning over wrapping attribute element. - */ + */ wrap( range, attribute ) { if ( !( attribute instanceof AttributeElement ) ) { throw new CKEditorError( 'view-writer-wrap-invalid-attribute', this.document ); @@ -1108,6 +1139,7 @@ export default class DowncastWriter { const isAttribute = child.is( 'attributeElement' ); const isEmpty = child.is( 'emptyElement' ); const isUI = child.is( 'uiElement' ); + const isRaw = child.is( 'rawElement' ); // // (In all examples, assume that `wrapElement` is `` element.) @@ -1126,8 +1158,7 @@ export default class DowncastWriter { // //

abc

-->

abc

//

abc

-->

abc

- // - else if ( isText || isEmpty || isUI || ( isAttribute && shouldABeOutsideB( wrapElement, child ) ) ) { + else if ( isText || isEmpty || isUI || isRaw || ( isAttribute && shouldABeOutsideB( wrapElement, child ) ) ) { // Clone attribute. const newAttribute = wrapElement._clone(); @@ -1572,6 +1603,16 @@ export default class DowncastWriter { throw new CKEditorError( 'view-writer-cannot-break-ui-element', this.document ); } + // If position is placed inside RawElement - throw an exception as we cannot break inside. + if ( position.parent.is( 'rawElement' ) ) { + /** + * Cannot break inside RawElement instance. + * + * @error view-writer-cannot-break-raw-element + */ + throw new CKEditorError( 'view-writer-cannot-break-raw-element', this.document ); + } + // There are no attributes to break and text nodes breaking is not forced. if ( !forceSplitText && positionParent.is( 'text' ) && isContainerOrFragment( positionParent.parent ) ) { return position.clone(); @@ -1872,7 +1913,7 @@ function validateNodesToInsert( nodes, errorContext ) { } } -const validNodesToInsert = [ Text, AttributeElement, ContainerElement, EmptyElement, UIElement ]; +const validNodesToInsert = [ Text, AttributeElement, ContainerElement, EmptyElement, RawElement, UIElement ]; // Checks if node is ContainerElement or DocumentFragment, because in most cases they should be treated the same way. // diff --git a/packages/ckeditor5-engine/tests/view/downcastwriter/breakattributes.js b/packages/ckeditor5-engine/tests/view/downcastwriter/breakattributes.js index eb7177c1bd5..c681e3b9553 100644 --- a/packages/ckeditor5-engine/tests/view/downcastwriter/breakattributes.js +++ b/packages/ckeditor5-engine/tests/view/downcastwriter/breakattributes.js @@ -10,6 +10,7 @@ import ContainerElement from '../../../src/view/containerelement'; import AttributeElement from '../../../src/view/attributeelement'; import EmptyElement from '../../../src/view/emptyelement'; import UIElement from '../../../src/view/uielement'; +import RawElement from '../../../src/view/rawelement'; import Range from '../../../src/view/range'; import Position from '../../../src/view/position'; @@ -282,6 +283,27 @@ describe( 'DowncastWriter', () => { writer.breakAttributes( range ); }, 'view-writer-cannot-break-ui-element', writer ); } ); + + it( 'should throw if breaking inside a RawElement #1', () => { + const span = new RawElement( document, 'span' ); + new ContainerElement( document, 'p', null, span ); // eslint-disable-line no-new + const position = new Position( span, 0 ); + + expectToThrowCKEditorError( () => { + writer.breakAttributes( position ); + }, 'view-writer-cannot-break-raw-element', writer ); + } ); + + it( 'should throw if breaking inside a RawElement #2', () => { + const span = new RawElement( document, 'span' ); + const b = new AttributeElement( document, 'b' ); + new ContainerElement( document, 'p', null, [ span, b ] ); // eslint-disable-line no-new + const range = Range._createFromParentsAndOffsets( span, 0, b, 0 ); + + expectToThrowCKEditorError( () => { + writer.breakAttributes( range ); + }, 'view-writer-cannot-break-raw-element', writer ); + } ); } ); } ); } ); diff --git a/packages/ckeditor5-engine/tests/view/downcastwriter/clear.js b/packages/ckeditor5-engine/tests/view/downcastwriter/clear.js index eb5b01dcaad..94b5e4f2a24 100644 --- a/packages/ckeditor5-engine/tests/view/downcastwriter/clear.js +++ b/packages/ckeditor5-engine/tests/view/downcastwriter/clear.js @@ -10,6 +10,7 @@ import ContainerElement from '../../../src/view/containerelement'; import AttributeElement from '../../../src/view/attributeelement'; import EmptyElement from '../../../src/view/emptyelement'; import UIElement from '../../../src/view/uielement'; +import RawElement from '../../../src/view/rawelement'; import Document from '../../../src/view/document'; import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; @@ -166,6 +167,16 @@ describe( 'DowncastWriter', () => { ); } ); + it( 'should remove a RawElement', () => { + const elementToRemove = new RawElement( document, 'span' ); + + testDowncast( + elementToRemove, + 'f{ooba}r', + 'foobar' + ); + } ); + it( 'should remove ContainerElement', () => { const elementToRemove = new ContainerElement( document, 'p' ); diff --git a/packages/ckeditor5-engine/tests/view/downcastwriter/insert.js b/packages/ckeditor5-engine/tests/view/downcastwriter/insert.js index ba9ded24c05..be3854b4778 100644 --- a/packages/ckeditor5-engine/tests/view/downcastwriter/insert.js +++ b/packages/ckeditor5-engine/tests/view/downcastwriter/insert.js @@ -8,6 +8,7 @@ import ContainerElement from '../../../src/view/containerelement'; import Element from '../../../src/view/element'; import EmptyElement from '../../../src/view/emptyelement'; import UIElement from '../../../src/view/uielement'; +import RawElement from '../../../src/view/rawelement'; import Position from '../../../src/view/position'; import { stringify, parse } from '../../../src/dev-utils/view'; @@ -215,5 +216,16 @@ describe( 'DowncastWriter', () => { writer.insert( position, attributeElement ); }, 'view-writer-cannot-break-ui-element', document ); } ); + + it( 'should throw if trying to insert inside a RawElement', () => { + const rawElement = new RawElement( document, 'span' ); + new ContainerElement( document, 'p', null, rawElement ); // eslint-disable-line no-new + const position = new Position( rawElement, 0 ); + const attributeElement = new AttributeElement( document, 'i' ); + + expectToThrowCKEditorError( () => { + writer.insert( position, attributeElement ); + }, 'view-writer-cannot-break-raw-element', document ); + } ); } ); } ); diff --git a/packages/ckeditor5-engine/tests/view/downcastwriter/mergeattributes.js b/packages/ckeditor5-engine/tests/view/downcastwriter/mergeattributes.js index 7bf725dc132..a3581a80507 100644 --- a/packages/ckeditor5-engine/tests/view/downcastwriter/mergeattributes.js +++ b/packages/ckeditor5-engine/tests/view/downcastwriter/mergeattributes.js @@ -150,5 +150,12 @@ describe( 'DowncastWriter', () => { '[]' ); } ); + + it( 'should not merge when placed between RawElements', () => { + testMerge( + '[]', + '[]' + ); + } ); } ); } ); diff --git a/packages/ckeditor5-engine/tests/view/downcastwriter/move.js b/packages/ckeditor5-engine/tests/view/downcastwriter/move.js index 21acbb2de74..8d4d52c4e75 100644 --- a/packages/ckeditor5-engine/tests/view/downcastwriter/move.js +++ b/packages/ckeditor5-engine/tests/view/downcastwriter/move.js @@ -10,6 +10,7 @@ import AttributeElement from '../../../src/view/attributeelement'; import RootEditableElement from '../../../src/view/rooteditableelement'; import EmptyElement from '../../../src/view/emptyelement'; import UIElement from '../../../src/view/uielement'; +import RawElement from '../../../src/view/rawelement'; import Range from '../../../src/view/range'; import Position from '../../../src/view/position'; @@ -186,6 +187,29 @@ describe( 'DowncastWriter', () => { }, 'view-writer-cannot-break-ui-element', writer ); } ); + it( 'should move a RawElement', () => { + testMove( + 'foo[]bar', + 'baz{}quix', + 'foobar', + 'baz[]quix' + ); + } ); + + it( 'should throw if trying to move to a RawElement', () => { + const srcAttribute = new AttributeElement( document, 'b' ); + const srcContainer = new ContainerElement( document, 'p', null, srcAttribute ); + const srcRange = Range._createFromParentsAndOffsets( srcContainer, 0, srcContainer, 1 ); + + const dstRawElement = new RawElement( document, 'span' ); + new ContainerElement( document, 'p', null, dstRawElement ); // eslint-disable-line no-new + const dstPosition = new Position( dstRawElement, 0 ); + + expectToThrowCKEditorError( () => { + writer.move( srcRange, dstPosition ); + }, 'view-writer-cannot-break-raw-element', writer ); + } ); + it( 'should not break marker mappings if marker element was split and the original element was removed', () => { const mapper = new Mapper(); diff --git a/packages/ckeditor5-engine/tests/view/downcastwriter/remove.js b/packages/ckeditor5-engine/tests/view/downcastwriter/remove.js index 7a8c1d87425..0ec75674737 100644 --- a/packages/ckeditor5-engine/tests/view/downcastwriter/remove.js +++ b/packages/ckeditor5-engine/tests/view/downcastwriter/remove.js @@ -11,6 +11,7 @@ import { stringify, parse } from '../../../src/dev-utils/view'; import AttributeElement from '../../../src/view/attributeelement'; import EmptyElement from '../../../src/view/emptyelement'; import UIElement from '../../../src/view/uielement'; +import RawElement from '../../../src/view/rawelement'; import Document from '../../../src/view/document'; import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; @@ -160,6 +161,25 @@ describe( 'DowncastWriter', () => { }, 'view-writer-cannot-break-ui-element', document ); } ); + it( 'should remove a RawElement', () => { + testRemove( + 'foo[]bar', + 'foo{}bar', + '' + ); + } ); + + it( 'should throw if a range is placed inside a RawElement', () => { + const rawElement = new RawElement( document, 'span' ); + const attributeElement = new AttributeElement( document, 'b' ); + new ContainerElement( document, 'p', null, [ rawElement, attributeElement ] ); // eslint-disable-line no-new + const range = Range._createFromParentsAndOffsets( rawElement, 0, attributeElement, 0 ); + + expectToThrowCKEditorError( () => { + writer.remove( range ); + }, 'view-writer-cannot-break-raw-element', document ); + } ); + it( 'should remove single text node (as item)', () => { testRemove( '[foobar]', '', 'foobar', true ); } ); @@ -201,6 +221,15 @@ describe( 'DowncastWriter', () => { ); } ); + it( 'should remove a RawElement (as an item)', () => { + testRemove( + 'foo[]bar', + 'foobar', + '', + true + ); + } ); + it( 'should throw when item has no parent container', () => { const el = new AttributeElement( document, 'b' ); diff --git a/packages/ckeditor5-engine/tests/view/downcastwriter/unwrap.js b/packages/ckeditor5-engine/tests/view/downcastwriter/unwrap.js index 695430a01d9..0c1f3a88d62 100644 --- a/packages/ckeditor5-engine/tests/view/downcastwriter/unwrap.js +++ b/packages/ckeditor5-engine/tests/view/downcastwriter/unwrap.js @@ -9,6 +9,7 @@ import ContainerElement from '../../../src/view/containerelement'; import AttributeElement from '../../../src/view/attributeelement'; import EmptyElement from '../../../src/view/emptyelement'; import UIElement from '../../../src/view/uielement'; +import RawElement from '../../../src/view/rawelement'; import Position from '../../../src/view/position'; import Range from '../../../src/view/range'; import Text from '../../../src/view/text'; @@ -426,6 +427,25 @@ describe( 'DowncastWriter', () => { }, 'view-writer-cannot-break-ui-element', document ); } ); + it( 'should unwrap a RawElement', () => { + testUnwrap( + '[]', + '', + '[]' + ); + } ); + + it( 'should throw if a range is placed inside a RawElement', () => { + const rawElement = new RawElement( document, 'span' ); + const attribute = new AttributeElement( document, 'b' ); + const container = new ContainerElement( document, 'p', null, [ rawElement, attribute ] ); + const range = Range._createFromParentsAndOffsets( rawElement, 0, container, 2 ); + + expectToThrowCKEditorError( () => { + writer.unwrap( range, attribute ); + }, 'view-writer-cannot-break-raw-element', document ); + } ); + it( 'should unwrap if both elements have same id', () => { const unwrapper = writer.createAttributeElement( 'span', null, { id: 'foo' } ); const attribute = writer.createAttributeElement( 'span', null, { id: 'foo' } ); diff --git a/packages/ckeditor5-engine/tests/view/downcastwriter/wrap.js b/packages/ckeditor5-engine/tests/view/downcastwriter/wrap.js index 89d8a8ef52c..b8ccdae4e37 100644 --- a/packages/ckeditor5-engine/tests/view/downcastwriter/wrap.js +++ b/packages/ckeditor5-engine/tests/view/downcastwriter/wrap.js @@ -10,6 +10,7 @@ import Element from '../../../src/view/element'; import ContainerElement from '../../../src/view/containerelement'; import AttributeElement from '../../../src/view/attributeelement'; import EmptyElement from '../../../src/view/emptyelement'; +import RawElement from '../../../src/view/rawelement'; import UIElement from '../../../src/view/uielement'; import Position from '../../../src/view/position'; import Range from '../../../src/view/range'; @@ -422,6 +423,24 @@ describe( 'DowncastWriter', () => { }, 'view-writer-cannot-break-ui-element', document ); } ); + it( 'should wrap a RawElement', () => { + testWrap( + '[]', + '', + '[]' + ); + } ); + + it( 'should throw if a range is inside a RawElement', () => { + const rawElement = new RawElement( document, 'span' ); + const container = new ContainerElement( document, 'p', null, rawElement ); + const range = Range._createFromParentsAndOffsets( rawElement, 0, container, 1 ); + + expectToThrowCKEditorError( () => { + writer.wrap( range, new AttributeElement( document, 'b' ) ); + }, 'view-writer-cannot-break-raw-element', document ); + } ); + it( 'should keep stable hierarchy when wrapping with attribute with same priority', () => { testWrap( '[foo]', diff --git a/packages/ckeditor5-engine/tests/view/downcastwriter/writer.js b/packages/ckeditor5-engine/tests/view/downcastwriter/writer.js index 971f1d42804..6850d910287 100644 --- a/packages/ckeditor5-engine/tests/view/downcastwriter/writer.js +++ b/packages/ckeditor5-engine/tests/view/downcastwriter/writer.js @@ -136,6 +136,26 @@ describe( 'DowncastWriter', () => { } ); } ); + describe( 'createRawElement()', () => { + it( 'should create a RawElement', () => { + const element = writer.createRawElement( 'foo', attributes ); + + expect( element.is( 'rawElement' ) ).to.be.true; + expect( element.name ).to.equal( 'foo' ); + assertElementAttributes( element, attributes ); + } ); + + it( 'should allow to pass custom rendering method', () => { + const renderFn = function() {}; + const element = writer.createRawElement( 'foo', attributes, renderFn ); + + expect( element.is( 'rawElement' ) ).to.be.true; + expect( element.name ).to.equal( 'foo' ); + expect( element.render ).to.equal( renderFn ); + assertElementAttributes( element, attributes ); + } ); + } ); + describe( 'setAttribute()', () => { it( 'should set attribute on given element', () => { const element = writer.createAttributeElement( 'span' ); From 1932a7aa6e49d37e345c5b82f6ec63db85ffb11e Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 15:54:07 +0200 Subject: [PATCH 07/28] Made the Media Embed feature use RawElement instead of UIElement. --- .../src/mediaregistry.js | 2 +- packages/ckeditor5-media-embed/src/utils.js | 10 ------ .../tests/mediaembedediting.js | 32 +++++++++---------- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/packages/ckeditor5-media-embed/src/mediaregistry.js b/packages/ckeditor5-media-embed/src/mediaregistry.js index 3ea460e373f..5102fa6583b 100644 --- a/packages/ckeditor5-media-embed/src/mediaregistry.js +++ b/packages/ckeditor5-media-embed/src/mediaregistry.js @@ -234,7 +234,7 @@ class Media { const mediaHtml = this._getPreviewHtml( options ); - viewElement = writer.createUIElement( 'div', attributes, function( domDocument ) { + viewElement = writer.createRawElement( 'div', attributes, function( domDocument ) { const domElement = this.toDomElement( domDocument ); domElement.innerHTML = mediaHtml; diff --git a/packages/ckeditor5-media-embed/src/utils.js b/packages/ckeditor5-media-embed/src/utils.js index c5ce43fb7b5..9de070105dd 100644 --- a/packages/ckeditor5-media-embed/src/utils.js +++ b/packages/ckeditor5-media-embed/src/utils.js @@ -75,12 +75,6 @@ export function isMediaWidget( viewElement ) { export function createMediaFigureElement( writer, registry, url, options ) { const figure = writer.createContainerElement( 'figure', { class: 'media' } ); - // TODO: This is a hack. Without it, the figure in the data pipeline will contain   because - // its only child is the UIElement (wrapper). - // - // Note: The hack is a copy&paste from widget utils; it makes the figure act like it's a widget. - figure.getFillerOffset = getFillerOffset; - writer.insert( writer.createPositionAt( figure, 0 ), registry.getMediaViewElement( writer, url, options ) ); return figure; @@ -123,7 +117,3 @@ export function insertMedia( model, url, insertPosition ) { writer.setSelection( mediaElement, 'on' ); } ); } - -function getFillerOffset() { - return null; -} diff --git a/packages/ckeditor5-media-embed/tests/mediaembedediting.js b/packages/ckeditor5-media-embed/tests/mediaembedediting.js index ddaa46968f9..eb4ba180328 100644 --- a/packages/ckeditor5-media-embed/tests/mediaembedediting.js +++ b/packages/ckeditor5-media-embed/tests/mediaembedediting.js @@ -85,7 +85,7 @@ describe( 'MediaEmbedEditing', () => { } ).then( editor => { editor.setData( '
' ); - expect( getViewData( editor.editing.view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( '' ); + expect( getViewData( editor.editing.view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '' ); } ); } ); @@ -99,7 +99,7 @@ describe( 'MediaEmbedEditing', () => { } ).then( editor => { editor.setData( '
' ); - expect( getViewData( editor.editing.view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( editor.editing.view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'A, id=123' + @@ -109,7 +109,7 @@ describe( 'MediaEmbedEditing', () => { editor.setData( '
' ); - expect( getViewData( editor.editing.view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( editor.editing.view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'B, id=123' + @@ -119,7 +119,7 @@ describe( 'MediaEmbedEditing', () => { editor.setData( '
' ); - expect( getViewData( editor.editing.view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( editor.editing.view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'C, id=123' + @@ -350,7 +350,7 @@ describe( 'MediaEmbedEditing', () => { } ).then( editor => { editor.setData( '
' ); - expect( getViewData( editor.editing.view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( editor.editing.view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'A, id=123' + @@ -360,7 +360,7 @@ describe( 'MediaEmbedEditing', () => { editor.setData( '
' ); - expect( getViewData( editor.editing.view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( editor.editing.view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'extraB, id=123' + @@ -384,7 +384,7 @@ describe( 'MediaEmbedEditing', () => { '
' + '
' ); - expect( getViewData( editor.editing.view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( editor.editing.view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'B, id=123' + @@ -407,7 +407,7 @@ describe( 'MediaEmbedEditing', () => { '
' + '
' ); - expect( getViewData( editor.editing.view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( editor.editing.view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'B, id=123' + @@ -434,7 +434,7 @@ describe( 'MediaEmbedEditing', () => { '
' + '
' ); - expect( getViewData( editor.editing.view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( editor.editing.view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'B, id=123' + @@ -833,7 +833,7 @@ describe( 'MediaEmbedEditing', () => { it( 'should convert', () => { setModelData( model, '' ); - expect( getViewData( view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'allow-everything, id=https://ckeditor.com' + @@ -850,7 +850,7 @@ describe( 'MediaEmbedEditing', () => { writer.setAttribute( 'url', 'https://cksource.com', media ); } ); - expect( getViewData( view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'allow-everything, id=https://cksource.com' + @@ -867,7 +867,7 @@ describe( 'MediaEmbedEditing', () => { writer.removeAttribute( 'url', media ); } ); - expect( getViewData( view, { withoutSelection: true, renderUIElements: true } ) ) + expect( getViewData( view, { withoutSelection: true, renderRawElements: true } ) ) .to.equal( '
' + '
' + @@ -888,7 +888,7 @@ describe( 'MediaEmbedEditing', () => { writer.removeAttribute( 'url', media ); } ); - expect( getViewData( view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( view, { withoutSelection: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'allow-everything, id=https://ckeditor.com' + @@ -916,7 +916,7 @@ describe( 'MediaEmbedEditing', () => { writer.insert( writer.createPositionAt( widgetViewElement, 'end' ), externalUIElement ); } ); - expect( getViewData( view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( view, { withoutSelection: true, renderUIElements: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'allow-everything, id=https://ckeditor.com' + @@ -929,7 +929,7 @@ describe( 'MediaEmbedEditing', () => { writer.setAttribute( 'url', 'https://cksource.com', media ); } ); - expect( getViewData( view, { withoutSelection: true, renderUIElements: true } ) ).to.equal( + expect( getViewData( view, { withoutSelection: true, renderUIElements: true, renderRawElements: true } ) ).to.equal( '
' + '
' + 'allow-everything, id=https://cksource.com' + @@ -947,7 +947,7 @@ describe( 'MediaEmbedEditing', () => { for ( const url of urls ) { editor.setData( `
` ); - const viewData = getViewData( view, { withoutSelection: true, renderUIElements: true } ); + const viewData = getViewData( view, { withoutSelection: true, renderRawElements: true } ); let expectedRegExp; const expectedUrl = url.match( /^https?:\/\// ) ? url : 'https://' + url; From 50fbc806e030ecdac6ca245ad033f97ff493ff99 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 16:22:50 +0200 Subject: [PATCH 08/28] Tests: Added a test that verifies the placeholder feature no longer collides with media embed. --- .../tests/integration.js | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 packages/ckeditor5-media-embed/tests/integration.js diff --git a/packages/ckeditor5-media-embed/tests/integration.js b/packages/ckeditor5-media-embed/tests/integration.js new file mode 100644 index 00000000000..67d3d24bacd --- /dev/null +++ b/packages/ckeditor5-media-embed/tests/integration.js @@ -0,0 +1,64 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* global document */ + +import MediaEmbed from '../src/mediaembed'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; +import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; +import { enablePlaceholder } from '@ckeditor/ckeditor5-engine/src/view/placeholder'; + +describe( 'MediaEmbed integration', () => { + let element, clock; + + beforeEach( () => { + clock = sinon.useFakeTimers(); + element = document.createElement( 'div' ); + document.body.appendChild( element ); + } ); + + afterEach( () => { + element.remove(); + clock.restore(); + } ); + + describe( 'with the placeholder feature', () => { + it( 'should make the placeholder CSS class disappear when pasting a new media into an empty editing root', async () => { + const editor = await ClassicTestEditor.create( element, { + plugins: [ MediaEmbed, Paragraph ] + } ); + + enablePlaceholder( { + view: editor.editing.view, + element: editor.editing.view.document.getRoot(), + text: 'foo', + isDirectHost: false + } ); + + editor.editing.view.document.fire( 'paste', { + dataTransfer: { + getData() { + return 'https://www.youtube.com/watch?v=H08tGjXNHO4'; + } + }, + stopPropagation() {}, + preventDefault() {} + } ); + + clock.tick( 100 ); + + expect( getViewData( editor.editing.view ) ).to.equal( + '[
' + + '
' + + '
' + + '
]' + ); + + await editor.destroy(); + } ); + } ); +} ); From b56be5d7cdbbc924841b0eaddab6b8353bae15b5 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 16:39:45 +0200 Subject: [PATCH 09/28] Docs: Used the RawElement in the "Using a React component in a block widget" guide. --- docs/_snippets/framework/tutorials/using-react-in-widget.js | 2 +- docs/framework/guides/tutorials/using-react-in-a-widget.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/_snippets/framework/tutorials/using-react-in-widget.js b/docs/_snippets/framework/tutorials/using-react-in-widget.js index d04a25472e0..4aac4006846 100644 --- a/docs/_snippets/framework/tutorials/using-react-in-widget.js +++ b/docs/_snippets/framework/tutorials/using-react-in-widget.js @@ -114,7 +114,7 @@ class ProductPreviewEditing extends Plugin { // The inner
element. // This element will host a React component. - const reactWrapper = viewWriter.createUIElement( 'div', { + const reactWrapper = viewWriter.createRawElement( 'div', { class: 'product__react-wrapper' }, function( domDocument ) { const domElement = this.toDomElement( domDocument ); diff --git a/docs/framework/guides/tutorials/using-react-in-a-widget.md b/docs/framework/guides/tutorials/using-react-in-a-widget.md index 29cea3b5859..acb68d78c26 100644 --- a/docs/framework/guides/tutorials/using-react-in-a-widget.md +++ b/docs/framework/guides/tutorials/using-react-in-a-widget.md @@ -365,7 +365,7 @@ export default class ProductPreviewEditing extends Plugin { // The inner
element. // This element will host a React component. - const reactWrapper = viewWriter.createUIElement( 'div', { + const reactWrapper = viewWriter.createRawElement( 'div', { class: 'product__react-wrapper' }, function( domDocument ) { const domElement = this.toDomElement( domDocument ); @@ -1184,7 +1184,7 @@ export default class ProductPreviewEditing extends Plugin { // The inner
element. // This element will host a React component. - const reactWrapper = viewWriter.createUIElement( 'div', { + const reactWrapper = viewWriter.createRawElement( 'div', { class: 'product__react-wrapper' }, function( domDocument ) { const domElement = this.toDomElement( domDocument ); From c467a3b64bc96121fb760ba5a2a64a7e8f6b8d35 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 16:57:54 +0200 Subject: [PATCH 10/28] Docs: Mentioned RawElement in the "Editing engine" guide. --- docs/framework/guides/architecture/editing-engine.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/framework/guides/architecture/editing-engine.md b/docs/framework/guides/architecture/editing-engine.md index 671849cb39b..a493150c27f 100644 --- a/docs/framework/guides/architecture/editing-engine.md +++ b/docs/framework/guides/architecture/editing-engine.md @@ -224,7 +224,7 @@ editor.data; // The data pipeline (DataController). ### Element types and custom data -The structure of the view resembles the structure in the DOM very closely. The semantics of HTML is defined in its specification. The view structure comes "DTD-free", so in order to provide additional information and better express the semantics of the content, the view structure implements 5 element types ({@link module:engine/view/containerelement~ContainerElement}, {@link module:engine/view/attributeelement~AttributeElement}, {@link module:engine/view/emptyelement~EmptyElement}, {@link module:engine/view/uielement~UIElement}, and {@link module:engine/view/editableelement~EditableElement}) and so called {@link module:engine/view/element~Element#getCustomProperty "custom properties"} (i.e. custom element properties which are not rendered). This additional information provided by editor features is then used by the {@link module:engine/view/renderer~Renderer} and [converters](#conversion). +The structure of the view resembles the structure in the DOM very closely. The semantics of HTML is defined in its specification. The view structure comes "DTD-free", so in order to provide additional information and better express the semantics of the content, the view structure implements 6 element types ({@link module:engine/view/containerelement~ContainerElement}, {@link module:engine/view/attributeelement~AttributeElement}, {@link module:engine/view/emptyelement~EmptyElement}, {@link module:engine/view/rawelement~RawElement}, {@link module:engine/view/uielement~UIElement}, and {@link module:engine/view/editableelement~EditableElement}) and so called {@link module:engine/view/element~Element#getCustomProperty "custom properties"} (i.e. custom element properties which are not rendered). This additional information provided by editor features is then used by the {@link module:engine/view/renderer~Renderer} and [converters](#conversion). The element types can be defined as follows: @@ -232,6 +232,7 @@ The element types can be defined as follows: * **Attribute element** – The elements that cannot contain container elements inside them. Most model text attributes are converted to view attribute elements. They are used mostly for inline styling elements such as ``, ``, ``, ``. Similar attribute elements are flattened by the view writer, so e.g. `x` would automatically be optimized to `x`. * **Empty element** – The elements that must not have any child nodes, for example ``. * **UI elements** – The elements that are not a part of the "data" but need to be "inlined" in the content. They are ignored by the selection (it jumps over them) and the view writer in general. The contents of these elements and events coming from them are filtered out, too. +* **Raw elements** – The elements that work as data containers ("sandboxes") but their children are transparent to the editor. Useful when non-standard data must be rendered but the editor should not be concerned what it it is and how it works. Users cannot put the selection inside a raw element, split it into smaller chunks or directly modify its content. * **Editable element** – The elements used as "nested editables" of non-editable fragments of the content, for example a caption in the image widget, where the `
` wrapping the image is not editable (it is a widget) and the `
` inside it is an editable element. Additionally, you can define {@link module:engine/view/element~Element#getCustomProperty custom properties} which can be used to store information like: From a5208bfae3f7d61c583948396e8dc91848a825d8 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 16:59:56 +0200 Subject: [PATCH 11/28] Tests: Added a comment in media embed integration test. --- packages/ckeditor5-media-embed/tests/integration.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/ckeditor5-media-embed/tests/integration.js b/packages/ckeditor5-media-embed/tests/integration.js index 67d3d24bacd..8a9b6afe3f9 100644 --- a/packages/ckeditor5-media-embed/tests/integration.js +++ b/packages/ckeditor5-media-embed/tests/integration.js @@ -27,6 +27,7 @@ describe( 'MediaEmbed integration', () => { } ); describe( 'with the placeholder feature', () => { + // https://github.com/ckeditor/ckeditor5/issues/1684 it( 'should make the placeholder CSS class disappear when pasting a new media into an empty editing root', async () => { const editor = await ClassicTestEditor.create( element, { plugins: [ MediaEmbed, Paragraph ] From 827151380908eeb5efe0695cf11be7f0037c44d9 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jul 2020 17:00:32 +0200 Subject: [PATCH 12/28] Docs: Minor docs correction. --- docs/framework/guides/architecture/editing-engine.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/framework/guides/architecture/editing-engine.md b/docs/framework/guides/architecture/editing-engine.md index a493150c27f..74a555d6d08 100644 --- a/docs/framework/guides/architecture/editing-engine.md +++ b/docs/framework/guides/architecture/editing-engine.md @@ -232,7 +232,7 @@ The element types can be defined as follows: * **Attribute element** – The elements that cannot contain container elements inside them. Most model text attributes are converted to view attribute elements. They are used mostly for inline styling elements such as ``, ``, ``, ``. Similar attribute elements are flattened by the view writer, so e.g. `x` would automatically be optimized to `x`. * **Empty element** – The elements that must not have any child nodes, for example ``. * **UI elements** – The elements that are not a part of the "data" but need to be "inlined" in the content. They are ignored by the selection (it jumps over them) and the view writer in general. The contents of these elements and events coming from them are filtered out, too. -* **Raw elements** – The elements that work as data containers ("sandboxes") but their children are transparent to the editor. Useful when non-standard data must be rendered but the editor should not be concerned what it it is and how it works. Users cannot put the selection inside a raw element, split it into smaller chunks or directly modify its content. +* **Raw element** – The elements that work as data containers ("sandboxes") but their children are transparent to the editor. Useful when non-standard data must be rendered but the editor should not be concerned what it it is and how it works. Users cannot put the selection inside a raw element, split it into smaller chunks or directly modify its content. * **Editable element** – The elements used as "nested editables" of non-editable fragments of the content, for example a caption in the image widget, where the `
` wrapping the image is not editable (it is a widget) and the `
` inside it is an editable element. Additionally, you can define {@link module:engine/view/element~Element#getCustomProperty custom properties} which can be used to store information like: From b3f21a12d22769306ae961f52082d4ea426d086c Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 14 Jul 2020 10:26:49 +0200 Subject: [PATCH 13/28] Tests: Added tests for RawElement class. --- .../ckeditor5-engine/tests/view/rawelement.js | 165 ++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 packages/ckeditor5-engine/tests/view/rawelement.js diff --git a/packages/ckeditor5-engine/tests/view/rawelement.js b/packages/ckeditor5-engine/tests/view/rawelement.js new file mode 100644 index 00000000000..dd5f58f9cce --- /dev/null +++ b/packages/ckeditor5-engine/tests/view/rawelement.js @@ -0,0 +1,165 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* global document, HTMLElement */ + +import RawElement from '../../src/view/rawelement'; +import Element from '../../src/view/element'; +import Document from '../../src/view/document'; +import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; +import { StylesProcessor } from '../../src/view/stylesmap'; + +describe( 'RawElement', () => { + let rawElement, doc; + + beforeEach( () => { + doc = new Document( new StylesProcessor() ); + + rawElement = new RawElement( doc, 'span', { + foo: 'bar', + style: 'margin-top: 2em;color: white;', + class: 'foo bar' + } ); + } ); + + describe( 'constructor()', () => { + it( 'should create instance', () => { + expect( rawElement.name ).to.equal( 'span' ); + expect( rawElement.getAttribute( 'foo' ) ).to.equal( 'bar' ); + expect( rawElement.getStyle( 'margin-top' ) ).to.equal( '2em' ); + expect( rawElement.getStyle( 'color' ) ).to.equal( 'white' ); + expect( rawElement.hasClass( 'foo' ) ).to.true; + expect( rawElement.hasClass( 'bar' ) ).to.true; + } ); + + it( 'should throw if child elements are passed to constructor', () => { + expectToThrowCKEditorError( () => { + new RawElement( doc, 'img', null, [ new Element( doc, 'i' ) ] ); // eslint-disable-line no-new + }, 'view-rawelement-cannot-add: Cannot add child nodes to RawElement instance.' ); + } ); + } ); + + describe( 'is()', () => { + let el; + + before( () => { + el = new RawElement( doc, 'span' ); + } ); + + it( 'should return true for rawElement/element, also with correct name and element name', () => { + expect( el.is( 'rawElement' ) ).to.be.true; + expect( el.is( 'view:rawElement' ) ).to.be.true; + expect( el.is( 'rawElement', 'span' ) ).to.be.true; + expect( el.is( 'view:rawElement', 'span' ) ).to.be.true; + expect( el.is( 'element' ) ).to.be.true; + expect( el.is( 'view:element' ) ).to.be.true; + expect( el.is( 'node' ) ).to.be.true; + expect( el.is( 'view:node' ) ).to.be.true; + expect( el.is( 'element', 'span' ) ).to.be.true; + expect( el.is( 'view:element', 'span' ) ).to.be.true; + expect( el.is( 'span' ) ).to.be.true; + expect( el.is( 'view:span' ) ).to.be.true; + } ); + + it( 'should return false for other accept values', () => { + expect( el.is( 'rawElement', 'p' ) ).to.be.false; + expect( el.is( 'view:rawElement', 'p' ) ).to.be.false; + expect( el.is( 'element', 'p' ) ).to.be.false; + expect( el.is( 'view:element', 'p' ) ).to.be.false; + expect( el.is( 'p' ) ).to.be.false; + expect( el.is( 'view:p' ) ).to.be.false; + expect( el.is( 'text' ) ).to.be.false; + expect( el.is( 'textProxy' ) ).to.be.false; + expect( el.is( 'containerElement' ) ).to.be.false; + expect( el.is( 'attributeElement' ) ).to.be.false; + expect( el.is( 'emptyElement' ) ).to.be.false; + expect( el.is( 'rootElement' ) ).to.be.false; + expect( el.is( 'documentFragment' ) ).to.be.false; + expect( el.is( 'model:element' ) ).to.be.false; + expect( el.is( 'model:span' ) ).to.be.false; + expect( el.is( 'model:node' ) ).to.be.false; + } ); + } ); + + describe( '_appendChild()', () => { + it( 'should throw when try to append new child element', () => { + expectToThrowCKEditorError( () => { + rawElement._appendChild( new Element( doc, 'i' ) ); + }, 'view-rawelement-cannot-add: Cannot add child nodes to RawElement instance.' ); + } ); + } ); + + describe( '_insertChild()', () => { + it( 'should throw when try to insert new child element', () => { + expectToThrowCKEditorError( () => { + rawElement._insertChild( 0, new Element( doc, 'i' ) ); + }, 'view-rawelement-cannot-add: Cannot add child nodes to RawElement instance.' ); + } ); + } ); + + describe( '_clone()', () => { + it( 'should be properly cloned', () => { + const newUIElement = rawElement._clone(); + + expect( newUIElement.name ).to.equal( 'span' ); + expect( newUIElement.getAttribute( 'foo' ) ).to.equal( 'bar' ); + expect( newUIElement.getStyle( 'margin-top' ) ).to.equal( '2em' ); + expect( newUIElement.getStyle( 'color' ) ).to.equal( 'white' ); + expect( newUIElement.hasClass( 'foo' ) ).to.true; + expect( newUIElement.hasClass( 'bar' ) ).to.true; + expect( newUIElement.isSimilar( rawElement ) ).to.true; + } ); + } ); + + describe( 'getFillerOffset()', () => { + it( 'should return null', () => { + expect( rawElement.getFillerOffset() ).to.null; + } ); + } ); + + describe( 'render()', () => { + let domElement; + + beforeEach( () => { + domElement = rawElement.render( document ); + } ); + + it( 'should return DOM element', () => { + expect( domElement ).to.be.instanceOf( HTMLElement ); + } ); + + it( 'should use element name', () => { + expect( domElement.tagName.toLowerCase() ).to.equal( rawElement.name ); + } ); + + it( 'should render attributes', () => { + for ( const key of rawElement.getAttributeKeys() ) { + expect( domElement.getAttribute( key ) ).to.equal( rawElement.getAttribute( key ) ); + } + } ); + + it( 'should allow to change render() method', () => { + rawElement.render = function( domDocument ) { + return domDocument.createElement( 'b' ); + }; + + expect( rawElement.render( document ).tagName.toLowerCase() ).to.equal( 'b' ); + } ); + + it( 'should allow to add new elements inside', () => { + rawElement.render = function( domDocument ) { + const element = this.toDomElement( domDocument ); + const text = domDocument.createTextNode( 'foo bar' ); + element.appendChild( text ); + + return element; + }; + + const rendered = rawElement.render( document ); + expect( rendered.tagName.toLowerCase() ).to.equal( 'span' ); + expect( rendered.textContent ).to.equal( 'foo bar' ); + } ); + } ); +} ); From bbe1ffc6e1dba5d37cab10ff35eb4e8802eaefc8 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 14 Jul 2020 10:27:37 +0200 Subject: [PATCH 14/28] Tests: Code refactoring. --- .../ckeditor5-engine/tests/view/rawelement.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/rawelement.js b/packages/ckeditor5-engine/tests/view/rawelement.js index dd5f58f9cce..5035846ed6a 100644 --- a/packages/ckeditor5-engine/tests/view/rawelement.js +++ b/packages/ckeditor5-engine/tests/view/rawelement.js @@ -101,15 +101,15 @@ describe( 'RawElement', () => { describe( '_clone()', () => { it( 'should be properly cloned', () => { - const newUIElement = rawElement._clone(); - - expect( newUIElement.name ).to.equal( 'span' ); - expect( newUIElement.getAttribute( 'foo' ) ).to.equal( 'bar' ); - expect( newUIElement.getStyle( 'margin-top' ) ).to.equal( '2em' ); - expect( newUIElement.getStyle( 'color' ) ).to.equal( 'white' ); - expect( newUIElement.hasClass( 'foo' ) ).to.true; - expect( newUIElement.hasClass( 'bar' ) ).to.true; - expect( newUIElement.isSimilar( rawElement ) ).to.true; + const newRawElement = rawElement._clone(); + + expect( newRawElement.name ).to.equal( 'span' ); + expect( newRawElement.getAttribute( 'foo' ) ).to.equal( 'bar' ); + expect( newRawElement.getStyle( 'margin-top' ) ).to.equal( '2em' ); + expect( newRawElement.getStyle( 'color' ) ).to.equal( 'white' ); + expect( newRawElement.hasClass( 'foo' ) ).to.true; + expect( newRawElement.hasClass( 'bar' ) ).to.true; + expect( newRawElement.isSimilar( rawElement ) ).to.true; } ); } ); From 1fa577f35a98627305f3876e04b73da24614b01f Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jul 2020 15:59:10 +0200 Subject: [PATCH 15/28] Simplified RawElement#render(). Added docs to the class. --- .../ckeditor5-engine/src/view/rawelement.js | 58 +++++++------------ 1 file changed, 20 insertions(+), 38 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/rawelement.js b/packages/ckeditor5-engine/src/view/rawelement.js index 40158c55b24..86b6bf51099 100644 --- a/packages/ckeditor5-engine/src/view/rawelement.js +++ b/packages/ckeditor5-engine/src/view/rawelement.js @@ -14,7 +14,15 @@ import Node from './node'; /** * The raw element class. * - * It is used to represent elements that TODO. + * Raw elements work as data containers ("wrappers", "sandboxes") but their children are not managed or + * even recognized by the editor. This encapsulation allows integrations to maintain custom DOM structures + * in the editor content without, for instance, worrying about compatibility with other editor features. + * Raw elements make a perfect tool for integration with external frameworks and data sources. + * + * Unlike {@link module:engine/view/uielement~UIElement ui elements}, raw elements act like a real editor + * content (similar to {@link module:engine/view/containerelement~ContainerElement} or + * {@link module:engine/view/emptyelement~EmptyElement}), they are considered by the editor selection and + * {@link module:widget/utils.toWidget they can work as widgets}. * * To create a new raw element use the * {@link module:engine/view/downcastwriter~DowncastWriter#createRawElement `downcastWriter#createRawElement()`} method. @@ -100,59 +108,33 @@ export default class RawElement extends Element { _insertChild( index, nodes ) { if ( nodes && ( nodes instanceof Node || Array.from( nodes ).length > 0 ) ) { /** - * Cannot add children to a {@link module:engine/view/rawelement~RawElement}. + * Cannot add children to a {@link module:engine/view/rawelement~RawElement} instance. * * @error view-rawelement-cannot-add */ throw new CKEditorError( - 'view-rawelement-cannot-add: Cannot add child nodes to RawElement instance.', + 'view-rawelement-cannot-add: Cannot add child nodes to a RawElement instance.', [ this, nodes ] ); } } /** - * Renders this {@link module:engine/view/rawelement~RawElement} to DOM. This method is called by - * {@link module:engine/view/domconverter~DomConverter}. - * Do not use inheritance to create custom rendering method, replace `render()` method instead: + * Allows rendering the children of a {@link module:engine/view/rawelement~RawElement} on the DOM level. + * This method is called by the {@link module:engine/view/domconverter~DomConverter} with the raw DOM element + * passed as an argument leaving the number and shape of the children up to the integrator. * - * const myRawElement = downcastWriter.createRawElement( 'span' ); + * This method **must be defined** for the `RawElement` to work: * - * myRawElement.render = function( domDocument ) { - * const domElement = this.toDomElement( domDocument ); - * domElement.innerHTML = 'this is ui element'; + * const myRawElement = downcastWriter.createRawElement( 'div' ); * - * return domElement; + * myRawElement.render = function( domElement ) { + * domElement.innerHTML = 'This is the raw content of myRawElement.'; * }; * - * If changes in your raw element should trigger some editor UI update you should call - * the {@link module:core/editor/editorui~EditorUI#update `editor.ui.update()`} method - * after rendering your UI element. - * - * @param {Document} domDocument - * @returns {HTMLElement} - */ - render( domDocument ) { - return this.toDomElement( domDocument ); - } - - /** - * Creates DOM element based on this view `RawElement`. - * - * **Note**: Each time this method is called new DOM element is created. - * - * @param {Document} domDocument - * @returns {HTMLElement} + * @method #render + * @param {HTMLElement} domElement The native DOM element representing the raw view element. */ - toDomElement( domDocument ) { - const domElement = domDocument.createElement( this.name ); - - for ( const key of this.getAttributeKeys() ) { - domElement.setAttribute( key, this.getAttribute( key ) ); - } - - return domElement; - } } // Returns `null` because block filler is not needed for RawElements. From 5e9cefbb88796437596b3a9660bf663a077b2c3e Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jul 2020 15:59:45 +0200 Subject: [PATCH 16/28] Tests: Updated RawElement tests after #render() simplification. --- .../ckeditor5-engine/tests/view/rawelement.js | 52 ++----------------- 1 file changed, 3 insertions(+), 49 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/rawelement.js b/packages/ckeditor5-engine/tests/view/rawelement.js index 5035846ed6a..82cf83fc67c 100644 --- a/packages/ckeditor5-engine/tests/view/rawelement.js +++ b/packages/ckeditor5-engine/tests/view/rawelement.js @@ -3,8 +3,6 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* global document, HTMLElement */ - import RawElement from '../../src/view/rawelement'; import Element from '../../src/view/element'; import Document from '../../src/view/document'; @@ -37,7 +35,7 @@ describe( 'RawElement', () => { it( 'should throw if child elements are passed to constructor', () => { expectToThrowCKEditorError( () => { new RawElement( doc, 'img', null, [ new Element( doc, 'i' ) ] ); // eslint-disable-line no-new - }, 'view-rawelement-cannot-add: Cannot add child nodes to RawElement instance.' ); + }, 'view-rawelement-cannot-add: Cannot add child nodes to a RawElement instance.' ); } ); } ); @@ -87,7 +85,7 @@ describe( 'RawElement', () => { it( 'should throw when try to append new child element', () => { expectToThrowCKEditorError( () => { rawElement._appendChild( new Element( doc, 'i' ) ); - }, 'view-rawelement-cannot-add: Cannot add child nodes to RawElement instance.' ); + }, 'view-rawelement-cannot-add: Cannot add child nodes to a RawElement instance.' ); } ); } ); @@ -95,7 +93,7 @@ describe( 'RawElement', () => { it( 'should throw when try to insert new child element', () => { expectToThrowCKEditorError( () => { rawElement._insertChild( 0, new Element( doc, 'i' ) ); - }, 'view-rawelement-cannot-add: Cannot add child nodes to RawElement instance.' ); + }, 'view-rawelement-cannot-add: Cannot add child nodes to a RawElement instance.' ); } ); } ); @@ -118,48 +116,4 @@ describe( 'RawElement', () => { expect( rawElement.getFillerOffset() ).to.null; } ); } ); - - describe( 'render()', () => { - let domElement; - - beforeEach( () => { - domElement = rawElement.render( document ); - } ); - - it( 'should return DOM element', () => { - expect( domElement ).to.be.instanceOf( HTMLElement ); - } ); - - it( 'should use element name', () => { - expect( domElement.tagName.toLowerCase() ).to.equal( rawElement.name ); - } ); - - it( 'should render attributes', () => { - for ( const key of rawElement.getAttributeKeys() ) { - expect( domElement.getAttribute( key ) ).to.equal( rawElement.getAttribute( key ) ); - } - } ); - - it( 'should allow to change render() method', () => { - rawElement.render = function( domDocument ) { - return domDocument.createElement( 'b' ); - }; - - expect( rawElement.render( document ).tagName.toLowerCase() ).to.equal( 'b' ); - } ); - - it( 'should allow to add new elements inside', () => { - rawElement.render = function( domDocument ) { - const element = this.toDomElement( domDocument ); - const text = domDocument.createTextNode( 'foo bar' ); - element.appendChild( text ); - - return element; - }; - - const rendered = rawElement.render( document ); - expect( rendered.tagName.toLowerCase() ).to.equal( 'span' ); - expect( rendered.textContent ).to.equal( 'foo bar' ); - } ); - } ); } ); From bc1a607b5003147107b82b98b471ef8bd7e73b38 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jul 2020 16:00:47 +0200 Subject: [PATCH 17/28] Simplified the DowncastWriter#createRawElement() method. --- .../src/view/downcastwriter.js | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/downcastwriter.js b/packages/ckeditor5-engine/src/view/downcastwriter.js index 3f41eb94d65..5932b18226a 100644 --- a/packages/ckeditor5-engine/src/view/downcastwriter.js +++ b/packages/ckeditor5-engine/src/view/downcastwriter.js @@ -258,6 +258,11 @@ export default class DowncastWriter { * return domElement; * } ); * + * Unlike {@link #createRawElement raw elements}, UI elements are by no means editor content, for instance, + * they are ignored by the editor selection system and they cannot {@link module:widget/utils.toWidget become a widget}. + * + * You should not use UI elements as data containers. Check out {@link #createRawElement} instead. + * * @param {String} name Name of the element. * @param {Object} [attributes] Elements attributes. * @param {Function} [renderFunction] Custom render function. @@ -279,15 +284,23 @@ export default class DowncastWriter { * writer.createRawElement( 'span' ); * writer.createRawElement( 'span', { id: 'foo-1234' } ); * - * Custom render function can be provided as third parameter: - * - * writer.createRawElement( 'span', null, function( domDocument ) { - * const domElement = this.toDomElement( domDocument ); - * domElement.innerHTML = 'this is ui element'; + * Custom render function should be provided as third parameter: * - * return domElement; + * writer.createRawElement( 'span', null, function( domElement ) { + * domElement.innerHTML = 'This is the raw content of the raw element.'; * } ); * + * Raw elements work as data containers ("wrappers", "sandboxes") but their children are not managed or + * even recognized by the editor. This encapsulation allows integrations to maintain custom DOM structures + * in the editor content without, for instance, worrying about compatibility with other editor features. + * Raw elements make a perfect tool for integration with external frameworks and data sources. + * + * Unlike {@link #createUIElement ui elements}, raw elements act like a "real" editor content (similar to + * {@link module:engine/view/containerelement~ContainerElement} or {@link module:engine/view/emptyelement~EmptyElement}), + * they are considered by the editor selection and {@link module:widget/utils.toWidget they can work as widgets}. + * + * You should not use raw elements to render UI in the editor content. Check out {@link #createUIElement} instead. + * * @param {String} name Name of the element. * @param {Object} [attributes] Elements attributes. * @param {Function} [renderFunction] Custom render function. @@ -296,9 +309,7 @@ export default class DowncastWriter { createRawElement( name, attributes, renderFunction ) { const rawElement = new RawElement( this.document, name, attributes ); - if ( renderFunction ) { - rawElement.render = renderFunction; - } + rawElement.render = renderFunction || ( () => {} ); return rawElement; } @@ -1610,7 +1621,7 @@ export default class DowncastWriter { * * @error view-writer-cannot-break-raw-element */ - throw new CKEditorError( 'view-writer-cannot-break-raw-element', this.document ); + throw new CKEditorError( 'view-writer-cannot-break-raw-element: Cannot break inside a RawElement instance.', this.document ); } // There are no attributes to break and text nodes breaking is not forced. From d7483a1d0fcc1aa30b7adf61c499e1b6a6d7f2e3 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jul 2020 16:01:56 +0200 Subject: [PATCH 18/28] Made sure editing view creates the RawElement. Updated docs. --- .../ckeditor5-engine/src/view/domconverter.js | 17 ++++++++++------- .../tests/view/domconverter/rawelement.js | 8 +++----- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/domconverter.js b/packages/ckeditor5-engine/src/view/domconverter.js index bff23bbea84..5fb92a01484 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.js +++ b/packages/ckeditor5-engine/src/view/domconverter.js @@ -229,17 +229,19 @@ export default class DomConverter { return domElement; } else { - // RawElement has its own render() method (see https://github.com/ckeditor/ckeditor5/issues/4469). - if ( viewNode.is( 'rawElement' ) ) { - domElement = viewNode.render( domDocument ); - } // Create DOM element. - else if ( viewNode.hasAttribute( 'xmlns' ) ) { + if ( viewNode.hasAttribute( 'xmlns' ) ) { domElement = domDocument.createElementNS( viewNode.getAttribute( 'xmlns' ), viewNode.name ); } else { domElement = domDocument.createElement( viewNode.name ); } + // RawElement take care of their children in RawElement#render() method which can be customized + // (see https://github.com/ckeditor/ckeditor5/issues/4469). + if ( viewNode.is( 'rawElement' ) ) { + viewNode.render( domElement ); + } + if ( options.bind ) { this.bindElements( domElement, viewNode ); } @@ -895,9 +897,10 @@ export default class DomConverter { * Checks if given selection's boundaries are at correct places. * * The following places are considered as incorrect for selection boundaries: + * * * before or in the middle of the inline filler sequence, - * * inside the DOM element which represents {@link module:engine/view/uielement~UIElement a view UI element}. - * * inside the DOM element which represents {@link module:engine/view/rawelement~RawElement a view raw element}. + * * inside a DOM element which represents {@link module:engine/view/uielement~UIElement a view UI element}, + * * inside a DOM element which represents {@link module:engine/view/rawelement~RawElement a view raw element}. * * @param {Selection} domSelection DOM Selection object to be checked. * @returns {Boolean} `true` if the given selection is at a correct place, `false` otherwise. diff --git a/packages/ckeditor5-engine/tests/view/domconverter/rawelement.js b/packages/ckeditor5-engine/tests/view/domconverter/rawelement.js index 2b6fa9c7163..67289f3f206 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/rawelement.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/rawelement.js @@ -17,11 +17,8 @@ describe( 'DOMConverter RawElement integration', () => { function createRawElement( name ) { const element = new ViewRawElement( viewDocument, name ); - element.render = function( domDocument ) { - const root = this.toDomElement( domDocument ); - root.innerHTML = '

foo bar

'; - - return root; + element.render = function( domElement ) { + domElement.innerHTML = '

foo bar

'; }; return element; @@ -35,6 +32,7 @@ describe( 'DOMConverter RawElement integration', () => { describe( 'viewToDom()', () => { it( 'should create a DOM element from a RawElement', () => { const rawElement = new ViewRawElement( viewDocument, 'div' ); + rawElement.render = () => {}; const domElement = converter.viewToDom( rawElement, document ); expect( domElement ).to.be.instanceOf( HTMLElement ); From 60476ef067fc307995c0b4ebb6372664c4fbdd3f Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jul 2020 16:02:57 +0200 Subject: [PATCH 19/28] Tests: Updated view stringify test helper after changes to the RawElement rendering strategy. --- packages/ckeditor5-engine/src/dev-utils/view.js | 14 +++++++++++--- packages/ckeditor5-engine/tests/dev-utils/view.js | 6 +----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/ckeditor5-engine/src/dev-utils/view.js b/packages/ckeditor5-engine/src/dev-utils/view.js index 0be96687ebd..0045a64411d 100644 --- a/packages/ckeditor5-engine/src/dev-utils/view.js +++ b/packages/ckeditor5-engine/src/dev-utils/view.js @@ -680,8 +680,15 @@ class ViewStringify { callback( this._stringifyElementOpen( root ) ); } - if ( ( this.renderUIElements && root.is( 'uiElement' ) ) || ( this.renderRawElements && root.is( 'rawElement' ) ) ) { + if ( ( this.renderUIElements && root.is( 'uiElement' ) ) ) { callback( root.render( document ).innerHTML ); + } else if ( this.renderRawElements && root.is( 'rawElement' ) ) { + // There's no DOM element for "root" to pass to render(). Creating + // a surrogate container to render the children instead. + const rawContentContainer = document.createElement( 'div' ); + root.render( rawContentContainer ); + + callback( rawContentContainer.innerHTML ); } else { let offset = 0; callback( this._stringifyElementRanges( root, offset ) ); @@ -834,8 +841,9 @@ class ViewStringify { * Returns: * * 'attribute' for {@link module:engine/view/attributeelement~AttributeElement attribute elements}, * * 'container' for {@link module:engine/view/containerelement~ContainerElement container elements}, - * * 'empty' for {@link module:engine/view/emptyelement~EmptyElement empty elements}. - * * 'ui' for {@link module:engine/view/uielement~UIElement UI elements}. + * * 'empty' for {@link module:engine/view/emptyelement~EmptyElement empty elements}, + * * 'ui' for {@link module:engine/view/uielement~UIElement UI elements}, + * * 'raw' for {@link module:engine/view/rawelement~RawElement raw elements}, * * an empty string when the current configuration is preventing showing elements' types. * * @private diff --git a/packages/ckeditor5-engine/tests/dev-utils/view.js b/packages/ckeditor5-engine/tests/dev-utils/view.js index e983535b30e..c2e7c8d6531 100644 --- a/packages/ckeditor5-engine/tests/dev-utils/view.js +++ b/packages/ckeditor5-engine/tests/dev-utils/view.js @@ -431,12 +431,8 @@ describe( 'view test utils', () => { it( 'should stringify a RawElement, (renderRawElements=true)', () => { const span = new RawElement( viewDocument, 'span' ); - span.render = function( domDocument ) { - const domElement = this.toDomElement( domDocument ); - + span.render = function( domElement ) { domElement.innerHTML = 'foo'; - - return domElement; }; const p = new ContainerElement( viewDocument, 'p', null, span ); From 777a1b3c6efb96521db61b5b8a6845b2cd25fcad Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jul 2020 16:03:21 +0200 Subject: [PATCH 20/28] Tests: Updated view renderer tests after changes to RawElement rendering strategy. --- .../ckeditor5-engine/tests/view/renderer.js | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/renderer.js b/packages/ckeditor5-engine/tests/view/renderer.js index cc4e86ac623..7a71b9aa428 100644 --- a/packages/ckeditor5-engine/tests/view/renderer.js +++ b/packages/ckeditor5-engine/tests/view/renderer.js @@ -3301,13 +3301,11 @@ describe( 'Renderer', () => { '

FooUI2 Bar

' ) ); } ); - it( 'should handle rawElement rendering', () => { + it( 'should handle RawElement rendering', () => { function createRawElement( id, text ) { const element = new ViewRawElement( viewDocument, 'span' ); - element.render = function( domDocument ) { - const domElement = this.toDomElement( domDocument ); + element.render = function( domElement ) { domElement.innerText = `${ text }`; - return domElement; }; return element; @@ -3339,6 +3337,36 @@ describe( 'Renderer', () => { '

FooRAW2 Bar

' ) ); } ); + it( 'should manage RawElement attributes', () => { + const rawElement = new ViewRawElement( viewDocument, 'span', { + foo: 'foo1', + baz: 'baz1' + } ); + + rawElement.render = function( domElement ) { + domElement.innerHTML = 'foo'; + }; + + const viewP = new ViewContainerElement( viewDocument, 'p', null, [ rawElement ] ); + viewRoot._appendChild( viewP ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + expect( normalizeHtml( domRoot.innerHTML ) ).to.equal( normalizeHtml( + '

foo

' ) ); + + rawElement._setAttribute( 'foo', 'foo2' ); + rawElement._setAttribute( 'new', 'new-value' ); + rawElement._removeAttribute( 'baz' ); + + renderer.markToSync( 'attributes', rawElement ); + renderer.render(); + + expect( normalizeHtml( domRoot.innerHTML ) ).to.equal( normalizeHtml( + '

foo

' ) ); + } ); + it( 'should handle linking entire content', () => { viewRoot._appendChild( parse( 'FooBar' ) ); From f298535dba3c8712c20c704b4a60ba0a009a20b9 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jul 2020 16:03:37 +0200 Subject: [PATCH 21/28] Tests: Updated mutation observer tests after changes to RawElement rendering strategy. --- .../tests/view/observer/mutationobserver.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/observer/mutationobserver.js b/packages/ckeditor5-engine/tests/view/observer/mutationobserver.js index dd17d87e590..1be30672d41 100644 --- a/packages/ckeditor5-engine/tests/view/observer/mutationobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/mutationobserver.js @@ -597,11 +597,8 @@ describe( 'MutationObserver', () => { function createRawElement( name ) { const element = new RawElement( name ); - element.render = function( domDocument ) { - const root = this.toDomElement( domDocument ); - root.innerHTML = 'foo bar'; - - return root; + element.render = function( domElement ) { + domElement.innerHTML = 'foo bar'; }; return element; From 1d771af67089eab4535e4b4f1830e05fd31b3f22 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jul 2020 16:04:04 +0200 Subject: [PATCH 22/28] Used the new DowncastWriter#createRawElement API in the MediaRegistry. --- packages/ckeditor5-media-embed/src/mediaregistry.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/ckeditor5-media-embed/src/mediaregistry.js b/packages/ckeditor5-media-embed/src/mediaregistry.js index 5102fa6583b..fdb6673dc62 100644 --- a/packages/ckeditor5-media-embed/src/mediaregistry.js +++ b/packages/ckeditor5-media-embed/src/mediaregistry.js @@ -234,12 +234,8 @@ class Media { const mediaHtml = this._getPreviewHtml( options ); - viewElement = writer.createRawElement( 'div', attributes, function( domDocument ) { - const domElement = this.toDomElement( domDocument ); - + viewElement = writer.createRawElement( 'div', attributes, function( domElement ) { domElement.innerHTML = mediaHtml; - - return domElement; } ); } else { if ( this.url ) { From ff1d7e2e21344acedcb23229cd4117c0d8e67f42 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jul 2020 16:04:35 +0200 Subject: [PATCH 23/28] Docs: Used the new DowncastWriter#createRawElement API in the "Using a React component in a block widget" guide. --- .../framework/tutorials/using-react-in-widget.js | 6 +----- .../guides/tutorials/using-react-in-a-widget.md | 12 ++---------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/docs/_snippets/framework/tutorials/using-react-in-widget.js b/docs/_snippets/framework/tutorials/using-react-in-widget.js index 4aac4006846..5f101b01588 100644 --- a/docs/_snippets/framework/tutorials/using-react-in-widget.js +++ b/docs/_snippets/framework/tutorials/using-react-in-widget.js @@ -116,15 +116,11 @@ class ProductPreviewEditing extends Plugin { // This element will host a React component. const reactWrapper = viewWriter.createRawElement( 'div', { class: 'product__react-wrapper' - }, function( domDocument ) { - const domElement = this.toDomElement( domDocument ); - + }, function( domElement ) { // This the place where React renders the actual product preview hosted // by a UIElement in the view. you are using a function (renderer) passed as // editor.config.products#productRenderer. renderProduct( id, domElement ); - - return domElement; } ); viewWriter.insert( viewWriter.createPositionAt( section, 0 ), reactWrapper ); diff --git a/docs/framework/guides/tutorials/using-react-in-a-widget.md b/docs/framework/guides/tutorials/using-react-in-a-widget.md index acb68d78c26..29064d87095 100644 --- a/docs/framework/guides/tutorials/using-react-in-a-widget.md +++ b/docs/framework/guides/tutorials/using-react-in-a-widget.md @@ -367,15 +367,11 @@ export default class ProductPreviewEditing extends Plugin { // This element will host a React component. const reactWrapper = viewWriter.createRawElement( 'div', { class: 'product__react-wrapper' - }, function( domDocument ) { - const domElement = this.toDomElement( domDocument ); - + }, function( domElement ) { // This the place where React renders the actual product preview hosted // by a UIElement in the view. You are using a function (renderer) passed as // editor.config.products#productRenderer. renderProduct( id, domElement ); - - return domElement; } ); viewWriter.insert( viewWriter.createPositionAt( section, 0 ), reactWrapper ); @@ -1186,15 +1182,11 @@ export default class ProductPreviewEditing extends Plugin { // This element will host a React component. const reactWrapper = viewWriter.createRawElement( 'div', { class: 'product__react-wrapper' - }, function( domDocument ) { - const domElement = this.toDomElement( domDocument ); - + }, function( domElement ) { // This the place where React renders the actual product preview hosted // by a UIElement in the view. You are using a function (renderer) passed as // editor.config.products#productRenderer. renderProduct( id, domElement ); - - return domElement; } ); viewWriter.insert( viewWriter.createPositionAt( section, 0 ), reactWrapper ); From 9969d1e3819b622dd7abd3a9f10d68cc18a9a37b Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jul 2020 16:05:00 +0200 Subject: [PATCH 24/28] Docs: Improved the description of raw elements in the "Editing engine" guide. --- docs/framework/guides/architecture/editing-engine.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/framework/guides/architecture/editing-engine.md b/docs/framework/guides/architecture/editing-engine.md index 74a555d6d08..85512f400a6 100644 --- a/docs/framework/guides/architecture/editing-engine.md +++ b/docs/framework/guides/architecture/editing-engine.md @@ -232,7 +232,7 @@ The element types can be defined as follows: * **Attribute element** – The elements that cannot contain container elements inside them. Most model text attributes are converted to view attribute elements. They are used mostly for inline styling elements such as ``, ``, ``, ``. Similar attribute elements are flattened by the view writer, so e.g. `x` would automatically be optimized to `x`. * **Empty element** – The elements that must not have any child nodes, for example ``. * **UI elements** – The elements that are not a part of the "data" but need to be "inlined" in the content. They are ignored by the selection (it jumps over them) and the view writer in general. The contents of these elements and events coming from them are filtered out, too. -* **Raw element** – The elements that work as data containers ("sandboxes") but their children are transparent to the editor. Useful when non-standard data must be rendered but the editor should not be concerned what it it is and how it works. Users cannot put the selection inside a raw element, split it into smaller chunks or directly modify its content. +* **Raw element** – The elements that work as data containers ("wrappers", "sandboxes") but their children are transparent to the editor. Useful when non-standard data must be rendered but the editor should not be concerned what it is and how it works. Users cannot put the selection inside a raw element, split it into smaller chunks or directly modify its content. * **Editable element** – The elements used as "nested editables" of non-editable fragments of the content, for example a caption in the image widget, where the `
` wrapping the image is not editable (it is a widget) and the `
` inside it is an editable element. Additionally, you can define {@link module:engine/view/element~Element#getCustomProperty custom properties} which can be used to store information like: From ea42b0dadb5c2c88d782236f070e8053d5bc732e Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 21 Jul 2020 09:57:24 +0200 Subject: [PATCH 25/28] toWidget() should throw when attempting to create a widget out of a non-ContainerElement instance. --- packages/ckeditor5-widget/src/utils.js | 16 ++++++++++++++++ packages/ckeditor5-widget/tests/utils.js | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/packages/ckeditor5-widget/src/utils.js b/packages/ckeditor5-widget/src/utils.js index acfaf2d1311..c3723a6435e 100644 --- a/packages/ckeditor5-widget/src/utils.js +++ b/packages/ckeditor5-widget/src/utils.js @@ -12,6 +12,7 @@ import IconView from '@ckeditor/ckeditor5-ui/src/icon/iconview'; import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview'; import global from '@ckeditor/ckeditor5-utils/src/dom/global'; +import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import dragHandleIcon from '../theme/icons/drag-handle.svg'; import { getTypeAroundFakeCaretPosition } from './widgettypearound/utils'; @@ -93,6 +94,21 @@ export function isWidget( node ) { */ /* eslint-enable max-len */ export function toWidget( element, writer, options = {} ) { + if ( !element.is( 'containerElement' ) ) { + /** + * The element passed to `toWidget()` must be a {@link module:engine/view/containerelement~ContainerElement} + * instance. + * + * @error widget-to-widget-wrong-element-type + * @param {String} element The view element passed to `toWidget()`. + */ + throw new CKEditorError( + 'widget-to-widget-wrong-element-type: The element passed to toWidget() must be a container element instance.', + null, + { element } + ); + } + writer.setAttribute( 'contenteditable', 'false', element ); writer.addClass( WIDGET_CLASS_NAME, element ); diff --git a/packages/ckeditor5-widget/tests/utils.js b/packages/ckeditor5-widget/tests/utils.js index 42f2a244e6e..b654fc671cd 100644 --- a/packages/ckeditor5-widget/tests/utils.js +++ b/packages/ckeditor5-widget/tests/utils.js @@ -126,6 +126,24 @@ describe( 'widget utils', () => { expect( icon.classList.contains( 'ck' ) ).to.be.true; expect( icon.classList.contains( 'ck-icon' ) ).to.be.true; } ); + + it( 'should throw when attempting to create a widget out of anything but ContainerElement', () => { + expect( () => { + toWidget( writer.createRawElement( 'div' ), writer ); + }, 'raw element' ).to.throw( /^widget-to-widget-wrong-element-type/ ); + + expect( () => { + toWidget( writer.createEmptyElement( 'img' ), writer ); + }, 'empty element' ).to.throw( /^widget-to-widget-wrong-element-type/ ); + + expect( () => { + toWidget( writer.createAttributeElement( 'a' ), writer ); + }, 'attribute element' ).to.throw( /^widget-to-widget-wrong-element-type/ ); + + expect( () => { + toWidget( writer.createUIElement( 'span' ), writer ); + }, 'UI element' ).to.throw( /^widget-to-widget-wrong-element-type/ ); + } ); } ); describe( 'isWidget()', () => { From 4885381905c72daeb0fe6a91433a03a99c225b3c Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 21 Jul 2020 10:44:34 +0200 Subject: [PATCH 26/28] Tests: Added a DowncastWriter#createRawElement() test to check if RawElement gets a default render() method if not specified. --- .../tests/view/downcastwriter/writer.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/ckeditor5-engine/tests/view/downcastwriter/writer.js b/packages/ckeditor5-engine/tests/view/downcastwriter/writer.js index 6850d910287..797ee204da5 100644 --- a/packages/ckeditor5-engine/tests/view/downcastwriter/writer.js +++ b/packages/ckeditor5-engine/tests/view/downcastwriter/writer.js @@ -143,6 +143,18 @@ describe( 'DowncastWriter', () => { expect( element.is( 'rawElement' ) ).to.be.true; expect( element.name ).to.equal( 'foo' ); assertElementAttributes( element, attributes ); + + expect( element.render ).to.be.a( 'function' ); + } ); + + it( 'should provide a default empty render() method', () => { + const element = writer.createRawElement( 'foo' ); + + expect( element.render ).to.be.a( 'function' ); + + expect( () => { + element.render(); + } ).to.not.throw(); } ); it( 'should allow to pass custom rendering method', () => { From a13b28860b1ef06e1776b8d9da2e01edc21abf30 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 22 Jul 2020 09:48:17 +0200 Subject: [PATCH 27/28] Docs: Updated DowncastWriter#createUIElement() and #createRawElement() docs. --- packages/ckeditor5-engine/src/view/downcastwriter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/downcastwriter.js b/packages/ckeditor5-engine/src/view/downcastwriter.js index 7a62f776f5f..7c5c0eceb00 100644 --- a/packages/ckeditor5-engine/src/view/downcastwriter.js +++ b/packages/ckeditor5-engine/src/view/downcastwriter.js @@ -259,7 +259,7 @@ export default class DowncastWriter { * } ); * * Unlike {@link #createRawElement raw elements}, UI elements are by no means editor content, for instance, - * they are ignored by the editor selection system and they cannot {@link module:widget/utils.toWidget become a widget}. + * they are ignored by the editor selection system. * * You should not use UI elements as data containers. Check out {@link #createRawElement} instead. * @@ -297,7 +297,7 @@ export default class DowncastWriter { * * Unlike {@link #createUIElement ui elements}, raw elements act like a "real" editor content (similar to * {@link module:engine/view/containerelement~ContainerElement} or {@link module:engine/view/emptyelement~EmptyElement}), - * they are considered by the editor selection and {@link module:widget/utils.toWidget they can work as widgets}. + * and they are considered by the editor selection. * * You should not use raw elements to render UI in the editor content. Check out {@link #createUIElement} instead. * From b224e9718048e07acf9f53d53da6a985f1ceea4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Wed, 22 Jul 2020 22:40:54 +0200 Subject: [PATCH 28/28] Removed confusing examples. --- packages/ckeditor5-engine/src/view/downcastwriter.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/downcastwriter.js b/packages/ckeditor5-engine/src/view/downcastwriter.js index 7c5c0eceb00..cf0e83eb1ac 100644 --- a/packages/ckeditor5-engine/src/view/downcastwriter.js +++ b/packages/ckeditor5-engine/src/view/downcastwriter.js @@ -281,12 +281,7 @@ export default class DowncastWriter { /** * Creates a new {@link module:engine/view/rawelement~RawElement}. * - * writer.createRawElement( 'span' ); - * writer.createRawElement( 'span', { id: 'foo-1234' } ); - * - * Custom render function should be provided as third parameter: - * - * writer.createRawElement( 'span', null, function( domElement ) { + * writer.createRawElement( 'span', { id: 'foo-1234' }, function( domElement ) { * domElement.innerHTML = 'This is the raw content of the raw element.'; * } ); * @@ -299,7 +294,7 @@ export default class DowncastWriter { * {@link module:engine/view/containerelement~ContainerElement} or {@link module:engine/view/emptyelement~EmptyElement}), * and they are considered by the editor selection. * - * You should not use raw elements to render UI in the editor content. Check out {@link #createUIElement} instead. + * You should not use raw elements to render UI in the editor content. Check out {@link #createUIElement `#createUIElement()`} instead. * * @param {String} name Name of the element. * @param {Object} [attributes] Elements attributes.