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

Commit

Permalink
Expose more granular utils so we can optimize code outside the positi…
Browse files Browse the repository at this point in the history
…on module.
  • Loading branch information
Reinmar committed Apr 10, 2020
1 parent c25ad5f commit bd3924a
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 22 deletions.
38 changes: 21 additions & 17 deletions src/model/position.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,16 +225,10 @@ export default class Position {
* @type {module:engine/model/node~Node|null}
*/
get nodeAfter() {
// Cache parent and reuse for performance reasons. This also means that we cannot use the #index property.
// See #6579.
// Cache the parent and reuse for performance reasons. See #6579 and #6582.
const parent = this.parent;
const textNode = getTextNode( this, parent );

if ( textNode !== null ) {
return null;
}

return parent.getChild( parent.offsetToIndex( this.offset ) );
return getNodeAfter( this, parent, getTextNode( this, parent ) );
}

/**
Expand All @@ -244,16 +238,10 @@ export default class Position {
* @type {module:engine/model/node~Node|null}
*/
get nodeBefore() {
// Cache parent and reuse for performance reasons. This also means that we cannot use the #index property.
// See #6579.
// Cache the parent and reuse for performance reasons. See #6579 and #6582.
const parent = this.parent;
const textNode = getTextNode( this, parent );

if ( textNode !== null ) {
return null;
}

return parent.getChild( parent.offsetToIndex( this.offset ) - 1 );
return getNodeBefore( this, parent, getTextNode( this, parent ) );
}

/**
Expand Down Expand Up @@ -1078,7 +1066,7 @@ export default class Position {
// Helper function used to inline text node access by using a cached parent.
// Reduces the access to the Position#parent property 3 times (in total, when taken into account what #nodeAfter and #nodeBefore do).
// See #6579.
function getTextNode( position, positionParent ) {
export function getTextNode( position, positionParent ) {
const node = positionParent.getChild( positionParent.offsetToIndex( position.offset ) );

if ( node && node.is( 'text' ) && node.startOffset < position.offset ) {
Expand All @@ -1087,3 +1075,19 @@ function getTextNode( position, positionParent ) {

return null;
}

export function getNodeAfter( position, positionParent, textNode ) {
if ( textNode !== null ) {
return null;
}

return positionParent.getChild( positionParent.offsetToIndex( position.offset ) );
}

export function getNodeBefore( position, positionParent, textNode ) {
if ( textNode !== null ) {
return null;
}

return positionParent.getChild( positionParent.offsetToIndex( position.offset ) - 1 );
}
19 changes: 14 additions & 5 deletions src/model/treewalker.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@
import Text from './text';
import TextProxy from './textproxy';
import Element from './element';
import Position from './position';
import {
default as Position,
getTextNode as getTextNodeAtPosition,
getNodeAfter as getNodeAfterPosition,
getNodeBefore as getNodeBeforePosition
} from './position';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';

/**
Expand Down Expand Up @@ -226,8 +231,10 @@ export default class TreeWalker {
}

// Get node just after the current position.
const textNodeAtPosition = position.textNode;
const node = textNodeAtPosition ? textNodeAtPosition : position.nodeAfter;
// 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 ) {
Expand Down Expand Up @@ -302,8 +309,10 @@ export default class TreeWalker {
}

// Get node just before the current position.
const textNodeAtPosition = position.textNode;
const node = textNodeAtPosition ? textNodeAtPosition : position.nodeBefore;
// 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--;
Expand Down

0 comments on commit bd3924a

Please sign in to comment.