[wp-trac] [WordPress Trac] #24990: Nested Shortcode Inside [caption]

WordPress Trac noreply at wordpress.org
Mon May 11 10:14:42 UTC 2015


#24990: Nested Shortcode Inside [caption]
----------------------------------------+-----------------------------
 Reporter:  prionkor                    |       Owner:
     Type:  defect (bug)                |      Status:  new
 Priority:  normal                      |   Milestone:  Future Release
Component:  Media                       |     Version:  3.6
 Severity:  normal                      |  Resolution:
 Keywords:  needs-unit-tests has-patch  |     Focuses:
----------------------------------------+-----------------------------

Comment (by tychay):

 media.php line 889 and 915

 {{{
 #!php
         if ( current_theme_supports( 'html5', 'caption' ) ) {
                 return '<figure ' . $atts['id'] . 'style="width: ' . (int)
 $atts['width'] . 'px;" class="' . esc_attr( $class ) . '">'
                 . do_shortcode( $content ) . '<figcaption class="wp-
 caption-text">' . $atts['caption'] . '</figcaption></figure>';
         }
 }}}

 {{{
 #!php
         return '<div ' . $atts['id'] . $style . 'class="' . esc_attr(
 $class ) . '">'
         . do_shortcode( $content ) . '<p class="wp-caption-text">' .
 $atts['caption'] . '</p></div>';
 }}}

 At first glance, it seems to be the usual way of supporting nesting
 shortcodes.

 These look right, but they're not. (Or at least I think they're not.) It
 took me a while to notice this because `$content` is actually the `<a><img
 /></a>` and `$atts['caption']` is the old  `$content` minus the image. Why
 would you `do_shortcode()` on the part that surely must be valid HTML? One
 possibility: a shortcode to set properties, which I've never seen (but is
 doable). You want to actually `do_shortcode()` in the stuff '''other'''
 than the image.

 Here's my thinking: when it was originally written a long time ago, the
 programmer didn't realize that an earlier line rewrites `$content` and
 keeps the image part in the content and puts the content part in the
 attribute. But in all normal cases if you want to support nesting of
 shortcodes, you run `do_shortcode()` on `$content` so they put that in
 thinking they're supporting nesting. It's an honest mistake, and I didn't
 notice it the first couple times I glanced at it until I finally decided
 to really look at why shortcodes inside captions are not being processed.

 My suggestion is a very simple fix (much simpler than the regex stuff
 above). on these two lines replace replace `$atts['caption']` with
 `do_shortcode($atts['caption'])`. At this point, I think it's been too
 long to replace  `do_shortcode($content)` with just `$content` as it might
 break some plugins.

 I can do a patch, but I wanted to get feedback in case I missed something
 about the state of plugin development.

 ~

 To the notes 7 months ago about reverse nesting shortcodes that output
 images so they can be put safely in captions. The way I handle that case
 in my plugin is the same way embed is handled to beat `wpautop`: register
 a dummy function and a `the_content` handler that beats it to do: save
 shortcodes, register the real one, do_shortcode, and restore shortcodes.

 In fact the pattern is so common (embed, syntaxhighligher, etc.). I
 honestly think we should add a feature to core to improve
 `add_shortcode()` to create the pattern, like: `add_shortcode( $tag,
 $function, $before_wpautop=false )` so people wouldn't have to constantly
 be writing all these extra handlers and functions to emulate this
 behavior. It'll also be slightly more efficient than running the trick
 serially. But that's another bug entirely :-D

 (Ironically if we supported that, the codes that are run earlier cannot
 correctly support nesting do_shortcode, but I've thought about that, and
 they shouldn't. The only times you actually want to run earlier are to
 beat things like wpautop or another shortcode in which case you aren't
 nested, but more likely nested **inside** something.

 I can do a patch for that also, if there's interest. But, we should open a
 different bug on that one.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/24990#comment:12>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list