Canonical URL - is it injected correctly or should it?

[benjaoming]

A canonical URL such as <link rel="canonical" href="https://docs.readthedocs.io/en/stable/index.html" /> is already inserted in themes.

However, this extension proposes to add it again.

readthedocs-sphinx-ext/readthedocs_ext/_templates/readthedocs-insert.html.tmpl

Lines 4 to 19 in f1145b1

	{%- if canonical_url %}
	{%- if pagename == "index" %}
	{%- set canonical_page = "" %}
	{%- elif pagename.endswith("/index") %}
	{%- set canonical_page = pagename[:-("/index"|length)] + "/" %}
	{%- else %}
	{%- set ending = "/" if 'dirhtml' in builder else ".html" %}
	{%- set canonical_page = pagename + ending %}
	{%- endif %}
	<!--
	Always link to the latest version, as canonical.
	http://docs.readthedocs.org/en/latest/canonical.html
	-->
	<link rel="canonical" href="{{ canonical_url|e }}{{ canonical_page }}" />
	{%- endif %}

Adding this issue to clarify what this approach is about and if it's a potential solution to upstream Sphinx issues.

CC: @ericholscher

[ericholscher]

That is where we can inject logic to fix the Sphinx bug, as needed, instead of in the theme. It will then apply to all RTD sphinx projects.

[benjaoming]

Won't it be a problem to inject <link rel="canonical" href="{{ canonical_url|e }}{{ canonical_page }}" /> if it's already defined previously by the theme?

[ericholscher]

Perhaps? Seems like something we should hopefully be able to check for, and be smart about, but it's definitely a hack.