Canonical ref does not match URL for builds that use dirhtml builder

[benjaoming]

Details

Upstream issue: sphinx-doc/sphinx#9730

• Read the Docs project URL: https://packaging.python.org/en/latest/tutorials/packaging-projects/
• Build URL (if applicable): https://readthedocs.org/projects/{your_project_slug}/builds/{build_id}/
• Config: https://github.com/pypa/packaging.python.org/blob/main/.readthedocs.yaml

Expected Result

<link rel="canonical" href="[https://packaging.python.org/en/latest/tutorials/packaging-projects.html" />

Actual Result

<link rel="canonical" href="[https://packaging.python.org/en/latest/tutorials/packaging-projects/" />

[benjaoming]

All this might be a Sphinx issue though. I haven't had the time to check.

[stsewd]

I think this was a problem when installing an outdated version of sphinx, and then installing a new one over it. They have their requirements set explicitly https://github.com/pypa/packaging.python.org/blob/main/requirements.txt, so we should be fine enabling the install latest feature flag.

[benjaoming]

I think this was a problem when installing an outdated version of sphinx, and then installing a new one over it.

So the issue originates from this behavior?

https://github.com/readthedocs/readthedocs.org/blob/fc7f68b9848e4c6a39014801829d395df2aede86/readthedocs/doc_builder/python_environments.py#L203-L222

How does sphinx==4.5.0 end up having its build outputs affected by the previous Sphinx installation?

[benjaoming]

Seems that the theme, python-docs-theme, is the reason for being stuck with sphinx 4.x until the work by Adam is merged - https://github.com/python/python-docs-theme/pull/99

[stsewd]

o we should be fine enabling the install latest feature flag.

And... that didn't work :D

[benjaoming]

Similar issue here: https://test-builds.readthedocs.io/en/htmldir/folder/

[benjaoming]

I upgraded Sphinx to the latest version in https://test-builds.readthedocs.io/en/htmldir/ but the issue persists.

[ericholscher]

Yea, this is a bug, we should prioritize fixing it. It's likely causing lots of search issues.

[benjaoming]

@ericholscher I just saw that the issue description here has broken down where the bug might be found: #1303

[benjaoming]

This is indeed an issue in the theme. .html is hard-coded regardless of the builder.

https://github.com/readthedocs/sphinx_rtd_theme/blob/master/sphinx_rtd_theme/layout.html#L54

Moving this issue and opening a PR since I'm already in the area 👍

[benjaoming]

This is the upstream issue by the way: sphinx-doc/sphinx#9730

[benjaoming]

This is only fixing the theme.

If we want to implement a work-around on everything hosted on RTD, we should figure out why canonical_url as defined here doesn't seem to be applied: https://github.com/readthedocs/readthedocs-sphinx-ext/blob/main/readthedocs_ext/_templates/readthedocs-insert.html.tmpl

[ericholscher]

I was able to fix teh canonical issue with a HTML -> HTMLDir redirect: https://docs.readthedocs.io/en/latest/user-defined-redirects.html#sphinx-redirects

[humitos]

The easiest fix for users here is to remove the theme configuration canonical_url (which is already deprecated) and use html_baseurl Sphinx's option instead. When html_baseurl is defined, Sphinx automatically generates the pageurl and inject it into the context.

Read the Docs already does this "automagically" at https://github.com/readthedocs/readthedocs.org/blob/4b2eb75bebfce04390ac3f84d1c57ef7f7126fe8/readthedocs/doc_builder/templates/doc_builder/conf.py.tmpl#L151-L156

Besides, I think that now Read the Docs is exposing READTHEDOCS_CANONICAL_URL we should use that environment variable from this theme to automatically set html_baseurl from this theme instead of relying on Read the Docs itself.

[benjaoming]

@humitos did something get fixed in Sphinx? The upstream issue here is that Sphinx appends .html to pageurl when it shouldn't do that for the DirHTML builder.

If we want to fix the issue quickly in the theme, I proposed #1407 as a backwards and forwards compatible workaround.

[humitos]

@benjaoming gotcha! I miss-read the Sphinx code and thought that self.out_suffix on its code will do exactly what we need, but it will probably just adds .html always instead of only when it's required.

[benjaoming]

@humitos yes, that's it.. and there's also a PR to fix exactly that part... I'd put in a proposal to add a canonical_url to the page context.. but holding back with more work, since the simple fix in that PR has been sitting stale for a long time now.

[humitos]

I'm a little lost here about how to solve this issue. What are the actionable steps to move forward from our side?

[ericholscher]

I think it's a Sphinx issue that we could work around, but I think Sphinx should likely fix it.