DirHTML builder generates wrong canonical links

[lexming]

Describe the bug

Using the dirhtml builder results in pages with canonical links of the form

<link rel="canonical" href="https://www.url.com/search.html" />

for the index document _website/search/index.html

How to Reproduce

1. Set html_baseurl

html_baseurl = 'https://www.url.com'

2. Build documentation using dirhtml builder

sphinx-build -b dirhtml . _website/

Expected behavior

The canonical link has either the form

<link rel="canonical" href="https://www.url.com/search/" />

or

<link rel="canonical" href="https://www.url.com/search/index.html" />

Your project

sphinx-canonical-links.zip

Screenshots

No response

OS

Unix

Python version

3.9

Sphinx version

4.x

Sphinx extensions

No response

Extra tools

No response

Additional context

sphinx-canonical-links.zip

[davidism]

This is affecting the Pallets docs on Read the Docs. Apparently some browsers "Share Link" feature uses a canonical URL if it's available, so sharing links to docs shares invalid links. Is there anything I can do to help get this merged and released?

Also, somehow I didn't realize this has been generated by Sphinx since 1.8 in 2018, both Read the Docs and Pallets-Sphinx-Theme have code for generating this tag as well.

[davidism]

I added a workaround for this issue to Pallets-Sphinx-Themes. A hook will detect if the dirhtml builder is in use and if the pageurl value ends with .html and replace it with the correct URL. pallets/pallets-sphinx-themes#53

[benjaoming]

A quick suggestion for fixing this bug:

Since a lot of projects and themes may have developed behavior based on how the currently undocumented pageurl context variable is implemented, it might be better to define and implement an additional context variable that is 100% intended to render the canonical URL?

For instance: tldr; I don't really know if pageurl is supposed to link to /search/index.html or /search/.. this might be useful for local builds (that don't have an HTTP server in front). I also don't know if (when using it for canonical URLs), it should contain the baseurl or if indeed we need a different canonical domain from baseurl. For instance, we might have CSP enabled which means having the current domain as baseurl for assets and hrefs and a different canonical domain for search engines to index.

Idea? 💡

Starting from scratch with 3 new variables might be easier to handle and customize canonical URLs?

• canonical_baseurl (default value: baseurl)
• canonical_pageurl (default value: similar to pageurl but with DirHTML fixes applied)
• canonical_absolute_url is a helper function that combines canonical_baseurl and canonical_pageurl.
• pageurl is fixed for DirHTML by setting it to /search/ and then it's added to documentation.

[lexming]

@benjaoming thanks for the suggestion, having a specific variable for the canonical URL can indeed be useful. Let me clarify though that the core of this issue is that the current pageurl with DirHTML is broken and points to a non-existing document. For instance, /search.html instead of /search or /search/index.html. So any developed behavior based on current pageurl is not being questioned, since it is not working.

[benjaoming]

So any developed behavior based on current pageurl is not being questioned, since it is not working.

I wrote this, and I think it works... but not sure if we'll merge it nor if it's a recommendable approach https://github.com/readthedocs/sphinx_rtd_theme/blob/6beecce44759257ed584f2fdb0357f2aec0ad01a/sphinx_rtd_theme/layout.html#L62-L77

(afaict, it's also going to keep on working, even if pageurl becomes consistent with DirHTML builder)

[benjaoming]

@AA-Turner I would like to move forwards with a fix for this issue. What do you think about the approach to add new context variables specifically for canonical URLs as laid out in #9730 (comment) ? It would be nice to have some level of assurance that the approach is well-understood before implementing it.