Fix multiple Mermaid/Graphviz diagrams not rendering#363
Merged
Conversation
The rendering loops in mermaid.init.js and viz.init.js navigated two levels up from the <code> element and set innerHTML on the shared parent wrapper. This replaced the entire parent's content — including all subsequent diagram <pre> elements — on the first iteration. Later iterations found their elements detached with a null parentElement, causing them to fail silently. Only the first diagram in any document ever rendered. Fix: replace only the specific <pre> element via outerHTML instead of clobbering the shared parent's innerHTML. Each diagram's <pre> is replaced in isolation, leaving sibling elements intact. Also adds a tagName guard in mermaid.init.js to skip gracefully if the DOM structure is ever unexpected. Related to #331 Related to #332
Matches the defensive guard already present in mermaid.init.js. If a <code> element is somehow detached or not inside a <pre>, log a warning and skip rather than throw a TypeError. Related to #332
Contributor
Code Coverage ReportCurrent Coverage: 58.04% Coverage Details (Summary) |
Owner
Author
Confidence NoteThe root cause analysis is solid and the fix is logically correct, but I want to flag the limits of what we can verify here. What we're confident about: What we haven't verified:
Recommendation: Ship in v3000.0.6 and ask @rcuisnier and @gino-santerre-telus to confirm. If both diagrams now render, we're done. If not, the next debugging step is inspecting the live DOM structure in the WebView console. |
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
1 participant
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.
Summary
mermaid.init.js: the rendering loop navigated to the parent of each<pre>and set itsinnerHTML, replacing the entire parent's content on the first iteration and destroying all subsequent diagram elements. Changed topre.outerHTML = result.svgto replace only the specific<pre>. Added null/tagName guard for unexpected DOM structures.viz.init.js: identical root cause (dom.parentElement.parentElement.innerHTML). Changed todom.parentElement.outerHTML. Added matching null/tagName guard.plans/test_coverage_improvement_plan.md:MPGraphvizRenderingTests.mwas missing from the tracking document.Root Cause
PR #333 (v3000.0.5) fixed the MutationObserver path so Mermaid renders at all on DOM replacement, but the rendering loop itself was broken regardless of how it was triggered.
querySelectorAllreturns a static snapshot, so the loop correctly collects all elements — but the first iteration'sinnerHTMLassignment on the shared parent element removed all siblings from the DOM, leaving subsequent elements withparentElement === nulland silently failing.Related Issues
Related to #331
Related to #332
Manual Testing Plan
13 scenarios covering: multiple Mermaid diagrams, multiple Graphviz diagrams, mixed engines, invalid syntax alongside valid diagrams, live-edit re-render, and Mermaid+Graphviz together in one document. Full checklist:
Review Notes
No Obj-C changes. No automated JS tests possible (no WKWebView test harness exists in this project). The fix is a mechanical change to DOM replacement strategy; the
renderingguard, MutationObserver, and unique ID scheme are unchanged.