-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try: parse shortcode blocks outside the content #37545
Conversation
Adds do_shortcode() to parse the shortcode outside the content (block templates, block patterns)
Hi, @carolinan I think we should resolve this by adding It is how we handle it for Template Parts. A related conversation - #17626 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works as expected. Thanks, @carolinan.
P.S. I've updated the PR description, so it correctly links the issue.
Don't we need to run shortcode_unautop before do_shortcode? |
Good point, @carlomanf. I can see empty |
OK, how about now, I added both convert_smilies and shortcode_unautop. |
I'm assuming you're using Also, let's run |
Well, it matches what's in the template part: |
Good call. Feel free to merge once all checks are green. Thanks again for working on this, @carolinan. |
@noisysocks, should we backport this to WP 5.9? |
Missed this, sorry. I think it's too late for WP 5.9 as RC 1 was released this morning. Since it isn't landing in WP 5.9 we should move the changes out of |
After some thought, I am doubtful if the approach here is sound. The reason is that many of the filters here ( This is inefficient at best and could cause unexpected behaviour at worst. I imagine that if you double-wrap a shortcode to escape it (e.g. What might be a better approach is to add a condition to some of the default filters to only be enqueued if a block template is not active, and if a block template is active, apply them just once to the entire filled-in template. By the way, another issue has been opened: https://core.trac.wordpress.org/ticket/54759 |
@carlomanf, I agree there's some issue with this approach in general (not with this PR). The |
Now that I think about it, maybe the same could also potentially happen with a double-wrapped shortcode inside a post content block inside a query block inside |
I will try to do more testing on Monday. |
@Mamaduka @noisysocks we should probably backport this to WP 5.9, not in the next minor :) |
@audrasjb, I tested shortcode escaping as @carlomanf suggested, and What do you think about moving I'm pro including this into 5.9 if we can resolve these issues. Example shortcode: add_shortcode( 'note', function() {
ob_start();
?>
<div class="note" style="margin: 30px 0; padding: 27px 40px; background: #fcf3dc; border-radius: 5px;">
<p>This is a note!</p>
</div>
<?php
return ob_get_clean();
} ); |
Can you elaborate more about the mixed results? I would expect shortcodes directly in the template to escape correctly, but ones in a template part or a post content within the template to not work, because those blocks apply I would also expect moving |
I think this will require a complex fix and I personally don't see any problem with it being left out of 5.9 because block templates were introduced in 5.8 and have been lacking shortcode support for a full release cycle already. |
Here are my results for escaped shortcut tests:
|
OK I trust y'all 🙂 |
* Try: parse shortcode blocks outside the content Adds do_shortcode() to parse the shortcode outside the content (block templates, block patterns)
I misread some of the conversation here and thought the consensus was to yes to backporting this. Thanks @Mamaduka for pointing out to me that the consensus was actually no. It was included in WordPress/wordpress-develop#2184 which will land in 5.9 RC 3. Sorry about that. Since it's a PHP-only change, it's very easy to revert. (No need to publish packages.) I think we can leave it in 5.9 RC 3 and if there are any bug reports to do with this then we can revert the changes before the final release next week. Ping me if you see any such reports. It would also be helpful, of course, to download 5.9 RC 3 when it's published today and test it 😀 |
The report was lodged right here above before you asked for it. If this change was included mistakenly, then I don't understand why it wasn't reverted immediately after the mistake was noticed. To be crystal clear, this is the kind of bug we are talking about:
The above is based on my own testing, and although I wasn't able to reproduce the mixed results that @Mamaduka did, the escaping did fail on my first test, which is evidence enough that this is not ready to be released. |
Could you open a PR to include in 5.9.1? |
If Shortcode support can be fixed instead of reverted in 5.9.1 that would be preferred/appreciated by myself and any plugin developers who use shortcodes to render their Block contents. We build our Block & Widget on top of our Shortcode for easier cross-context support. Until WP5.9RC3 was release it was broken in the Site Editor context due to the Shortcode not rendering. |
Description
Adds do_shortcode() to parse the shortcode when it is placed outside the content (block templates, block patterns).
Closes #35258. Partially #37312
How has this been tested?
Sample shortcode block and smilies:
Tested manually by:
Screenshots
Types of changes
Bug fix
Checklist:
*.native.js
files for terms that need renaming or removal).