minimal-mistakes icon indicating copy to clipboard operation
minimal-mistakes copied to clipboard

Do not markdownify title - Fixes #3094

Open lsolesen opened this issue 2 years ago • 7 comments

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

Summary

Context

lsolesen avatar Aug 08 '21 09:08 lsolesen

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 avatar Aug 08 '21 10:08 iBug

@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.

lsolesen avatar Aug 08 '21 10:08 lsolesen

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.

iBug avatar Aug 08 '21 10:08 iBug

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/

Screen Shot 2021-08-08 at 1 54 14 PM

The title has a word italized and bolded, and the <title> element is readable without Markdown characters appearing. Screen Shot 2021-08-08 at 1 51 59 PM

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.

Screen Shot 2021-08-08 at 1 53 32 PM

mmistakes avatar Aug 08 '21 17:08 mmistakes

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.

mmistakes avatar Aug 08 '21 17:08 mmistakes

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

lsolesen avatar Aug 13 '21 07:08 lsolesen

@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>

Screen Shot 2022-05-27 at 10 29 24 AM

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.

mmistakes avatar May 27 '22 14:05 mmistakes