WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 3 months ago

Last modified 3 months ago

#46743 closed defect (bug) (wontfix)

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

Reported by: SergeyBiryukov Owned by:
Milestone: Priority: high
Severity: normal Version: 5.2
Component: Themes Keywords: has-patch needs-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 3 months ago.
46743.1.diff (5.7 KB) - added by whyisjake 3 months ago.
Explicitly declare the theme supports in the function files of the existing themes.

Download all attachments as: .zip

Change History (19)

This ticket was mentioned in Slack in #themereview by dannycooper. View the logs.


4 months ago

@desrosj
3 months ago

#2 @desrosj
3 months ago

  • Keywords has-patch added

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

#3 in reply to: ↑ description ; follow-up: @ramiy
3 months 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.


3 months ago

#5 in reply to: ↑ 3 @SergeyBiryukov
3 months 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 3 months ago by SergeyBiryukov (previous) (diff)

#6 @JeffPaul
3 months 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
3 months 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.


3 months ago

#9 @rabmalin
3 months 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
3 months 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 3 months ago by johnbillion (previous) (diff)

#11 @williampatton
3 months 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 3 months ago by williampatton (previous) (diff)

@whyisjake
3 months ago

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

#12 @pento
3 months 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
3 months 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
3 months ago

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

#15 @pento
3 months 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
3 months 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
3 months 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.

Note: See TracTickets for help on using tickets.