webapp/bookstack
1
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2025-07-19 22:03:19 +03:00
Code Activity

Comments: Switched to lexical editor

Browse Source 
Required a lot of changes to provide at least a decent attempt at proper
editor teardown control.
Also updates HtmlDescriptionFilter and testing to address issue with bad
child iteration which could lead to missed items.
Renamed editor version from comments to basic as it'll also be used for
item descriptions.
This commit is contained in:
Dan Brown
2025-06-25 14:16:01 +01:00
parent c606970e38
commit b80992ca59
16 changed files with 176 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
app/Util/HtmlDescriptionFilter.php
View File

@ -4,7 +4,6 @@ namespace BookStack\Util;
use DOMAttr;
use DOMElement;
use DOMNamedNodeMap;
use DOMNode;
/**
@ -25,6 +24,7 @@ class HtmlDescriptionFilter
'ul' => [],
'li' => [],
'strong' => [],
'span' => [],
'em' => [],
'br' => [],
];
@ -59,7 +59,6 @@ class HtmlDescriptionFilter
return;
}
/** @var DOMNamedNodeMap $attrs */
$attrs = $element->attributes;
for ($i = $attrs->length - 1; $i >= 0; $i--) {
/** @var DOMAttr $attr */
@ -70,7 +69,8 @@ class HtmlDescriptionFilter
}
}
foreach ($element->childNodes as $child) {
$childNodes = [...$element->childNodes];
foreach ($childNodes as $child) {
if ($child instanceof DOMElement) {
static::filterElement($child);
}

34
resources/js/components/page-comment.ts
View File

@ -1,8 +1,9 @@
import {Component} from './component';
import {getLoading, htmlToDom} from '../services/dom';
import {buildForInput} from '../wysiwyg-tinymce/config';
import {PageCommentReference} from "./page-comment-reference";
import {HttpError} from "../services/http";
import {SimpleWysiwygEditorInterface} from "../wysiwyg";
import {el} from "../wysiwyg/utils/dom";
export interface PageCommentReplyEventData {
id: string; // ID of comment being replied to
@ -21,8 +22,7 @@ export class PageComment extends Component {
protected updatedText!: string;
protected archiveText!: string;
protected wysiwygEditor: any = null;
protected wysiwygLanguage!: string;
protected wysiwygEditor: SimpleWysiwygEditorInterface|null = null;
protected wysiwygTextDirection!: string;
protected container!: HTMLElement;
@ -44,7 +44,6 @@ export class PageComment extends Component {
this.archiveText = this.$opts.archiveText;
// Editor reference and text options
this.wysiwygLanguage = this.$opts.wysiwygLanguage;
this.wysiwygTextDirection = this.$opts.wysiwygTextDirection;
// Element references
@ -90,7 +89,7 @@ export class PageComment extends Component {
this.form.toggleAttribute('hidden', !show);
}
protected startEdit() : void {
protected async startEdit(): Promise<void> {
this.toggleEditMode(true);
if (this.wysiwygEditor) {
@ -98,21 +97,20 @@ export class PageComment extends Component {
return;
}
const config = buildForInput({
language: this.wysiwygLanguage,
containerElement: this.input,
type WysiwygModule = typeof import('../wysiwyg');
const wysiwygModule = (await window.importVersioned('wysiwyg')) as WysiwygModule;
const editorContent = this.input.value;
const container = el('div', {class: 'comment-editor-container'});
this.input.parentElement?.appendChild(container);
this.input.hidden = true;
this.wysiwygEditor = wysiwygModule.createBasicEditorInstance(container as HTMLElement, editorContent, {
darkMode: document.documentElement.classList.contains('dark-mode'),
textDirection: this.wysiwygTextDirection,
drawioUrl: '',
pageId: 0,
translations: {},
translationMap: (window as unknown as Record<string, Object>).editor_translations,
textDirection: this.$opts.textDirection,
translations: (window as unknown as Record<string, Object>).editor_translations,
});
(window as unknown as {tinymce: {init: (arg0: Object) => Promise<any>}}).tinymce.init(config).then(editors => {
this.wysiwygEditor = editors[0];
setTimeout(() => this.wysiwygEditor.focus(), 50);
});
this.wysiwygEditor.focus();
}
protected async update(event: Event): Promise<void> {
@ -121,7 +119,7 @@ export class PageComment extends Component {
this.form.toggleAttribute('hidden', true);
const reqData = {
html: this.wysiwygEditor.getContent(),
html: await this.wysiwygEditor?.getContentAsHtml() || '',
};
try {

33
resources/js/components/page-comments.ts
View File

@ -1,10 +1,11 @@
import {Component} from './component';
import {getLoading, htmlToDom} from '../services/dom';
import {buildForInput} from '../wysiwyg-tinymce/config';
import {Tabs} from "./tabs";
import {PageCommentReference} from "./page-comment-reference";
import {scrollAndHighlightElement} from "../services/util";
import {PageCommentArchiveEventData, PageCommentReplyEventData} from "./page-comment";
import {el} from "../wysiwyg/utils/dom";
import {SimpleWysiwygEditorInterface} from "../wysiwyg";
export class PageComments extends Component {
@ -28,9 +29,8 @@ export class PageComments extends Component {
private hideFormButton!: HTMLElement;
private removeReplyToButton!: HTMLElement;
private removeReferenceButton!: HTMLElement;
private wysiwygLanguage!: string;
private wysiwygTextDirection!: string;
private wysiwygEditor: any = null;
private wysiwygEditor: SimpleWysiwygEditorInterface|null = null;
private createdText!: string;
private countText!: string;
private archivedCountText!: string;
@ -63,7 +63,6 @@ export class PageComments extends Component {
this.removeReferenceButton = this.$refs.removeReferenceButton;
// WYSIWYG options
this.wysiwygLanguage = this.$opts.wysiwygLanguage;
this.wysiwygTextDirection = this.$opts.wysiwygTextDirection;
// Translations
@ -107,7 +106,7 @@ export class PageComments extends Component {
}
}
protected saveComment(event: SubmitEvent): void {
protected async saveComment(event: SubmitEvent): Promise<void> {
event.preventDefault();
event.stopPropagation();
@ -117,7 +116,7 @@ export class PageComments extends Component {
this.form.toggleAttribute('hidden', true);
const reqData = {
html: this.wysiwygEditor.getContent(),
html: (await this.wysiwygEditor?.getContentAsHtml()) || '',
parent_id: this.parentId || null,
content_ref: this.contentReference,
};
@ -189,27 +188,25 @@ export class PageComments extends Component {
this.addButtonContainer.toggleAttribute('hidden', false);
}
protected loadEditor(): void {
protected async loadEditor(): Promise<void> {
if (this.wysiwygEditor) {
this.wysiwygEditor.focus();
return;
}
const config = buildForInput({
language: this.wysiwygLanguage,
containerElement: this.formInput,
type WysiwygModule = typeof import('../wysiwyg');
const wysiwygModule = (await window.importVersioned('wysiwyg')) as WysiwygModule;
const container = el('div', {class: 'comment-editor-container'});
this.formInput.parentElement?.appendChild(container);
this.formInput.hidden = true;
this.wysiwygEditor = wysiwygModule.createBasicEditorInstance(container as HTMLElement, '', {
darkMode: document.documentElement.classList.contains('dark-mode'),
textDirection: this.wysiwygTextDirection,
drawioUrl: '',
pageId: 0,
translations: {},
translationMap: (window as unknown as Record<string, Object>).editor_translations,
translations: (window as unknown as Record<string, Object>).editor_translations,
});
(window as unknown as {tinymce: {init: (arg0: Object) => Promise<any>}}).tinymce.init(config).then(editors => {
this.wysiwygEditor = editors[0];
setTimeout(() => this.wysiwygEditor.focus(), 50);
});
this.wysiwygEditor.focus();
}
protected removeEditor(): void {

38
resources/js/wysiwyg/index.ts
View File

@ -2,9 +2,9 @@ import {createEditor, LexicalEditor} from 'lexical';
import {createEmptyHistoryState, registerHistory} from '@lexical/history';
import {registerRichText} from '@lexical/rich-text';
import {mergeRegister} from '@lexical/utils';
import {getNodesForPageEditor, registerCommonNodeMutationListeners} from './nodes';
import {getNodesForBasicEditor, getNodesForPageEditor, registerCommonNodeMutationListeners} from './nodes';
import {buildEditorUI} from "./ui";
import {getEditorContentAsHtml, setEditorContentFromHtml} from "./utils/actions";
import {focusEditor, getEditorContentAsHtml, setEditorContentFromHtml} from "./utils/actions";
import {registerTableResizer} from "./ui/framework/helpers/table-resizer";
import {EditorUiContext} from "./ui/framework/core";
import {listen as listenToCommonEvents} from "./services/common-events";
@ -15,7 +15,7 @@ import {registerShortcuts} from "./services/shortcuts";
import {registerNodeResizer} from "./ui/framework/helpers/node-resizer";
import {registerKeyboardHandling} from "./services/keyboard-handling";
import {registerAutoLinks} from "./services/auto-links";
import {contextToolbars, getMainEditorFullToolbar} from "./ui/defaults/toolbars";
import {contextToolbars, getBasicEditorToolbar, getMainEditorFullToolbar} from "./ui/defaults/toolbars";
import {modals} from "./ui/defaults/modals";
import {CodeBlockDecorator} from "./ui/decorators/code-block";
import {DiagramDecorator} from "./ui/decorators/diagram";
@ -90,20 +90,20 @@ export function createPageEditorInstance(container: HTMLElement, htmlContent: st
registerCommonNodeMutationListeners(context);
return new SimpleWysiwygEditorInterface(editor);
return new SimpleWysiwygEditorInterface(context);
}
export function createCommentEditorInstance(container: HTMLElement, htmlContent: string, options: Record<string, any> = {}): SimpleWysiwygEditorInterface {
export function createBasicEditorInstance(container: HTMLElement, htmlContent: string, options: Record<string, any> = {}): SimpleWysiwygEditorInterface {
const editor = createEditor({
namespace: 'BookStackCommentEditor',
nodes: getNodesForPageEditor(),
namespace: 'BookStackBasicEditor',
nodes: getNodesForBasicEditor(),
onError: console.error,
theme: theme,
});
const context: EditorUiContext = buildEditorUI(container, editor, options);
editor.setRootElement(context.editorDOM);
mergeRegister(
const editorTeardown = mergeRegister(
registerRichText(editor),
registerHistory(editor, createEmptyHistoryState(), 300),
registerShortcuts(context),
@ -111,23 +111,33 @@ export function createCommentEditorInstance(container: HTMLElement, htmlContent:
);
// Register toolbars, modals & decorators
context.manager.setToolbar(getMainEditorFullToolbar(context)); // TODO - Create comment toolbar
context.manager.setToolbar(getBasicEditorToolbar(context));
context.manager.registerContextToolbar('link', contextToolbars.link);
context.manager.registerModal('link', modals.link);
context.manager.onTeardown(editorTeardown);
setEditorContentFromHtml(editor, htmlContent);
return new SimpleWysiwygEditorInterface(editor);
return new SimpleWysiwygEditorInterface(context);
}
export class SimpleWysiwygEditorInterface {
protected editor: LexicalEditor;
protected context: EditorUiContext;
constructor(editor: LexicalEditor) {
this.editor = editor;
constructor(context: EditorUiContext) {
this.context = context;
}
async getContentAsHtml(): Promise<string> {
return await getEditorContentAsHtml(this.editor);
return await getEditorContentAsHtml(this.context.editor);
}
focus(): void {
focusEditor(this.context.editor);
}
remove() {
this.context.editorDOM.remove();
this.context.manager.teardown();
}
}

14
resources/js/wysiwyg/nodes.ts
View File

@ -20,9 +20,6 @@ import {HeadingNode} from "@lexical/rich-text/LexicalHeadingNode";
import {QuoteNode} from "@lexical/rich-text/LexicalQuoteNode";
import {CaptionNode} from "@lexical/table/LexicalCaptionNode";
/**
* Load the nodes for lexical.
*/
export function getNodesForPageEditor(): (KlassConstructor<typeof LexicalNode> | LexicalNodeReplacement)[] {
return [
CalloutNode,
@ -45,6 +42,15 @@ export function getNodesForPageEditor(): (KlassConstructor<typeof LexicalNode> |
];
}
export function getNodesForBasicEditor(): (KlassConstructor<typeof LexicalNode> | LexicalNodeReplacement)[] {
return [
ListNode,
ListItemNode,
ParagraphNode,
LinkNode,
];
}
export function registerCommonNodeMutationListeners(context: EditorUiContext): void {
const decorated = [ImageNode, CodeBlockNode, DiagramNode];
@ -53,7 +59,7 @@ export function registerCommonNodeMutationListeners(context: EditorUiContext): v
if (mutation === "destroyed") {
const decorator = context.manager.getDecoratorByNodeKey(nodeKey);
if (decorator) {
decorator.destroy(context);
decorator.teardown();
}
}
}

10
resources/js/wysiwyg/ui/defaults/toolbars.ts
View File

@ -221,6 +221,16 @@ export function getMainEditorFullToolbar(context: EditorUiContext): EditorContai
]);
}
export function getBasicEditorToolbar(context: EditorUiContext): EditorContainerUiElement {
return new EditorSimpleClassContainer('editor-toolbar-main', [
new EditorButton(bold),
new EditorButton(italic),
new EditorButton(link),
new EditorButton(bulletList),
new EditorButton(numberList),
]);
}
export const contextToolbars: Record<string, EditorContextToolbarDefinition> = {
image: {
selector: 'img:not([drawio-diagram] img)',

17
resources/js/wysiwyg/ui/framework/core.ts
View File

@ -30,6 +30,7 @@ export function isUiBuilderDefinition(object: any): object is EditorUiBuilderDef
export abstract class EditorUiElement {
protected dom: HTMLElement|null = null;
private context: EditorUiContext|null = null;
private abortController: AbortController = new AbortController();
protected abstract buildDOM(): HTMLElement;
@ -79,9 +80,16 @@ export abstract class EditorUiElement {
if (target) {
target.addEventListener('editor::' + name, ((event: CustomEvent) => {
callback(event.detail);
}) as EventListener);
}) as EventListener, { signal: this.abortController.signal });
}
}
teardown(): void {
if (this.dom && this.dom.isConnected) {
this.dom.remove();
}
this.abortController.abort('teardown');
}
}
export class EditorContainerUiElement extends EditorUiElement {
@ -129,6 +137,13 @@ export class EditorContainerUiElement extends EditorUiElement {
child.setContext(context);
}
}
teardown() {
for (const child of this.children) {
child.teardown();
}
super.teardown();
}
}
export class EditorSimpleClassContainer extends EditorContainerUiElement {

2
resources/js/wysiwyg/ui/framework/decorator.ts
View File

@ -48,7 +48,7 @@ export abstract class EditorDecorator {
* Destroy this decorator. Used for tear-down operations upon destruction
* of the underlying node this decorator is attached to.
*/
destroy(context: EditorUiContext): void {
teardown(): void {
for (const callback of this.onDestroyCallbacks) {
callback();
}

15
resources/js/wysiwyg/ui/framework/helpers/dropdowns.ts
View File

@ -41,11 +41,18 @@ export class DropDownManager {
constructor() {
this.onMenuMouseOver = this.onMenuMouseOver.bind(this);
this.onWindowClick = this.onWindowClick.bind(this);
window.addEventListener('click', (event: MouseEvent) => {
const target = event.target as HTMLElement;
this.closeAllNotContainingElement(target);
});
window.addEventListener('click', this.onWindowClick);
}
teardown(): void {
window.removeEventListener('click', this.onWindowClick);
}
protected onWindowClick(event: MouseEvent): void {
const target = event.target as HTMLElement;
this.closeAllNotContainingElement(target);
}
protected closeAllNotContainingElement(element: HTMLElement): void {

48
resources/js/wysiwyg/ui/framework/manager.ts
View File

@ -12,6 +12,8 @@ export type SelectionChangeHandler = (selection: BaseSelection|null) => void;
export class EditorUIManager {
public dropdowns: DropDownManager = new DropDownManager();
protected modalDefinitionsByKey: Record<string, EditorFormModalDefinition> = {};
protected activeModalsByKey: Record<string, EditorFormModal> = {};
protected decoratorConstructorsByType: Record<string, typeof EditorDecorator> = {};
@ -21,12 +23,12 @@ export class EditorUIManager {
protected contextToolbarDefinitionsByKey: Record<string, EditorContextToolbarDefinition> = {};
protected activeContextToolbars: EditorContextToolbar[] = [];
protected selectionChangeHandlers: Set<SelectionChangeHandler> = new Set();
public dropdowns: DropDownManager = new DropDownManager();
protected domEventAbortController = new AbortController();
protected teardownCallbacks: (()=>void)[] = [];
setContext(context: EditorUiContext) {
this.context = context;
this.setupEventListeners(context);
this.setupEventListeners();
this.setupEditor(context.editor);
}
@ -99,7 +101,7 @@ export class EditorUIManager {
setToolbar(toolbar: EditorContainerUiElement) {
if (this.toolbar) {
this.toolbar.getDOMElement().remove();
this.toolbar.teardown();
}
this.toolbar = toolbar;
@ -170,10 +172,40 @@ export class EditorUIManager {
return this.getContext().options.textDirection === 'rtl' ? 'rtl' : 'ltr';
}
onTeardown(callback: () => void): void {
this.teardownCallbacks.push(callback);
}
teardown(): void {
this.domEventAbortController.abort('teardown');
for (const [_, modal] of Object.entries(this.activeModalsByKey)) {
modal.teardown();
}
for (const [_, decorator] of Object.entries(this.decoratorInstancesByNodeKey)) {
decorator.teardown();
}
if (this.toolbar) {
this.toolbar.teardown();
}
for (const toolbar of this.activeContextToolbars) {
toolbar.teardown();
}
this.dropdowns.teardown();
for (const callback of this.teardownCallbacks) {
callback();
}
}
protected updateContextToolbars(update: EditorUiStateUpdate): void {
for (let i = this.activeContextToolbars.length - 1; i >= 0; i--) {
const toolbar = this.activeContextToolbars[i];
toolbar.destroy();
toolbar.teardown();
this.activeContextToolbars.splice(i, 1);
}
@ -253,9 +285,9 @@ export class EditorUIManager {
});
}
protected setupEventListeners(context: EditorUiContext) {
protected setupEventListeners() {
const layoutUpdate = this.triggerLayoutUpdate.bind(this);
window.addEventListener('scroll', layoutUpdate, {capture: true, passive: true});
window.addEventListener('resize', layoutUpdate, {passive: true});
window.addEventListener('scroll', layoutUpdate, {capture: true, passive: true, signal: this.domEventAbortController.signal});
window.addEventListener('resize', layoutUpdate, {passive: true, signal: this.domEventAbortController.signal});
}
}

2
resources/js/wysiwyg/ui/framework/modals.ts
View File

@ -34,8 +34,8 @@ export class EditorFormModal extends EditorContainerUiElement {
}
hide() {
this.getDOMElement().remove();
this.getContext().manager.setModalInactive(this.key);
this.teardown();
}
getForm(): EditorForm {

13
resources/js/wysiwyg/ui/framework/toolbars.ts
View File

@ -60,17 +60,4 @@ export class EditorContextToolbar extends EditorContainerUiElement {
const dom = this.getDOMElement();
dom.append(...children.map(child => child.getDOMElement()));
}
protected empty() {
const children = this.getChildren();
for (const child of children) {
child.getDOMElement().remove();
}
this.removeChildren(...children);
}
destroy() {
this.empty();
this.getDOMElement().remove();
}
}

2
resources/js/wysiwyg/utils/actions.ts
View File

@ -64,6 +64,6 @@ export function getEditorContentAsHtml(editor: LexicalEditor): Promise<string> {
});
}
export function focusEditor(editor: LexicalEditor) {
export function focusEditor(editor: LexicalEditor): void {
editor.focus(() => {}, {defaultSelection: "rootStart"});
}

1
resources/views/comments/comment.blade.php
View File

@ -7,7 +7,6 @@
option:page-comment:updated-text="{{ trans('entities.comment_updated_success') }}"
option:page-comment:deleted-text="{{ trans('entities.comment_deleted_success') }}"
option:page-comment:archive-text="{{ $comment->archived ? trans('entities.comment_unarchive_success') : trans('entities.comment_archive_success') }}"
option:page-comment:wysiwyg-language="{{ $locale->htmlLang() }}"
option:page-comment:wysiwyg-text-direction="{{ $locale->htmlDirection() }}"
id="comment{{$comment->local_id}}"
class="comment-box">

2
resources/views/comments/comments.blade.php
View File

@ -3,7 +3,6 @@
option:page-comments:created-text="{{ trans('entities.comment_created_success') }}"
option:page-comments:count-text="{{ trans('entities.comment_thread_count') }}"
option:page-comments:archived-count-text="{{ trans('entities.comment_archived_count') }}"
option:page-comments:wysiwyg-language="{{ $locale->htmlLang() }}"
option:page-comments:wysiwyg-text-direction="{{ $locale->htmlDirection() }}"
class="comments-list tab-container"
aria-label="{{ trans('entities.comments') }}">
@ -73,7 +72,6 @@
@if(userCan('comment-create-all') || $commentTree->canUpdateAny())
@push('body-end')
<script src="{{ versioned_asset('libs/tinymce/tinymce.min.js') }}" nonce="{{ $cspNonce }}" defer></script>
@include('form.editor-translations')
@include('entities.selector-popup')
@endpush

31
tests/Entity/CommentStoreTest.php
View File

@ -193,13 +193,14 @@ class CommentStoreTest extends TestCase
{
$page = $this->entities->page();
$script = '<script>const a = "script";</script><p onclick="1">My lovely comment</p>';
$script = '<script>const a = "script";</script><script>const b = "sneakyscript";</script><p onclick="1">My lovely comment</p>';
$this->asAdmin()->postJson("/comment/$page->id", [
'html' => $script,
]);
$pageView = $this->get($page->getUrl());
$pageView->assertDontSee($script, false);
$pageView->assertDontSee('sneakyscript', false);
$pageView->assertSee('<p>My lovely comment</p>', false);
$comment = $page->comments()->first();
@ -209,6 +210,7 @@ class CommentStoreTest extends TestCase
$pageView = $this->get($page->getUrl());
$pageView->assertDontSee($script, false);
$pageView->assertDontSee('sneakyscript', false);
$pageView->assertSee('<p>My lovely comment</p><p>updated</p>');
}
@ -216,7 +218,7 @@ class CommentStoreTest extends TestCase
{
$page = $this->entities->page();
Comment::factory()->create([
'html' => '<script>superbadscript</script><p onclick="superbadonclick">scriptincommentest</p>',
'html' => '<script>superbadscript</script><script>superbadscript</script><p onclick="superbadonclick">scriptincommentest</p>',
'entity_type' => 'page', 'entity_id' => $page
]);
@ -229,7 +231,7 @@ class CommentStoreTest extends TestCase
public function test_comment_html_is_limited()
{
$page = $this->entities->page();
$input = '<h1>Test</h1><p id="abc" href="beans">Content<a href="#cat" data-a="b">a</a><section>Hello</section></p>';
$input = '<h1>Test</h1><p id="abc" href="beans">Content<a href="#cat" data-a="b">a</a><section>Hello</section><section>there</section></p>';
$expected = '<p>Content<a href="#cat">a</a></p>';
$resp = $this->asAdmin()->post("/comment/{$page->id}", ['html' => $input]);
@ -248,4 +250,27 @@ class CommentStoreTest extends TestCase
'html' => $expected,
]);
}
public function test_comment_html_spans_are_cleaned()
{
$page = $this->entities->page();
$input = '<p><span class="beans">Hello</span> do you have <span style="white-space: discard;">biscuits</span>?</p>';
$expected = '<p><span>Hello</span> do you have <span>biscuits</span>?</p>';
$resp = $this->asAdmin()->post("/comment/{$page->id}", ['html' => $input]);
$resp->assertOk();
$this->assertDatabaseHas('comments', [
'entity_type' => 'page',
'entity_id' => $page->id,
'html' => $expected,
]);
$comment = $page->comments()->first();
$resp = $this->put("/comment/{$comment->id}", ['html' => $input]);
$resp->assertOk();
$this->assertDatabaseHas('comments', [
'id' => $comment->id,
'html' => $expected,
]);
}
}