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

Add a way to defer loading of MathJax #9450

Closed
cpitclaudel opened this issue Jul 14, 2021 · 7 comments
Closed

Add a way to defer loading of MathJax #9450

cpitclaudel opened this issue Jul 14, 2021 · 7 comments

Comments

@cpitclaudel
Copy link
Contributor

@cpitclaudel cpitclaudel commented Jul 14, 2021

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

It is quite tricky to configure MathJax to work with Sphinx currently.

Sphinx loads MathJax asynchronously since #3606 and #5005. While this was fine for MathJax 2, because of the special kind of <script> blocks mentioned in #5616 , it doesn't work well with MathJax 3.

Indeed, in MathJax 3, MathJax expect a config <script> block to be present before MathJax is loaded. Sphinx 4 added mathjax3_config parameter:

        if app.config.mathjax3_config:
            body = 'window.MathJax = %s' % json.dumps(app.config.mathjax3_config)
            app.add_js_file(None, body=body)

This assumes that the config is a simple dictionary, which isn't sufficient: that configuration should be able to contain functions, for example.

The only possibility at the moment is to add a separate script file containing a MathJax configuration and to load it with app.add_js_file.

Describe the solution you'd like

There are three possibilities:

  • Allow arbitrary strings for mathjax3_config, and in that case don't JSON-serialize them.

  • Change async to defer when loading MathJax.

  • Make it possible for users to change async to defer themselves. At the moment this isn't possible because the async flags is unconditionally added:

        if app.registry.html_assets_policy == 'always' or domain.has_equations(pagename):
          # Enable mathjax only if equations exists
          options = {'async': 'async'}
          if app.config.mathjax_options:
              options.update(app.config.mathjax_options)
    

The latter two are preferable because they would allow individual pages to use different MathJax config by using a .. raw:: block to override the default MathJax configuration on a given page (the script in that raw block will run before MathJax loads thanks to the defer option).

CC @jfbu , the author of #5616.

Thanks!

cpitclaudel added a commit to cpitclaudel/MyST-Parser that referenced this issue Jul 19, 2021
The global approach in 395 isn't really sustainable: it requires all-ways
cooperation between all projects that want to customize MathJax.  Additionally,
when processing a MyST document without Sphinx, the MathJax configuration
changes are not performed (part of executablebooks#347).  And, of course, this approach of
overriding the MathJax object causes issues down the line for projects that need
to customize MathJax (the setting in Sphinx isn't sufficient, see sphinx-doc/sphinx#9450)

The following two approaches would not cause these issues:

1. Add a custom script instead of touching the mathjax3_config variable;
   something like this, essentially:

   ```js
   app.add_js_file(None, priority=0, body="""
      var MathJax = window.MathJax || MathJax;
      MathJax.options = MathJax.options || {};
      MathJax.options.processHtmlClass = (MathJax.options.processHtmlClass || "")
      + "|math";
   """)
   ```

- Don't touch MathJax_config at all; instead, add an explicit `mathjax_process`
  class on all math nodes, either by changing `docutils_renderer` (this PR) or by
  adding a Docutils transform to processes all math nodes:

  ```python
  class ActivateMathJaxTransform(Transform):
      default_priority = 800

      @staticmethod
      def is_math(node):
          return isinstance(node, (math, math_block))

      def apply(self, **kwargs):
          for node in self.document.traverse(self.is_math):
              node.attributes.setdefault("classes", []).append("mathjax_process")
  ```

This PR isn't ready for merging; it's just to start a discussion.
@tk0miya tk0miya added this to the 4.2.0 milestone Jul 23, 2021
@tk0miya
Copy link
Member

@tk0miya tk0miya commented Jul 23, 2021

I'm not good at loading JS. Could you let me know the impact of changing async to defer? Are there any incompatible change for users? If not, we can change the loading option for MathJax to defer in the next version.

@tk0miya tk0miya removed this from the 4.2.0 milestone Sep 12, 2021
@tk0miya tk0miya added this to the 4.3.0 milestone Sep 12, 2021
@cpitclaudel
Copy link
Contributor Author

@cpitclaudel cpitclaudel commented Sep 13, 2021

I don't think it's an incompatible change. For MDN:

  • If the async attribute is present, then the script will be executed asynchronously as soon as it downloads.
  • If the async attribute is absent but the defer attribute is present, then the script is executed when the page has finished parsing.

So changing async to defer just rules out certain behaviors (the script starts executing before the whole page is parsed), it shouldn't add any new ones.

@tk0miya
Copy link
Member

@tk0miya tk0miya commented Nov 8, 2021

I found an explanation for this topic:

Note that here we use the defer attribute on both scripts so that they will execute in order, but still not block the rest of the page while the files are being downloaded to the browser. If the async attribute were used, there is no guarantee that the configuration would run first, and so you could get instances where MathJax doesn’t get properly configured, and they would seem to occur randomly.
https://docs.mathjax.org/en/latest/web/configuration.html#using-a-local-file-for-configuration

I believe using defer option instead is a good alternative.

@tk0miya tk0miya closed this in 4cd19b8 Nov 9, 2021
tk0miya added a commit that referenced this issue Nov 9, 2021
Close #9450: mathjax: Load MathJax via "defer" strategy
@raphaeltimbo
Copy link

@raphaeltimbo raphaeltimbo commented Nov 12, 2021

Is there a way to have the option to choose async instead of defer?
I am using plotly in my docs and, as discussed in #9563, plotly uses mathjax v2, so I have to force sphinx to also use mathjax v2.
The problem is that with 'defer' plotly graphs stopped rendering.

@tk0miya
Copy link
Member

@tk0miya tk0miya commented Nov 17, 2021

@raphaeltimbo Could you file a new issue, please? I'll fix it soon.

@raphaeltimbo
Copy link

@raphaeltimbo raphaeltimbo commented Nov 18, 2021

@tk0miya, I have filed a new issue as requested. Let me know if you need additional information.
Thank you!

@tk0miya
Copy link
Member

@tk0miya tk0miya commented Nov 18, 2021

Thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants