readthedocs flyout not visible on https://www.sphinx-doc.org/

[mgeier]

Describe the bug

IIRC, the site https://www.sphinx-doc.org/ previously had the readthedocs flyout menu in the bottom right of the page, which seems to be gone now.

I think there have been some changes regarding this on RTD recently, and it may be broken from their side.

I just wanted to report this, but maybe it will be fixed on its own soon?

How to Reproduce

Go to https://www.sphinx-doc.org/

Environment Information

website

Sphinx extensions

No response

Additional context

No response

[mgeier]

BTW, the search functionality also doesn't seem to work: https://www.sphinx-doc.org/en/master/search.html?q=search

[AA-Turner]

cc: @stsewd @humitos; please may you check that read the docs will be compatible with Sphinx 7.2 (to be released next week)?

A

[mattip]

Hmm. Maybe the project could be configured to add older versions which might aid debugging issues like these.

[humitos]

I took a look at this and found that the readthedocs-doc-embed.js is not injected in the resulting HTML. This file is automatically injected by the readthedocs-sphinx-ext dependency installed and configured in the project by using app.add_js_file(...): https://github.com/rtfd/readthedocs-sphinx-ext/blob/7c60d1646c12ac0b83d61abfbdd5bcd77d324124/readthedocs_ext/readthedocs.py#L112-L127

Does something changed in the way that these js files are injected? Note this is not a file on disk, but a URL: https://assets.readthedocs.org/static/javascript/readthedocs-doc-embed.js

It uses html-page-context event to trigger that code. Does this event change in some way?

[picnixz]

We changed the way Javascript and CSS files are represented internally. Before there was the Stylesheet and Javascript classes that behaved like a string containing the name of the file but now it's no more the case (see 1db8cba).

By the way, the sphinx_rtd_theme is failing as well because it uses pathto(css, 1) (sphinx_rtd_theme/layout.html) but css here is no more a string, so the pathto call fails.

[humitos]

This is the test that I'm doing:

1. install readthedocs-sphinx-ext

pip install readthedocs-sphinx-ext

2. configure it as a Sphinx extension

diff --git a/doc/conf.py b/doc/conf.py
index 85607681c..9e0c3dcc0 100644
--- a/doc/conf.py
+++ b/doc/conf.py
@@ -184,6 +184,8 @@ nitpick_ignore = {
 }
 
 
+extensions.insert(0, "readthedocs_ext.readthedocs")
+
 # -- Extension interface -------------------------------------------------------
 
 from sphinx import addnodes  # noqa: E402

3. build the documentation with extra verbosity

rm -rf _build
python3 ../sphinx/cmd/build.py -M html "." "_build" -vvv | grep "adding js_file"
...

[app] adding js_file: 'https://assets.readthedocs.org/static/javascript/readthedocs-doc-embed.js', {'priority': 500, 'async': 'async'}

....

4. Grep the resulting documentation to find that file

rg "readthedocs-doc-embed.js" _html/build

The file is not there for some reason.

[picnixz]

Are you able to bisect the culprit commit?

[humitos]

We changed the way Javascript and CSS files are represented internally. Before there was the Stylesheet and Javascript classes that behaved like a string containing the name of the file but now it's no more the case (see 1db8cba).

I understand this should not be a problem since we are using the app.add_js_file exposed API to add our js file, right?

[humitos]

By the way, the sphinx_rtd_theme is failing as well because it uses pathto(css, 1) (sphinx_rtd_theme/layout.html) but css here is no more a string, so the pathto call fails.

Can you open an issue on https://github.com/readthedocs/sphinx_rtd_theme about this?

[humitos]

Are you able to bisect the culprit commit?

v7.1.2 does not have this problem. I started the bisect between 7.1.2 and 7.2.0

I think I found the commit that breaks this integration: fa17437

code/sphinx/doc  fa1743725 ✔                                                                                                                                                         5d3h ◒  
▶ git bisect bad
fa17437254291a2a3a41da9dc0de6a768cec87d4 is the first bad commit
commit fa17437254291a2a3a41da9dc0de6a768cec87d4
Author: Adam Turner <9087854+aa-turner@users.noreply.github.com>
Date:   Sat Aug 12 08:50:33 2023 +0100

    Remove hidden pass-through calls when adding asset files
    
    ``Sphinx.add_css_file()`` called ``Builder.add_css_file()``
    and``Sphinx.add_js_file()`` called ``Builder.add_js_file()``.

 sphinx/application.py            |  6 ------
 sphinx/builders/html/__init__.py | 10 ++++++++--
 sphinx/ext/mathjax.py            |  8 +++++---
 3 files changed, 13 insertions(+), 11 deletions(-)
(python-3.10.8)
code/sphinx/doc  fa1743725 ✔                                                                                                                                                         5d3h ◒

[picnixz]

After some investigation, it appears that the script_files of the HTML context does not contain the new stylesheet.

Prior to the culprit commit, ctx['script_files'] is correctly updated and contains just the external URL string. After the commit, it no more contains it. This is what I think happens:

• app.add_js_files is called by RTD event handler.
• The file is added to app's registry.
• However, it is not injected into the HTML context.
• Why: before, app.add_js_config automatically added to the builder (namely, it modified builder._js_files as well). Now, ctx['script_files'] is actually a reference to builder._js_files. So, when using app.add_js_config, it also changed builder._js_config and ultimately ctx['script_files'] even if you never added explicitly to it.

@AA-Turner This is quite serious issue I think. I think that the previous behaviour is fine and it should be fine that app.add_js_file also adds the js files for the builder (unless you have a counterargument).

In addition, I saw that app.registry.js_files grows indefinitely which may causes a MemoryError if there are many pages that should be added.

[AA-Turner]

Fixed in Sphinx 7.2.1