From 22cd08ff1ca417eb3e70dea3c3d61f7e5eb026eb Mon Sep 17 00:00:00 2001 From: Kai Welke Date: Sun, 16 Apr 2023 07:24:16 +0200 Subject: [PATCH] fix: canonical links (#1262) Upstream Sphinx doesn't construct canonical links correctly for the DirectoryHTMLBuilder. When I introduced #650, it included a bug for `index.html` pages. This PR adds a proper fix with tests this time to make sure that canonical links are what they should be. This PR is a follow up of #1258 and reverts the change in `layout.html`. Checking the canonical link shouldn't be done in the template. It's reported here: [#9730](https://github.com/sphinx-doc/sphinx/issues/9730) --- CHANGELOG.md | 4 + constraints.txt | 2 +- pyproject.toml | 2 +- requirements.txt | 2 +- src/sphinxawesome_theme/jinja_functions.py | 17 ++- src/sphinxawesome_theme/layout.html | 121 ++++++++++----------- src/theme-src/yarn.lock | 28 ++--- tests/test_basic.py | 2 +- tests/test_canonical.py | 77 +++++++++++++ 9 files changed, 171 insertions(+), 84 deletions(-) create mode 100644 tests/test_canonical.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 761bac61e..7d05c6e05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 4.0.3 + +- Fix canonical links + ## 4.0.2 - Restore missing logo (#1214) diff --git a/constraints.txt b/constraints.txt index cddf577f7..8ed086e14 100644 --- a/constraints.txt +++ b/constraints.txt @@ -2,4 +2,4 @@ nox==2022.11.21 nox-poetry==1.0.2 pip==23.0.1 pipx==1.2.0 -poetry==1.4.1 +poetry==1.4.2 diff --git a/pyproject.toml b/pyproject.toml index 1bd080351..d1a75ad82 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "sphinxawesome-theme" -version = "4.0.2" +version = "4.0.3" description = "An awesome theme for the Sphinx documentation generator" readme = "README.md" authors = ["Kai Welke "] diff --git a/requirements.txt b/requirements.txt index df5f36e8c..3134feadf 100644 --- a/requirements.txt +++ b/requirements.txt @@ -39,7 +39,7 @@ python-dotenv==1.0.0; python_version >= "3.8" and python_version < "4.0" pytz==2023.3; python_version >= "3.8" and python_version < "3.9" pyyaml==6.0; python_version >= "3.8" and python_version < "4.0" requests==2.28.2; python_version >= "3.8" and python_version < "4" -ruff==0.0.260; python_version >= "3.8" and python_version < "4.0" +ruff==0.0.261; python_version >= "3.8" and python_version < "4.0" setuptools==67.6.1; python_version >= "3.8" and python_version < "4.0" six==1.16.0; python_version >= "3.8" and python_version < "4.0" snowballstemmer==2.2.0; python_version >= "3.8" and python_version < "4.0" diff --git a/src/sphinxawesome_theme/jinja_functions.py b/src/sphinxawesome_theme/jinja_functions.py index e1b9b5664..6b8f4762f 100644 --- a/src/sphinxawesome_theme/jinja_functions.py +++ b/src/sphinxawesome_theme/jinja_functions.py @@ -44,8 +44,6 @@ def _get_manifest_json(app: Sphinx) -> Any: if path.isfile(manifest): with open(manifest) as m: return json.load(m) - else: - return {} else: return {} @@ -58,6 +56,17 @@ def _make_asset_url(app: Sphinx, asset: str) -> Any: return manifest.get(asset, asset) +def _make_canonical(app: Sphinx, pagename: str) -> str: + """Turn a filepath into the correct canonical link. + + Upstream Sphinx builds the wrong canonical links for the ``dirhtml`` builder. + """ + canonical = posixpath.join(app.config.html_baseurl, pagename.replace("index", "")) + if not canonical.endswith("/"): + canonical += "/" + return canonical + + def setup_jinja( app: Sphinx, pagename: str, @@ -70,8 +79,8 @@ def setup_jinja( app.builder.templates.environment.filters["sanitize"] = _make_id_from_title context["asset"] = partial(_make_asset_url, app) # must override `pageurl` for directory builder - if app.builder.name == "dirhtml": - context["pageurl"] = posixpath.join(app.config.html_baseurl, pagename + "/") + if app.builder.name == "dirhtml" and app.config.html_baseurl: + context["pageurl"] = _make_canonical(app, pagename) def setup(app: Sphinx) -> Dict[str, Any]: diff --git a/src/sphinxawesome_theme/layout.html b/src/sphinxawesome_theme/layout.html index 24120f546..a7ff098f9 100644 --- a/src/sphinxawesome_theme/layout.html +++ b/src/sphinxawesome_theme/layout.html @@ -12,76 +12,73 @@ {%- set lang_attr = "en" if language == None else (language|replace('_','-')) -%} {%- if not embedded and docstitle -%} - {%- if title == docstitle -%} - {%- set titlesuffix = "" -%} - {%- else -%} - {%- set titlesuffix = " | "|safe + docstitle|e -%} - {%- endif -%} +{%- if title == docstitle -%} +{%- set titlesuffix = "" -%} {%- else -%} - {%- set titlesuffix = "" -%} +{%- set titlesuffix = " | "|safe + docstitle|e -%} +{%- endif -%} +{%- else -%} +{%- set titlesuffix = "" -%} {%- endif -%} - - - - {{ metatags }} - {%- block htmltitle %} - {{ title|striptags|e if title else docstitle }}{{ titlesuffix }} - {%- endblock %} - {#- Extra CSS files for overriding stuff #} - {%- for css in css_files %} - - {%- endfor %} - {%- if docsearch %} - - {% endif %} - {%- if pageurl and pageurl.endswith("index/") %} - - {%- elif pageurl %} - - {%- endif %} - {%- set _favicon_url = favicon_url | default(pathto('_static/' + (favicon or ""), 1)) %} - {%- if favicon_url or favicon %} - - {%- endif %} - {%- block linktags %} - {%- if hasdoc('search') and not docsearch %} - - {%- endif %} - {%- if hasdoc('genindex') %} - - {%- endif %} - {%- if next %} - - {%- endif %} - {%- if prev %} - - {%- endif %} - {%- endblock %} - - -
- {% include "skip.html" %} + + + + {{ metatags }} + {%- block htmltitle %} + {{ title|striptags|e if title else docstitle }}{{ titlesuffix }} + {%- endblock %} + {#- Extra CSS files for overriding stuff #} + {%- for css in css_files %} + + {%- endfor %} + {%- if docsearch %} + + {% endif %} + {%- if pageurl %} + + {%- endif %} + {%- set _favicon_url = favicon_url | default(pathto('_static/' + (favicon or ""), 1)) %} + {%- if favicon_url or favicon %} + + {%- endif %} + {%- block linktags %} + {%- if hasdoc('search') and not docsearch %} + + {%- endif %} + {%- if hasdoc('genindex') %} + + {%- endif %} + {%- if next %} + + {%- endif %} + {%- if prev %} + + {%- endif %} + {%- endblock %} + -
- {% include "header.html" %} -
+ +
+ {% include "skip.html" %} - {% block document %} - {% endblock %} +
+ {% include "header.html" %} +
- {% include "modals.html" %} -
- {% block scripts %} - {{ script() }} + {% block document %} {% endblock %} - + + {% include "modals.html" %} +
+ {% block scripts %} + {{ script() }} + {% endblock %} + + diff --git a/src/theme-src/yarn.lock b/src/theme-src/yarn.lock index e899847a4..9292fabcc 100644 --- a/src/theme-src/yarn.lock +++ b/src/theme-src/yarn.lock @@ -1066,7 +1066,7 @@ resolved "https://registry.yarnpkg.com/@csstools/css-tokenizer/-/css-tokenizer-2.1.1.tgz#07ae11a0a06365d7ec686549db7b729bc036528e" integrity sha512-GbrTj2Z8MCTUv+52GE0RbFGM527xuXZ0Xa5g0Z+YN573uveS4G0qi6WNOMyz3yrFM/jaILTTwJ0+umx81EzqfA== -"@csstools/media-query-list-parser@^2.0.1", "@csstools/media-query-list-parser@^2.0.4": +"@csstools/media-query-list-parser@^2.0.2", "@csstools/media-query-list-parser@^2.0.4": version "2.0.4" resolved "https://registry.yarnpkg.com/@csstools/media-query-list-parser/-/media-query-list-parser-2.0.4.tgz#466bd254041530dfd1e88bcb1921e8ca4af75b6a" integrity sha512-GyYot6jHgcSDZZ+tLSnrzkR7aJhF2ZW6d+CXH66mjy5WpAQhZD4HDke2OQ36SivGRWlZJpAz7TzbW6OKlEpxAA== @@ -2490,7 +2490,7 @@ eslint-webpack-plugin@^4.0.0: normalize-path "^3.0.0" schema-utils "^4.0.0" -eslint@^8.36.0: +eslint@^8.38.0: version "8.38.0" resolved "https://registry.yarnpkg.com/eslint/-/eslint-8.38.0.tgz#a62c6f36e548a5574dd35728ac3c6209bd1e2f1a" integrity sha512-pIdsD2jwlUGf/U38Jv97t8lq6HpaU/G9NKbYmpWpZGw3LdTNhZLbJePqxOXGB5+JEKfOPU/XLxYxFh03nr1KTg== @@ -2860,7 +2860,7 @@ hosted-git-info@^4.0.1: dependencies: lru-cache "^6.0.0" -html-tags@^3.2.0: +html-tags@^3.3.1: version "3.3.1" resolved "https://registry.yarnpkg.com/html-tags/-/html-tags-3.3.1.tgz#a04026a18c882e4bba8a01a3d39cfe465d40b5ce" integrity sha512-ztqyC3kLto0e9WbNp0aeP+M3kTt+nbaIveGmUxAtZa+8iFgKLUOD4YKM5j+f3QD89bra7UeumolZHKuOXnTmeQ== @@ -4534,13 +4534,13 @@ stylelint-webpack-plugin@^4.1.0: schema-utils "^4.0.0" stylelint@^15.4.0: - version "15.4.0" - resolved "https://registry.yarnpkg.com/stylelint/-/stylelint-15.4.0.tgz#3958fff41fbcd68cf947fdecb329762d45f87013" - integrity sha512-TlOvpG3MbcFwHmK0q2ykhmpKo7Dq892beJit0NPdpyY9b1tFah/hGhqnAz/bRm2PDhDbJLKvjzkEYYBEz7Dxcg== + version "15.5.0" + resolved "https://registry.yarnpkg.com/stylelint/-/stylelint-15.5.0.tgz#f16c238231f3f32e62da8a88969821d237eae8a6" + integrity sha512-jyMO3R1QtE5mUS4v40+Gg+sIQBqe7CF1xPslxycDzNVkIBCUD4O+5F1vLPq16VmunUTv4qG9o2rUKLnU5KkVeQ== dependencies: "@csstools/css-parser-algorithms" "^2.1.0" "@csstools/css-tokenizer" "^2.1.0" - "@csstools/media-query-list-parser" "^2.0.1" + "@csstools/media-query-list-parser" "^2.0.2" "@csstools/selector-specificity" "^2.2.0" balanced-match "^2.0.0" colord "^2.9.3" @@ -4554,7 +4554,7 @@ stylelint@^15.4.0: global-modules "^2.0.0" globby "^11.1.0" globjoin "^0.1.4" - html-tags "^3.2.0" + html-tags "^3.3.1" ignore "^5.2.4" import-lazy "^4.0.0" imurmurhash "^0.1.4" @@ -4809,9 +4809,9 @@ unicode-property-aliases-ecmascript@^2.0.0: integrity sha512-6t3foTQI9qne+OZoVQB/8x8rk2k1eVy1gRXhV3oFQ5T6R1dqQ1xtin3XqSlx3+ATBkliTaR/hHyJBm+LVPNM8w== update-browserslist-db@^1.0.10: - version "1.0.10" - resolved "https://registry.yarnpkg.com/update-browserslist-db/-/update-browserslist-db-1.0.10.tgz#0f54b876545726f17d00cd9a2561e6dade943ff3" - integrity sha512-OztqDenkfFkbSG+tRxBeAnCVPckDBcvibKd35yDONx6OU8N7sqgwc7rCbkJ/WcYtVRZ4ba68d6byhC21GFh7sQ== + version "1.0.11" + resolved "https://registry.yarnpkg.com/update-browserslist-db/-/update-browserslist-db-1.0.11.tgz#9a2a641ad2907ae7b3616506f4b977851db5b940" + integrity sha512-dCwEFf0/oT85M1fHBg4F0jtLwJrutGoHSQXCh7u4o2t1drG+c0a9Flnqww6XUKSfQMPpJBRjU8d4RXB09qtvaA== dependencies: escalade "^3.1.1" picocolors "^1.0.0" @@ -4898,9 +4898,9 @@ webpack-sources@^3.2.3: integrity sha512-/DyMEOrDgLKKIG0fmvtz+4dUX/3Ghozwgm6iPp8KRhvn+eQf9+Q7GWxVNMk3+uCPWfdXYC4ExGBckIXdFEfH1w== webpack@^5.78.0: - version "5.78.0" - resolved "https://registry.yarnpkg.com/webpack/-/webpack-5.78.0.tgz#836452a12416af2a7beae906b31644cb2562f9e6" - integrity sha512-gT5DP72KInmE/3azEaQrISjTvLYlSM0j1Ezhht/KLVkrqtv10JoP/RXhwmX/frrutOPuSq3o5Vq0ehR/4Vmd1g== + version "5.79.0" + resolved "https://registry.yarnpkg.com/webpack/-/webpack-5.79.0.tgz#8552b5da5a26e4e25842c08a883e08fc7740547a" + integrity sha512-3mN4rR2Xq+INd6NnYuL9RC9GAmc1ROPKJoHhrZ4pAjdMFEkJJWrsPw8o2JjCIyQyTu7rTXYn4VG6OpyB3CobZg== dependencies: "@types/eslint-scope" "^3.7.3" "@types/estree" "^1.0.0" diff --git a/tests/test_basic.py b/tests/test_basic.py index bcc66ccdc..f4048955e 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -10,7 +10,7 @@ def test_returns_version() -> None: """It has the correct version.""" - assert sphinxawesome_theme.__version__ == "4.0.2" + assert sphinxawesome_theme.__version__ == "4.0.3" @pytest.mark.sphinx("dummy") diff --git a/tests/test_canonical.py b/tests/test_canonical.py new file mode 100644 index 000000000..2e49aafd4 --- /dev/null +++ b/tests/test_canonical.py @@ -0,0 +1,77 @@ +"""Test the construction of canonical links.""" + +import os +from pathlib import Path + +import pytest +from sphinx.application import Sphinx + +from .util import parse_html + + +@pytest.mark.sphinx( + "html", + confoverrides={ + "extensions": ["sphinxawesome_theme"], + "html_theme": "sphinxawesome_theme", + }, +) +def test_no_canonical_links_in_html(app: Sphinx) -> None: + """It doesn't add canonical links by default.""" + app.build() + assert os.path.exists(Path(app.outdir) / "index.html") + tree = parse_html(Path(app.outdir) / "index.html") + link = tree.select("[rel=canonical]") + assert len(link) == 0 + + +@pytest.mark.sphinx( + "dirhtml", + confoverrides={ + "extensions": ["sphinxawesome_theme"], + "html_theme": "sphinxawesome_theme", + }, +) +def test_no_canonical_links_in_dirhtml(app: Sphinx) -> None: + """It doesn't add canonical links by default.""" + app.build() + assert os.path.exists(Path(app.outdir) / "index.html") + tree = parse_html(Path(app.outdir) / "index.html") + link = tree.select("[rel=canonical]") + assert len(link) == 0 + + +@pytest.mark.sphinx( + "html", + confoverrides={ + "extensions": ["sphinxawesome_theme"], + "html_theme": "sphinxawesome_theme", + "html_baseurl": "https://test.org", + }, +) +def test_canonical_links_in_html(app: Sphinx) -> None: + """It adds the correct canonical link for the HTML builder.""" + app.build() + assert os.path.exists(Path(app.outdir) / "index.html") + tree = parse_html(Path(app.outdir) / "index.html") + link = tree.select("[rel=canonical]") + assert len(link) == 1 + assert link[0]["href"] == "https://test.org/index.html" + + +@pytest.mark.sphinx( + "dirhtml", + confoverrides={ + "extensions": ["sphinxawesome_theme"], + "html_theme": "sphinxawesome_theme", + "html_baseurl": "https://test.org", + }, +) +def test_canonical_links_in_dirhtml(app: Sphinx) -> None: + """It adds the correct canonical link for the DirectoryHTML builder.""" + app.build() + assert os.path.exists(Path(app.outdir) / "index.html") + tree = parse_html(Path(app.outdir) / "index.html") + link = tree.select("[rel=canonical]") + assert len(link) == 1 + assert link[0]["href"] == "https://test.org/"