Opened 6 years ago
Closed 6 years 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: |
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
@
6 years 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
@
6 years 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
@
6 years ago
Core functions looks consistent in the theme, and without any parameter. Needed action:)
wp_head(); wp_body_open(); wp_footer();
#6
@
6 years 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.
6 years ago
#8
@
6 years 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
@
6 years 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
@
6 years 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
@
6 years 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
@
6 years 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.