Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strip tags from page title #5

Merged
merged 2 commits into from
Sep 13, 2023
Merged

Strip tags from page title #5

merged 2 commits into from
Sep 13, 2023

Conversation

waylan
Copy link
Owner

@waylan waylan commented Sep 13, 2023

There shouldn't be any HTML tags in an HTML <title> so there is no harm in stripping them. And we get the benefit of not having the tags rendered in the browser's title bar/tab. See this comment in Python-Markdown/markdown#1379.

There shouldn't be any HTML tags in an HTML `<title>` so there is no harm
in stripping them. And we get the benefit of not having the tags rendered
in the browser's title. See this [comment](Python-Markdown/markdown#1379 (review)).
@oprypin
Copy link

oprypin commented Sep 14, 2023

Something I noticed - probably in <link rel="next" it should get the same treatment

{% if page and page.next_page %}<link rel="next" title="{{ page.next_page.title }}" href="{{ page.next_page.url|url }}" />{% endif %}
{% if page and page.previous_page %}<link rel="prev" title="{{ page.previous_page.title }}" href="{{ page.previous_page.url|url }}" />{% endif %}

Anyway this is partly a new issue with MkDocs 1.5 and I'm looking into it generally, maybe this shouldn't be necessary mkdocs/mkdocs#3357 (comment)

@waylan
Copy link
Owner Author

waylan commented Sep 15, 2023

Actually in our case we don't want to strip the <code> tags on the page. Only in the <title> tag where HTML does not get rendered does this matter. So actually, the raw HTML not being stripped is desirable in our case. In fact, we added the <code> tags specifically to avoid tripping up the spellchecker in our CI tests (as the contents of <code> tags are skipped). Therefore, if the current behavior was reverted, we would lose a feature.

I realize that you may have inadvertently introduced a breaking change here, but I'm inclined to suggest that you leave it as-is and document that themes should use the striptags filter where needed. But then again, it may be that that doesn't work for all edge cases.

@oprypin
Copy link

oprypin commented Sep 15, 2023

@waylan Thanks for your input, you can comment on the issue too.

But what I'm saying is still true and separate-

title="{{ page.next_page.title }}" <- raw HTML gets thrown into the quoted part here (and Firefox at least underlines it in red in view-source:), it should in fact be stripped same as inside <title>

In the body sure, you don't need to strip it

@waylan
Copy link
Owner Author

waylan commented Sep 16, 2023

Ah, right, in the title. Yeah, that is an issue. Thanks for pointing that out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants