WordPress.org

Make WordPress Core

Opened 6 months ago

Closed 5 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:

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)

46679.patch (4.2 KB) - added by ramiy 6 months ago.
46679-back-comp.patch (6.8 KB) - added by ramiy 6 months ago.
46679-back-comp.2.patch (6.8 KB) - added by ramiy 6 months ago.

Download all attachments as: .zip

Change History (24)

#1 @adamsilverstein
6 months ago

  • Owner set to adamsilverstein
  • Status changed from new to assigned

#2 @lgedeon
6 months ago

Proposed solution to add this to each of the default themes.

<?php
if ( ! function_exists( 'wp_body_open' ) ) {
        function wp_body_open() {
                do_action( 'wp_body_open' );
        }
}

We might also add the doc block. Not sure on expectations there.

#3 follow-up: @johnbillion
6 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?

Last edited 6 months ago by johnbillion (previous) (diff)

#4 in reply to: ↑ 3 @timph
6 months ago

Replying to johnbillion:

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?

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 @klihelp
6 months ago

Core functions looks consistent in the theme, and without any parameter. Needed action:)

wp_head();

wp_body_open();

wp_footer();

#6 @adamsilverstein
6 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.


6 months ago

#8 @ramiy
6 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+.

@ramiy
6 months ago

#9 @ramiy
6 months ago

  • Keywords has-patch added; needs-patch removed

#10 @ramiy
6 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 @adamsilverstein
6 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 @pento
6 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 the wp_head() / wp_footer() pattern. Particularly as this is the actual function call that works in WP 5.2+, using a do_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 @ramiy
6 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.

#14 @ramiy
6 months ago

  • Keywords needs-refresh removed

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


5 months ago

#16 @ramiy
5 months ago

@adamsilverstein @pento please review the new patch.

#17 @adamsilverstein
5 months ago

Thanks @ramiy - patch looks good to me. @pento - does this look good for commit?

Last edited 5 months ago by adamsilverstein (previous) (diff)

#18 @adamsilverstein
5 months ago

  • Keywords 2nd-opinion commit added

#19 @ramiy
5 months ago

@pento ?

#20 @pento
5 months ago

  • Keywords 2nd-opinion removed

Thanks, @ramiy! Patch looks good.

#21 @adamsilverstein
5 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 45256:

Bundled Theme: add a wp_body_open shim for older WordPress versions.

WordPress 5.2 adds the wp_body_open() function, and the default themes make use of it. This patch adds a shim for wp_body_open to bundled themes so this function will also work in older versions of WordPress.

Props lgedeon, johnbillion, timph, ramiy, pento.
Fixes #46679.

Note: See TracTickets for help on using tickets.