Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge branch 'master' into i/6501
Browse files Browse the repository at this point in the history
  • Loading branch information
Reinmar committed Apr 14, 2020
2 parents e4fea16 + 08e8294 commit c0364a2
Show file tree
Hide file tree
Showing 32 changed files with 291 additions and 86 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ language: node_js
services:
- xvfb
node_js:
- '8'
- '10'
cache:
yarn: true
branches:
Expand Down
2 changes: 1 addition & 1 deletion src/model/documentfragment.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export default class DocumentFragment {
* @returns {Boolean}
*/
is( type ) {
return type == 'documentFragment' || type == 'model:documentFragment';
return type === 'documentFragment' || type === 'model:documentFragment';
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/model/documentselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
11 changes: 6 additions & 5 deletions src/model/element.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/model/liveposition.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/model/liverange.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/model/markercollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ class Marker {
* @returns {Boolean}
*/
is( type ) {
return type == 'marker' || type == 'model:marker';
return type === 'marker' || type === 'model:marker';
}

/**
Expand Down
11 changes: 1 addition & 10 deletions src/model/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -432,7 +431,7 @@ export default class Node {
* @returns {Boolean}
*/
is( type ) {
return type == 'node' || type == 'model:node';
return type === 'node' || type === 'model:node';
}

/**
Expand Down Expand Up @@ -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
*/
130 changes: 113 additions & 17 deletions src/model/position.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -141,7 +143,7 @@ export default class Position {
* @type {Number}
*/
get offset() {
return last( this.path );
return this.path[ this.path.length - 1 ];
}

/**
Expand Down Expand Up @@ -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 );
}

/**
Expand All @@ -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 ) );
}

/**
Expand Down Expand Up @@ -339,10 +345,12 @@ export default class Position {
* @returns {Array.<module:engine/model/item~Item>} 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 } );
}
}

Expand Down Expand Up @@ -540,7 +548,7 @@ export default class Position {
* @returns {Boolean}
*/
is( type ) {
return type == 'position' || type == 'model:position';
return type === 'position' || type === 'model:position';
}

/**
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 );
}
2 changes: 1 addition & 1 deletion src/model/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ export default class Range {
* @returns {Boolean}
*/
is( type ) {
return type == 'range' || type == 'model:range';
return type === 'range' || type === 'model:range';
}

/**
Expand Down
16 changes: 12 additions & 4 deletions src/model/rootelement.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
}

/**
Expand All @@ -105,3 +112,4 @@ export default class RootElement extends Element {
// @if CK_DEBUG_ENGINE // console.log( 'ModelRootElement: ' + this );
// @if CK_DEBUG_ENGINE // }
}

2 changes: 1 addition & 1 deletion src/model/selection.js
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ export default class Selection {
* @returns {Boolean}
*/
is( type ) {
return type == 'selection' || type == 'model:selection';
return type === 'selection' || type === 'model:selection';
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/model/text.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/model/textproxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export default class TextProxy {
* @returns {Boolean}
*/
is( type ) {
return type == 'textProxy' || type == 'model:textProxy';
return type === 'textProxy' || type === 'model:textProxy';
}

/**
Expand Down
Loading

0 comments on commit c0364a2

Please sign in to comment.