Skip to content

Commit

Permalink
Merge pull request #14029 from ckeditor/ck/13596-multi-root-crashing
Browse files Browse the repository at this point in the history 
Fix: Added multi-root support for: title, HTML comments, table resize, word count and block toolbar.
  • Loading branch information
@scofalik
scofalik committed May 24, 2023
2 parents d65b6df + 9841c05 commit 95ef94b
Show file tree
Hide file tree
Showing 12 changed files with 349 additions and 130 deletions.
1 change: 1 addition & 0 deletions packages/ckeditor5-heading/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"@ckeditor/ckeditor5-clipboard": "^38.0.1",
"@ckeditor/ckeditor5-dev-utils": "^37.0.0",
"@ckeditor/ckeditor5-editor-classic": "^38.0.1",
"@ckeditor/ckeditor5-editor-multi-root": "^38.0.1",
"@ckeditor/ckeditor5-engine": "^38.0.1",
"@ckeditor/ckeditor5-enter": "^38.0.1",
"@ckeditor/ckeditor5-image": "^38.0.1",
Expand Down
210 changes: 125 additions & 85 deletions packages/ckeditor5-heading/src/title.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default class Title extends Plugin {
* A reference to an empty paragraph in the body
* created when there is no element in the body for the placeholder purposes.
*/
private _bodyPlaceholder?: null | Element;
private _bodyPlaceholder = new Map<string, Element>();

/**
* @inheritDoc
Expand All @@ -66,8 +66,6 @@ export default class Title extends Plugin {
const editor = this.editor;
const model = editor.model;

this._bodyPlaceholder = null;

// To use the schema for disabling some features when the selection is inside the title element
// it is needed to create the following structure:
//
Expand Down Expand Up @@ -139,7 +137,8 @@ export default class Title extends Plugin {
* @returns The title of the document.
*/
public getTitle( options: Record<string, unknown> = {} ): string {
const titleElement = this._getTitleElement();
const rootName = options.rootName ? options.rootName as string : undefined;
const titleElement = this._getTitleElement( rootName );
const titleContentElement = titleElement!.getChild( 0 ) as Element;

return this.editor.data.stringify( titleContentElement, options );
Expand All @@ -160,7 +159,8 @@ export default class Title extends Plugin {
const editor = this.editor;
const data = editor.data;
const model = editor.model;
const root = editor.model.document.getRoot()!;
const rootName = options.rootName ? options.rootName as string : undefined;
const root = editor.model.document.getRoot( rootName )!;
const view = editor.editing.view;
const viewWriter = new DowncastWriter( view.document );

Expand Down Expand Up @@ -196,8 +196,8 @@ export default class Title extends Plugin {
/**
* Returns the `title` element when it is in the document. Returns `undefined` otherwise.
*/
private _getTitleElement(): Element | undefined {
const root = this.editor.model.document.getRoot()!;
private _getTitleElement( rootName?: string ): Element | undefined {
const root = this.editor.model.document.getRoot( rootName )!;

for ( const child of root.getChildren() as IterableIterator<Element> ) {
if ( isTitle( child ) ) {
Expand All @@ -211,103 +211,131 @@ export default class Title extends Plugin {
* All additional children should be moved after the `title` element and renamed to a paragraph.
*/
private _fixTitleContent( writer: Writer ) {
const title = this._getTitleElement();
let changed = false;

// There's no title in the content - it will be created by _fixTitleElement post-fixer.
if ( !title || title.maxOffset === 1 ) {
return false;
}
for ( const rootName of this.editor.model.document.getRootNames() ) {
const title = this._getTitleElement( rootName );

// If there is no title in the content it will be created by `_fixTitleElement` post-fixer.
// If the title has just one element, then it is correct. No fixing.
if ( !title || title.maxOffset === 1 ) {
continue;
}

const titleChildren = Array.from( title.getChildren() ) as Array<Element>;

const titleChildren = Array.from( title.getChildren() as IterableIterator<Element> );
// Skip first child because it is an allowed element.
titleChildren.shift();

// Skip first child because it is an allowed element.
titleChildren.shift();
for ( const titleChild of titleChildren ) {
writer.move( writer.createRangeOn( titleChild ), title, 'after' );
writer.rename( titleChild, 'paragraph' );
}

for ( const titleChild of titleChildren ) {
writer.move( writer.createRangeOn( titleChild ), title, 'after' );
writer.rename( titleChild, 'paragraph' );
changed = true;
}

return true;
return changed;
}

/**
* Model post-fixer callback that creates a title element when it is missing,
* takes care of the correct position of it and removes additional title elements.
*/
private _fixTitleElement( writer: Writer ) {
let changed = false;
const model = this.editor.model;
const modelRoot = model.document.getRoot()!;

const titleElements = Array.from( modelRoot.getChildren() as IterableIterator<Element> ).filter( isTitle );
const firstTitleElement = titleElements[ 0 ];
const firstRootChild = modelRoot.getChild( 0 ) as Element;
for ( const rootName of this.editor.model.document.getRootNames() ) {
const modelRoot = model.document.getRoot( rootName )!;

// When title element is at the beginning of the document then try to fix additional
// title elements (if there are any) and stop post-fixer as soon as possible.
if ( firstRootChild.is( 'element', 'title' ) ) {
return fixAdditionalTitleElements( titleElements, writer, model );
}
const titleElements = Array.from( modelRoot.getChildren() as IterableIterator<Element> ).filter( isTitle );
const firstTitleElement = titleElements[ 0 ];
const firstRootChild = modelRoot.getChild( 0 ) as Element;

// When there is no title in the document and first element in the document cannot be changed
// to the title then create an empty title element at the beginning of the document.
if ( !firstTitleElement && !titleLikeElements.has( firstRootChild.name ) ) {
const title = writer.createElement( 'title' );
// When title element is at the beginning of the document then try to fix additional title elements (if there are any).
if ( firstRootChild.is( 'element', 'title' ) ) {
if ( titleElements.length > 1 ) {
fixAdditionalTitleElements( titleElements, writer, model );

writer.insert( title, modelRoot );
writer.insertElement( 'title-content', title );
changed = true;
}

return true;
}
continue;
}

// At this stage, we are sure the title is somewhere in the content. It has to be fixed.
// When there is no title in the document and first element in the document cannot be changed
// to the title then create an empty title element at the beginning of the document.
if ( !firstTitleElement && !titleLikeElements.has( firstRootChild.name ) ) {
const title = writer.createElement( 'title' );

// Change the first element in the document to the title if it can be changed (is title-like).
if ( titleLikeElements.has( firstRootChild.name ) ) {
changeElementToTitle( firstRootChild, writer, model );
// Otherwise, move the first occurrence of the title element to the beginning of the document.
} else {
writer.move( writer.createRangeOn( firstTitleElement ), modelRoot, 0 );
}
writer.insert( title, modelRoot );
writer.insertElement( 'title-content', title );

changed = true;

fixAdditionalTitleElements( titleElements, writer, model );
continue;
}

if ( titleLikeElements.has( firstRootChild.name ) ) {
// Change the first element in the document to the title if it can be changed (is title-like).
changeElementToTitle( firstRootChild, writer, model );
} else {
// Otherwise, move the first occurrence of the title element to the beginning of the document.
writer.move( writer.createRangeOn( firstTitleElement ), modelRoot, 0 );
}

fixAdditionalTitleElements( titleElements, writer, model );

changed = true;
}

return true;
return changed;
}

/**
* Model post-fixer callback that adds an empty paragraph at the end of the document
* when it is needed for the placeholder purposes.
*/
private _fixBodyElement( writer: Writer ) {
const modelRoot = this.editor.model.document.getRoot()!;
let changed = false;

if ( modelRoot.childCount < 2 ) {
this._bodyPlaceholder = writer.createElement( 'paragraph' );
writer.insert( this._bodyPlaceholder, modelRoot, 1 );
for ( const rootName of this.editor.model.document.getRootNames() ) {
const modelRoot = this.editor.model.document.getRoot( rootName )!;

return true;
if ( modelRoot.childCount < 2 ) {
const placeholder = writer.createElement( 'paragraph' );

writer.insert( placeholder, modelRoot, 1 );
this._bodyPlaceholder.set( rootName, placeholder );

changed = true;
}
}

return false;
return changed;
}

/**
* Model post-fixer callback that removes a paragraph from the end of the document
* if it was created for the placeholder purposes and is not needed anymore.
*/
private _fixExtraParagraph( writer: Writer ) {
const root = this.editor.model.document.getRoot()!;
const placeholder = this._bodyPlaceholder!;
let changed = false;

for ( const rootName of this.editor.model.document.getRootNames() ) {
const root = this.editor.model.document.getRoot( rootName )!;
const placeholder = this._bodyPlaceholder.get( rootName )!;

if ( shouldRemoveLastParagraph( placeholder, root ) ) {
this._bodyPlaceholder = null;
writer.remove( placeholder );
if ( shouldRemoveLastParagraph( placeholder, root ) ) {
this._bodyPlaceholder.delete( rootName );
writer.remove( placeholder );

return true;
changed = true;
}
}

return false;
return changed;
}

/**
Expand All @@ -317,7 +345,6 @@ export default class Title extends Plugin {
const editor: Editor & Partial<ElementApi> = this.editor;
const t = editor.t;
const view = editor.editing.view;
const viewRoot = view.document.getRoot();
const sourceElement = editor.sourceElement;

const titlePlaceholder = editor.config.get( 'title.placeholder' ) || t( 'Type your title' );
Expand All @@ -336,35 +363,44 @@ export default class Title extends Plugin {
} );

// Attach placeholder to first element after a title element and remove it if it's not needed anymore.
// First element after title can change so we need to observe all changes keep placeholder in sync.
let oldBody: ViewElement;
// First element after title can change, so we need to observe all changes keep placeholder in sync.
const bodyViewElements = new Map<string, ViewElement>();

// This post-fixer runs after the model post-fixer so we can assume that
// the second child in view root will always exist.
// This post-fixer runs after the model post-fixer, so we can assume that the second child in view root will always exist.
view.document.registerPostFixer( writer => {
const body = viewRoot!.getChild( 1 ) as ViewElement;
let hasChanged = false;

// If body element has changed we need to disable placeholder on the previous element
// and enable on the new one.
if ( body !== oldBody ) {
if ( oldBody ) {
hidePlaceholder( writer, oldBody );
writer.removeAttribute( 'data-placeholder', oldBody );
for ( const viewRoot of view.document.roots ) {
// `viewRoot` can be empty despite the model post-fixers if the model root was detached.
if ( viewRoot.isEmpty ) {
continue;
}

writer.setAttribute( 'data-placeholder', bodyPlaceholder, body );
oldBody = body;
hasChanged = true;
}
// If `viewRoot` is not empty, then we can expect at least two elements in it.
const body = viewRoot!.getChild( 1 ) as ViewElement;
const oldBody = bodyViewElements.get( viewRoot.rootName );

// Then we need to display placeholder if it is needed.
// See: https://github.com/ckeditor/ckeditor5/issues/8689.
if ( needsPlaceholder( body, true ) && viewRoot!.childCount === 2 && body!.name === 'p' ) {
hasChanged = showPlaceholder( writer, body ) ? true : hasChanged;
// Or hide if it is not needed.
} else {
hasChanged = hidePlaceholder( writer, body ) ? true : hasChanged;
// If body element has changed we need to disable placeholder on the previous element and enable on the new one.
if ( body !== oldBody ) {
if ( oldBody ) {
hidePlaceholder( writer, oldBody );
writer.removeAttribute( 'data-placeholder', oldBody );
}

writer.setAttribute( 'data-placeholder', bodyPlaceholder, body );
bodyViewElements.set( viewRoot.rootName, body );

hasChanged = true;
}

// Then we need to display placeholder if it is needed.
// See: https://github.com/ckeditor/ckeditor5/issues/8689.
if ( needsPlaceholder( body, true ) && viewRoot!.childCount === 2 && body!.name === 'p' ) {
hasChanged = showPlaceholder( writer, body ) ? true : hasChanged;
} else {
// Or hide if it is not needed.
hasChanged = hidePlaceholder( writer, body ) ? true : hasChanged;
}
}

return hasChanged;
Expand All @@ -385,8 +421,11 @@ export default class Title extends Plugin {
const selectedElements = Array.from( selection.getSelectedBlocks() );

if ( selectedElements.length === 1 && selectedElements[ 0 ].is( 'element', 'title-content' ) ) {
const firstBodyElement = model.document.getRoot()!.getChild( 1 );
const root = selection.getFirstPosition()!.root;
const firstBodyElement = root.getChild( 1 );

writer.setSelection( firstBodyElement!, 0 );

cancel();
}
} );
Expand All @@ -401,15 +440,16 @@ export default class Title extends Plugin {
return;
}

const root = editor.model.document.getRoot()!;
const selectedElement = first( selection.getSelectedBlocks() );
const selectionPosition = selection.getFirstPosition()!;
const root = editor.model.document.getRoot( selectionPosition.root.rootName! )!;

const title = root.getChild( 0 ) as Element;
const body = root.getChild( 1 );

if ( selectedElement === body && selectionPosition.isAtStart ) {
writer.setSelection( title.getChild( 0 )!, 0 );

cancel();
}
} );
Expand Down Expand Up @@ -503,6 +543,7 @@ function fixAdditionalTitleElements( titleElements: Array<Element>, writer: Writ
for ( const title of titleElements ) {
if ( title.index !== 0 ) {
fixTitleElement( title, writer, model );

hasChanged = true;
}
}
Expand Down Expand Up @@ -573,4 +614,3 @@ export interface TitleConfig {
*/
placeholder?: string;
}

0 comments on commit 95ef94b

Please sign in to comment.