Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to render an Ampersand (&) on a MindMap #6308

Open
awhitford opened this issue Feb 20, 2025 · 2 comments · May be fixed by #6315
Open

Unable to render an Ampersand (&) on a MindMap #6308

awhitford opened this issue Feb 20, 2025 · 2 comments · May be fixed by #6315
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect

Comments

@awhitford
Copy link

Description

Imagine a mind map like this:

mindmap
  root((Domain))
      Domain Specific Tools
        Markit EDM["S&P Markit EDM"]

The S&P is rendered as S&P -- I can't seem to figure out a way around it.

This problem may be applicable to other graph types (not just mindmap).

Steps to reproduce

  1. Render the above code to see the issue.

Screenshots

Image

Code Sample

mindmap
  root((Domain))
      Domain Specific Tools
        Markit EDM["S&P Markit EDM"]

Setup

  • Mermaid version: 11.4.0
  • Browser and Version: Chrome Version 133.0.6943.100 (Official Build) (x86_64)

Suggested Solutions

No response

Additional Context

It appears that the ampersand is being encoded twice: & --> & --> &

I tried using %26 instead of the ampersand &, but that just rendered as %26.

@awhitford awhitford added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Feb 20, 2025
@nour0205
Copy link

nour0205 commented Feb 23, 2025

Bug Confirmation: Ampersand (&) Not Rendering Correctly in MindMap

I have tested this issue and confirm that the ampersand (&) character is incorrectly rendered in MindMap diagrams due to double encoding (& → & → &). Instead of displaying "S&P Markit EDM", the output appears as "S&P Markit EDM", leading to incorrect text display.

The issue was tested using two different MindMap structures:
First Test: Simple MindMap Structure

  • Code Used:
    mermaid mindmap root((Domain)) Domain Specific Tools Markit EDM["S&P Markit EDM"]

Image

Second Test: Extended MindMap with Nested Nodes

  • Code Used:
    mermaid mindmap root(("S&P Global")) Domain Specific Tools S&P Data Markit EDM["S&P Market Research"]

Image

  • Observation: The issue persists across:
    • Root node text (S&P Global).
    • Sub-nodes without brackets (S&P Data).
    • Bracket-wrapped labels (S&P Market Research).

Additional Findings

  • This bug only affects MindMaps. It does not occur in:
    • Flowcharts
    • Sequence Diagrams
    • Gantt Charts
    • Pie Charts
    • Entity-Relationship Diagrams : Fails due to syntax error on &

A probable cause could be that text is being encoded:

  1. During data storage or parsing – In mindmapDb.ts, where node text is stored and processed before rendering.
  2. Within the rendering pipeline – In mindmapRender.ts or svgDraw.ts, where additional transformations might be applied before displaying the text in an SVG element.
  3. As part of a global escaping function – If MermaidJS applies a text sanitization function as part of its universal rendering process, MindMaps might be unintentionally inheriting redundant encoding logic.

@nour0205 nour0205 linked a pull request Feb 23, 2025 that will close this issue
@nour0205
Copy link

Hi @awhitford,

I have implemented a fix to properly render the ampersand (&) character in mind maps. Previously, the ampersand was being double-encoded (&&&), which resulted in incorrect rendering.

Before and After Screenshots

Before

Image

After

Image

Pull Request: #6315

Changes and Fix

  • Adjusted the text rendering logic to ensure & is correctly displayed.
  • This fix should also apply to other diagrams using similar text processing, but the primary focus was on mindmaps.

Pending Argos Snapshot Approval

There are Argos visual test changes awaiting approval due to this fix. The updated snapshots correctly reflect the fixed ampersand rendering. A reviewer with the right permissions needs to approve these changes to move forward.

Let me know if you have any questions or if any additional refinements are needed. Thank you for reporting this issue, and I look forward to your feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants