WordPress.org
Get WordPress

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#46743 closed defect (bug) (wontfix)

A way to detect whether the current theme supports wp_body_open()

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by:
Milestone: Priority: high
Severity: normal Version: 5.2
Component: Themes Keywords: has-patch has-dev-note
Focuses: Cc:

Description

Background: #12563, #46679.

With a new wp_body_open() template tag introduced in [45042], what's the recommended way for plugin authors to detect whether the current theme supports wp_body_open(), and use a fallback code otherwise?

Should we add body-open to the list of theme features registered with ​add_theme_support(), so that plugin authors could check it with current_theme_supports()?

Attachments (2)

46743.diff​ (1.1 KB) - added by desrosj 6 years ago.
46743.1.diff​ (5.7 KB) - added by whyisjake 6 years ago.
Explicitly declare the theme supports in the function files of the existing themes.

Download all attachments as: .zip

Change History (21)

This ticket was mentioned in ​Slack in #themereview by dannycooper. ​View the logs.
6 years ago

@desrosj
6 years ago

#2 @desrosj
6 years ago

  • Keywords has-patch added

46743.diff​ adds documentation noting body-open theme support.

#3 in reply to: ↑ description ; follow-up: @ramiy
6 years ago

  • Keywords 2nd-opinion added

Replying to SergeyBiryukov:

Should we add body-open to the list of theme features registered with ​add_theme_support(), so that plugin authors could check it with current_theme_supports()?

Not sure we should add a theme features for the new wp_body_open() function.

