Explicitly close void elements

[remcohaszing]

📑 Summary

Previously <br> elements were explicitly closed. This was then undone by dompurify due to a misconfiguration.

Now all void elements are explicitly autoclosed and compurify is configured to treat content as SVG, not HTML.

Resolves remcohaszing/rehype-mermaid#5

📏 Design Decisions

Possibly a better solution might be to use XMLSerializer instead of innerHTML to create svgCode on line 496.

📋 Tasks

Make sure you

• 📖 have read the contribution guidelines
• 💻 have added necessary unit/e2e tests.
• 📓 have added documentation. Make sure MERMAID_RELEASE_VERSION is used for all new features.
• 🔖 targeted develop branch

[netlify]

✅ Deploy Preview for mermaid-js ready!

Name	Link
🔨 Latest commit	1c8dccb
🔍 Latest deploy log	https://app.netlify.com/sites/mermaid-js/deploys/65215170e4008500092cb600
😎 Deploy Preview	https://deploy-preview-4926--mermaid-js.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[codecov]

Codecov Report

Merging #4926 (1c8dccb) into develop (12a4707) will decrease coverage by 36.65%.
The diff coverage is n/a.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #4926       +/-   ##
============================================
- Coverage    79.94%   43.29%   -36.65%     
============================================
  Files          164       20      -144     
  Lines        13623     4721     -8902     
  Branches       693       21      -672     
============================================
- Hits         10891     2044     -8847     
- Misses        2583     2676       +93     
+ Partials       149        1      -148     

Flag	Coverage Δ
e2e	?
unit	43.29% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 159 files with indirect coverage changes

[remcohaszing]

Actually I don’t think this really fixes the issue.

Should mermaid even produce valid SVG (what I need for mermaid-isomorphic / rehype-mermaidjs) or is it sufficient for mermaid to generate SVG that works in the current document (sufficient for people that use mermaid in the browser directly)?

[remcohaszing]

This approach doesn’t work if the elements have a class name.

[remcohaszing]

So down here svgCode may still be invalid SVG (but valid HTML).

[remcohaszing]

I decided to resolve this in mermaid-isomorphic instead. remcohaszing/mermaid-isomorphic#3

[aloisklink]

Actually I don’t think this really fixes the issue.

Should mermaid even produce valid SVG (what I need for mermaid-isomorphic / rehype-mermaidjs) or is it sufficient for mermaid to generate SVG that works in the current document (sufficient for people that use mermaid in the browser directly)?

I'd love mermaid to produce valid SVG all the time! Although we'd probably want some way to test it.

Especially if we can get mermaid to produce SVG 1.1-compliant SVGs (the latest SVG spec that most non-browser environments, like Inkscape support).

One issue is that Mermaid supports giving diagrams custom CSS, so users could add new CSS features that aren't part of SVG 1.1 (e.g. CSS Variables for light/dark-mode)).

---

I think you're right, XMLSerializer is probably the best way to handle this (it's what I did in the mermaid-cli project: mermaid-js/mermaid-cli@578e294), but I don't know how difficult that would be implement in mermaid, and if it would break anything else in a browser.

[remcohaszing]

For SVG to be valid, it needs to be valid XML. For SVG to be safe to embed, it needs to be valid HTML. So typically this the SVG is semantically valid all the time (at least semantically) if it can be parsed by both an XML parser and an HTML parser.

Currently cleanUpSvgCode does a naive attempt to make void elements self-closing. This is then undone by dompurify.

Why is the output sanitized? I don’t think this is very useful if everything is generated by mermaid.

I think you're right, XMLSerializer is probably the best way to handle this (it's what I did in the mermaid-cli project: mermaid-js/mermaid-cli@578e294), but I don't know how difficult that would be implement in mermaid, and if it would break anything else in a browser.

If you look at the diffs of the SVG files in remcohaszing/mermaid-isomorphic#3, you can see that this makes all HTML void elements and empty SVG elements self-closing, but keeps other HTML elements inside <foreignObject>s intact. So this is safe as far as I can tell.

[aloisklink]

Why is the output sanitized? I don’t think this is very useful if everything is generated by mermaid.

I believe it's so users can add HTML to their mermaid diagrams, e.g.

```mermaid
flowchart TD
    A[<i>italics</i> <b>bold</b>]
```

will be rendered as

flowchart TD
    A[<i>italics</i> <b>bold</b>]

Loading

And mermaid needs to sanitize to avoid XSS attacks.

---

If you look at the diffs of the SVG files in remcohaszing/mermaid-isomorphic#3, you can see that this makes all HTML void elements and empty SVG elements self-closing, but keeps other HTML elements inside <foreignObject>s intact. So this is safe as far as I can tell.

👍 That's good to hear! I think some very very very old browsers might not support self-closing HTML elements, but even internet explorer supports it, so it shouldn't be anything we need to worry about!