From a9f4a11b63ec8388816a0baccda3b99f08986575 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 30 Aug 2023 03:48:28 +0100 Subject: [PATCH 1/7] Special case ``MathJax.js?config=`` --- sphinx/builders/html/_assets.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sphinx/builders/html/_assets.py b/sphinx/builders/html/_assets.py index a72c5000bbc..26eff503b8d 100644 --- a/sphinx/builders/html/_assets.py +++ b/sphinx/builders/html/_assets.py @@ -134,6 +134,12 @@ def _file_checksum(outdir: Path, filename: str | os.PathLike[str]) -> str: # As we cannot safely strip the query string, # raise an error to the user. if '?' in filename: + if 'MathJax.js?' in filename: + # MathJax v2 reads a ``?config=...`` query parameter, + # special case this and just skip adding the checksum. + # https://docs.mathjax.org/en/v2.7-latest/configuration.html#considerations-for-using-combined-configuration-files + # https://github.com/sphinx-doc/sphinx/issues/11658 + return '' msg = f'Local asset file paths must not contain query strings: {filename!r}' raise ThemeError(msg) try: From 15c460120ad6ba51860861d867b3b3c83f7ba9f2 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 30 Aug 2023 03:54:33 +0100 Subject: [PATCH 2/7] Alternate implementation: warning --- sphinx/builders/html/_assets.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/sphinx/builders/html/_assets.py b/sphinx/builders/html/_assets.py index 26eff503b8d..93c17b1beaf 100644 --- a/sphinx/builders/html/_assets.py +++ b/sphinx/builders/html/_assets.py @@ -6,7 +6,10 @@ from typing import TYPE_CHECKING from sphinx.deprecation import RemovedInSphinx90Warning -from sphinx.errors import ThemeError +from sphinx.locale import __ +from sphinx.util import logging + +logger = logging.getLogger(__name__) if TYPE_CHECKING: from pathlib import Path @@ -134,14 +137,9 @@ def _file_checksum(outdir: Path, filename: str | os.PathLike[str]) -> str: # As we cannot safely strip the query string, # raise an error to the user. if '?' in filename: - if 'MathJax.js?' in filename: - # MathJax v2 reads a ``?config=...`` query parameter, - # special case this and just skip adding the checksum. - # https://docs.mathjax.org/en/v2.7-latest/configuration.html#considerations-for-using-combined-configuration-files - # https://github.com/sphinx-doc/sphinx/issues/11658 - return '' - msg = f'Local asset file paths must not contain query strings: {filename!r}' - raise ThemeError(msg) + logger.warning(__('Local asset file paths must not contain query strings: %r'), + filename, type='misc', subtype='asset_checksum') + return '' try: # Remove all carriage returns to avoid checksum differences content = outdir.joinpath(filename).read_bytes().translate(None, b'\r') From abbf657a50f103283bc827c380d8ef4284a6bd50 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 30 Aug 2023 19:02:06 +0100 Subject: [PATCH 3/7] Revert "Alternate implementation: warning" This reverts commit 15c460120ad6ba51860861d867b3b3c83f7ba9f2. --- sphinx/builders/html/_assets.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/sphinx/builders/html/_assets.py b/sphinx/builders/html/_assets.py index 93c17b1beaf..26eff503b8d 100644 --- a/sphinx/builders/html/_assets.py +++ b/sphinx/builders/html/_assets.py @@ -6,10 +6,7 @@ from typing import TYPE_CHECKING from sphinx.deprecation import RemovedInSphinx90Warning -from sphinx.locale import __ -from sphinx.util import logging - -logger = logging.getLogger(__name__) +from sphinx.errors import ThemeError if TYPE_CHECKING: from pathlib import Path @@ -137,9 +134,14 @@ def _file_checksum(outdir: Path, filename: str | os.PathLike[str]) -> str: # As we cannot safely strip the query string, # raise an error to the user. if '?' in filename: - logger.warning(__('Local asset file paths must not contain query strings: %r'), - filename, type='misc', subtype='asset_checksum') - return '' + if 'MathJax.js?' in filename: + # MathJax v2 reads a ``?config=...`` query parameter, + # special case this and just skip adding the checksum. + # https://docs.mathjax.org/en/v2.7-latest/configuration.html#considerations-for-using-combined-configuration-files + # https://github.com/sphinx-doc/sphinx/issues/11658 + return '' + msg = f'Local asset file paths must not contain query strings: {filename!r}' + raise ThemeError(msg) try: # Remove all carriage returns to avoid checksum differences content = outdir.joinpath(filename).read_bytes().translate(None, b'\r') From 5535f07f09e42225c414b47595be9cc50d6fb4f2 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 30 Aug 2023 19:04:07 +0100 Subject: [PATCH 4/7] move special casing --- sphinx/builders/html/__init__.py | 8 +++++++- sphinx/builders/html/_assets.py | 6 ------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/sphinx/builders/html/__init__.py b/sphinx/builders/html/__init__.py index 3515edd44aa..d677a1b44c9 100644 --- a/sphinx/builders/html/__init__.py +++ b/sphinx/builders/html/__init__.py @@ -1085,7 +1085,13 @@ def js_tag(js: _JavaScript | str) -> str: return f'' uri = pathto(os.fspath(js.filename), resource=True) - if checksum := _file_checksum(outdir, js.filename): + if 'MathJax.js?' not in js.filename: + # MathJax v2 reads a ``?config=...`` query parameter, + # special case this and just skip adding the checksum. + # https://docs.mathjax.org/en/v2.7-latest/configuration.html#considerations-for-using-combined-configuration-files + # https://github.com/sphinx-doc/sphinx/issues/11658 + pass + elif checksum := _file_checksum(outdir, js.filename): uri += f'?v={checksum}' if attrs: return f'' diff --git a/sphinx/builders/html/_assets.py b/sphinx/builders/html/_assets.py index 26eff503b8d..a72c5000bbc 100644 --- a/sphinx/builders/html/_assets.py +++ b/sphinx/builders/html/_assets.py @@ -134,12 +134,6 @@ def _file_checksum(outdir: Path, filename: str | os.PathLike[str]) -> str: # As we cannot safely strip the query string, # raise an error to the user. if '?' in filename: - if 'MathJax.js?' in filename: - # MathJax v2 reads a ``?config=...`` query parameter, - # special case this and just skip adding the checksum. - # https://docs.mathjax.org/en/v2.7-latest/configuration.html#considerations-for-using-combined-configuration-files - # https://github.com/sphinx-doc/sphinx/issues/11658 - return '' msg = f'Local asset file paths must not contain query strings: {filename!r}' raise ThemeError(msg) try: From 599090c46254b454687d94ac42cefc85e6bf49e6 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 30 Aug 2023 19:21:23 +0100 Subject: [PATCH 5/7] tests --- sphinx/builders/html/__init__.py | 2 +- tests/test_ext_math.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/sphinx/builders/html/__init__.py b/sphinx/builders/html/__init__.py index d677a1b44c9..46e66be6d72 100644 --- a/sphinx/builders/html/__init__.py +++ b/sphinx/builders/html/__init__.py @@ -1085,7 +1085,7 @@ def js_tag(js: _JavaScript | str) -> str: return f'' uri = pathto(os.fspath(js.filename), resource=True) - if 'MathJax.js?' not in js.filename: + if 'MathJax.js?' in js.filename: # MathJax v2 reads a ``?config=...`` query parameter, # special case this and just skip adding the checksum. # https://docs.mathjax.org/en/v2.7-latest/configuration.html#considerations-for-using-combined-configuration-files diff --git a/tests/test_ext_math.py b/tests/test_ext_math.py index 9e2367fe3fd..d5331f839bf 100644 --- a/tests/test_ext_math.py +++ b/tests/test_ext_math.py @@ -283,6 +283,34 @@ def test_mathjax_options_defer_for_mathjax2(app, status, warning): assert ('' in content + + +@pytest.mark.sphinx( + 'html', testroot='ext-math', + confoverrides={ + 'extensions': ['sphinx.ext.mathjax'], + 'mathjax_path': 'MathJax.js?config=scipy-mathjax', + }, +) +def test_mathjax_path_config(app): + app.builder.build_all() + + content = (app.outdir / 'index.html').read_text(encoding='utf8') + assert '' in content + + @pytest.mark.sphinx('html', testroot='ext-math', confoverrides={'extensions': ['sphinx.ext.mathjax']}) def test_mathjax_is_installed_only_if_document_having_math(app, status, warning): From b207d0dd564b2cb75462f6e2f0f2da6050b20a01 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 30 Aug 2023 19:26:12 +0100 Subject: [PATCH 6/7] typing --- sphinx/builders/html/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/builders/html/__init__.py b/sphinx/builders/html/__init__.py index 46e66be6d72..85067be0178 100644 --- a/sphinx/builders/html/__init__.py +++ b/sphinx/builders/html/__init__.py @@ -1085,7 +1085,7 @@ def js_tag(js: _JavaScript | str) -> str: return f'' uri = pathto(os.fspath(js.filename), resource=True) - if 'MathJax.js?' in js.filename: + if 'MathJax.js?' in os.fspath(js.filename): # MathJax v2 reads a ``?config=...`` query parameter, # special case this and just skip adding the checksum. # https://docs.mathjax.org/en/v2.7-latest/configuration.html#considerations-for-using-combined-configuration-files From 4a63651088ebcee8cdb799ae996ede9bcdf8a803 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 30 Aug 2023 19:34:11 +0100 Subject: [PATCH 7/7] CHANGES --- CHANGES | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES b/CHANGES index 464cd71fec0..109266dd33c 100644 --- a/CHANGES +++ b/CHANGES @@ -20,6 +20,7 @@ Bugs fixed packages that make use of ``if typing.TYPE_CHECKING:`` to guard circular imports needed by type checkers. Patch by Matt Wozniski. +* #11659: Allow ``?config=...`` in :confval:`mathjax_path`. Testing -------