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

Math is occasionally rendered incorrectly (e.g., fraction inside sqrt) #5435

Closed
pbreheny opened this issue Apr 2, 2024 · 8 comments · Fixed by #5483
Closed

Math is occasionally rendered incorrectly (e.g., fraction inside sqrt) #5435

pbreheny opened this issue Apr 2, 2024 · 8 comments · Fixed by #5483
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect

Comments

@pbreheny
Copy link

pbreheny commented Apr 2, 2024

Description

I'm not entirely sure on the scope of the problem, but it seems that for more complex equations involving nested representations (e.g., a square root of a fraction), mermaid does not render the math correctly.

Steps to reproduce

Here's a minimal reproducible example:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <script type="module">
      import mermaid from 'https://cdn.jsdelivr.net/npm/mermaid@10/dist/mermaid.esm.min.mjs';
    </script>
</head>
<body>

<div class="mermaid">
  graph LR
  B("$$\sqrt{\frac{\pi(1-\pi)}{n}}$$")
  A-->B
</div>

</body>
</html>

The fraction is inside the square root, but this is not how mermaid renders the equation (see screenshot).

Screenshots

Screenshot from 2024-04-02 14-01-56
Screenshot from 2024-04-02 14-15-18

Code Sample

No response

Setup

  • Mermaid version: 10.9.0
  • Browser and Version: Chrome

Suggested Solutions

No response

Additional Context

No response

@pbreheny pbreheny added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Apr 2, 2024
@NicolasNewman
Copy link
Contributor

@pbreheny Do you happen to know which version of chrome this was?

On v123.0.6312.123:
image

And on Firefox v124.0.2:
image

Granted the sqrt goes further down then it should.

@NicolasNewman
Copy link
Contributor

I agree that this is an issue on Mermaid's end. Here's the same formula displayed in Joplin (Electron based app with KaTeX integration)

KaTeX v0.16.9
Electron v29.1.0 (Chromium v122.0.6261.39)

image

@pbreheny
Copy link
Author

Oh, interesting...it is indeed browser dependent. The mistake shows up on Chromium (Linux) Version 124.0.6367.60 (Official Build) snap (64-bit). When I go over to a Windows machine and run on Google Chrome / Edge / Firefox, the equation is correctly displayed on all three browsers.

@NicolasNewman
Copy link
Contributor

Does this effect how it renders on Linux?

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/katex@0.16.10/dist/katex.min.css"
        integrity="sha384-wcIxkf4k558AjM3Yz3BBFQUbk/zgIYC2R0QpeeYb+TwlBVMrlgLqwRjRtGZiK7ww" crossorigin="anonymous">
    <script type="module">
        import mermaid from 'https://cdn.jsdelivr.net/npm/mermaid@10/dist/mermaid.esm.min.mjs';
        mermaid.initialize({
            theme: 'forest',
            legacyMathML: true,
        });
    </script>
</head>
<body>
    <div class="mermaid">
        graph LR
        B("$$\sqrt{\frac{\pi(1-\pi)}{n}}$$")
        A-->B
    </div>
</body>
</html>

@pbreheny
Copy link
Author

No; sqrt is still misplaced:

Screenshot from 2024-04-22 11-46-17

@NicolasNewman
Copy link
Contributor

Could you try opening the following? Does one of them render the same as your 2nd screenshot?

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/katex@0.16.10/dist/katex.min.css"
        integrity="sha384-wcIxkf4k558AjM3Yz3BBFQUbk/zgIYC2R0QpeeYb+TwlBVMrlgLqwRjRtGZiK7ww" crossorigin="anonymous">

    <!-- The loading of KaTeX is deferred to speed up page rendering -->
    <script defer src="https://cdn.jsdelivr.net/npm/katex@0.16.10/dist/katex.min.js"
        integrity="sha384-hIoBPJpTUs74ddyc4bFZSM1TVlQDA60VBbJS0oA934VSz82sBx1X7kSx2ATBDIyd"
        crossorigin="anonymous"></script>
    <script type="module">
        import mermaid from 'https://cdn.jsdelivr.net/npm/mermaid@10/dist/mermaid.esm.min.mjs';
        const element = document.querySelector('div.math')
        const element2 = document.querySelector('div.math2')
        katex.render(String.raw`\sqrt{\frac{\pi(1-\pi)}{n}}`, element, {
            throwOnError: false,
            displayMode: true,
            output: 'mathml'
        }
        );
        katex.render(String.raw`\sqrt{\frac{\pi(1-\pi)}{n}}`, element2, {
            throwOnError: false,
            displayMode: true,
            output: 'htmlAndMathml'
        }
        );

    </script>
</head>
<body>
    <div class="math"></div>
    <div class="math2"></div>
    <div class="mermaid">
        graph LR
        B("$$\sqrt{\frac{\pi(1-\pi)}{n}}$$")
        A-->B
    </div>
</body>
</html>

@pbreheny
Copy link
Author

Yes; first one is incorrect, second one is correct:

Screenshot from 2024-04-22 12-03-37

@NicolasNewman
Copy link
Contributor

NicolasNewman commented Apr 22, 2024

Great! The issue comes down OSes using different fonts and browsers having different MathML implementations. On Windows the first shows as being elongated while the 2nd is correct.

image.

Unfortunately it was a bit of an oversight as I didn't think to test different fonts but the useLegacyMathML option only defaults to using the stylesheet if MathML is not supported at all in a browser. Going forward the config field should be re-worked to also allow using the KaTeX's stylesheet if specified, and never the browser's implementation. I'll hopefully have a PR opened by this weekend to address the problem.

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