minimal-mistakes

minimal-mistakes copied to clipboard

Title should not be markdownified. Pull request to fix gh-3094.

Summary

Context

You can add Fixes #3094 to your PR description so it's automatically closed when this is merged. See https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

@iBug But why should the vertical bar | be escaped in the seo_title?

If it needs replacing then the replace should probably take place on seo_title together with escape_once instead, as the user might use it as part of the title also.

Well you're right. I couldn't find any necessity to escape the vertical bar. Maybe it was related to Markdown parsing where it would be interpreted as table (it trolled me once upon a time). Now that you've removed markdownify, this escape shouldn't be needed anymore.

The markdownify filter does serve a purpose as it normalizes titles that may use Markdown to apply emphasis or other "light" formatting.

For example check this test post on master. https://mmistakes.github.io/minimal-mistakes/markdown/markup-title-with-markup/

The title has a word italized and bolded, and the <title> element is readable without Markdown characters appearing.

This PR breaks that. Now the <title> element has all the Markdown syntax there. Markdown titles are probably an edge case, but the theme has had support for them for a long time and this breaks that.

A better fix might be to apply the Markdown and replace transforms on page.title before the separator and site.title are appended. Then it won't do unintended things on the | or any other characters that serve a purpose in Markdown.

Thanks for the comments @iBug and @mmistakes. Here is a new stab on solving the issue.

@lsolesen sorry for the delay on this, finally got around to testing it. Titles with markdown parse correctly now, but the "home" index.html page doesn't output a title now. There's an empty <title></title>

Will probably want to test a paginated home index as well as there's some logic to spit out page numbers in the title element.