diff --git a/.travis.yml b/.travis.yml index 6a542fc7b..a143860b6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,7 +7,7 @@ language: node_js services: - xvfb node_js: -- '8' +- '10' cache: yarn: true branches: diff --git a/src/model/documentfragment.js b/src/model/documentfragment.js index 79926943e..2224237f6 100644 --- a/src/model/documentfragment.js +++ b/src/model/documentfragment.js @@ -133,7 +133,7 @@ export default class DocumentFragment { * @returns {Boolean} */ is( type ) { - return type == 'documentFragment' || type == 'model:documentFragment'; + return type === 'documentFragment' || type === 'model:documentFragment'; } /** diff --git a/src/model/documentselection.js b/src/model/documentselection.js index 214bfd48b..b97752846 100644 --- a/src/model/documentselection.js +++ b/src/model/documentselection.js @@ -382,7 +382,7 @@ export default class DocumentSelection { * @returns {Boolean} */ is( type ) { - return type == 'selection' || + return type === 'selection' || type == 'model:selection' || type == 'documentSelection' || type == 'model:documentSelection'; diff --git a/src/model/element.js b/src/model/element.js index 59c14e1c8..202771e8f 100644 --- a/src/model/element.js +++ b/src/model/element.js @@ -116,13 +116,14 @@ export default class Element extends Node { * @returns {Boolean} */ is( type, name = null ) { - const cutType = type.replace( /^model:/, '' ); - if ( !name ) { - return cutType == 'element' || cutType == this.name || super.is( type ); - } else { - return cutType == 'element' && name == this.name; + return type === 'element' || type === 'model:element' || + type === this.name || type === 'model:' + this.name || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'node' || type === 'model:node'; } + + return name === this.name && ( type === 'element' || type === 'model:element' ); } /** diff --git a/src/model/liveposition.js b/src/model/liveposition.js index 8aaccbc68..a577d7b67 100644 --- a/src/model/liveposition.js +++ b/src/model/liveposition.js @@ -80,7 +80,9 @@ export default class LivePosition extends Position { * @returns {Boolean} */ is( type ) { - return type == 'livePosition' || type == 'model:livePosition' || super.is( type ); + return type === 'livePosition' || type === 'model:livePosition' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type == 'position' || type === 'model:position'; } /** diff --git a/src/model/liverange.js b/src/model/liverange.js index b11121cba..a6b6402d3 100644 --- a/src/model/liverange.js +++ b/src/model/liverange.js @@ -57,7 +57,9 @@ export default class LiveRange extends Range { * @returns {Boolean} */ is( type ) { - return type == 'liveRange' || type == 'model:liveRange' || super.is( type ); + return type === 'liveRange' || type === 'model:liveRange' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type == 'range' || type === 'model:range'; } /** diff --git a/src/model/markercollection.js b/src/model/markercollection.js index 4d9a5a7d6..33def0b15 100644 --- a/src/model/markercollection.js +++ b/src/model/markercollection.js @@ -463,7 +463,7 @@ class Marker { * @returns {Boolean} */ is( type ) { - return type == 'marker' || type == 'model:marker'; + return type === 'marker' || type === 'model:marker'; } /** diff --git a/src/model/node.js b/src/model/node.js index d85bb8055..42d5976d6 100644 --- a/src/model/node.js +++ b/src/model/node.js @@ -10,7 +10,6 @@ import toMap from '@ckeditor/ckeditor5-utils/src/tomap'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays'; - // To check if component is loaded more than once. import '@ckeditor/ckeditor5-utils/src/version'; @@ -432,7 +431,7 @@ export default class Node { * @returns {Boolean} */ is( type ) { - return type == 'node' || type == 'model:node'; + return type === 'node' || type === 'model:node'; } /** @@ -500,11 +499,3 @@ export default class Node { this._attrs.clear(); } } - -/** - * The node's parent does not contain this node. - * - * This error may be thrown from corrupted trees. - * - * @error model-node-not-found-in-parent - */ diff --git a/src/model/position.js b/src/model/position.js index 6bc5f2206..795d6d12e 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -10,8 +10,6 @@ import TreeWalker from './treewalker'; import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; -import Text from './text'; -import { last } from 'lodash-es'; // To check if component is loaded more than once. import '@ckeditor/ckeditor5-utils/src/version'; @@ -81,9 +79,13 @@ export default class Position { ); } - // Normalize the root and path (if element was passed). - path = root.getPath().concat( path ); - root = root.root; + // Normalize the root and path when element (not root) is passed. + if ( root.is( 'rootElement' ) ) { + path = path.slice(); + } else { + path = [ ...root.getPath(), ...path ]; + root = root.root; + } /** * Root of the position path. @@ -141,7 +143,7 @@ export default class Position { * @type {Number} */ get offset() { - return last( this.path ); + return this.path[ this.path.length - 1 ]; } /** @@ -216,9 +218,7 @@ export default class Position { * @type {module:engine/model/text~Text|null} */ get textNode() { - const node = this.parent.getChild( this.index ); - - return ( node instanceof Text && node.startOffset < this.offset ) ? node : null; + return getTextNodeAtPosition( this, this.parent ); } /** @@ -228,17 +228,23 @@ export default class Position { * @type {module:engine/model/node~Node|null} */ get nodeAfter() { - return this.textNode === null ? this.parent.getChild( this.index ) : null; + // Cache the parent and reuse for performance reasons. See #6579 and #6582. + const parent = this.parent; + + return getNodeAfterPosition( this, parent, getTextNodeAtPosition( this, parent ) ); } /** * Node directly before this position or `null` if this position is in text node. * * @readonly - * @type {Node} + * @type {module:engine/model/node~Node|null} */ get nodeBefore() { - return this.textNode === null ? this.parent.getChild( this.index - 1 ) : null; + // Cache the parent and reuse for performance reasons. See #6579 and #6582. + const parent = this.parent; + + return getNodeBeforePosition( this, parent, getTextNodeAtPosition( this, parent ) ); } /** @@ -339,10 +345,12 @@ export default class Position { * @returns {Array.} Array with ancestors. */ getAncestors() { - if ( this.parent.is( 'documentFragment' ) ) { - return [ this.parent ]; + const parent = this.parent; + + if ( parent.is( 'documentFragment' ) ) { + return [ parent ]; } else { - return this.parent.getAncestors( { includeSelf: true } ); + return parent.getAncestors( { includeSelf: true } ); } } @@ -540,7 +548,7 @@ export default class Position { * @returns {Boolean} */ is( type ) { - return type == 'position' || type == 'model:position'; + return type === 'position' || type === 'model:position'; } /** @@ -840,7 +848,7 @@ export default class Position { // Then, add the rest of the path. // If this position is at the same level as `from` position nothing will get added. - combined.path = combined.path.concat( this.path.slice( i + 1 ) ); + combined.path = [ ...combined.path, ...this.path.slice( i + 1 ) ]; return combined; } @@ -1057,3 +1065,91 @@ export default class Position { * * @typedef {String} module:engine/model/position~PositionStickiness */ + +/** + * Returns a text node at the given position. + * + * This is a helper function optimized to reuse the position parent instance for performance reasons. + * + * Normally, you should use {@link module:engine/model/position~Position#textNode `Position#textNode`}. + * If you start hitting performance issues with {@link module:engine/model/position~Position#parent `Position#parent`} + * check if your algorithm does not access it multiple times (which can happen directly or indirectly via other position properties). + * + * See https://github.com/ckeditor/ckeditor5/issues/6579. + * + * See also: + * + * * {@link module:engine/model/position~getNodeAfterPosition} + * * {@link module:engine/model/position~getNodeBeforePosition} + * + * @param {module:engine/model/position~Position} position + * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} positionParent The parent of the + * given position. + * @returns {module:engine/model/text~Text|null} + */ +export function getTextNodeAtPosition( position, positionParent ) { + const node = positionParent.getChild( positionParent.offsetToIndex( position.offset ) ); + + if ( node && node.is( 'text' ) && node.startOffset < position.offset ) { + return node; + } + + return null; +} + +/** + * Returns the node after the given position. + * + * This is a helper function optimized to reuse the position parent instance and the calculation of the text node at the + * specific position for performance reasons. + * + * Normally, you should use {@link module:engine/model/position~Position#nodeAfter `Position#nodeAfter`}. + * If you start hitting performance issues with {@link module:engine/model/position~Position#parent `Position#parent`} and/or + * {@link module:engine/model/position~Position#textNode `Position#textNode`} + * check if your algorithm does not access those properties multiple times + * (which can happen directly or indirectly via other position properties). + * + * See https://github.com/ckeditor/ckeditor5/issues/6579 and https://github.com/ckeditor/ckeditor5/issues/6582. + * + * See also: + * + * * {@link module:engine/model/position~getTextNodeAtPosition} + * * {@link module:engine/model/position~getNodeBeforePosition} + * + * @param {module:engine/model/position~Position} position + * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} positionParent The parent of the + * given position. + * @param {module:engine/model/text~Text|null} textNode Text node at the given position. + * @returns {module:engine/model/node~Node|null} + */ +export function getNodeAfterPosition( position, positionParent, textNode ) { + if ( textNode !== null ) { + return null; + } + + return positionParent.getChild( positionParent.offsetToIndex( position.offset ) ); +} + +/** + * Returns the node before the given position. + * + * Refer to {@link module:engine/model/position~getNodeBeforePosition} for documentation on when to use this util method. + * + * See also: + * + * * {@link module:engine/model/position~getTextNodeAtPosition} + * * {@link module:engine/model/position~getNodeAfterPosition} + * + * @param {module:engine/model/position~Position} position + * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} positionParent The parent of the + * given position. + * @param {module:engine/model/text~Text|null} textNode Text node at the given position. + * @returns {module:engine/model/node~Node|null} + */ +export function getNodeBeforePosition( position, positionParent, textNode ) { + if ( textNode !== null ) { + return null; + } + + return positionParent.getChild( positionParent.offsetToIndex( position.offset ) - 1 ); +} diff --git a/src/model/range.js b/src/model/range.js index 559fb20cb..743f09bd7 100644 --- a/src/model/range.js +++ b/src/model/range.js @@ -158,7 +158,7 @@ export default class Range { * @returns {Boolean} */ is( type ) { - return type == 'range' || type == 'model:range'; + return type === 'range' || type === 'model:range'; } /** diff --git a/src/model/rootelement.js b/src/model/rootelement.js index 2d29cac2d..39e21239a 100644 --- a/src/model/rootelement.js +++ b/src/model/rootelement.js @@ -80,12 +80,19 @@ export default class RootElement extends Element { * @returns {Boolean} */ is( type, name ) { - const cutType = type.replace( 'model:', '' ); if ( !name ) { - return cutType == 'rootElement' || super.is( type ); - } else { - return ( cutType == 'rootElement' && name == this.name ) || super.is( type, name ); + return type === 'rootElement' || type === 'model:rootElement' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'element' || type === 'model:element' || + type === this.name || type === 'model:' + this.name || + type === 'node' || type === 'model:node'; } + + return name === this.name && ( + type === 'rootElement' || type === 'model:rootElement' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'element' || type === 'model:element' + ); } /** @@ -105,3 +112,4 @@ export default class RootElement extends Element { // @if CK_DEBUG_ENGINE // console.log( 'ModelRootElement: ' + this ); // @if CK_DEBUG_ENGINE // } } + diff --git a/src/model/selection.js b/src/model/selection.js index 18e7ebef4..b1a38af56 100644 --- a/src/model/selection.js +++ b/src/model/selection.js @@ -632,7 +632,7 @@ export default class Selection { * @returns {Boolean} */ is( type ) { - return type == 'selection' || type == 'model:selection'; + return type === 'selection' || type === 'model:selection'; } /** diff --git a/src/model/text.js b/src/model/text.js index 56bcafd33..7e9c0edf6 100644 --- a/src/model/text.js +++ b/src/model/text.js @@ -82,7 +82,9 @@ export default class Text extends Node { * @returns {Boolean} */ is( type ) { - return type == 'text' || type == 'model:text' || super.is( type ); + return type === 'text' || type === 'model:text' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'node' || type === 'model:node'; } /** diff --git a/src/model/textproxy.js b/src/model/textproxy.js index f3946f8c4..91610f364 100644 --- a/src/model/textproxy.js +++ b/src/model/textproxy.js @@ -178,7 +178,7 @@ export default class TextProxy { * @returns {Boolean} */ is( type ) { - return type == 'textProxy' || type == 'model:textProxy'; + return type === 'textProxy' || type === 'model:textProxy'; } /** diff --git a/src/model/treewalker.js b/src/model/treewalker.js index f70da7700..bcf0b5288 100644 --- a/src/model/treewalker.js +++ b/src/model/treewalker.js @@ -10,7 +10,12 @@ import Text from './text'; import TextProxy from './textproxy'; import Element from './element'; -import Position from './position'; +import { + default as Position, + getTextNodeAtPosition, + getNodeAfterPosition, + getNodeBeforePosition +} from './position'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; /** @@ -225,7 +230,11 @@ export default class TreeWalker { return { done: true }; } - const node = position.textNode ? position.textNode : position.nodeAfter; + // Get node just after the current position. + // Use a highly optimized version instead of checking the text node first and then getting the node after. See #6582. + const positionParent = position.parent; + const textNodeAtPosition = getTextNodeAtPosition( position, positionParent ); + const node = textNodeAtPosition ? textNodeAtPosition : getNodeAfterPosition( position, positionParent, textNodeAtPosition ); if ( node instanceof Element ) { if ( !this.shallow ) { @@ -299,8 +308,11 @@ export default class TreeWalker { return { done: true }; } - // Get node just before current position - const node = position.textNode ? position.textNode : position.nodeBefore; + // Get node just before the current position. + // Use a highly optimized version instead of checking the text node first and then getting the node before. See #6582. + const positionParent = position.parent; + const textNodeAtPosition = getTextNodeAtPosition( position, positionParent ); + const node = textNodeAtPosition ? textNodeAtPosition : getNodeBeforePosition( position, positionParent, textNodeAtPosition ); if ( node instanceof Element ) { position.offset--; diff --git a/src/model/writer.js b/src/model/writer.js index d5b26cee6..9b3cf5357 100644 --- a/src/model/writer.js +++ b/src/model/writer.js @@ -1360,14 +1360,14 @@ export default class Writer { const markerRange = marker.getRange(); let isAffected = false; - if ( type == 'move' ) { + if ( type === 'move' ) { isAffected = positionOrRange.containsPosition( markerRange.start ) || positionOrRange.start.isEqual( markerRange.start ) || positionOrRange.containsPosition( markerRange.end ) || positionOrRange.end.isEqual( markerRange.end ); } else { - // if type == 'merge'. + // if type === 'merge'. const elementBefore = positionOrRange.nodeBefore; const elementAfter = positionOrRange.nodeAfter; diff --git a/src/view/attributeelement.js b/src/view/attributeelement.js index 626e470d1..2e7a46fb4 100644 --- a/src/view/attributeelement.js +++ b/src/view/attributeelement.js @@ -157,12 +157,18 @@ export default class AttributeElement extends Element { * @returns {Boolean} */ is( type, name = null ) { - const cutType = type && type.replace( /^view:/, '' ); - if ( !name ) { - return cutType == 'attributeElement' || super.is( type ); + return type === 'attributeElement' || type === 'view:attributeElement' || + // 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 ( cutType == 'attributeElement' && name == this.name ) || super.is( type, name ); + return name === this.name && ( + type === 'attributeElement' || type === 'view:attributeElement' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'element' || type === 'view:element' + ); } } diff --git a/src/view/containerelement.js b/src/view/containerelement.js index 753b71867..a4ae85727 100644 --- a/src/view/containerelement.js +++ b/src/view/containerelement.js @@ -83,11 +83,18 @@ export default class ContainerElement extends Element { * @returns {Boolean} */ is( type, name = null ) { - const cutType = type && type.replace( /^view:/, '' ); if ( !name ) { - return cutType == 'containerElement' || super.is( type ); + return type === 'containerElement' || type === 'view:containerElement' || + // 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 ( cutType == 'containerElement' && name == this.name ) || super.is( type, name ); + return name === this.name && ( + type === 'containerElement' || type === 'view:containerElement' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'element' || type === 'view:element' + ); } } } diff --git a/src/view/documentfragment.js b/src/view/documentfragment.js index 1bda12313..7e42a1af5 100644 --- a/src/view/documentfragment.js +++ b/src/view/documentfragment.js @@ -118,7 +118,7 @@ export default class DocumentFragment { * @returns {Boolean} */ is( type ) { - return type == 'documentFragment' || type == 'view:documentFragment'; + return type === 'documentFragment' || type === 'view:documentFragment'; } /** diff --git a/src/view/documentselection.js b/src/view/documentselection.js index 0e6d5d644..2b9341737 100644 --- a/src/view/documentselection.js +++ b/src/view/documentselection.js @@ -292,7 +292,7 @@ export default class DocumentSelection { * @returns {Boolean} */ is( type ) { - return type == 'selection' || + return type === 'selection' || type == 'documentSelection' || type == 'view:selection' || type == 'view:documentSelection'; diff --git a/src/view/editableelement.js b/src/view/editableelement.js index b6ca91c6f..293b69b34 100644 --- a/src/view/editableelement.js +++ b/src/view/editableelement.js @@ -95,11 +95,20 @@ export default class EditableElement extends ContainerElement { * @returns {Boolean} */ is( type, name = null ) { - const cutType = type && type.replace( /^view:/, '' ); if ( !name ) { - return cutType == 'editableElement' || super.is( type ); + return type === 'editableElement' || type === 'view:editableElement' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'containerElement' || type === 'view:containerElement' || + type === this.name || type === 'view:' + this.name || + type === 'element' || type === 'view:element' || + type === 'node' || type === 'view:node'; } else { - return ( cutType == 'editableElement' && name == this.name ) || super.is( type, name ); + return name === this.name && ( + type === 'editableElement' || type === 'view:editableElement' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'containerElement' || type === 'view:containerElement' || + type === 'element' || type === 'view:element' + ); } } diff --git a/src/view/element.js b/src/view/element.js index 9bd3efebb..ae5f93b9b 100644 --- a/src/view/element.js +++ b/src/view/element.js @@ -176,11 +176,13 @@ export default class Element extends Node { * @returns {Boolean} */ is( type, name = null ) { - const cutType = type.replace( /^view:/, '' ); if ( !name ) { - return cutType == 'element' || cutType == this.name || super.is( type ); + return type === this.name || type === 'view:' + this.name || + type === 'element' || type === 'view:element' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'node' || type === 'view:node'; } else { - return cutType == 'element' && name == this.name; + return name === this.name && ( type === 'element' || type === 'view:element' ); } } diff --git a/src/view/emptyelement.js b/src/view/emptyelement.js index 39182de91..76b8c0b3d 100644 --- a/src/view/emptyelement.js +++ b/src/view/emptyelement.js @@ -74,11 +74,17 @@ export default class EmptyElement extends Element { * @returns {Boolean} */ is( type, name = null ) { - const cutType = type.replace( /^view:/, '' ); if ( !name ) { - return cutType == 'emptyElement' || super.is( type ); + return type === 'emptyElement' || type === 'view:emptyElement' || + // 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 ( cutType == 'emptyElement' && name == this.name ) || super.is( type, name ); + return name === this.name && ( + type === 'emptyElement' || type === 'view:emptyElement' || + type === 'element' || type === 'view:element' + ); } } diff --git a/src/view/node.js b/src/view/node.js index 719120930..6347b00e0 100644 --- a/src/view/node.js +++ b/src/view/node.js @@ -339,7 +339,7 @@ export default class Node { * @returns {Boolean} */ is( type ) { - return type == 'node' || type == 'view:node'; + return type === 'node' || type === 'view:node'; } /** diff --git a/src/view/position.js b/src/view/position.js index 782673309..b925b201d 100644 --- a/src/view/position.js +++ b/src/view/position.js @@ -222,7 +222,7 @@ export default class Position { * @returns {Boolean} */ is( type ) { - return type == 'position' || type == 'view:position'; + return type === 'position' || type === 'view:position'; } /** diff --git a/src/view/range.js b/src/view/range.js index d140a6ac6..6286cee2b 100644 --- a/src/view/range.js +++ b/src/view/range.js @@ -449,7 +449,7 @@ export default class Range { * @returns {Boolean} */ is( type ) { - return type == 'range' || type == 'view:range'; + return type === 'range' || type === 'view:range'; } /** diff --git a/src/view/rooteditableelement.js b/src/view/rooteditableelement.js index 75337cb21..577bab8d8 100644 --- a/src/view/rooteditableelement.js +++ b/src/view/rooteditableelement.js @@ -68,11 +68,22 @@ export default class RootEditableElement extends EditableElement { * @returns {Boolean} */ is( type, name = null ) { - const cutType = type.replace( /^view:/, '' ); if ( !name ) { - return cutType == 'rootElement' || super.is( type ); + return type === 'rootElement' || type === 'view:rootElement' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'editableElement' || type === 'view:editableElement' || + type === 'containerElement' || type === 'view:containerElement' || + type === this.name || type === 'view:' + this.name || + type === 'element' || type === 'view:element' || + type === 'node' || type === 'view:node'; } else { - return ( cutType == 'rootElement' && name == this.name ) || super.is( type, name ); + return name === this.name && ( + type === 'rootElement' || type === 'view:rootElement' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'editableElement' || type === 'view:editableElement' || + type === 'containerElement' || type === 'view:containerElement' || + type === 'element' || type === 'view:element' + ); } } diff --git a/src/view/selection.js b/src/view/selection.js index f9d213d62..d00c82ab3 100644 --- a/src/view/selection.js +++ b/src/view/selection.js @@ -590,7 +590,7 @@ export default class Selection { * @returns {Boolean} */ is( type ) { - return type == 'selection' || type == 'view:selection'; + return type === 'selection' || type === 'view:selection'; } /** diff --git a/src/view/text.js b/src/view/text.js index 27636d59f..3e6ef0190 100644 --- a/src/view/text.js +++ b/src/view/text.js @@ -60,7 +60,9 @@ export default class Text extends Node { * @returns {Boolean} */ is( type ) { - return type == 'text' || type == 'view:text' || super.is( type ); + return type === 'text' || type === 'view:text' || + // From super.is(). This is highly utilised method and cannot call super. See ckeditor/ckeditor5#6529. + type === 'node' || type === 'view:node'; } /** diff --git a/src/view/textproxy.js b/src/view/textproxy.js index ab829088e..cc98f3803 100644 --- a/src/view/textproxy.js +++ b/src/view/textproxy.js @@ -156,7 +156,7 @@ export default class TextProxy { * @returns {Boolean} */ is( type ) { - return type == 'textProxy' || type == 'view:textProxy'; + return type === 'textProxy' || type === 'view:textProxy'; } /** diff --git a/src/view/uielement.js b/src/view/uielement.js index c2f2a0718..3e39586d4 100644 --- a/src/view/uielement.js +++ b/src/view/uielement.js @@ -87,11 +87,17 @@ export default class UIElement extends Element { * @returns {Boolean} */ is( type, name = null ) { - const cutType = type.replace( /^view:/, '' ); if ( !name ) { - return cutType == 'uiElement' || super.is( type ); + return type === 'uiElement' || type === 'view:uiElement' || + // 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 ( cutType == 'uiElement' && name == this.name ) || super.is( type, name ); + return name === this.name && ( + type === 'uiElement' || type === 'view:uiElement' || + type === 'element' || type === 'view:element' + ); } } diff --git a/tests/model/position.js b/tests/model/position.js index 043221db5..2c6d3b6e6 100644 --- a/tests/model/position.js +++ b/tests/model/position.js @@ -8,7 +8,12 @@ import DocumentFragment from '../../src/model/documentfragment'; import Element from '../../src/model/element'; import Text from '../../src/model/text'; import TextProxy from '../../src/model/textproxy'; -import Position from '../../src/model/position'; +import { + default as Position, + getTextNodeAtPosition, + getNodeAfterPosition, + getNodeBeforePosition +} from '../../src/model/position'; import Range from '../../src/model/range'; import MarkerOperation from '../../src/model/operation/markeroperation'; import AttributeOperation from '../../src/model/operation/attributeoperation'; @@ -485,10 +490,12 @@ describe( 'Position', () => { } ); } ); - it( 'should have proper parent path', () => { - const position = new Position( root, [ 1, 2, 3 ] ); + describe( 'getParentPath()', () => { + it( 'should have proper parent path', () => { + const position = new Position( root, [ 1, 2, 3 ] ); - expect( position.getParentPath() ).to.deep.equal( [ 1, 2 ] ); + expect( position.getParentPath() ).to.deep.equal( [ 1, 2 ] ); + } ); } ); describe( 'isBefore()', () => { @@ -1275,4 +1282,39 @@ describe( 'Position', () => { expect( positionB.getCommonAncestor( positionA ) ).to.equal( lca ); } } ); + + describe( 'getTextNodeAtPosition() util', () => { + it( 'returns a text node at the given position', () => { + const position = new Position( root, [ 1, 0, 1 ] ); + const positionParent = position.parent; + + expect( getTextNodeAtPosition( position, positionParent ) ).to.equal( foz ); + } ); + + // This util is covered with tests by Position#textNode tests. + } ); + + describe( 'getNodeAfterPosition() util', () => { + it( 'returns a node after the position', () => { + const position = new Position( root, [ 1, 0 ] ); + const positionParent = position.parent; + const textNode = getTextNodeAtPosition( position, positionParent ); + + expect( getNodeAfterPosition( position, positionParent, textNode ) ).to.equal( li1 ); + } ); + + // This util is covered with tests by Position#nodeAfter tests. + } ); + + describe( 'getNodeBeforePosition() util', () => { + it( 'returns a node before the position', () => { + const position = new Position( root, [ 1, 1 ] ); + const positionParent = position.parent; + const textNode = getTextNodeAtPosition( position, positionParent ); + + expect( getNodeBeforePosition( position, positionParent, textNode ) ).to.equal( li1 ); + } ); + + // This util is covered with tests by Position#nodeBefore tests. + } ); } );