Why not?

  1. We don't register theme features for wp_head() and wp_footer().
  1. When hooking to the wp_body_open action, if the action does not exist (pre 5.3 versions), it won't do nothing and won't break nothing as the action does not exist.
  1. We should educate theme developers how to use wp_body_open() function (see #46679).
  1. Besides, we can't add theme features for every new function.

If plugin developers want to check if the wp_body_open() function exist, they can check it using:

if ( ! function_exists( 'wp_body_open' ) ) { ... }

The same way the default functions do that in ticket #46679.

This ticket was mentioned in ​Slack in #core by jeffpaul. ​View the logs.
6 years ago

#5 in reply to: ↑ 3 @SergeyBiryukov
6 years ago

Replying to ramiy:

Thanks for the feedback! I'll try to address the items one by one.

We don't register theme features for wp_head() and wp_footer().

wp_head() and wp_footer() are long-established functions, it's safe to assume every theme has them, as a lot of things break otherwise (admin bar, stylesheets, etc.).

When hooking to the wp_body_open action, if the action does not exist (pre 5.3 versions), it won't do nothing and won't break nothing as the action does not exist.

That means plugin authors cannot rely on the new action, as there's no guarantee that the current theme supports it. Unless I'm missing something, that makes the new action largely useless.

Let's consider the use case from #12563: adding asynchronous JS code right after the opening <body> tag. How would the plugin author know if the code was successfully added? If the current theme does not support wp_body_open(), they would need to use some fallback code.

We should educate theme developers how to use wp_body_open() function (see #46679)

I agree, but that does not solve the immediate issue for plugin authors.

Besides, we can't add theme features for every new function.

In this case, we don't actually need to add anything except some documentation, as in 46743.diff​. If a theme declares support with add_theme_support( 'body-open' ), it gets added to the $_wp_theme_features global, and then any plugin developer can check it with current_theme_supports( 'body-open' ).

If plugin developers want to check if the wp_body_open() function exist, they can check it using:

if ( ! function_exists( 'wp_body_open' ) ) { ... }

The point was not to check if the function exists (it always does in 5.2+), but whether the current theme supports it.

Last edited 6 years ago by SergeyBiryukov (previous) (diff)

#6 @JeffPaul
6 years ago

  • Milestone changed from 5.2 to 5.3

Per input from @williampatton in today's bug scrub, we're punting this to 5.3 as this still has details to iron out.

#7 @williampatton
6 years ago

If we can get these details ironed out real fast (like in next 1-2 days) then we could maybe be cheeky and sneak this back in on the 5.2 milestone.

I don't know of the best way to implement the feature this proposes though myself - but I do know that such a feature is wanted by plugin (and theme) devs. I'll ask for some extra eyes on this ticket from theme-review channel and see if they can figure it out.

This ticket was mentioned in ​Slack in #themereview by williampatton. ​View the logs.
6 years ago

#9 @rabmalin
6 years ago

I would go with the add_theme_support() approach so that theme author will have method to declare support for the wp_body_open(). In ideal world, all theme will have support for this, but it will take long time to have that support in the themes available around the world.

#10 @johnbillion
6 years ago

  • Milestone changed from 5.3 to 5.2
  • Priority changed from normal to high

This needs to be addressed in 5.2 really because the feature was introduced in 5.2.

I imagine theme developers will want to start using this straight away and without clear guidance for best usage it's not a reliable feature.

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

#11 @williampatton
6 years ago

A method involving the use of add_theme_support() is doable but I don't like the idea of having to define both the hook in the template and a support string in my setup functions. It seems redundant to need both.

Could we instead add a support method late (directly inside the wp_body_open hook) If a theme includes the hook in their template then it auto-adds the support. This would mean that plugin developers would need to do late detection - late detection may be preferable than redundant registration boilerplate code.

Last edited 6 years ago by williampatton (previous) (diff)

@whyisjake
6 years ago

Explicitly declare the theme supports in the function files of the existing themes.

#12 @pento
6 years ago

  • Keywords reporter-feedback added; 2nd-opinion removed

I don't really like the idea of new theme feature if we don't have to.

Are there some examples of how plugins put their code as soon after the <body> tag as possible? Legit examples only, please, I have little interest in supporting ​hacks like this.

It seems like plugins would be able to add their hook to the wp_body_open action, but leave their current practice in place, and check did_action( 'wp_body_open' ) to decide if they need to fall back to their current practice.

#13 @azaozz
6 years ago

It seems like plugins would be able to add their hook to the wp_body_open action, but leave their current practice in place, and check did_action( 'wp_body_open' ) to decide if they need to fall back to their current practice.

This makes the most sense imho. Plugins will need to keep their existing code for themes that don't have wp_body_open() and can decide whether to run it by looking at did_action( 'wp_body_open' ).

@SergeyBiryukov @johnbillion perhaps we can add some more docs describing this and close the ticket?

#14 @desrosj
6 years ago

This can be put in a dev note. @earnjam has a Miscellaneous Developer dev note that this could be dropped in.

#15 @pento
6 years ago

  • Keywords needs-dev-note added; reporter-feedback removed
  • Milestone 5.2 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Sounds like a plan. I assume @earnjam is working off the needs-dev-note keyword, so I'll close this ticket with that.

#16 @earnjam
6 years ago

I already had a sentence in the dev note that this ticket was being worked on, but I'll change that now to clarify how plugins should check for it.

#17 @spacedmonkey
6 years ago

Couldn't we hook into wp_body_open and set a flag in options, has_wp_body_open = true. This flag could be cleared on switch of theme. All that would require that flag to be set, is all page load on the front end.

#18 @danieliser
5 years ago

Seems a simpler solution may have eluded us.

<?php
add_action( 'wp_body_open', 'my_function' );
add_action( 'wp_footer', 'my_function' );

function my_function () {
    if ( doing_action( 'wp_body_open' ) ) {
        remove_action ( 'wp_footer', 'my_function' );
    }

    // do stuff.
}

#19 @desrosj
4 years ago

  • Keywords has-dev-note added; needs-dev-note removed

The correct practices for using the new wp_body_open() function while maintaining backwards compatibility was documented in the following dev note: ​https://make.wordpress.org/core/2019/04/24/miscellaneous-developer-updates-in-5-2/.

Note: See TracTickets for help on using tickets.