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

Extra paragraphs turned into entities around all text #48

Closed einkoro closed 10 years ago

einkoro commented 10 years ago

Any input in bbPress topics and replies are wrapped with extra paragraphs which are turned into entities. This occurs with 1.5.1 and the latest from master today.

test

becomes:

<p>&lt;p&gt;test&lt;/p&gt;</p>
einkoro commented 10 years ago

Seems like wpmarkdown_markdown_to_html() is executed twice when a reply or topic is submitted.

einkoro commented 10 years ago

Its called from both bbp_reply_pre_content and wp_insert_post_data when posting a new reply. Is there any reason for the other functions hooked into the following filters if wp_insert_post_data is already being called on comments, replies, and topics?

add_filter('pre_comment_content',array($this,'pre_comment_content'),5);
add_filter('bbp_new_reply_pre_content',array( $this, 'bbp_reply_pre_content' ), 5, 2 );
add_filter('bbp_edit_reply_pre_content',array( $this, 'bbp_reply_pre_content' ), 5, 2 );
add_filter('bbp_new_topic_pre_content',array( $this, 'bbp_topic_pre_content' ), 5, 2 );
add_filter('bbp_edit_topic_pre_content',array( $this, 'bbp_topic_pre_content' ), 5, 2 );
einkoro commented 10 years ago

Looks like that was the entirely wrong direction as its the function bbp_encode_bad hooked into both topic and replies causing the issue. Seems like the issue in #25 of which your gist helps with: https://gist.github.com/stephenharris/6078242

einkoro commented 10 years ago

And forgot that only helps when preventing it from being called twice otherwise backticks and blockquotes seem to have issues still. Quick fix I'm using is adding a check for the post type:

    //For posts
    public function wp_insert_post_data( $data, $postarr ) {
        $ignored_post_types = array('comment', 'reply', 'topic');

        if ( !in_array( $data['post_type'], $ignored_post_types ) ) {

            ...

        }

        return $data;
    }
stephenharris commented 10 years ago

@Einkoro I haven't been able to replicate this at all. And in theory the markdown to HTML conversion can run more than once without issue as it skips any HTML it finds.

Which version of bbPress are you using?

einkoro commented 10 years ago

bbPress 2.5.3. Only thing that comes to mind that might cause a difference, although doubtful, is that I'm running HHVM rather than PHP-FPM.

stephenharris commented 10 years ago

I've just updated and re-tested, still fine my end. So the error definiately occurs at wp_insert_post_data() then? What is the before & after (and expected) HTML?

I'm going to right some unit tests to double check that multiple runs of wp_markdown_to_html() isn't harmful. In any case, if I recall, it was necessary to use bbp_reply_pre_content to jump in before bbPress started playing with the content. That it is run through wp_markdown_to_html() a second time was probably an oversight on my part.

einkoro commented 10 years ago

I'm starting to think its some sort of caching issue of either the allowed tags in bbPress with memcached obj cache or HHVM's JIT. Not sure if that bbPress call uses a transient. Undoing the conditional I added no longer produces the issue after restarting everything. I'll let you know if I see this again when the site goes through QA tomorrow and if not I'll close the ticket out.

My original notes chasing this issue:

I added logging before and after each call to wp_markdown_to_html() to note the function, post type and content going in and coming out from it and this was the result for a post with this markdown:

asdf  `code` asdf

Log:

bbp_reply_pre_content():
before: asdf  `code` asdf
after: <p>asdf  <code>code</code> asdf</p>\n
wp_insert_post_data(): reply:
before: &lt;p&gt;asdf  <code>code</code> asdf&lt;/p&gt;\n
after: &lt;p&gt;asdf  <code>code</code> asdf&lt;/p&gt;\n

From there I made my way to bbp_encode_bad() and found issue #25. Either removing the hook or adding the list of allowed tags from the gist linked to in #25 didn't fix it on its own as the next post with more markdown had issues. After that I tried dropped in the conditional check for the post type and then this started to parse correctly for me.

Markdown before submitting:

I'm a regular ol' paragraph.

> I'm a blockquote.

I'm a paragraph with `<code>` in it and some *other* random **markdown**.

    <dl>
      <dt>I'm some HTML code that a user is trying to post</dt>
      <dd>I'm indented four spaces NOTE the missing closing tag
    </dl>

I'm a [link](http://google.com) in a paragraph

HTML after submitting:

<p>&lt;p&gt;I’m a regular ol’ paragraph.&lt;/p&gt;</p>
<blockquote><p>
  &lt;p&gt;I’m a blockquote.&lt;/p&gt;
</p></blockquote>
<p>&lt;p&gt;I’m a paragraph with <code></code><code></code> in it and some <em>other</em> random <strong>markdown</strong>.&lt;/p&gt;</p>
<pre class="prettyprint prettyprinted"><code><span class="tag">&lt;dl&gt;</span><span class="pln">
  </span><span class="tag">&lt;dt&gt;</span><span class="pln">I'm some HTML code that a user is trying to post</span><span class="tag">&lt;/dt&gt;</span><span class="pln">
  </span><span class="tag">&lt;dd&gt;</span><span class="pln">I'm indented four spaces NOTE the missing closing tag
</span><span class="tag">&lt;/dl&gt;</span></code></pre>
<p>&lt;p&gt;I'm a <a href="http://google.com" rel="nofollow">link</a> in a paragraph&lt;/p&gt;</p>

Expected HTML:

<p>I'm a regular ol' paragraph.</p>

<blockquote>
  <p>I'm a blockquote.</p>
</blockquote>

<p>I'm a paragraph with <code>&lt;code&gt;</code> in it and some <em>other</em> random <strong>markdown</strong>.</p>

<pre class="prettyprint prettyprinted"><code><span class="tag">&lt;dl&gt;</span><span class="pln">
  </span><span class="tag">&lt;dt&gt;</span><span class="pln">I'm some HTML code that a user is trying to post</span><span class="tag">&lt;/dt&gt;</span><span class="pln">
  </span><span class="tag">&lt;dd&gt;</span><span class="pln">I'm indented four spaces NOTE the missing closing tag
</span><span class="tag">&lt;/dl&gt;</span></code></pre>

<p>I'm a <a href="http://google.com">link</a> in a paragraph</p>
einkoro commented 10 years ago

Doesn't reproduce anymore since clearing memcached and restarting HHVM so I'm going to assume this is non-issue.