fix(mermaid): parse SVG via innerHTML instead of DOMParser xml (#1131) #1134
Merged
Conversation
Two tests: - baseline: normal flowchart renders SVG - regression: flowchart with <img> in node label must show SVG or the 'Unable to render' error UI, never raw XML parseerror text The second test currently FAILS if the DOMParser (image/svg+xml) path is hit with HTML void elements — documents the bug before the fix.
DOMParser with 'image/svg+xml' is a strict XML parser. When Mermaid emits SVG containing HTML void elements (e.g. <img> inside <foreignObject> nodes), the parser fails and returns a <parseerror> document element. Inserting that element into the DOM renders the raw XML error text instead of the 'Unable to render' error UI. Switching to innerHTML parsing (HTML parser) handles void elements correctly and is safe — Mermaid's own sanitisation has already run on the SVG string before we receive it.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Mermaid preview injection logic in the LeafWiki UI to avoid XML parse errors when Mermaid emits HTML void elements (e.g. <img>) inside <foreignObject>, and adds an end-to-end regression test to ensure raw XML parser errors never leak into the rendered page.
Changes:
- Switch Mermaid SVG parsing from
DOMParser(..., 'image/svg+xml')to HTML parsing viainnerHTML+querySelector('svg'). - Throw a controlled error when Mermaid output doesn’t contain an
<svg>element (so the existing error UI is shown instead of raw parse output). - Add Playwright e2e coverage for basic Mermaid rendering and for the
<img>/<foreignObject>regression scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| ui/leafwiki-ui/src/features/preview/useMermaidInjector.ts | Changes how Mermaid’s rendered SVG string is parsed and injected into the preview container to avoid XML parseerror nodes. |
| e2e/tests/mermaid.spec.ts | Adds Playwright coverage for Mermaid rendering, including a regression test for the reported parseerror leak. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
DOMParser with 'image/svg+xml' is a strict XML parser. When Mermaid emits SVG containing HTML void elements (e.g. <img> inside <foreignObject> nodes), the parser fails and inserts a raw XML parseerror element into the DOM — showing confusing error text instead of the expected 'Unable to render' UI. Fix: parse the rendered SVG string via innerHTML (HTML parser), which handles void elements correctly. Also change securityLevel from 'strict' to 'antiscript' so that HTML in node labels (e.g. <img>) is preserved by Mermaid and rendered via foreignObject, while script tags and event handlers are still stripped by DOMPurify. Correct Mermaid syntax for images in nodes: A[label <img src='...' />] (single quotes inside src attribute) Reported in #1131.
- Use <template> instead of <div> for inert SVG parsing (no side-effect fetches from <img src=...> before the node is inserted into the DOM) - Replace external shields.io URL in e2e test with a data URL to keep the test suite hermetic - Replace fixed waitForTimeout(3000) with expect.poll to avoid flakiness under variable CI/local speeds
The single-quote variant only showed the fix works for src='...'. Both quote styles must be covered since HTML allows both; the template-based parser handles them equally. Refactors the two img-node tests into a parameterised loop to avoid duplicating the poll/assert logic.
- Add assertion that <img> inside the rendered SVG is actually visible, covering the core #1131 regression (not just that no error text appears) - Add article.toBeVisible() pre-check before the poll so a blank page produces an actionable failure instead of a 15 s timeout - Remove redundant article svg visibility check after poll; replaced by the more specific article svg img assertion - Extract getCsrfScript/createPage to e2e/helpers/api.ts — previously copy-pasted verbatim in mermaid.spec.ts, wikilinks.spec.ts, editor.spec.ts - Add avoidEscape: true to the ESLint quotes rule so strings containing single quotes can use double quotes instead of escaped single quotes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.