Skip to content

Commit

Permalink
Merge pull request #14359 from ckeditor/ck/14346-fix-styles-when-spli…
Browse files Browse the repository at this point in the history 
…tting-lists-in-ghs

Fix (html-support): Preserve HTML attributes when list is split or its item is removed. Closes #14346 and #14349.

Fix (engine): Fix shallow view and model TreeWalkers returning elements outside of specified boundaries.
  • Loading branch information
@niegowski
niegowski committed Jun 19, 2023
2 parents a5908d4 + cd1d3f8 commit c519feb
Show file tree
Hide file tree
Showing 5 changed files with 349 additions and 105 deletions.
75 changes: 42 additions & 33 deletions packages/ckeditor5-engine/src/model/treewalker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,21 +216,28 @@ export default class TreeWalker implements Iterable<TreeWalkerValue> {
// 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 textNodeAtPosition = getTextNodeAtPosition( position, parent );
const node = textNodeAtPosition ? textNodeAtPosition : getNodeAfterPosition( position, parent, textNodeAtPosition );
const node = textNodeAtPosition || getNodeAfterPosition( position, parent, textNodeAtPosition );

if ( node instanceof Element ) {
if ( !this.shallow ) {
// Manual operations on path internals for optimization purposes. Here and in the rest of the method.
( position.path as Array<number> ).push( 0 );
this._visitedParent = node;
} else {
// We are past the walker boundaries.
if ( this.boundaries && this.boundaries.end.isBefore( position ) ) {
return { done: true, value: undefined };
}

position.offset++;
}

this._position = position;

return formatReturnValue( 'elementStart', node, previousPosition, position, 1 );
} else if ( node instanceof Text ) {
}

if ( node instanceof Text ) {
let charactersCount;

if ( this.singleCharacters ) {
Expand All @@ -252,19 +259,19 @@ export default class TreeWalker implements Iterable<TreeWalkerValue> {
this._position = position;

return formatReturnValue( 'text', item, previousPosition, position, charactersCount );
} else {
// `node` is not set, we reached the end of current `parent`.
( position.path as Array<number> ).pop();
position.offset++;
this._position = position;
this._visitedParent = parent.parent!;
}

if ( this.ignoreElementEnd ) {
return this._next();
} else {
return formatReturnValue( 'elementEnd', parent as Element, previousPosition, position );
}
// `node` is not set, we reached the end of current `parent`.
( position.path as Array<number> ).pop();
position.offset++;
this._position = position;
this._visitedParent = parent.parent!;

if ( this.ignoreElementEnd ) {
return this._next();
}

return formatReturnValue( 'elementEnd', parent as Element, previousPosition, position );
}

/**
Expand All @@ -289,27 +296,29 @@ export default class TreeWalker implements Iterable<TreeWalkerValue> {
// 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 );
const node = textNodeAtPosition || getNodeBeforePosition( position, positionParent, textNodeAtPosition );

if ( node instanceof Element ) {
position.offset--;

if ( !this.shallow ) {
( position.path as Array<number> ).push( node.maxOffset );
this._position = position;
this._visitedParent = node;

if ( this.ignoreElementEnd ) {
return this._previous();
} else {
return formatReturnValue( 'elementEnd', node, previousPosition, position );
}
} else {
if ( this.shallow ) {
this._position = position;

return formatReturnValue( 'elementStart', node, previousPosition, position, 1 );
}
} else if ( node instanceof Text ) {

( position.path as Array<number> ).push( node.maxOffset );
this._position = position;
this._visitedParent = node;

if ( this.ignoreElementEnd ) {
return this._previous();
}

return formatReturnValue( 'elementEnd', node, previousPosition, position );
}

if ( node instanceof Text ) {
let charactersCount;

if ( this.singleCharacters ) {
Expand All @@ -331,14 +340,14 @@ export default class TreeWalker implements Iterable<TreeWalkerValue> {
this._position = position;

return formatReturnValue( 'text', item, previousPosition, position, charactersCount );
} else {
// `node` is not set, we reached the beginning of current `parent`.
( position.path as Array<number> ).pop();
this._position = position;
this._visitedParent = parent.parent!;

return formatReturnValue( 'elementStart', parent as Element, previousPosition, position, 1 );
}

// `node` is not set, we reached the beginning of current `parent`.
( position.path as Array<number> ).pop();
this._position = position;
this._visitedParent = parent.parent!;

return formatReturnValue( 'elementStart', parent as Element, previousPosition, position, 1 );
}
}

Expand Down
137 changes: 75 additions & 62 deletions packages/ckeditor5-engine/src/view/treewalker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,38 +212,47 @@ export default class TreeWalker implements IterableIterator<TreeWalkerValue> {
if ( !this.shallow ) {
position = new Position( node, 0 );
} else {
// We are past the walker boundaries.
if ( this.boundaries && this.boundaries.end.isBefore( position ) ) {
return { done: true, value: undefined };
}

position.offset++;
}

this._position = position;

return this._formatReturnValue( 'elementStart', node, previousPosition, position, 1 );
} else if ( node instanceof Text ) {
}

if ( node instanceof Text ) {
if ( this.singleCharacters ) {
position = new Position( node, 0 );
this._position = position;

return this._next();
} else {
let charactersCount = node.data.length;
let item;

// If text stick out of walker range, we need to cut it and wrap in TextProxy.
if ( node == this._boundaryEndParent ) {
charactersCount = this.boundaries!.end.offset;
item = new TextProxy( node, 0, charactersCount );
position = Position._createAfter( item );
} else {
item = new TextProxy( node, 0, node.data.length );
// If not just keep moving forward.
position.offset++;
}
}

this._position = position;
let charactersCount = node.data.length;
let item;

return this._formatReturnValue( 'text', item, previousPosition, position, charactersCount );
// If text stick out of walker range, we need to cut it and wrap in TextProxy.
if ( node == this._boundaryEndParent ) {
charactersCount = this.boundaries!.end.offset;
item = new TextProxy( node, 0, charactersCount );
position = Position._createAfter( item );
} else {
item = new TextProxy( node, 0, node.data.length );
// If not just keep moving forward.
position.offset++;
}
} else if ( typeof node == 'string' ) {

this._position = position;

return this._formatReturnValue( 'text', item, previousPosition, position, charactersCount );
}

if ( typeof node == 'string' ) {
let textLength;

if ( this.singleCharacters ) {
Expand All @@ -261,17 +270,17 @@ export default class TreeWalker implements IterableIterator<TreeWalkerValue> {
this._position = position;

return this._formatReturnValue( 'text', textProxy, previousPosition, position, textLength );
} else {
// `node` is not set, we reached the end of current `parent`.
position = Position._createAfter( parent as any );
this._position = position;
}

if ( this.ignoreElementEnd ) {
return this._next();
} else {
return this._formatReturnValue( 'elementEnd', parent as any, previousPosition, position );
}
// `node` is not set, we reached the end of current `parent`.
position = Position._createAfter( parent as any );
this._position = position;

if ( this.ignoreElementEnd ) {
return this._next();
}

return this._formatReturnValue( 'elementEnd', parent as any, previousPosition, position );
}

/**
Expand Down Expand Up @@ -310,49 +319,53 @@ export default class TreeWalker implements IterableIterator<TreeWalkerValue> {
}

if ( node instanceof Element ) {
if ( !this.shallow ) {
position = new Position( node, node.childCount );
this._position = position;

if ( this.ignoreElementEnd ) {
return this._previous();
} else {
return this._formatReturnValue( 'elementEnd', node, previousPosition, position );
}
} else {
if ( this.shallow ) {
position.offset--;
this._position = position;

return this._formatReturnValue( 'elementStart', node, previousPosition, position, 1 );
}
} else if ( node instanceof Text ) {

position = new Position( node, node.childCount );
this._position = position;

if ( this.ignoreElementEnd ) {
return this._previous();
}

return this._formatReturnValue( 'elementEnd', node, previousPosition, position );
}

if ( node instanceof Text ) {
if ( this.singleCharacters ) {
position = new Position( node, node.data.length );
this._position = position;

return this._previous();
} else {
let charactersCount = node.data.length;
let item;
}

// If text stick out of walker range, we need to cut it and wrap in TextProxy.
if ( node == this._boundaryStartParent ) {
const offset = this.boundaries!.start.offset;
let charactersCount = node.data.length;
let item;

item = new TextProxy( node, offset, node.data.length - offset );
charactersCount = item.data.length;
position = Position._createBefore( item );
} else {
item = new TextProxy( node, 0, node.data.length );
// If not just keep moving backward.
position.offset--;
}
// If text stick out of walker range, we need to cut it and wrap in TextProxy.
if ( node == this._boundaryStartParent ) {
const offset = this.boundaries!.start.offset;

this._position = position;

return this._formatReturnValue( 'text', item, previousPosition, position, charactersCount );
item = new TextProxy( node, offset, node.data.length - offset );
charactersCount = item.data.length;
position = Position._createBefore( item );
} else {
item = new TextProxy( node, 0, node.data.length );
// If not just keep moving backward.
position.offset--;
}
} else if ( typeof node == 'string' ) {

this._position = position;

return this._formatReturnValue( 'text', item, previousPosition, position, charactersCount );
}

if ( typeof node == 'string' ) {
let textLength;

if ( !this.singleCharacters ) {
Expand All @@ -371,13 +384,13 @@ export default class TreeWalker implements IterableIterator<TreeWalkerValue> {
this._position = position;

return this._formatReturnValue( 'text', textProxy, previousPosition, position, textLength );
} else {
// `node` is not set, we reached the beginning of current `parent`.
position = Position._createBefore( parent as any );
this._position = position;

return this._formatReturnValue( 'elementStart', parent as Element, previousPosition, position, 1 );
}

// `node` is not set, we reached the beginning of current `parent`.
position = Position._createBefore( parent as any );
this._position = position;

return this._formatReturnValue( 'elementStart', parent as Element, previousPosition, position, 1 );
}

/**
Expand Down

0 comments on commit c519feb

Please sign in to comment.