From c241dcf8fed738bd09e96cabc7e625c94f6ee0b8 Mon Sep 17 00:00:00 2001 From: Dimitri POSTOLOV Date: Wed, 14 Oct 2026 18:08:39 -0500 Subject: [PATCH] fix(visimer): restore the rendered label when an edit is cancelled (#6509) * fix(visimer): restore the rendered label when an in-place edit is cancelled Escape on an in-place label edit wrote the raw label text from the diagram source back into the label with innerHTML. That text never passed through mermaid's sanitizer. The canvas now saves the label nodes that mermaid rendered when the edit opens and puts them back on Escape. If a render replaced the label during the edit, the canvas also repaints it from source. The reinject after a mid-edit render now moves the typed nodes into the new label instead of re-parsing them as HTML. * refactor(visimer): repaint a swapped label with a direct render call The Escape path set renderHeldByEdit only to reach the held-render block below it. Call scheduleRender directly instead. scheduleRender restarts one debounce timer, so a second call from the held-render block is harmless. * refactor(visimer): drop prose comments and pin the label swap in the repaint test The new and rewritten prose comments in view.ts and inplace.test.ts are outside the default comment allowlist. The Escape rule stays as a single STOP marker. The comments that described the removed innerHTML reinject are deleted with it. The repaint test now checks that the in-flight render replaced the label, and that the new label is still editable, before it sends Escape. Without that check, a late swap would let the test pass without reaching the repaint branch. GitOrigin-RevId: ca5cd6619147dd31d292ad6bd3282c5125156669 --- .changeset/escape-label-restore.md | 5 ++ packages/dom/src/view.ts | 44 ++++++++++-------- packages/dom/test/inplace.test.ts | 75 +++++++++++++++++++++++++++++- 3 files changed, 102 insertions(+), 22 deletions(-) create mode 100644 .changeset/escape-label-restore.md diff --git a/.changeset/escape-label-restore.md b/.changeset/escape-label-restore.md new file mode 100644 index 0000000..1be779e --- /dev/null +++ b/.changeset/escape-label-restore.md @@ -0,0 +1,5 @@ +--- +"@visimer/dom": patch +--- + +Cancelling an in-place label edit with Escape now restores the label exactly as mermaid rendered it. Before, the canvas parsed the raw label text from the diagram source as HTML, which skipped mermaid's sanitizer. If your canvas renders diagrams that you did not write, upgrade. diff --git a/packages/dom/src/view.ts b/packages/dom/src/view.ts index f3d695b..1738f32 100644 --- a/packages/dom/src/view.ts +++ b/packages/dom/src/view.ts @@ -270,21 +270,16 @@ function readLabelHtml(label: HTMLElement): string { .trim() } -/** - * DOM-faithful read of the same label, used ONLY for the mid-flight snapshot - * that a mermaid re-render immediately reinjects into the freshly rendered - * label (`label.innerHTML = liveEdit.value`). The snapshot has to preserve - * exactly what was on screen: an in-flight render that resolves the same - * instant the user types a trailing space would otherwise lose that space - * to `readLabelHtml`'s canonicalisation + trim, and the next keystroke - * would land against the previous word. `
` normalisation is still - * safe here because mermaid emits `
` in the source and both browsers - * accept either serialisation. - */ function readLabelSnapshot(label: HTMLElement): string { return (label.innerHTML ?? '').replace(//gi, '
') } +interface LiveEdit { + value: string + nodes: Node[] + caret: number +} + /** * Absolute caret offset within `root`, counted across all of its text nodes. * `Selection.anchorOffset` alone is relative to the anchor node — a label that @@ -391,6 +386,8 @@ export class MermaidCanvasView { entityId: string labelSelector: string original: string + originalNodes: Node[] + openedOn: HTMLElement lastCommitted: string caretOffset: number commitValue: (v: string) => void @@ -809,15 +806,15 @@ export class MermaidCanvasView { // destroys them, then reinject below so no keystroke is lost // a session may have OPENED while the render was in flight — re-read it // through the accessor so the early-return narrowing above doesn't apply - let liveEdit: { value: string; caret: number } | null = null + let liveEdit: LiveEdit | null = null const midFlightSession = this.activeInPlaceSession() const editingLabel = midFlightSession?.label if (midFlightSession && editingLabel?.isConnected) { - // Snapshot the label's raw innerHTML (DOM-faithful, no trim, no - // ` ` canonicalisation) so `resumeInPlaceSession` below can - // reinject exactly what was on screen. Using `readLabelHtml` here - // would strip a just-typed trailing space out of the snapshot. - liveEdit = { value: readLabelSnapshot(editingLabel), caret: caretOffsetWithin(editingLabel) } + liveEdit = { + value: readLabelSnapshot(editingLabel), + nodes: [...editingLabel.childNodes], + caret: caretOffsetWithin(editingLabel), + } // a pending live-commit timer closes over the label we are about to // detach; the reinjection below reschedules it against the new one if (midFlightSession.liveTimer) { @@ -1418,6 +1415,8 @@ export class MermaidCanvasView { entityId, labelSelector, original: current, + originalNodes: [...label.childNodes].map((node) => node.cloneNode(true)), + openedOn: label, lastCommitted: current, caretOffset: -1, commitValue, @@ -1481,8 +1480,13 @@ export class MermaidCanvasView { // states. Restore the label DOM directly too — when the reverted code // equals the last rendered code, render() short-circuits and would // leave the typed text on screen. - label.innerHTML = session.original + // STOP: never parse session.original as HTML; it is unsanitized diagram source. + label.replaceChildren(...session.originalNodes) if (session.lastCommitted !== session.original) session.commitValue(session.original) + if (label !== session.openedOn) { + this.lastRenderedCode = '' + this.scheduleRender() + } } // apply any canvas render held back while the session was typing if (this.renderHeldByEdit) { @@ -1527,7 +1531,7 @@ export class MermaidCanvasView { } /** re-attach a live in-place editing session after a render replaced the DOM */ - private resumeInPlaceSession(liveEdit?: { value: string; caret: number } | null) { + private resumeInPlaceSession(liveEdit?: LiveEdit | null) { const session = this.inPlaceSession if (!session) return if (!this.editor.entityExists(session.entityId)) { @@ -1554,7 +1558,7 @@ export class MermaidCanvasView { liveEdit.value !== session.lastCommitted && liveEdit.value !== readLabelSnapshot(label) ) { - label.innerHTML = liveEdit.value + label.replaceChildren(...liveEdit.nodes) // run the carried-over delta through the normal live-commit debounce label.dispatchEvent(new Event('input')) } diff --git a/packages/dom/test/inplace.test.ts b/packages/dom/test/inplace.test.ts index 8d03332..2c882fc 100644 --- a/packages/dom/test/inplace.test.ts +++ b/packages/dom/test/inplace.test.ts @@ -19,7 +19,7 @@ const CODE = 'flowchart TD\n A[Write a doc] --> B[End]\n' * Set `gate` to hold the next render un-resolved — that models the async gap * between a live commit and the SVG swap landing. */ -function makeFakeMermaid() { +function makeFakeMermaid(renderLabel: (label: string) => string = (label) => label) { let release: (() => void) | null = null const fake: MermaidLike & { gate(): void; release(): void } = { initialize() {}, @@ -41,7 +41,7 @@ function makeFakeMermaid() { ...nodes.map( ([, id, label], i) => `` + - `

${label}

` + + `

${renderLabel(label)}

` + ``, ), '', @@ -223,6 +223,77 @@ describe('in-place label editing survives the live-commit re-render', () => { // rendered code, in which case no re-render will repaint it expect(labelOf(container, 'node:A').textContent).toBe('Write a doc') }) + + it('Escape after an in-flight render swapped the label repaints it from source', async () => { + mermaid.gate() + editor.dispatch({ type: 'renameNode', id: 'A', label: 'Renamed' }) + await vi.advanceTimersByTimeAsync(1) + + view.editEntityLabel('node:A') + const openedOn = labelOf(container, 'node:A') + mermaid.release() + for (let i = 0; i < 5; i++) await Promise.resolve() + await vi.advanceTimersByTimeAsync(0) + + const swapped = labelOf(container, 'node:A') + expect(swapped).not.toBe(openedOn) + expect(swapped.getAttribute('contenteditable')).toBe('true') + swapped.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })) + await vi.advanceTimersByTimeAsync(500) + expect(editor.code).toContain('A[Renamed]') + expect(labelOf(container, 'node:A').textContent).toBe('Renamed') + }) +}) + +describe('Escape restores what mermaid rendered, never the raw source label', () => { + let editor: MermaidWysiwygEditor + let container: HTMLElement + let view: MermaidCanvasView + + const escapeHtml = (s: string) => s.replace(/&/g, '&').replace(//g, '>') + + beforeEach(async () => { + vi.useFakeTimers() + editor = new MermaidWysiwygEditor({ code: 'flowchart TD\n A[raw] --> B[End]\n' }) + container = document.createElement('div') + document.body.appendChild(container) + view = new MermaidCanvasView({ editor, container, mermaid: makeFakeMermaid(escapeHtml), debounceMs: 0 }) + await view.render() + }) + + afterEach(() => { + view.destroy() + container.remove() + vi.useRealTimers() + }) + + it('cancelling an untouched edit does not parse the source label as HTML', async () => { + view.editEntityLabel('node:A') + const label = labelOf(container, 'node:A') + label.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })) + expect(label.querySelector('i')).toBeNull() + + await vi.advanceTimersByTimeAsync(500) + expect(labelOf(container, 'node:A')).toBe(label) + expect(label.querySelector('i')).toBeNull() + expect(label.textContent).toBe('raw') + }) + + it('cancelling after a live commit does not parse the source label as HTML', async () => { + view.editEntityLabel('node:A') + typeInto(labelOf(container, 'node:A'), 'Half typed') + await vi.advanceTimersByTimeAsync(500) + expect(editor.code).toContain('A[Half typed]') + + const label = labelOf(container, 'node:A') + label.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })) + expect(label.querySelector('i')).toBeNull() + expect(label.textContent).toBe('raw') + + await vi.advanceTimersByTimeAsync(500) + expect(editor.code).toContain('raw') + expect(editor.code).not.toContain('Half typed') + }) }) /**