Make WordPress Core

Opened 8 days ago

Last modified 5 days ago

#58366 new defect (bug)

Shortcode Support Regained but Content Filters are messing with Shortcode HTML

Reported by: domainsupport's profile domainsupport Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.2.2
Component: Shortcodes Keywords: needs-testing has-testing-info needs-unit-tests
Focuses: Cc:

Description

I am extremely grateful that the Security Team were able to quickly regain support for shortcodes in the Block Theme templates. However, whatever change has been agreed and pushed out means that the filters to automatically inject <br> and <p> tags into the content are now affecting shortcodes and we are seeing text being automatically wrapped in <p> tags and carriage returns replaced with <br> tags.

Rather than revert to the insecure v6.2.1 we are going through shortcodes to remove any carriage returns.

Please advise.

Oliver

Attachments (2)

58366-poc.diff (2.4 KB) - added by peterwilsoncc 5 days ago.
POC -- not for commit
58366.diff (929 bytes) - added by bekk 5 days ago.
For review

Download all attachments as: .zip

Change History (41)

#1 @johnbillion
8 days ago

  • Version changed from trunk to 6.2.2

This ticket was mentioned in Slack in #core by desrosj. View the logs.


8 days ago

#3 @kubiq
7 days ago

Here you can see all possible usecases and current problems:
https://demo.kubiq.sk/sample-page/

This demo uses [xxx] shortcode that should render A B C

<?php
add_shortcode( 'xxx', function(){
        return "A\nB\nC";
});
  1. Shortcode block in Template and Template part is doing redundant autop
  2. Shortcodes are not working when they are in reusable block in Template and Template part (group, row, whatever)

This is clean WP install without any plugin and default 2023 theme - I've just added functions.php in it, with the code from above

Last edited 7 days ago by kubiq (previous) (diff)

This ticket was mentioned in Slack in #core by kubiq. View the logs.


7 days ago

#5 @Clorith
6 days ago

I'm not sure if this scenario would be considered a bug, or that the previous behavior would be considered an unintentional lack of processing.

The \n symbol represents a newline after all, but wasn't being treated as such inline since processing has already happened to the post content at the time of the shortcode being called.

Given that the site editor is a whole other beast, that shortcodes do not work exactly the same as in post content feels like something we can communicate and make clear that they are two very different use cases 🤔

As for the reusable block scenario, that does appear unintentional, and those may need to also intentionally call do_shortcode and make it an explicit expectation that shortcodes within them are rendered (this is the recommendation for other blocks as well, so seems like a sensible thing to introduce I think).

#6 @kubiq
6 days ago

@Clorith shortcodes should never do any autop! Imagine pretty common output for input with a lot of attributes

<input
    type="number"
    name="age"
    value=""
    placeholder="18"
    id="age"
    class="form-control"
    min="18"
    max="80"
    step="1"
/>

any many other scenarios... \n inside the shortcodes has to stay \n

#7 @domainsupport
6 days ago

@Clorith I think you'll find that @kubiq is correct on this one I'm afraid; and I have a better example ...

<textarea>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</textarea>

... would have been perfectly fine FSE template shortcode output in v6.2 but in 6.2.2 it is now filtered and is output something like this ...

<textarea>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

<p>Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.</p>

Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
<br>
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</textarea>

... and there's nothing that can be done about it.

I'm not sure why there haven't been more complaints about this yet but I am sure that there will be.

Oliver

#8 @aflijja
6 days ago

I second @domainsupport; here is another one ...

