Filter adds a /p after a blockquote where it shouldn't.

The problem is in function _filter_autop($text).
When I comment out this line, the problem disappears:

$chunk = str_replace('</blockquote></p>', '</p></blockquote>', $chunk);

To reproduce:

The code in question is unchanged between D5 and D6, so you can test with either.
Create a node, select the 'php' input format, and insert the following code in the node:

$text = "<blockquote class=\"quote\"><p>Something</p>
<p class=\"author\">Al Gore, The Assault on Reason</p>
</blockquote>";

$text2 = str_replace('<', '&lt;' ,$text);
$text2 = str_replace('>', '&gt;', $text2);
echo "Before:<br/><pre>";
echo $text2;
echo "</pre>";

$text = _filter_autop($text);
$text2 = str_replace('<', '&lt;' ,$text);
$text2 = str_replace('>', '&gt;', $text2);
echo "After:<br/><pre>";
echo $text2;
echo "</pre>";

comment and uncomment the said php line, and see the difference.

See this chunck of code:

      $chunk = preg_replace('|\n*$|', '', $chunk) ."\n\n"; // just to make things a little easier, pad the end
      $chunk = preg_replace('|<br />\s*<br />|', "\n\n", $chunk);
      $chunk = preg_replace('!(<'. $block .'[^>]*>)!', "\n$1", $chunk); // Space things out a little
      $chunk = preg_replace('!(</'. $block .'>)!', "$1\n\n", $chunk); // Space things out a little
      $chunk = preg_replace("/\n\n+/", "\n\n", $chunk); // take care of duplicates
      $chunk = preg_replace('/\n?(.+?)(?:\n\s*\n|\z)/s', "<p>$1</p>\n", $chunk); // make paragraphs, including one at the end
      $chunk = preg_replace('|<p>\s*</p>\n|', '', $chunk); // under certain strange conditions it could create a P of entirely whitespace
      $chunk = preg_replace("|<p>(<li.+?)</p>|", "$1", $chunk); // problem with nested lists
      $chunk = preg_replace('|<p><blockquote([^>]*)>|i', "<blockquote$1><p>", $chunk);
// CHECK THE NEXT TWO LINES
      $chunk = str_replace('</blockquote></p>', '</p></blockquote>', $chunk); // ------------------------------> This line is the troublemaker.
      $chunk = str_replace('<p></p>', '', $chunk); // --------------------------------------------------------------------> ADDED THIS LINE. 
      $chunk = preg_replace('!<p>\s*(</?'. $block .'[^>]*>)!', "$1", $chunk);
      $chunk = preg_replace('!(</?'. $block .'[^>]*>)\s*</p>!', "$1", $chunk);
      $chunk = preg_replace('|(?<!<br />)\s*\n|', "<br />\n", $chunk); // make line breaks
      $chunk = preg_replace('!(</?'. $block .'[^>]*>)\s*<br />!', "$1", $chunk);
      $chunk = preg_replace('!<br />(\s*</?(?:p|li|div|th|pre|td|ul|ol)>)!', '$1', $chunk);
      $chunk = preg_replace('/&([^#])(?![A-Za-z0-9]{1,8};)/', '&amp;$1', $chunk);

We start with the following text:

<blockquote class="quote"><p>Something</p>
<p class="author">Al Gore, The Assault on Reason</p>
</blockquote>

The closing blockquote can be on a separate line or at the end of the previous line. It doesn't seem to matter.

Before the troublemaking line is executed, we text has become thus:

<blockquote class="quote"><p>
<p>Something</p></p>
<p><p class="author">Al Gore, The Assault on Reason</p></p>
<p></blockquote></p>

I.e. plenty of <p>'s and </p>'s have been added.

The troublemaker changes the text to:

<blockquote class="quote"><p>
<p>Something</p></p>
<p><p class="author">Al Gore, The Assault on Reason</p></p>
<p></p></blockquote>

I.e. it prepares the ground for the rest of the of the code to remove the redundant p's.

The problem is that in this situation, the current code misses one /p, thus:

<blockquote class="quote"><p>Something</p>
<p class="author">Al Gore, The Assault on Reason</p>
</p></blockquote>

Rather than removing the troublemaking line which must be there for a reason, we add a str_replace() to remove code that is bound to be useless in every circumstances.

The added line removes the empty pair of <p></p> and solves the problem.

<blockquote class="quote"><p>Something</p>
<p class="author">Al Gore, The Assault on Reason</p>
</blockquote>

I tried moving the more generic line down to where I added the new line as you suggested, but it doesn't work. I still end up with the extraneous /p.

I don't know what you mean by the generated HTML code being sub-optimal . Without the patch I have invalid html but with it the html validates.

The blockquote lines must be there for a reason. It would be foolhardy to remove them and expect no to break anything. Actually, they seem to be here to ensure that p's are nested within blockquote and not the other way around. <p><blockquote></blockquote></p> is not valid.

Having tried some more, I can state:
1) the patch fixes a validation bug.
2) the patch is so simple (removing an empty paragraph) that it cannot break anything else.

The only question in my mind is: is it worth to add a comment, with a reference to this issue?

- the patch is trivial
- it can't possibly break anything
- it fixes a bug
- alternative methods suggested have more chances to break something. Anyway: they have been tried but they failed.

Ergo: RTBC.

It is said in many places and by many people that in case of a very trivial patch, it is ok to RTBC one's own patch.
The rest of your comment also strongly suggests that you didn't even have a look at the patch :-/

1) I replied to Dries's concerns.
2) how tricky can removing an empty paragraph (<p></p>) be??

The patch only removed a completely empty <p></p>.
If someone uses an empty P for CSS purposes, then adding a space would do the trick: <p> </p>.

Does this mean that for this one liner to get committed, we need to create an upgrade function system_update_7009(), searching the DB for <p></p> and replacing all instances with <p> </p>?

I don't see why a real bug should be won't fixed.