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 9, 2020
1 parent b0d2574 commit 10823d0
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 14 deletions.
22 changes: 13 additions & 9 deletions src/model/position.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +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 );

return textNode === null ? parent.getChild( parent.offsetToIndex( this.offset ) ) : null;
return getNodeAfter( this, parent, getTextNode( this, parent ) );
}

/**
Expand All @@ -240,12 +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 );

return textNode === null ? parent.getChild( parent.offsetToIndex( this.offset ) - 1 ) : null;
return getNodeBefore( this, parent, getTextNode( this, parent ) );
}

/**
Expand Down Expand Up @@ -1070,8 +1066,16 @@ 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 ) );

return ( node && node.is( 'text' ) && node.startOffset < position.offset ) ? node : null;
}

export function getNodeAfter( position, positionParent, textNode ) {
return textNode === null ? positionParent.getChild( positionParent.offsetToIndex( position.offset ) ) : null;
}

export function getNodeBefore( position, positionParent, textNode ) {
return textNode === null ? positionParent.getChild( positionParent.offsetToIndex( position.offset ) - 1 ) : null;
}
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 10823d0

Please sign in to comment.