shortcode_unautop() should also remove the <br /> added after shortcodes

[aaroncampbell]

Currently wpautop() wraps a shortcode in <p> tags as well as adding a <br /> tag after the shortcode. We then use shortcode_unautop() to remove the <p> tags, but the <br /> stays.

To replicate, just drop a few caption shortcodes into a post and set them all to align left or right. You'll see that even though they all float (assuming that's how your theme handles them) they stair step down because of the extra <br />

I'm not a regex expert so someone should probably double check my patch, but it seems to work for me.

Please take note of existing wpautop() tickets that are releated to the <p> and <br> tag. I just compiled a list of related tickets in here: #13340 maybe it's of use, and I think there is one ticket related to unautop() already but don't blame me if not.

Hi, I'm new to this Trac system but I have a couple of comments.

1. This problem is documented as having been fixed in WP 2.5.1.

extract from ​http://codex.wordpress.org/Shortcode_API#Output

wpautop recognizes shortcode syntax and will attempt not to wrap p or br tags around shortcodes that stand alone on a line by themselves. Shortcodes intended for use in this manner should ensure that the output is wrapped in an appropriate block tag such as <p> or <div>.

Note: in WordPress 2.5.0, shortcodes were parsed before post formatting was applied, so the shortcode output HTML was sometimes wrapped in p or br tags. This was incorrect behaviour that has been fixed in 2.5.1.

2. Shouldn't someone update the above text to say it's still a problem? Or is there some etiquette that says WordPress should not air its dirty washing.
3. I've applied the fix and it's fine for me.
4. I'm not sure if I agree with the comments in #13340 as they suggest that this fix will need a lot of regression testing. I agree that there may be some sites that will appear to break by the removal of the unwanted <br />'s BUT in my opinion their continued presence is particularly annoying given the above documentation.
5. I would like to see this fix as a candidate for 3.0.6 (3.2 is far too late)
6. If not then I would like to see the workaround documented. Actually I've had a go myself. See ​http://www.bobbingwidewebdesign.com/2011/02/21/shortcode-breaks-and-wordpress-bug-14050/

1. This is similar to the problem that was fixed in 2.5.1, but it's not quite the same thing. At that point all shortcode content was being run through autop and it caused ALL THE CONTENT of the shortcode to be treated accordingly. Thus if your shortcode returned 3 paragraphs of text all three paragraphs would be wrapped in p tags. Right now if you return the same three paragraphs, they are left untouched except that a br tag is added at the end.
2. You are more than welcome to update the codex as long as you are clear in describing what exactly is happening, AND clear that this behavior *may* change in the future (as this ticket is not yet "accepted" you shouldn't say *will*).
3. I'm glad this fix works for you!
4. In #13340 hakre is talking about changes to wpautop() and in this ticket we're not suggesting a change to that. Instead I want to make an adjustment to shortcode_unautop(). The former is used in many places and a small change can GREATLY affect things on the front end of a thousands (or even hundreds of thousands) of sites. It's used on such a vast array of content that it's almost mind blowing to consider. However, shortcode_unautop() is used far less, and since it's specifically meant to undo what wpautop() does to shorcodes, I think it will be safe to effect this change.
5. This fix will definitely not make it into 3.0.6 because there won't be a 3.0.6 (and if for some reason there is, it will only be for security issues). Likewise, this was punted from 3.1, which I agree with. There just isn't time to get something like this in there. Likewise it probably won't go in 3.1.1 because while I consider it a bug, it's not a regression. This is how it's worked ever since shortcode_unautop() was introduced. In my opinion 3.2 is the perfect time to try to get this fixed.
6. I think the article you linked to is fine. When you update the codex to point out that you get a trailing br on shortcodes, make sure to mention that while this *may* be fixed in the future, right now you can put the shortcodes on the same line as a workaround.

Same patch, just refreshed for current trunk.

Unit Tests

Before patch:
Tests: 27, Assertions: 51, Failures: 3, Skipped: 4.

After patch:
Tests: 27, Assertions: 51, Failures: 2, Skipped: 4.

The two that are failing were failing when I first set everything up. I'm not sure if we need to look into them or if they were already like that (test_utf8_whitespace_1 and test_utf8_whitespace_2)

Moving to 3.3. See also #15600

Is this still a problem? I have been trying to reproduce but do not get a <br/> anywhere in the returned shortcode or after the shortcode. Just wondering if the patch needs refreshed again or if the ticket is obsolete.

shortcode_unautop() was rewritten in [18952], however the associated test still fails:

1) Tests_Shortcode::test_multiple_shortcode_unautop
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'[footag]
-[footag]
+'<p>[footag]<br />
+[footag]</p>
 '

Related: #12061