Opened 7 months ago
Closed 6 months ago
#46679 closed defect (bug) (fixed)
Themes: Add a shim for wp_body_open()
Reported by: | pento | Owned by: | adamsilverstein |
---|---|---|---|
Milestone: | 5.2 | Priority: | normal |
Severity: | normal | Version: | 5.2 |
Component: | Bundled Theme | Keywords: | has-patch commit |
Focuses: | Cc: | ||
PR Number: |
Description
Split off from #12563.
WordPress 5.2 adds the wp_body_open()
function, and the default themes make use of it.
They don't have a shim for this function, however, so will trigger fatal errors if they're run on an older version of WordPress.
Attachments (3)
Change History (24)
#3
follow-up:
↓ 4
@
7 months ago
- Version set to trunk
What's the real purpose of the wp_body_open()
wrapper function? It presents consistency with other functions such as wp_head()
and wp_footer()
, but would placing a direct call to do_action( 'wp_body_open' )
be so bad?
#4
in reply to:
↑ 3
@
7 months ago
Replying to johnbillion:
What's the real purpose of the
wp_body_open()
wrapper function? It presents consistency with other functions such aswp_head()
andwp_footer()
, but would placing a direct call todo_action( 'wp_body_open' )
be so bad?
I understand the consistency part, and agree with this. Just adding that do_action( 'wp_body_open' )
is 100% more clear about what it actually does as well. It's frustrating when you have to search through code just to figure out something is only a wrapper around an action.
#5
@
7 months ago
Core functions looks consistent in the theme, and without any parameter. Needed action:)
wp_head(); wp_body_open(); wp_footer();
#6
@
7 months ago
What's the real purpose of the wp_body_open() wrapper function?
Mainly this was added this way for consistency with other function calls/existing pattern for header/footer calls. It was discussed a bit in #12563. I do see how switching to triggering an action directly would remove the need for a shim though, which we didn;t consider previously.
This ticket was mentioned in Slack in #themereview by tim. View the logs.
7 months ago
#8
@
7 months ago
The default themes, and any other theme should use:
<?php if ( function_exists( 'wp_body_open' ) ) wp_body_open(); ?>
instead of just:
<?php wp_body_open(); ?>
Unless the theme "Minimum Required WordPress Version" is 5.2+.
#10
@
7 months ago
The second patch implements the solution suggested by @lgedeon.
Note that some themes has a separate file with backward compatibility functions (inc/back-compat.php
). In other themes the code implemented in functions.php
.
#11
@
7 months ago
- Keywords 2nd-opinion added
Thanks for the patches @ramiy! I like 46679-back-comp.patch - it brings the functionality wherever these themes are used, benefiting users.
Does this look good to you @pento?
#12
@
7 months ago
- Keywords needs-refresh added; 2nd-opinion removed
Yah, I like the direction of 46679-back-comp.patch. The function shouldn't go in inc/back-compat.php
, though, as that file is only loaded on versions of WordPress where the theme won't work, and it'll prevent the theme from being activated. Where the file exists, lets put it in inc/template-tags.php
.
To address the other options:
- I think the
wp_body_open()
function call fits better with thewp_head()
/wp_footer()
pattern. Particularly as this is the actual function call that works in WP 5.2+, using ado_action()
call solely for back compat is presenting a sub-optimal example for folks to base their themes off. - Adding a
function_exists( 'wp_body_open' )
check is an unnecessary logic check in a template file, which should be avoided where possible.
#13
@
7 months ago
@pento Thanks for the feedback. I've updated the patch.
In 2010-2014 I used the functions.php
file, In 2015-2019 I used the inc/template-tags.php
file.
Also, I replaced if () { ... }
with if () : ... endif;
, to match the current code convention in theme files.
Proposed solution to add this to each of the default themes.
We might also add the doc block. Not sure on expectations there.