search

stephenharris / WP-MarkDown

WP-MarkDown plug-in. Allows Markdown to be enabled in posts, comments and bbPress forums.
http://wordpress.org/extend/plugins/wp-markdown/
112 stars 19 forks source link

p Tags being added to Post Data #58

Closed shazahm1 closed 9 years ago

shazahm1 commented 9 years ago

I'm only really using this for the bbPress support but I noticed something immediately. The markdown to HTML was adding <p> tags. These should not be applied since wpauto takes care of this when rendering the page.

The issue that it really created for me was that since bbpress users are not permitted to use <p> by default, those <p> were being rendered on page.

Since the <p> tags should not really be saved to the db I added this

$content = preg_replace( '~<p>\\s*?(.*?)?\\s*</p>~s', '\1', $content );

Right before this line: https://github.com/stephenharris/WP-MarkDown/blob/master/wp-markdown.php#L289

That stripped the <p> tags from the markdown rendered output before saving it to the db which solved my issue. Not sure if it'll cause any other issues.

I also added before these lines of code too, probably not needed ... but since many topic/replies had the <p> tags in the post data, it ensured they were removed (wpauto fixes it any way) and not displayed in the topic/reply. https://github.com/stephenharris/WP-MarkDown/blob/master/wp-markdown.php#L307 https://github.com/stephenharris/WP-MarkDown/blob/master/wp-markdown.php#L317 https://github.com/stephenharris/WP-MarkDown/blob/master/wp-markdown.php#L355

stephenharris commented 9 years ago

Hi @shazahm1

Are you seeing <p> tags - or are they in the mark-up and you don't want them to be?

If the former, this is a known issue (#52) and relates to an overzealous whitelist filter.

The (generated) paragraph tags are part of the Markdown specification, and wpautop only replaces double line-breaks with paragraph elements, so it won't be the case that paragraph tags are needlessly inserted.

shazahm1 commented 9 years ago

@stephenh1988

Yeap, this is a dupe of #52, sort of. When WordPress saves the post data to the db it is without the <p> and <br> tags. Activating WP Markdown saves them in the db and they shouldn't be, wpautop will add them in the the_content filter.

Stripping them before the post data is saved will ensure consistency with WP core.

I think the only change that really needs to be done is the first ... but to ensure backward compatibility you may need to add the others. ... if you feel this is a bug, that is ...

I did apply a filter to the bbPress tag whitelist to allow other tags supported by WP Markdown already.

With the changes I've made, including the bbPress tag whitelist filter, I'm rockin' and very happy with the result!

stephenharris commented 9 years ago

Yeap, this is a dupe of # 52, sort of. When WordPress saves the post data to the db it is without the <p> and <br> tags.

This is not quite true. For instance, if I use the text tab on the post editor, and use paragraph tags, these are present in the database, and wpautop has no work to do. Normally there are no <p> tags present, as these aren't present when using the tinymce editor. The purpose of wpautop is to 'fix' that for purposes of HTML rendering.

The philosophy of this plug-in is to act as a sort of layer between the user and the post editor. The Markdown is converted to HTML, and then to all intents and purposes you may as well have entered that HTML yourself into the 'text' tab.

The 'bug' here, in my opinion, is with a whitelist that excludes <p> tags. But equally I think it would be wrong of the plug-in to add allowed tags to that whitelist (even if it's something as innocuous as a paragraph). So the 'fix' here would be to include <p> tags into that whitelist via the gist: https://gist.github.com/stephenharris/6078242

shazahm1 commented 9 years ago

Ok, yep, you're right! Thanks for your time and consideration!