Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config to add JS/CSS to all generated HTML pages #9115

Closed
humitos opened this issue Apr 19, 2021 · 5 comments · Fixed by #9174
Closed

Config to add JS/CSS to all generated HTML pages #9115

humitos opened this issue Apr 19, 2021 · 5 comments · Fixed by #9174
Labels
api type:enhancement enhance or introduce a new feature
Milestone

Comments

@humitos
Copy link
Contributor

humitos commented Apr 19, 2021

Is your feature request related to a problem? Please describe.

After #8631 was merged, JS/CSS files for MathJax are only added to those pages that contains math formulas. This is not a problem unless you want to embed the content of one of those pages where the JS/CSS are included from a different page.

This problem is clear on my extension sphinx-hoverxref that shows a tooltip with the content of the linked page when hovering over a link. Example: if you hover on "tooltip with Mathjax" in https://sphinx-hoverxref.readthedocs.io/en/sphinx-35/usage.html#tooltip-with-mathjax you won't see the MathJax rendered properly inside the tooltip

Screenshot_2021-04-19_11-02-47

and an error in the console "Not triggering MathJax because it is not defined". This is because MathJax is not included in usage.html page but it's included in target page (mathjax.html)

Note that I'm mentioning the problem only with MathJax for now since it's a known extension that it's using this new feature, but this applies to all the extension that start including the JS/CSS only on pages where the extension is present

Describe the solution you'd like

I think a config to force the old behavior that includes all the JS/CSS in all the pages could work. I know this has some drawbacks, but as far is optional, I think it's good.

Describe alternatives you've considered

  1. sphinx-hoverxref uses an endpoint from Read the Docs' API to get the content to embed in the tooltip. We are discussing if it's possible to return "extras": ["mathjax.js", "mathjax.css"] together with the content to be included so the JS client knows it has to include these assets before rendering the content. See Embed: design doc for new embed API readthedocs/readthedocs.org#8039 (comment)
  2. Another option could be to detect from the Sphinx extension "what are the assets required in the target page and add them into this page as well". Note that this won't work when using intersphinx.

Additional context

Reference: #8631 (comment)
Downstream issue: readthedocs/sphinx-hoverxref#119

@tk0miya
Copy link
Member

tk0miya commented Apr 25, 2021

My thought:

  • Only extensions would like to enable JS/CSS in the all pages. Users would not. So it's better to provide an API to switch the behavior instead of a configuration.
  • It's difficult to control the behavior by sphinx-core because extensions can control the visibility of the JS/CSS on their code, not sphinx-core.
    Here is an example of mathjax's case. It enables JS entry only if the document contains equations at least one.
    if domain.has_equations(pagename):
    # Enable mathjax only if equations exists
    options = {'async': 'async'}
    if app.config.mathjax_options:
    options.update(app.config.mathjax_options)
    app.add_js_file(app.config.mathjax_path, **options) # type: ignore
    if app.config.mathjax2_config:
    if app.config.mathjax_path == MATHJAX_URL:
    logger.warning(
    'mathjax_config/mathjax2_config does not work '
    'for the current MathJax version, use mathjax3_config instead')
    body = 'MathJax.Hub.Config(%s)' % json.dumps(app.config.mathjax2_config)
    app.add_js_file(None, type='text/x-mathjax-config', body=body)
    if app.config.mathjax3_config:
    body = 'window.MathJax = %s' % json.dumps(app.config.mathjax3_config)
    app.add_js_file(None, body=body)
  • So the following mechanism is needed to resolve this problem:
    • A new API like app.enable_html_assets_in_all_pages() to enable assets in all pages.
    • Extensions should refer the result of the API to control when they install assets in the specific pages.

@tk0miya tk0miya added this to the 4.1.0 milestone Apr 25, 2021
@tk0miya tk0miya added the api label Apr 25, 2021
@mgeier
Copy link
Contributor

mgeier commented Apr 25, 2021

BTW, a similar thing happens if there is :math:`a+b` in a page title, it will not be displayed in the TOC of other pages (if those don't use any math on their own).

I'm not sure if this should be a separate issue.
This is something that happens in Sphinx core, without extensions (except for sphinx.ext.mathjax).

@tk0miya
Copy link
Member

tk0miya commented Apr 25, 2021

It's a different topic. And it was already posted as #8139.

@humitos
Copy link
Contributor Author

humitos commented Apr 25, 2021

Thanks for sharing your thoughts @tk0miya! I think adding something like app.enable_html_assets_in_all_pages() is a good path forward. I understand the result of that would be able to be changed by one extension so all the other ones follow that rule, right? I imagine that if any extension defines enable_html_assets_in_all_pages = True every other extension will add assets to all the pages even if there is one extension that defines enable_html_assets_in_all_pages = False. So, True takes precedence.

humitos added a commit to humitos/sphinx that referenced this issue May 6, 2021
This new method in the `Sphinx` object allows extensions to communicate to
Sphinx that it's preferred to include HTML assets in all the pages. However,
it's extensions developers' responsability to follow this config and decide
whether or not include the assets required.

Extensions developers' can check `Sphinx.html_assets_in_all_pages` together with
any other logic they may have to decide if the assets will be included in the
rendered page or not.

Closes sphinx-doc#9115
humitos added a commit to humitos/sphinx that referenced this issue May 6, 2021
This new method in the `Sphinx` object allows extensions to communicate to
Sphinx that it's preferred to include HTML assets in all the pages. However,
it's extensions developers' responsability to follow this config and decide
whether or not include the assets required.

Extensions developers' can check `Sphinx.html_assets_in_all_pages` together with
any other logic they may have to decide if the assets will be included in the
rendered page or not.

Closes sphinx-doc#9115
@humitos
Copy link
Contributor Author

humitos commented May 6, 2021

@tk0miya I opened a PR with a proposal. Let's see if that's something similar to what you had in mind. Let me know 😄

humitos added a commit to humitos/sphinx-tabs that referenced this issue Jul 5, 2021
Sphinx v4.1.0 will include `Sphinx.registry.html_assets_policy` that defines the
policy to whether include or not HTML assets in pages where the extension is not
used.

This PR makes usage of this policy to decide if sphinx-tabs assets should be
included or not in the page that's being rendered.

Reference: sphinx-doc/sphinx#9115
humitos added a commit to humitos/sphinx-tabs that referenced this issue Jul 5, 2021
Sphinx v4.1.0 will include `Sphinx.registry.html_assets_policy` that defines the
policy to whether include or not HTML assets in pages where the extension is not
used.

This PR makes usage of this policy to decide if sphinx-tabs assets should be
included or not in the page that's being rendered.

Reference: sphinx-doc/sphinx#9115
humitos added a commit to humitos/sphinx-tabs that referenced this issue Jul 5, 2021
…sets

Sphinx v4.1.0 will include `Sphinx.registry.html_assets_policy` that defines the
policy to whether include or not HTML assets in pages where the extension is not
used.

This PR makes usage of this policy to decide if sphinx-tabs assets should be
included or not in the page that's being rendered.

Reference: sphinx-doc/sphinx#9115
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants