Fix Sphinx 9 compatibility for CSS and JS asset rendering by dpinte · Pull Request #25 · enthought/enthought-sphinx-theme

dpinte · 2026-02-10T15:40:49Z

Summary

In Sphinx 9, css_files and script_files contain _CascadingStyleSheet and _JavaScript objects instead of plain strings. Passing these to pathto() causes TypeError: argument of type '_CascadingStyleSheet' is not iterable.
Use css_tag() / js_tag() (available since Sphinx 7) to render asset objects, falling back to raw <link>/<script> tags for plain strings (older Sphinx). This mirrors Sphinx's own basic/layout.html approach.
Fixes the doc build for downstream projects (e.g., traits) that resolve to Sphinx 9 via Sphinx>=2.1.0.

Test plan

Verified traits doc build succeeds with Sphinx 9.1.0 on Python 3.13

🤖 Generated with Claude Code

In Sphinx 9, css_files and script_files contain _CascadingStyleSheet and _JavaScript objects instead of plain strings. Passing these objects to pathto() causes a TypeError because they don't support the `in` operator or string concatenation. Use css_tag() / js_tag() (available since Sphinx 7) to render asset objects, falling back to raw <link>/<script> tags for plain strings (older Sphinx versions). This mirrors the approach used in Sphinx's own basic/layout.html template. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

mdickinson · 2026-02-10T15:58:48Z

Verified traits doc build succeeds with Sphinx 9.1.0 on Python 3.13

It would be good to see evidence of this. :-)

I'll have a go at replicating the Traits doc build myself locally.

mdickinson · 2026-02-11T08:06:49Z

Test run for Traits documentation build here: enthought/traits#1879. The build succeeds modulo some referencing issues that have nothing to do with the theme.

mdickinson

LGTM

mdickinson · 2026-02-11T08:11:31Z

Note: pathto is documented here: https://github.com/sphinx-doc/sphinx/blob/master/doc/development/html_themes/templating.rst#helper-functions

mdickinson · 2026-02-11T08:21:06Z

Before this goes in, we should document the new requirement for Sphinx >= 7.0.

Copilot

Pull request overview

Updates the theme’s asset rendering to be compatible with Sphinx 9, where css_files / script_files may contain asset objects instead of plain strings, avoiding TypeError from passing objects into pathto().

Changes:

Render JS assets via js_tag() when a Sphinx asset object is provided; otherwise fall back to <script src="{{ pathto(...) }}">.
Render CSS assets via css_tag() when a Sphinx asset object is provided; otherwise fall back to <link href="{{ pathto(...) }}">.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T08:47:35Z

enthought_sphinx_theme/enthought/layout.html

+    {%- for js in script_files %}
+      {%- if js|attr("filename") is not none %}
+    {{ js_tag(js) }}
+      {%- else %}
+    <script type="text/javascript" src="{{ pathto(js, 1) }}"></script>
+      {%- endif %}
    {%- endfor %}


js|attr("filename") is not none will also be true for plain string entries because attr("filename") typically yields an Undefined value (which is not none). That would route strings into js_tag(js), likely failing and reintroducing the incompatibility. Prefer a type-based check (e.g., test whether js is a string and use pathto only in that case) or explicitly check that the attribute is defined (e.g., js|attr("filename") is defined and not none). Apply the same approach consistently to CSS/JS handling to avoid subtle differences in behavior.

Hmm. This does seem like a valid point. IIUC, the code is attempting to be backwards compatible with older versions of Sphinx - for those older versions, js is a plain string and so js|attr("filename") will be Undefined (at least according to the docs). Which is not none. If this analysis is correct, we never actually take the fallback path.

@dpinte What does Claude say about this?

Claude said you and copilot are right. It also inspected the standard upstream themes and verified how the checks were done. It seems they were not checking anything and thus just called js_tag(js). We'll remove those.

Thanks, @dpinte.

The fallback code was a good idea, though - it saves us from having to worry about pinning a minimum Sphinx version. (Of course, the upstream code doesn't need to worry about this - it's shipped with Sphinx, so it can assume the latest Sphinx bells and whistles.)

