Add "Mermaid Fixer" plugin

[Bunker-D]

I am submitting a new Community Plugin

Repo URL

Link to my plugin: https://github.com/Bunker-D/obsidian-mermaid-fixer

Release Checklist

• I have tested the plugin on
  • Windows
  • macOS
  • Linux
  • Android (if applicable)
  • iOS (if applicable)
• My GitHub release contains all required files
  • main.js
  • manifest.json
  • styles.css (optional)
• GitHub release name matches the exact version number specified in my manifest.json (Note: Use the exact version number, don't include a prefix v)
• The id in my manifest.json matches the id in the community-plugins.json file.
• My README.md describes the plugin's purpose and provides clear usage instructions.
• I have read the tips in https://github.com/obsidianmd/obsidian-releases/blob/master/plugin-review.md and have self-reviewed my plugin to avoid these common pitfalls. (💡 See below)
• I have added a license in the LICENSE file.
• My project respects and is compatible with the original license of any code from other plugins that I'm using.
  I have given proper attribution to these other projects in my README.md.

💡 The plugin does use innerHTML, but only with literal strings (defined by the plugin itself), thereby avoiding security issues. This choice is justified by the fact that it significantly facilitate the plugin maintenance, while modifications to Mermaid might require to update the plugin to mirror said modifications.

[joethei]

buttonIconEl.innerHTML = BUTTON_ICON;
You should use the addIcon and setIcon functions from Obsidian for this instead.

Can you explain more as to how your plugin fixes these issues?
And why is having the ribbon button important?

[Bunker-D]

“You should use the addIcon and setIcon”

I didn't know about setIcon. I'll update the code accordingly. (And remove the last, unrelated use of innerHTML, since I can easily do without it.)

I see that setIcon doesn't do anything if the same icon ID is used (even though the icon had been modified). But I can use the following hack:

removeIcon( 'mermaid-fixer' );
addIcon( 'mermaid-fixer', this.createIcon() );
setIcon( this.ribbonButton, '' );  // 👈
setIcon( this.ribbonButton, 'mermaid-fixer' );

Maybe there's a better way? (I could also toggle between to icon name… — Edit: I did that. To be cleaner and not reliant on how setIcon handles unknown icons nor on the preexisting icon set.)

“Can you explain more as to how your plugin fixes these issues?”

The developer's ReadMe should answer the question if you haven't seen it. Don't hesitate if you have any unanswered questions.

“And why is having the ribbon button important?”

As explained in the aforementioned document, it provides a safe and appropriated space to define to place the needed SVG marker definitions. Maybe there's a better place to put them, but I don't know where. (That's the first time I program something for Obsidian.)

When I experimented and tried to modify the DOM to insert these elements, the software stopped working properly. And I don't want to affect the DOM in a way that might break the software. So I chose to use what I though I could use within the Obsidian API — even though it's kind of a dirty hack.

If you know of a better way/place to insert the SVG marker definitions, I would gladly implement that instead.

[Bunker-D]

The requested changes have been done. There is no more innerHTML in the code (release 1.0.1).

(And I updated the community-plugins.json file in the pull request to include the latest included plugins.)

[lishid]

@joethei Remember that setIcon is also just an innerHTML.

@Bunker-D I am looking at your CSS styles and seeing if there's any of them we can integrate. I couldn't find some of them (or at least, where the go) in testing. I wrote some comments between the lines:

/* This seems to fix a #fff? If that's the case I can fix easily */
.mermaid [aria-roledescription="sequence"] .sequenceNumber {
    fill:var(--background-primary)!important;
}
/* This was set to background-modifier-border before, right? If that's the case I can fix easily */
.mermaid [aria-roledescription="classDiagram"] .relation {
    stroke:var(--text-normal)!important;
}
/* Couldn't find this rule anywhere */
.mermaid [aria-roledescription="er"] path.er {
    stroke:var(--text-muted)!important;
}
/* Why is this #000 */
.mermaid [aria-roledescription="journey"] .label {
    color:#000!important;
}
/* Couldn't find this rule anywhere */
.mermaid [aria-roledescription="requirement"] path.er{
    stroke:var(--text-normal)!important;
}
/* I can fix both here */
.mermaid [aria-roledescription="requirement"] .reqBox{
    fill:var(--background-primary-alt)!important;
    stroke:var(--text-muted)!important
}
/* I can fix the color here, but what is the stroke for? */
.mermaid [aria-roledescription="requirement"] .reqLabelBox{
    fill:var(--background-secondary)!important;
    stroke:none!important
}
/* I can fix */
.mermaid [aria-roledescription="requirement"] .req-title-line{
    stroke:var(--text-muted)!important
}
/* I can fix */
.mermaid [aria-roledescription="requirement"] .relationshipLabel{
    fill:var(--text-normal)!important
}

[joethei]

Just to add, for the arrow thing, @Bunker-D you should send a patch to mermaid upstream.
That way everyone else can enjoy the fix too.

[Bunker-D]

@lishid

/* This seems to fix a #fff? If that's the case I can fix easily */
.mermaid [aria-roledescription="sequence"] .sequenceNumber {
    fill:var(--background-primary)!important;
}

→ Yes. It gives the text color for the sequence numbers in sequence diagrams. It avoids having white on almost-white. (You can see the effect the Readme.)

Note that the <style> elements in the Mermaid <svg> sets this fill to both var(--text-muted) then #fff later on. I think var(--text-muted) isn't a safe choice: the lines are set to var(--text-normal) (ensuring proper contrast), and my marker matches that for the little circle behind the number. With var(--background-primary), there will always be a good contrast.

/* This was set to background-modifier-border before, right? If that's the case I can fix easily */
.mermaid [aria-roledescription="classDiagram"] .relation {
    stroke:var(--text-normal)!important;
}

→ The SVG <style> elements first defines it as var(--text-normal) in a specific rule, then as var(--background-modifier-border) with some markers. var(--background-modifier-border) doesn't ensure cross-theme legibility. And on the Obsidian dark them, it only gives a contrast of 1.37:1. (It should be typically at least 4.5:1 based on Web Content Accessibility Guidelines.)

Note that in the case of my plugin, the markers' style is fixed in the markers I insert (with the fill and stroke attributes). If your fixing the CSS for general use without my plugin, you should also style the markers. The CSS selector used is #… #aggregationEnd, #… #aggregationStart, #… #compositionEnd, #… #compositionStart, #… .relation, #… g.classGroup line, #… g.stateGroup line, but don't use that: it doesn't target all markers (see plugin for the list of ID's), and I don't think g.stateGroupe should be set to var(--text-normal), typically.

/* Couldn't find this rule anywhere */
.mermaid [aria-roledescription="er"] path.er {
    stroke:var(--text-muted)!important;
}

→ It fixes the color of the links between boxes. It is affected by .relationshipLine in the <style> (that sets stroke to var(--text-normal), and it is overwritten directly by the element definition: <path class="er relationshipLine" d="…" … style="stroke: gray; fill: none;"></path>.

/* Why is this #000 */
.mermaid [aria-roledescription="journey"] .label {
    color:#000!important;
}

→ User journeys uses big yellowish boxes and smileys. One possibility is to either keep that and fix a fitting text color, the other is to set theme-dependent backgrounds and text colors. I chose the former. (And I typically do the latter with requirement diagrams.)

/* I can fix the color here, but what is the stroke for? */
.mermaid [aria-roledescription="requirement"] .reqLabelBox{
    fill:var(--background-secondary)!important;
    stroke:none!important
}

→ The boxes (<rect>) in requirement diagrams are sized independently from text-length, so the text can overflow. When there is a border to the rectangle, it makes the overflowing text less readable. Those borders also don't bring much, so I removed them.

(On the contrary, there's the same issue with the main boxes. But in this case, I feel like the border are quite more important for the diagram legibility.)

[lishid]

Do you happen to have example mermaid diagrams I can test with for each of the cases?

[Bunker-D]

@joethei What I am proposing here cannot be included into Mermaid:

• The disappearing arrows are due to ID reuse, and I solve that with a hack that relies on inserting stuff outside of the Mermaid-created SVG.
• The other styling issues seem to be (for the most part?) on the Obsidian side, since they use Obsidian CSS variables. But @lishid seems to be onto it.

I might propose some solution to solve the issue on Mermaid side, but it would be a totally different solution (to ensure unique ID), and I would have to dig into their code to not forget anything. My issue is that it could break something for other users: changing the ID (to ensure unique IDs) might affect some styling.

It could be implemented as something optional, but then it would affect most current uses of Mermaid.

So yes, updating Mermaid. But I can't right now and it might not be so easy.

[Bunker-D]

@lishid You can use the notes in the test_notes folder in my repository. One note per diagram type.

For each note, going Edit view → Read view as the effect of triggering the arrow-disappearing bug. (I explain why in the dev readme.)

[Bunker-D]

@lishid If you can and want to modify Obsidian's code, maybe you could just add some code so that every time Mermaid is called to create an SVG, you make the IDs inside unique. (Although I would understand if you prefer to wait to have this done on Mermaid's side.)

This would work:

function makeMarkerIdsUnique( svg: Element ): void {
  ( new MermaidCleaner( svg ) ).makeAllMarkerIdsUnique();
}

class MermaidCleaner {
  private element: Element;
  private id: string;
  private style: Element;

  constructor( svg: Element ) {
    this.element = svg;
    this.id = svg.id;
    this.style = svg.getElementsByTagName( 'style' )[ 0 ];
  }

  makeAllMarkerIdsUnique() {
    for ( const marker of this.element.getElementsByTagName( 'marker' ) ) {
      this.makeMarkerIdUnique( marker );
    }
  }

  makeMarkerIdUnique( markerEl: Element ) {
    const oldMarkerId = markerEl.id;
    if ( !oldMarkerId ) return;
    const newMarkerId = this.makeNewId( oldMarkerId );
    markerEl.id = newMarkerId;
    this.updateMarkerCalls( oldMarkerId, newMarkerId );
    this.updateMarkerStyle( oldMarkerId, newMarkerId );
  }

  makeNewId( oldId: string ): string {
    // We use the diagram id — if any — as a suffix
    if ( this.id ) {
      return `${ oldId }-${ this.id }`;
    }
    // Else we use a random string at least 3 characters long
    // (Using a random suffix in any case minimises the risk of conflicts with other unprocessed id of any origin.)
    let id = oldId + '-';
    let length = 3;
    while ( length-- || document.getElementById( id ) ) {
      id += String.fromCharCode( 97 + Math.floor( 26 * Math.random() ) );
    }
    return id;
  }

  updateMarkerCalls( oldMarkerId: string, newMarkerId: string ): void {
    for ( const attribute of [ 'marker-start', 'marker-end' ] ) {
      for ( const el of [ ...this.element.querySelectorAll( `[${ attribute }="url(#${ oldMarkerId })"]` ) ] ) {
        el.setAttr( attribute, `url(#${ newMarkerId })` );
      }
    }
  }

  updateMarkerStyle( oldMarkerId: string, newMarkerId: string ): void {
    if ( !this.style ) return;
    const re = new RegExp( `#${ oldMarkerId }(?!\\w|-)`, 'g' );
    this.style.innerHTML = this.style.innerHTML.replace( re, '#' + newMarkerId );
  }

}

With that, you just have to use makeMarkerIdsUnique(svg) for every Mermaid-produced <svg>.

(I tested it works with a draft plugin applying the function to every element of document.querySelectorAll('.mermaid svg').)

Note that obviously it just takes care of the ID issue, not the styling issues.

[Bunker-D]

By the way, just ask if you want help so that all styling issues are covered through CSS, including those solved by the way I defined my markers. Typically, my plugin doesn't have to fix #arrowhead path for sequence diagram because it redefines #arrowhead; but there's an issue here (setting the fill and stroke to #333, overwriting the var(--text-normal) value from the parent <marker>).

[lishid]

I am aware of the missing IDs for the markers; I've just been waiting for someone to submit a patch upstream to fix it properly 🤣

[lishid]

mermaid-js/mermaid#1318
mermaid-js/mermaid#1871
mermaid-js/mermaid#3267

[Bunker-D]

Yeah. But Mermaid's code being as it is, I'm not sure I can code and commit a fix that wouldn't break anything in a timely manner. Plus, as it would change the ID, it could break somebody's use of Mermaid. So it might been argued that it would have to be a major release (while it fixes an issue visible only with some applications).

That's why I'm proposing a solution to fix Mermaid's result.

[lishid]

Ok I have implemented all of the style fixes, as well as a refactored version of your marker fix. Will credit you in the next release notes for the help with these!