<p>        <script>
        jQuery(document).ready(function($) {</p>
<p>            $('#wcps-558').slick({
                adaptiveHeight: true,
                slidesToShow: 4,
                slidesToScroll: 4,
                responsive: [{
                        breakpoint: 1200,
                        settings: {
                            slidesToShow: 4,
                            slidesToScroll: 4,
                            nav: true,
                        }
                    },
                    {
                        breakpoint: 768,
                        settings: {
                            slidesToShow: 2,
                            slidesToScroll: 2,
                            nav: true,
                        }
                    },
                    {
                        breakpoint: 576,
                        settings: {
                            slidesToShow: 1,
                            slidesToScroll: 1,
                            nav: true,
                            touchMove: true,
                            swipeToSlide: true,</p>
<p>                        }
                    }
                ],
                gutter: 10,
                arrows: true,
                appendArrows: '.controlsWrap-558',
                prevArrow: '</p>
<div class="prev"><i class="fas fa-chevron-left"></i></div>
<p>',

the strange part for me is this problem exist only on the front page.

#9 follow-up: @jim5471
6 days ago

I have a number of shortcode blocks in templates and template parts and am not seeing problems with the latest release.

Are you putting your custom HTML inside Custom HTML Blocks ?

I don't use reusable blocks so this may be a stupid question.

What is wrong with using "convert to blocks" ?

Then the shortcodes start working.

#10 in reply to: ↑ 9 @domainsupport
6 days ago

Replying to jim5471:

I have a number of shortcode blocks in templates and template parts and am not seeing problems with the latest release.

Are you putting your custom HTML inside Custom HTML Blocks ?

I don't use reusable blocks so this may be a stupid question.

What is wrong with using "convert to blocks" ?

Then the shortcodes start working.

HTML generated by our shortcodes is having <p> and <br> tags injected into it after it's been generated as if it were post content. This is not static HTML that can be moved into a Custom HTML block otherwise we wouldn't be using a shortcode at all. The HTML is dynamic and requires PHP to generate it.

Oliver

#11 @aflijja
6 days ago

a temp solution for me
patch "wp-includes/block-template.php"
with 2 lines

	$content = str_replace( ']]>', ']]&gt;', $content );
	$content = str_replace( '<p>', '', $content );
	$content = str_replace( '</p>', '', $content );

the second problem is new line generation inside script tag
i changed the single quote ' to backtik `
so accidental multiline script work

    prevArrow: `
<div class="prev"><i class="fas fa-chevron-left"></i></div>
`,

but this is temporary solution for me

#12 @bekk
6 days ago

This is also causing <p> tags to appear within inline javascript output by some shortcodes

#13 follow-up: @jim5471
6 days ago

@domainsupport, @bekk - Just trying to understand the issue..

If you return inline script in this way do you get problems ?

<?php
add_shortcode( 'xxx', function(){
        return "<!-- wp:html --><script><!-- code here /--></script><!-- /wp:html -->";
        });

#14 in reply to: ↑ 13 @domainsupport
6 days ago

Replying to jim5471:

If you return inline script in this way do you get problems ?

<?php
add_shortcode( 'xxx', function(){
        return "<!-- wp:html --><script><!-- code here /--></script><!-- /wp:html -->";
        });

I didn't wait to see if inline script was affected although I have no doubt that it is. I started trying to overcome the issue by putting all returned HTML on a single line but then realised that this wasn't possible for <textarea> input fields that have paragraphs of text within them and at that point I gave up.

I notice you are getting your example xxx shortcode to output an wp:html formatted block ... are you saying that this would be ignored by the content filters and could be a possible work-around?

If this is what you meant in your original comment then I apologise as I didn't realise.

Last edited 6 days ago by domainsupport (previous) (diff)

#15 @bekk
6 days ago

@jim5471 yes it gets rendered as

<p><script><!-- code here /--></script></p>

Likewise if you change it to:

<?php
add_shortcode( 'xxx', function() {
    return "<!-- wp:html --><script>
var obj = {
    'foo': 'bar',
    'baz': 'qux'
};

var json = JSON.stringify(obj);
</script>
    <!-- /wp:html -->";
});

It's rendered as:

<p><script>
var obj = {
    'foo': 'bar',
    'baz': 'qux'
};</p>
<p>var json = JSON.stringify(obj);
</script></p>
Last edited 6 days ago by bekk (previous) (diff)

#16 follow-up: @jim5471
6 days ago

@domainsupport

OK - now try the code without any line feeds.
It is being wrapped in <p> tags but the code is clean.

<?php
add_shortcode( 'xxx', function() { 
        return "<!-- wp:html --><script>var obj = { 'foo': 'bar', 'baz': 'qux'};var json = JSON.stringify(obj);</script><!-- /wp:html -->";
});
<p><script>var obj = { 'foo': 'bar', 'baz': 'qux'};var json = JSON.stringify(obj);</script></p>

#17 in reply to: ↑ 16 @bekk
6 days ago

Replying to jim5471:

@domainsupport

OK - now try the code without any line feeds.
It is being wrapped in <p> tags but the code is clean.

<?php
add_shortcode( 'xxx', function() { 
        return "<!-- wp:html --><script>var obj = { 'foo': 'bar', 'baz': 'qux'};var json = JSON.stringify(obj);</script><!-- /wp:html -->";
});
<p><script>var obj = { 'foo': 'bar', 'baz': 'qux'};var json = JSON.stringify(obj);</script></p>

Right, see https://developer.wordpress.org/reference/functions/wpautop/

#18 follow-up: @jim5471
6 days ago

OK - just found one difference between the example we have been looking at and the way I have coded my shortcodes.

<?php
function do_xxx() {
       $html = "<!-- wp:html --><script>
           var obj = { 'foo': 'bar', 'baz': 'qux'};
           var abc = 'xcxcfvgbcxbvcxbvxc';
           var json = JSON.stringify(obj);
           </script><!-- /wp:html -->";
        return $html;
    }
    add_shortcode('xxx', 'do_xxx');

Returns this...

<p><script>
           var obj = { 'foo': 'bar', 'baz': 'qux'};
           var abc = 'xcxcfvgbcxbvcxbvxc';
           var json = JSON.stringify(obj);
           </script></p> 

#19 in reply to: ↑ 18 @domainsupport
6 days ago

Replying to jim5471:

OK - just found one difference between the example we have been looking at and the way I have coded my shortcodes.

<?php
function do_xxx() {
       $html = "<!-- wp:html --><textarea>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</textarea><!-- /wp:html -->";
        return $html;
    }
    add_shortcode('xxx', 'do_xxx');

Returns this...

<p><textarea>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</p>
<p>Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.</p>
<p>Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.</p>
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</textarea></p>

#20 @davidsjptimoney
6 days ago

Another use case. Our site is for a non-profit film club. We use Eventbrite for ticketing, specifically their Embedded Checkout modal, which looks like this:

<!-- Noscript content for added SEO -->
<noscript><a href="https://www.eventbrite.co.uk/e/aftersun-12-tickets-590034829057" rel="noopener noreferrer" target="_blank">Buy Tickets on Eventbrite</a></noscript>
<!-- You can customise this button any way you like -->
<button id="eventbrite-widget-modal-trigger-590034829057" type="button">Buy Tickets</button>

<script src="https://www.eventbrite.co.uk/static/widgets/eb_widgets.js"></script>

<script type="text/javascript">
    var exampleCallback = function() {
        console.log('Order complete!');
    };

    window.EBWidgets.createWidget({
        widgetType: 'checkout',
        eventId: '590034829057',
        modal: true,
        modalTriggerElementId: 'eventbrite-widget-modal-trigger-590034829057',
        onOrderComplete: exampleCallback
    });
</script>

This is held in an ACF field for the film (a post). After the 6.2.2 fix, this is parsed and the line-feeds & carriage returns replaced with <br> and <p>.

To work around this, we've had to add extra code to the PHP snippet that pulls the ACF field and customises the button. Thus:

$searchf = array("\n","\r");
$content = str_replace($searchf,"",$content);

The other thing we noticed was that a table cell that wasn't properly closed (i.e. <tr><td></tr> instead of <tr><td></td></tr>) got a </p> injected, which caused layout issues.

We use a customised twenty twenty-two theme (not a child theme). Some of our site fell over with 6.2.1 but we got round that by encapsulating the shortcodes in the template within template parts, though I'm not clear if that is a sutainable solution, to judge from some of the other comments I've seen on here.

Last edited 5 days ago by davidsjptimoney (previous) (diff)

#21 @desrosj
6 days ago

  • Keywords needs-testing has-testing-info needs-unit-tests added

Marking has-testing-info, but please do provide as many detail as you can. If you've reported above and left out important details, feel free to clarify and expand.

Also marking needs-unit-tests. Ideally we start writing more tests for these edge cases to help us ensure they're all resolved appropriately. Probably a good situation for test first development.

#22 @jim5471
6 days ago

@domainsupport

OK now I get it !

#23 @ryansantschi
6 days ago

Having this issue as well. Not sure if this goes without saying, but any HTML comments (<!--) also are put in paragraph tags.

In my particular use case this is providing a temporary work around:

// Replace one or more consecutive whitespace characters with a single space
$shortcode_content = preg_replace('/\s+/', ' ', $shortcode_content);
// Remove HTML comments from the content
$shortcode_content = preg_replace('/<!--(.|\s)*?-->/', '', $shortcode_content);
Last edited 6 days ago by ryansantschi (previous) (diff)

#24 @tdrayson
6 days ago

I have been doing some testing, it seems that it's only the shortcode "block" that has this formatting issue.

If you put your shortcode into a template with a paragraph block, everything gets rendered correctly.

Backend - https://share.getcloudapp.com/kpul9w0K
Frontend - https://share.getcloudapp.com/YEu4qyLL

Not a perfect solution as your shortcode will get wrapped with a <p> tag.

#25 @kubiq
6 days ago

@tdrayson shortcode block in Template and Template part,
but also take a look on my demo:
https://demo.kubiq.sk/sample-page/
and comment here
https://core.trac.wordpress.org/ticket/58366#comment:3
Shortcodes in reusable blocks are completely broken, no matter how you add them, once they are in Template or Template part

#26 @Ov3rfly
6 days ago

Another use case similar to @davidsjptimoney, javascript snippets added via ACF field.

We have not (yet) the problem described here, but obviously only due to (still) use of classic theme instead of (planned) block theme.

We can not simply strip \n and \r from code as our javascript snippets also contain lines with // javascript comments similar to example here: https://stackoverflow.com/q/65274912

<script>
// Set to the same value as the web property used on the site
var gaProperty = 'UA-XXXX-Y';

// Disable tracking if the opt-out cookie exists.
var disableStr = 'ga-disable-' + gaProperty;
if (document.cookie.indexOf(disableStr + '=true') > -1) {
  window[disableStr] = true;
}

// Opt-out function
function gaOptout() {
  document.cookie = disableStr + '=true; expires=Thu, 31 Dec 2099 23:59:59 UTC; path=/';
  window[disableStr] = true;
}
</script>

These kind of code snippets with // javascript comments lines are widely in use and need their linefeeds present.

#27 follow-ups: @bekk
6 days ago

I haven't been able to come up with a simple enough patch that doesn't cause side effects. For anyone needing a temporary workaround mine is below it excludes the wpautop'ing for only the shortcode block, also @tdrayson's works too with the caveat that the shortcode is <p>'d

<?php
add_filter('register_block_type_args', function ($settings, $name) {
        if ($name === 'core/shortcode') {
                $settings['render_callback'] = function ($attributes, $content) {
                        return $content;
                };
        }
        return $settings;
}, 10, 2);
Last edited 6 days ago by bekk (previous) (diff)

#28 in reply to: ↑ 27 @kubiq
6 days ago

Replying to bekk:

Nice, that will fix that <p>s and <br>s... Thanks for the effort!

But there is still one big issue - shortcodes in reusable blocks in Templates and Templat parts are not rendering at all... Imagine some CTA section with Newsletter form from Contact Form 7 or anything like that = big problem on many many websites

#29 @andreasjr
6 days ago

I'm finding that 6.2.2 broke our shortcodes even more so than normal (we were at least able to fix rendering them by sticking them in Template Parts). As @kubiq mentioned, we use a lot of Newsletter signup forms in our CTA/Footer sections, and Gravity Forms' "block" is just a wrapper around a shortcode. This means all our footers just show a shortcode snippet instead of the form.

#30 follow-up: @andreasjr
6 days ago

Here is a more detailed finding of the problem me and my team are facing.

Some context: We are specifically experiencing problems with the Gravity Forms block. The GF block looks like it’s a wrapper for a shortcode. Obviously this is not WP team’s responsibility to fix, and ultimately GF will need to fix this, but it should be noted this block worked fine before the 6.2.x updates.

In a page:

  • GF block works
  • Shortcode works

Reusable block on a page:

  • GF block works
  • Shortcode works

In a template (e.g. page.html):

  • GF block does not work
  • Shortcode works

Reusable block in a template (e.g. page.html):

  • GF block does not work
  • Shortcode does not work

Template part in a template (e.g. part.html in page.html):

  • GF block does not work
  • Shortcode works

Reusable block in a template part in a template (e.g. part.html in page.html):

  • GF block does not work
  • Shortcode does not work

Additionally, when the shortcode does work, as others have mentioned there are extra <br> and <p> tags added.

This ticket was mentioned in Slack in #core by andreasjr. View the logs.


6 days ago

#32 in reply to: ↑ 27 @aflijja
6 days ago

Replying to bekk:
thank you it work.

#33 @coreyw
6 days ago

Also experiencing this with 6.2.2 and the WP Notification Bell plugin.

https://pastebin.com/bZjP5VAE

Notice errant paragraph tags inside script, surrounding spans, etc.

@peterwilsoncc
5 days ago

POC -- not for commit

#34 @peterwilsoncc
5 days ago

In [33469] for #33106, a placeholder comment was added to wpautop() to protect new lines within CDATA.

In 58366-poc.diff I've done something similar for protecting new lines within shortcodes for block themes. It uses the slightly more verbose wp-block-theme-line-break-placeholder to avoid collisions with wpautop placeholders.

I've done some minimal testing using the shortcode in comment 3 but if the approach is validated, it will need a rewrite as the POC uses closures unnecessarily.

#35 @christopherplus
5 days ago

Also experience this in 6.2.2 with a shortcode that outputs WP_Query.

@bekk
5 days ago

For review

#36 @bekk
5 days ago

My patch draws on the observation that the shortcode block should not be autop'ing its own content. It is noted in the_content filters that shortcodes should be processed after wpautop(), I don't see why the shortcode block is any different.

I also added shortcode processing to the core block, which restores shortcodes in template parts and reusable blocks.

My patch is attached above for review

#37 @stiofansisland
5 days ago

We are also having issues. Nested shortcodes no longer act as they did before, now nested shortcodes show the ending trailing shortcode. All issue seem to stem from this change: https://github.com/WordPress/WordPress/commit/2bb3a5169548d16173cf48ca9da1111efc428f86

The shortcode calls were put back in the wrong place, everything will be fixed for everyone if this is restored to how it was in 6.2.0. Unless there is a security reason not to do this, please do it?

This ticket was mentioned in Slack in #core-editor by kubiq. View the logs.


5 days ago

#39 in reply to: ↑ 30 @kaylam
5 days ago

Replying to andreasjr:

Here is a more detailed finding of the problem me and my team are facing.

Some context: We are specifically experiencing problems with the Gravity Forms block. The GF block looks like it’s a wrapper for a shortcode. Obviously this is not WP team’s responsibility to fix, and ultimately GF will need to fix this, but it should be noted this block worked fine before the 6.2.x updates.

I am also having ongoing issues with the Gravity Forms block from the recent update. At first I could get the form back by putting it in a Template Part. Now, as @andreasjr outlined, in a Template or Template Part, I can get the form to show up by using a core Shortcode Block instead, but there's <p> and <br> in the resulting html that should not be there.

Note: See TracTickets for help on using tickets.