I'll update the PR to do the right thing (which is to just remove the is not none check).

mdickinson · 2026-02-11T09:23:36Z

Use css_tag() / js_tag() (available since Sphinx 7)

The "Sphinx 7" here seems to be a bit of a hallucination. As I understand it, css_tag and js_tag go back at least as far as Sphinx 4.5: https://github.com/sphinx-doc/sphinx/blob/2329fdef8c20c6c75194f5d842b8f62ebad5c79d/sphinx/builders/html/__init__.py#L1154

…irement The js|attr("filename") is not none check was always true (Jinja2 Undefined is not none), so the pathto() fallback was never taken. Use js_tag() unconditionally for JS assets, matching Sphinx's own basic/layout.html. Add Sphinx >= 4.5 to install_requires and document the requirement. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

mdickinson · 2026-02-16T08:21:41Z

Before this goes in, we should document the new requirement for Sphinx >= 7.0.

No we shouldn't: I misunderstood what the new code is doing - there's a fallback path for plain string objects, which should make the code backwards compatible with earlier versions of Sphinx.

mdickinson · 2026-02-16T08:25:37Z

I misunderstood what the new code is doing - there's a fallback path for plain string objects

Ah, no: Claude has removed that fallback path in the latest commit (though it's only removed the fallback for JS files, and left it in for CSS files). I'll make a manual update to get us to the right place.

mdickinson · 2026-02-16T08:30:24Z

Fallback path restored in 88c6f4f. The treatment of JS files and CSS files are now consistent with one another.

mdickinson · 2026-02-16T08:46:23Z

Bumped the version number in setup.py so that this change is ready for immediate release (see 3f2ea65)

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-16T08:50:44Z

CHANGES.rst

+Release 0.7.4
+-------------
+
+Changes
+
+* Fix Sphinx 9 compatibility for CSS and JS asset rendering. (#25)
+


The new 0.7.4 changelog section is missing a “Release date:” line, while all existing releases in this file include one. To keep the changelog consistent (and avoid ambiguity around whether 0.7.4 is actually released), add a release date or explicitly mark the section as unreleased.

Addressed in acda488

dpinte mentioned this pull request Feb 10, 2026

Documentation build fails on Python 3.13 enthought/traits#1872

Open

mdickinson mentioned this pull request Feb 10, 2026

Update Python version for the documentation build to 3.13 enthought/traits#1879

Draft

mdickinson approved these changes Feb 11, 2026

View reviewed changes

mdickinson requested a review from Copilot February 11, 2026 08:46

Copilot AI reviewed Feb 11, 2026

View reviewed changes

mdickinson mentioned this pull request Feb 11, 2026

Can't build the documentation with Sphinx 9.1.0 - theme error happening enthought/apptools#368

Closed

Restore fallback path, but without the 'is not none'

88c6f4f

mdickinson added 3 commits February 16, 2026 08:42

Revert Sphinx requirement

0d83965

Revert unnecessary change in README.rst

f0e5c7f

Bump version number in setup.py

3f2ea65

mdickinson requested a review from Copilot February 16, 2026 08:47

Copilot started reviewing on behalf of mdickinson February 16, 2026 08:48 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

Add release date

acda488

mdickinson merged commit c154c7d into main Feb 16, 2026

mdickinson deleted the fix/sphinx-9-compatibility branch February 16, 2026 08:52

Conversation

dpinte commented Feb 10, 2026

Summary

Test plan

Uh oh!

mdickinson commented Feb 10, 2026

Uh oh!

mdickinson commented Feb 11, 2026

Uh oh!

mdickinson left a comment

Choose a reason for hiding this comment

Uh oh!

mdickinson commented Feb 11, 2026

Uh oh!

mdickinson commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

mdickinson Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

dpinte Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

mdickinson Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

mdickinson commented Feb 11, 2026

Uh oh!

mdickinson commented Feb 16, 2026

Uh oh!

mdickinson commented Feb 16, 2026

Uh oh!

mdickinson commented Feb 16, 2026

Uh oh!

mdickinson commented Feb 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

mdickinson Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants