Fix SVGs containing <title> not rendering in markdown-svg-renderer

[simonw]

Fix by Fable 5, bug identified by GPT-5.5.

Fable 5 in Claude Code says:

DOMPurify's SAFE_FOR_XML protection (default on in 3.x) strips any attribute whose value contains raw-text element closers like </title>, </style> or </script>. The sanitized SVG was stored in a data-svg attribute and then run through the second document-level DOMPurify pass, so any SVG containing one of those elements lost its data-svg attribute and was silently skipped by hydrateSvgBlocks, leaving a blank gap in the preview.

Store sanitized SVG sources in a JS Map keyed by id instead, and emit only a data-svg-id reference through the markdown sanitization pass, so DOMPurify never inspects the SVG markup as an attribute value.

https://claude.ai/code/session_01HTbzMyrEPHhHqGiFttbbqA

[claude]

Claude finished @simonw's task —— View job

---

Code Review for SVG Rendering Fix

Reviewing the changes to fix SVGs containing <title> not rendering in markdown-svg-renderer.

Todo List:

• Read and analyze the changed file
• Review code quality and implementation approach
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security implications
• Check test coverage needs
• Provide comprehensive feedback

Analysis

The fix addresses a subtle but critical issue with DOMPurify's SAFE_FOR_XML protection. The problem occurred when SVG content contained elements like <title>, <style>, or <script> - DOMPurify would strip the entire data-svg attribute containing these strings, causing the SVG to be silently dropped during rendering.

Code Quality & Implementation ✅

Excellent solution design:

• Root cause understanding: The fix correctly identifies that DOMPurify's attribute value scanning was the culprit
• Clean abstraction: Using a Map to store sanitized SVG content is elegant and avoids the attribute value problem entirely
• Minimal changes: Only touches the necessary code paths without over-engineering

Implementation quality:

• Clear variable naming (svgStore, data-svg-id)
• Good code organization with explanatory comments
• Proper error handling in hydrateSvgBlocks (removes placeholder if SVG not found)

Potential Bugs & Issues ✅

No significant issues found:

• ID generation using String(svgStore.size) is simple and reliable
• Map clearing in renderMarkdown() prevents memory leaks
• Graceful degradation when SVG content is missing

Minor considerations:

• ID collision risk is minimal since Map is cleared on each render
• No validation on retrieved SVG content, but this is acceptable since it was already sanitized before storage

Performance Considerations ✅

Positive impact:

• Eliminates HTML escaping overhead: No more escapeAttr() function calls
• Reduces DOM attribute size: Short ID strings instead of potentially large SVG markup
• Better memory usage: Map storage is more efficient than escaped attribute values

No performance concerns:

• Map operations are O(1) for the expected usage patterns
• Minimal overhead from ID generation and storage

Security Assessment ✅

Security remains intact:

• SVG content is still sanitized with svgSanitizeConfig before storage
• No new attack vectors introduced
• The indirection through Map doesn't bypass any existing protections
• DOMPurify still sanitizes the final HTML output

Improvement over previous approach:

• More reliable sanitization since DOMPurify no longer sees SVG content in attributes

Test Coverage Recommendations ⚠️

Critical test cases to add:

// SVGs with problematic elements should now render
const testCases = [
  '<svg><title>Chart Title</title><rect/></svg>',
  '<svg><style>.foo{fill:red}</style><circle/></svg>', 
  '<svg><path/></svg>'
];

Integration tests needed:

• Full markdown rendering pipeline with problematic SVGs
• Multiple SVGs in same document
• Edge cases (empty SVGs, malformed content)

Summary

This is a high-quality fix that elegantly solves the core problem. The solution is well-architected, secure, and performs better than the original approach. The only missing piece is comprehensive test coverage for the specific edge cases this fix addresses.

Recommendation: ✅ Approve - This fix properly addresses the identified issue without introducing new problems.

---

[simonw]

Fixed: https://claude-eloquent-tesla-l30aaz.tools-b1q.pages.dev/markdown-svg-renderer#url=https%3A%2F%2Fgist.github.com%2Fsimonw%2F94fde31c34a0400c1d29f57e6a708e6b (last pelican has <title> in the SVG)