Skip to content

🐛 Stop icon roles leaking SVG into toc labels and search#279

Merged
chrisjsewell merged 1 commit into
mainfrom
brief/12-icon-toctree
Jul 14, 2026
Merged

🐛 Stop icon roles leaking SVG into toc labels and search#279
chrisjsewell merged 1 commit into
mainfrom
brief/12-icon-toctree

Conversation

@chrisjsewell

@chrisjsewell chrisjsewell commented Jul 12, 2026

Copy link
Copy Markdown
Member

The octicon/material icon roles returned nodes.raw(format="html"), whose astext() is the entire <svg> markup — so an icon at the start of a section title poured SVG junk into every plain-text context derived via clean_astext. The measurable damage: the search index for such a page contained ~200 garbage title-terms (svg, path, viewbox, plus every path-coordinate fragment like 002l, 0zm8, …).

Changes

  • New sd_icon inline node carrying the markup in its svg attribute with no text children, so astext() == ""; the HTML visitor writes the SVG, non-HTML builders skip it. Octicon and material roles (and the _all-octicon table) migrated off nodes.raw. The fontawesome role keeps its own span-based node — it was never affected (already text-free); the two now share the design principle.
  • article-info icons migrated too, which fixes a latent bug: its sd-pr-2 padding class sat on the raw wrapper and was silently dropped by the HTML writer — it now lands on the <svg> element as intended.
  • Docs note for the icons not shown correctly in rst toctree #117 limitation: icon roles cannot appear in toctree entry titles (core Sphinx parses those as plain text) — the supported pattern is the icon in the target page's own H1.

Verification

  • Search index before/after: title-terms go from ~200 junk tokens to home, page, rocket; body/sidebar icon HTML is byte-identical (regenerated snippet fixtures are a pure rawsd_icon swap).
  • Independent builder sweep on Sphinx 7.4/8.2/9.x: html, dirhtml, singlehtml, epub (icon renders via the html handlers; nav/toc labels clean), json, pickle, latex, text, man, texinfo, gettext, linkcheck, xml (svg attribute escapes to valid, round-trippable XML), pseudoxml — no unknown-node warnings anywhere; env pickle/deepcopy round-trips preserve the icon; gettext extraction unchanged.
  • Full suite green on pinned Sphinx 7.4.7, 8.2.3, and 9.0.4 (142 passed each). Note: on Sphinx < 9 the search index's titles display field keeps the rendered HTML (upstream behaviour, predates this fix, harmless — icon shows in search results); the new test locks the cross-version titleterms fix and gates the Sphinx-9 plain-text-titles assertions accordingly.

Closes #99
Closes #117

@chrisjsewell chrisjsewell force-pushed the brief/12-icon-toctree branch from 7732ba1 to 30232c7 Compare July 12, 2026 17:49
@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.41%. Comparing base (c0acc48) to head (af77fb6).

Files with missing lines Patch % Lines
sphinx_design/icons.py 83.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #279      +/-   ##
==========================================
- Coverage   90.42%   90.41%   -0.02%     
==========================================
  Files          13       13              
  Lines        1149     1158       +9     
==========================================
+ Hits         1039     1047       +8     
- Misses        110      111       +1     
Flag Coverage Δ
pytests 90.41% <86.66%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Octicon and material icon roles returned a `nodes.raw` HTML node whose
`astext()` was the whole SVG markup, polluting `clean_astext` contexts
(toctree labels, search index titles/terms) when an icon began a title.
Introduce an inline `sd_icon` node that carries the SVG in an attribute
and has no Text children, rendered via an HTML visitor (other builders
skip it). Also migrate the article-info octicons, which restores their
`sd-pr-2` spacing class onto the rendered `<svg>` element.
@chrisjsewell chrisjsewell force-pushed the brief/12-icon-toctree branch from 30232c7 to af77fb6 Compare July 14, 2026 15:24
@chrisjsewell chrisjsewell merged commit 220407d into main Jul 14, 2026
25 checks passed
@chrisjsewell chrisjsewell deleted the brief/12-icon-toctree branch July 14, 2026 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

icons not shown correctly in rst toctree toctree skips an entry if its title begins with an inline icon directive

1 participant