Fix infinite update loop in editor with invalid input HTML (#1761)

* Use async function in PlainTextInput

* Clean up PlainTextInput

* Refactor logic from {Rich,Plain}TextInput into own files

* Remove prohibited tags on content.subscribe which also parses the html

ts/editor/DecoratedElements.svelte

@@ -4,18 +4,10 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
 <script context="module" lang="ts">
     import { CustomElementArray } from "../editable/decorated";
-    import contextProperty from "../sveltelib/context-property";
 
     const decoratedElements = new CustomElementArray();
 
-    const key = Symbol("decoratedElements");
-    const [context, setContextProperty] = contextProperty<CustomElementArray>(key);
-
-    export { context };
-</script>
-
-<script lang="ts">
-    setContextProperty(decoratedElements);
+    export { decoratedElements };
 </script>
 
 <slot />

ts/editor/MathjaxElement.svelte

@@ -2,11 +2,10 @@
 Copyright: Ankitects Pty Ltd and contributors
 License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
-<script lang="ts">
+<script lang="ts" context="module">
     import { Mathjax } from "../editable/mathjax-element";
-    import { context } from "./DecoratedElements.svelte";
+    import { decoratedElements } from "./DecoratedElements.svelte";
 
-    const decoratedElements = context.get();
     decoratedElements.push(Mathjax);
 
     import { parsingInstructions } from "./plain-text-input";

ts/editor/editor-toolbar/TemplateButtons.svelte

@@ -41,7 +41,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         }
 
         [mediaPromise, resolve] = promiseWithResolver<string>();
-        $focusedInput.focusHandler.focus.on(
+        $focusedInput.editable.focusHandler.focus.on(
             async () => setFormat("inserthtml", await mediaPromise),
             { once: true },
         );
@@ -61,7 +61,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         }
 
         [mediaPromise, resolve] = promiseWithResolver<string>();
