Parsedown 1.8 compatibility

[rhukster]

Parsedown 1.8 Compatibility

As you may or may not know, Parsedown has undergone some big changes recently.

First, there was the 1.7.0 release that addressed some XSS vulnerabilities in the code, but had the side effect of breaking some things, pretty much anything with generated HTML in the body of a markdown element.

Hot on the heels of this release has been the development of a pretty major 1.8.0 update. This version has a considerable amount of changes that focus on implement a better AST parser. erusev/parsedown#601

There is a 1.8.1-beta1 release available, but there are more fixes in the Parsedown master branch, and also accompanying fixes in Parsedown Extra master branch which has not seen any updates in quite some time.

Solution

After trying out the latest master branches for both Parsedown & Parsedown Extra, both of which Grav relies upon heavily, it became clear that some core fixes were required.

This PR contains a couple of changes:

1. It currently uses dev-master for both Parsedown & Parsedown Extra. This will be changed to released versions as soon as they are available.
2. It fixes the 'block' for the Twig tag we provide in ParsedownGravTrait which now requires a new structure.
3. It adds a new $allow attribute in the ParsedownGravTrait::addBlockType() method. This is required because unless 3rd party markdown plugins that use Parsedown Block Types are fixed and updated for Parsedown 1.8.0, Parsedown will throw an exception and bring down the whole Grav site.

Third-Party Plugin Updates

What this means is that after updating to this version of Grav, any Grav markdown type plugin that relies on Blocks, will not work until they are fixed and updated. I've already created fixes for two of my own plugins that require this:

• Markdown Notices - Feature/parsedown 1.8 compatibility grav-plugin-markdown-notices#7
• Shortcode Core - Feature/parsedown 1.8 compatibility grav-plugin-shortcode-core#45

As you can see the changes include:

1. a requirement for upcoming Grav release
2. passing the new $allow = true to the ParsedownGravTrait::addBlockType() method
3. various minor changes to bring it inline with Parsedown 1.8.0 internal AST structure changes

This means that after this code is merged and Grav is released, any other Grav plugin that needs this functionality should also be updated with similar changes, or it will stop working.

From my initial GPM search and testing, I've identified these plugins as having issues:

Throwing exceptions:

• Markdown Tasklists - @funkjedi - https://github.com/funkjedi/grav-plugin-markdown-tasklists
• Markdown Sections - @giansi - https://github.com/giansi/grav-plugin-markdown-sections

No errors, but not functioning:

• Markdown RubyText - @tidiview - https://github.com/tidiview/grav-plugin-markdown-rubytext
• Markdown Spoilers - @Regaez - https://github.com/Regaez/grav-plugin-markdown-spoilers

The most useful information can be found here: erusev/parsedown#601

This tutorial is a bit out of date but may provide some helpful information: https://github.com/erusev/parsedown/wiki/Tutorial:-Create-Extensions

Other Plugins?

If you know of other plugins that use Markdown block types, they are probably impacted also. Please let the authors know of this PR, or let me know and i'll ping them directly on their repos.

Thanks!!!

[mahagr]

I think this would be a perfect time to create Grav 1.5 branch instead of having the change in Grav 1.4.2

What do you think?

[rhukster]

It is definitely more impactful than a 1.4.3 would typically call for.. it probably is best to save this for 1.5. I have some other new things I would like to see in 1.5 too, but as we'll be moving to PHP 5.6+ compatibility, we could also look at updating Symfony to v3.2, but that will need some thorough testing.

[aidantwoods]

I've just seen this referencing the draft release notes.

I thought I'd put a note here to say that I'm keen to hear about any migration nits that Parsedown could perhaps make easier before 1.8.0 is released :) (hopefully along the lines of this kind of thing where there is a sensible compatibility layer that can be added: erusev/parsedown@40e7970).

[rhukster]

Hi @aidantwoods, yes, basically you can see the kinds of things i've had to fix in plugins that extend Parsedown in order for them to work with 1.8.0. Things like this:

https://github.com/getgrav/grav-plugin-markdown-notices/pull/7/files#diff-869a91a5a795c716917f30d3fb775b3cL69

and this:

https://github.com/getgrav/grav-plugin-shortcode-core/pull/45/files#diff-d0deca33c58addac7af23ae255bdf12aR202

If it could be possible for Parsedown to see the 'old' style for BlockTags and handle them automatically, it would be a huge help for migration. Unfortunately, it's very difficult to do this outside of Parsedown as we're effectively extending the core functionality, as there are no wrapper methods, we're effectively manipulating the Parsedown internals.

Moving forward it would be great if Parsedown itself had some methods for extending to allow the internal architecture to be hidden.

[aidantwoods]

The first one not working is a bug in the compatibility layer, and I've added support for the second so that the markup key is mapped over.

erusev/parsedown#615

Generally I think that updating to what you've got now is a good idea, but it shouldn't be required (maybe you can test to confirm? :) ).

---

Edit: Unfortunately for things that are say: having problems when directly overloading something like blockListComplete there isn't much that can be done other than the extension updating (since certain problems will presumably arise from the structure being output changing, e.g. https://github.com/funkjedi/grav-plugin-markdown-tasklists/blob/37e8d3c18c875fd2f821c928a677aebaa60c6e2e/markdown-tasklists.php#L31 relies on the text key containing an array, which is no longer done in core).

For completely new block/inline types (that don't rely on internal ones) there should hopefully be a lot we can do for compatibility.

---

Moving forward it would be great if Parsedown itself had some methods for extending to allow the internal architecture to be hidden.

PRs welcome :)

Something that should hopefully be easier already: as an extension, you can now optionally generate the entire AST prior to output and make modifications to generated elements directly (before they're converted to HTML).