wintercms / winter

Free, open-source, self-hosted CMS platform based on the Laravel PHP Framework.
https://wintercms.com
MIT License
1.38k stars 195 forks source link

Markdown parser fails in content file when using a content variable as the url #992

Open mjauvin opened 1 year ago

mjauvin commented 1 year ago

Winter CMS Build

dev-develop

PHP Version

8.1

Database engine

MySQL/MariaDB

Plugins installed

No response

Issue description

The new markdown parser fails in a contend file when using a content variable in place of a static url.

Steps to replicate

Using this markup in a CMS page:

{% content "test-content.md" url="https://wintercms.com" %}

And this content file:

[click here]({url})

Workaround

Using <a href="{url}">click here</a> tag in the content file works.

LukeTowers commented 1 year ago

Do the variables need to be parsed before the markdown gets parsed?

mjauvin commented 1 year ago

It used to work, so I would say yes.

LukeTowers commented 1 year ago

@bennothommo any thoughts?

bennothommo commented 1 year ago

I assume the new Markdown parser is probably applying some escaping on the URLs for links, whereas the old Parsedown library didn't. I do feel that those variables should be parsed before Markdown however, so if that's not the case, I'll fix that.

mjauvin commented 1 year ago

@bennothommo You must have had really good reasons to change the parser, but it has created quite a bit of BC... and now you're saying it can't even be extended easily because of final class/methods ?

bennothommo commented 1 year ago

@mjauvin I'd argue it's only been edge cases so far. Given how prevalent it's used in the Blog and Docs plugins, it's been surprisingly smooth if I'm being honest, especially since Parsedown was very loose with its following of standards whilst CommonMark is more strict.

mjauvin commented 1 year ago

Seems I've been hitting all those "edge cases" myself... ;)

bennothommo commented 1 year ago

True - which means either you have very bad luck, or people are silent on the matter. But either way, I still feel it was a net positive. :stuck_out_tongue:

LukeTowers commented 4 months ago

@mjauvin is this still an issue for you?

mjauvin commented 4 months ago

@LukeTowers it still is, but I'm now using Winter.Parsedown plugin to revert to the old parser when needed.