-        $focusedInput.focusHandler.focus.on(
+        $focusedInput.editable.focusHandler.focus.on(
             async () => setFormat("inserthtml", await mediaPromise),
             { once: true },
         );

ts/editor/mathjax-overlay/MathjaxHandle.svelte

@@ -17,7 +17,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     import MathjaxMenu from "./MathjaxMenu.svelte";
 
     const { container, api } = context.get();
-    const { focusHandler, preventResubscription } = api;
+    const { editable, preventResubscription } = api;
 
     const code = writable("");
 
@@ -41,7 +41,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     let selectAll = false;
 
     function placeHandle(after: boolean): void {
-        focusHandler.flushCaret();
+        editable.focusHandler.flushCaret();
 
         if (after) {
             (mathjaxElement as any).placeCaretAfter();

ts/editor/plain-text-input/PlainTextInput.svelte

@@ -30,15 +30,14 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     import { onMount, tick } from "svelte";
     import { writable } from "svelte/store";
 
+    import { singleCallback } from "../../lib/typing";
     import { pageTheme } from "../../sveltelib/theme";
     import { baseOptions, gutterOptions, htmlanki } from "../code-mirror";
     import CodeMirror from "../CodeMirror.svelte";
-    import { context as decoratedElementsContext } from "../DecoratedElements.svelte";
     import { context as editingAreaContext } from "../EditingArea.svelte";
     import { context as noteEditorContext } from "../NoteEditor.svelte";
     import removeProhibitedTags from "./remove-prohibited";
-
-    export let hidden = false;
+    import { storedToUndecorated, undecoratedToStored } from "./transform";
 
     const configuration = {
         mode: htmlanki,
@@ -49,31 +48,36 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     const { focusedInput } = noteEditorContext.get();
 
     const { editingInputs, content } = editingAreaContext.get();
-    const decoratedElements = decoratedElementsContext.get();
     const code = writable($content);
 
-    function focus(): void {
-        codeMirror.editor.then((editor) => editor.focus());
+    let codeMirror = {} as CodeMirrorAPI;
+
+    async function focus(): Promise<void> {
+        const editor = await codeMirror.editor;
+        editor.focus();
     }
 
-    function moveCaretToEnd(): void {
-        codeMirror.editor.then((editor) => editor.setCursor(editor.lineCount(), 0));
+    async function moveCaretToEnd(): Promise<void> {
+        const editor = await codeMirror.editor;
+        editor.setCursor(editor.lineCount(), 0);
     }
 
-    function refocus(): void {
-        codeMirror.editor.then((editor) => (editor as any).display.input.blur());
+    async function refocus(): Promise<void> {
+        const editor = (await codeMirror.editor) as any;
+        editor.display.input.blur();
+
         focus();
         moveCaretToEnd();
     }
 
+    export let hidden = false;
+
     function toggle(): boolean {
         hidden = !hidden;
         return hidden;
     }
 
-    let codeMirror = {} as CodeMirrorAPI;
-
-    export const api = {
+    export const api: PlainTextInputAPI = {
         name: "plain-text",
         focus,
         focusable: !hidden,
@@ -81,47 +85,46 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         refocus,
         toggle,
         codeMirror,
-    } as PlainTextInputAPI;
+    };
 
-    function pushUpdate(): void {
-        api.focusable = !hidden;
+    /**
+     * Communicate to editing area that input is not focusable
+     */
+    function pushUpdate(isFocusable: boolean): void {
+        api.focusable = isFocusable;
         $editingInputs = $editingInputs;
     }
 
-    function refresh(): void {
-        codeMirror.editor.then((editor) => editor.refresh());
+    async function refresh(): Promise<void> {
+        const editor = await codeMirror.editor;
+        editor.refresh();
     }
 
     $: {
-        hidden;
+        pushUpdate(!hidden);
         tick().then(refresh);
-        pushUpdate();
     }
 
-    function storedToUndecorated(html: string): string {
-        return decoratedElements.toUndecorated(html);
-    }
-
-    function undecoratedToStored(html: string): string {
-        return decoratedElements.toStored(html);
+    function onChange({ detail: html }: CustomEvent<string>): void {
+        code.set(removeProhibitedTags(html));
     }
 
     onMount(() => {
         $editingInputs.push(api);
         $editingInputs = $editingInputs;
 
-        const unsubscribeFromEditingArea = content.subscribe((value: string): void => {
-            code.set(storedToUndecorated(value));
-        });
-
-        const unsubscribeToEditingArea = code.subscribe((value: string): void => {
-            content.set(removeProhibitedTags(undecoratedToStored(value)));
-        });
-
-        return () => {
-            unsubscribeFromEditingArea();
-            unsubscribeToEditingArea();
-        };
+        return singleCallback(
+            content.subscribe((html: string): void =>
+                /* We call `removeProhibitedTags` here, because content might
+                 * have been changed outside the editor, and we need to parse
+                 * it to get the "neutral" value. Otherwise, there might be
+                 * conflicts with other editing inputs */
+                code.set(removeProhibitedTags(storedToUndecorated(html))),
+            ),
+            code.subscribe((html: string): void =>
+                content.set(undecoratedToStored(html)),
+            ),
+        );
     });
 
     setupLifecycleHooks(api);
@@ -133,12 +136,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     class:hidden
     on:focusin={() => ($focusedInput = api)}
 >
-    <CodeMirror
-        {configuration}
-        {code}
-        bind:api={codeMirror}
-        on:change={({ detail: html }) => code.set(removeProhibitedTags(html))}
-    />
+    <CodeMirror {configuration} {code} bind:api={codeMirror} on:change={onChange} />
 </div>
 
 <style lang="scss">

ts/editor/plain-text-input/remove-prohibited.ts

@@ -1,16 +1,9 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-const parser = new DOMParser();
+import { createDummyDoc } from "../../lib/parsing";
 
-function createDummyDoc(html: string): string {
-    return (
-        "<html><head></head><body>" +
-        // parsingInstructions.join("") +
-        html +
-        "</body>"
-    );
-}
+const parser = new DOMParser();
 
 function removeTag(element: HTMLElement, tagName: string): void {
     for (const elem of element.getElementsByTagName(tagName)) {

ts/editor/plain-text-input/transform.ts

@@ -0,0 +1,12 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+import { decoratedElements } from "../DecoratedElements.svelte";
+
+export function storedToUndecorated(html: string): string {
+    return decoratedElements.toUndecorated(html);
+}
+
+export function undecoratedToStored(html: string): string {
+    return decoratedElements.toStored(html);
+}

ts/editor/rich-text-input/RichTextInput.svelte

@@ -3,8 +3,8 @@ Copyright: Ankitects Pty Ltd and contributors
 License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
 <script context="module" lang="ts">
-    import type { FocusHandlerAPI } from "../../editable/content-editable";
     import type { ContentEditableAPI } from "../../editable/ContentEditable.svelte";
+    import { singleCallback } from "../../lib/typing";
     import useContextProperty from "../../sveltelib/context-property";
     import useDOMMirror from "../../sveltelib/dom-mirror";
     import type { InputHandlerAPI } from "../../sveltelib/input-handler";
@@ -13,15 +13,14 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     import type { EditingInputAPI } from "../EditingArea.svelte";
     import type CustomStyles from "./CustomStyles.svelte";
 
-    export interface RichTextInputAPI extends EditingInputAPI, ContentEditableAPI {
+    export interface RichTextInputAPI extends EditingInputAPI {
         name: "rich-text";
-        shadowRoot: Promise<ShadowRoot>;
         element: Promise<HTMLElement>;
         moveCaretToEnd(): void;
         toggle(): boolean;
         preventResubscription(): () => void;
         inputHandler: InputHandlerAPI;
-        focusHandler: FocusHandlerAPI;
+        editable: ContentEditableAPI;
     }
 
     export function editingInputIsRichText(
@@ -49,198 +48,102 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
     import { placeCaretAfterContent } from "../../domlib/place-caret";
     import ContentEditable from "../../editable/ContentEditable.svelte";
-    import type { DecoratedElement } from "../../editable/decorated";
-    import { bridgeCommand } from "../../lib/bridgecommand";
-    import {
-        fragmentToString,
-        nodeContainsInlineContent,
-        nodeIsElement,
-    } from "../../lib/dom";
-    import { on } from "../../lib/events";
-    import { promiseWithResolver } from "../../lib/promise";
-    import { nodeStore } from "../../sveltelib/node-store";
-    import { context as decoratedElementsContext } from "../DecoratedElements.svelte";
     import { context as editingAreaContext } from "../EditingArea.svelte";
     import { context as noteEditorContext } from "../NoteEditor.svelte";
+    import getNormalizingNodeStore from "./normalizing-node-store";
+    import useRichTextResolve from "./rich-text-resolve";
     import RichTextStyles from "./RichTextStyles.svelte";
     import SetContext from "./SetContext.svelte";
+    import { fragmentToStored, storedToFragment } from "./transform";
 
     export let hidden: boolean;
 
     const { focusedInput } = noteEditorContext.get();
-
     const { content, editingInputs } = editingAreaContext.get();
-    const decoratedElements = decoratedElementsContext.get();
-
-    function normalizeFragment(fragment: DocumentFragment): void {
-        fragment.normalize();
-
-        for (const decorated of decoratedElements) {
-            for (const element of fragment.querySelectorAll(
-                decorated.tagName,
-            ) as NodeListOf<DecoratedElement>) {
-                element.undecorate();
-            }
-        }
-    }
-
-    const nodes = nodeStore<DocumentFragment>(undefined, normalizeFragment);
-
-    function adjustInputHTML(html: string): string {
-        for (const component of decoratedElements) {
-            html = component.toUndecorated(html);
-        }
-
-        return html;
-    }
-
-    function adjustInputFragment(fragment: DocumentFragment): void {
-        if (nodeContainsInlineContent(fragment)) {
-            fragment.appendChild(document.createElement("br"));
-        }
-    }
-
-    function writeFromEditingArea(html: string): void {
-        /* We need .createContextualFragment so that customElements are initialized */
-        const fragment = document
-            .createRange()
-            .createContextualFragment(adjustInputHTML(html));
-        adjustInputFragment(fragment);
-        nodes.setUnprocessed(fragment);
-    }
-
-    function adjustOutputFragment(fragment: DocumentFragment): void {
-        if (
-            fragment.hasChildNodes() &&
-            nodeIsElement(fragment.lastChild!) &&
-            nodeContainsInlineContent(fragment) &&
-            fragment.lastChild!.tagName === "BR"
-        ) {
-            fragment.lastChild!.remove();
-        }
-    }
-
-    function adjustOutputHTML(html: string): string {
-        for (const component of decoratedElements) {
-            html = component.toStored(html);
-        }
-
-        return html;
-    }
-
-    function writeToEditingArea(fragment: DocumentFragment): void {
-        const clone = document.importNode(fragment, true);
-        adjustOutputFragment(clone);
-
-        const output = adjustOutputHTML(fragmentToString(clone));
-        content.set(output);
-    }
-
-    const [shadowPromise, shadowResolve] = promiseWithResolver<ShadowRoot>();
-
-    function attachShadow(element: Element): void {
-        shadowResolve(element.attachShadow({ mode: "open" }));
-    }
-
-    const [richTextPromise, richTextResolve] = promiseWithResolver<HTMLElement>();
-
-    function resolve(richTextInput: HTMLElement): { destroy: () => void } {
-        function onPaste(event: Event): void {
-            event.preventDefault();
-            bridgeCommand("paste");
-        }
-
-        function onCutOrCopy(): void {
-            bridgeCommand("cutOrCopy");
-        }
-
-        const removePaste = on(richTextInput, "paste", onPaste);
-        const removeCopy = on(richTextInput, "copy", onCutOrCopy);
-        const removeCut = on(richTextInput, "cut", onCutOrCopy);
-        richTextResolve(richTextInput);
-
-        return {
-            destroy() {
-                removePaste();
-                removeCopy();
-                removeCut();
-            },
-        };
-    }
 
+    const nodes = getNormalizingNodeStore();
+    const [richTextPromise, resolve] = useRichTextResolve();
     const { mirror, preventResubscription } = useDOMMirror();
     const [inputHandler, setupInputHandler] = useInputHandler();
 
-    function moveCaretToEnd() {
-        richTextPromise.then(placeCaretAfterContent);
+    export function attachShadow(element: Element): void {
+        element.attachShadow({ mode: "open" });
     }
 
-    export const api = {
+    async function moveCaretToEnd(): Promise<void> {
+        const richText = await richTextPromise;
+        placeCaretAfterContent(richText);
+    }
+
+    async function focus(): Promise<void> {
+        const richText = await richTextPromise;
+        richText.focus();
+    }
+
+    async function refocus(): Promise<void> {
+        const richText = await richTextPromise;
+        richText.blur();
+        richText.focus();
+        moveCaretToEnd();
+    }
+
+    function toggle(): boolean {
+        hidden = !hidden;
+        return hidden;
+    }
+
+    export const api: RichTextInputAPI = {
         name: "rich-text",
-        shadowRoot: shadowPromise,
         element: richTextPromise,
-        focus() {
-            richTextPromise.then((richText) => {
-                richText.focus();
-            });
-        },
-        refocus() {
-            richTextPromise.then((richText) => {
-                richText.blur();
-                richText.focus();
-                moveCaretToEnd();
-            });
-        },
+        focus,
+        refocus,
         focusable: !hidden,
-        toggle(): boolean {
-            hidden = !hidden;
-            return hidden;
-        },
+        toggle,
         moveCaretToEnd,
         preventResubscription,
         inputHandler,
-    } as RichTextInputAPI;
+        editable: {} as ContentEditableAPI,
+    };
 
     const allContexts = getAllContexts();
 
     function attachContentEditable(element: Element, { stylesDidLoad }): void {
-        stylesDidLoad.then(
-            () =>
-                new ContentEditable({
-                    target: element.shadowRoot,
-                    props: {
-                        nodes,
-                        resolve,
-                        mirrors: [mirror],
-                        inputHandlers: [setupInputHandler, setupGlobalInputHandler],
-                        api,
-                    },
-                    context: allContexts,
-                }),
-        );
+        (async () => {
+            await stylesDidLoad;
+
+            new ContentEditable({
+                target: element.shadowRoot!,
+                props: {
+                    nodes,
+                    resolve,
+                    mirrors: [mirror],
+                    inputHandlers: [setupInputHandler, setupGlobalInputHandler],
+                    api: api.editable,
+                },
+                context: allContexts,
+            });
+        })();
     }
 
-    function pushUpdate(): void {
-        api.focusable = !hidden;
+    function pushUpdate(isFocusable: boolean): void {
+        api.focusable = isFocusable;
         $editingInputs = $editingInputs;
     }
 
-    $: {
-        hidden;
-        pushUpdate();
-    }
+    $: pushUpdate(!hidden);
 
     onMount(() => {
         $editingInputs.push(api);
         $editingInputs = $editingInputs;
 
-        const unsubscribeFromEditingArea = content.subscribe(writeFromEditingArea);
-        const unsubscribeToEditingArea = nodes.subscribe(writeToEditingArea);
-
-        return () => {
-            unsubscribeFromEditingArea();
-            unsubscribeToEditingArea();
-        };
+        return singleCallback(
+            content.subscribe((html: string): void =>
+                nodes.setUnprocessed(storedToFragment(html)),
+            ),
+            nodes.subscribe((fragment: DocumentFragment): void =>
+                content.set(fragmentToStored(fragment)),
+            ),
+        );
     });
 </script>
 

ts/editor/rich-text-input/normalizing-node-store.ts

@@ -0,0 +1,25 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+import type { DecoratedElement } from "../../editable/decorated";
+import type { NodeStore } from "../../sveltelib/node-store";
+import { nodeStore } from "../../sveltelib/node-store";
+import { decoratedElements } from "../DecoratedElements.svelte";
+
+function normalizeFragment(fragment: DocumentFragment): void {
+    fragment.normalize();
+
+    for (const decorated of decoratedElements) {
+        for (const element of fragment.querySelectorAll(
+            decorated.tagName,
+        ) as NodeListOf<DecoratedElement>) {
+            element.undecorate();
+        }
+    }
+}
+
+function getStore(): NodeStore<DocumentFragment> {
+    return nodeStore<DocumentFragment>(undefined, normalizeFragment);
+}
+
+export default getStore;

ts/editor/rich-text-input/rich-text-resolve.ts

@@ -0,0 +1,43 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+import { bridgeCommand } from "../../lib/bridgecommand";
+import { on } from "../../lib/events";
+import { promiseWithResolver } from "../../lib/promise";
+
+function bridgeCopyPasteCommands(input: HTMLElement): { destroy(): void } {
+    function onPaste(event: Event): void {
+        event.preventDefault();
+        bridgeCommand("paste");
+    }
+
+    function onCutOrCopy(): void {
+        bridgeCommand("cutOrCopy");
+    }
+
+    const removePaste = on(input, "paste", onPaste);
+    const removeCopy = on(input, "copy", onCutOrCopy);
+    const removeCut = on(input, "cut", onCutOrCopy);
+
+    return {
+        destroy() {
+            removePaste();
+            removeCopy();
+            removeCut();
+        },
+    };
+}
+
+function useRichTextResolve(): [Promise<HTMLElement>, (input: HTMLElement) => void] {
+    const [promise, resolve] = promiseWithResolver<HTMLElement>();
+
+    function richTextResolve(input: HTMLElement): { destroy(): void } {
+        const destroy = bridgeCopyPasteCommands(input);
+        resolve(input);
+        return destroy;
+    }
+
+    return [promise, richTextResolve];
+}
+
+export default useRichTextResolve;

ts/editor/rich-text-input/transform.ts

@@ -0,0 +1,60 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+import {
+    fragmentToString,
+    nodeContainsInlineContent,
+    nodeIsElement,
+} from "../../lib/dom";
+import { createDummyDoc } from "../../lib/parsing";
+import { decoratedElements } from "../DecoratedElements.svelte";
+
+function adjustInputHTML(html: string): string {
+    for (const component of decoratedElements) {
+        html = component.toUndecorated(html);
+    }
+
+    return html;
+}
+
+function adjustInputFragment(fragment: DocumentFragment): void {
+    if (nodeContainsInlineContent(fragment)) {
+        fragment.appendChild(document.createElement("br"));
+    }
+}
+
+export function storedToFragment(html: string): DocumentFragment {
+    /* We need .createContextualFragment so that customElements are initialized */
+    const fragment = document
+        .createRange()
+        .createContextualFragment(createDummyDoc(adjustInputHTML(html)));
+
+    adjustInputFragment(fragment);
+    return fragment;
+}
+
+function adjustOutputFragment(fragment: DocumentFragment): void {
+    if (
+        fragment.hasChildNodes() &&
+        nodeIsElement(fragment.lastChild!) &&
+        nodeContainsInlineContent(fragment) &&
+        fragment.lastChild!.tagName === "BR"
+    ) {
+        fragment.lastChild!.remove();
+    }
+}
+
+function adjustOutputHTML(html: string): string {
+    for (const component of decoratedElements) {
+        html = component.toStored(html);
+    }
+
+    return html;
+}
+
+export function fragmentToStored(fragment: DocumentFragment): string {
+    const clone = document.importNode(fragment, true);
+    adjustOutputFragment(clone);
+
+    return adjustOutputHTML(fragmentToString(clone));
+}

ts/lib/parsing.ts

@@ -0,0 +1,12 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+/**
+ * Parsing with or without this dummy structure changes the output
+ * for both `DOMParser.parseAsString` and range.createContextualFragment`.
+ * Parsing without means that comments or meaningless html elements are dropped,
+ * which we want to avoid.
+ */
+export function createDummyDoc(html: string): string {
+    return `<html><head></head><body>${html}</body></html>`;
+}