WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 3 months ago

#51081 closed defect (bug) (fixed)

Fatal Error - Undefined get_page_templates() in Customizer

Reported by: Howdy_McGee Owned by: SergeyBiryukov
Milestone: 5.5.1 Priority: normal
Severity: normal Version: 5.5
Component: Customize Keywords: has-patch commit dev-reviewed fixed-major
Focuses: administration, template Cc:

Description

I'm using the display_post_states filter hook to display assigned page templates as "Post State" whenever they're listed. WordPress 5.5 breaks this functionality when viewing The Customizer. The easiest way to replicate this issue is to paste the following into any TwentyXX Theme:

/**
 * Add assigned templates to Post States
 * - https://developer.wordpress.org/reference/hooks/display_post_states/
 *
 * @param Array $post_states
 *
 * @return Array $post_states
 */
function prefix_page_template_labels( $post_states ) {
	
	$page_templates = get_page_templates();
	return $post_states;
	
}
add_filter( 'display_post_states', 'prefix_page_template_labels' );

At this point, when you go to The Customizer it throws a Fatal Error as get_page_templates() is not defined.

Attachments (2)

51081.2.diff (613 bytes) - added by audrasjb 3 months ago.
Docs: Properly document 5.5 changes in display_post_states filter doc block
51081.3.diff (648 bytes) - added by garrett-eclipse 3 months ago.
Updated verbiage; Capitalize Customizer, be extra clear indicate function_exists is the best way to check.

Download all attachments as: .zip

Change History (17)

#1 @desrosj
3 months ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 5.5.1

Thanks @Howdy_McGee! I am moving this to 5.5.1 for testing and verification.

#2 @SergeyBiryukov
3 months ago

Related:

This appears to be caused by the fact that get_post_states() (along with the display_post_states filter) is now used in the Customizer to denote special pages (per [47763] above), but does not have all of the admin context.

Per [48620] above, the Customizer requires wp-admin/includes/template.php if get_post_states() does not exist. However, get_page_templates() is defined in another file: wp-admin/includes/theme.php.

The easiest workaround for the example from the ticket description would be to check if the function exists:

function prefix_page_template_labels( $post_states ) {
	if ( function_exists( 'get_page_templates' ) ) {
		$page_templates = get_page_templates();
	}

	return $post_states;
	
}
add_filter( 'display_post_states', 'prefix_page_template_labels' );

I don't think we realistically can (or need to) require all of the admin files in the Customizer, so I'm not sure if anything needs changing here.

We could, however add some documentation for the display_post_states filter to explain that it is now also applied in the Customizer context, so if any admin functions are used, they need to be checked for existence.

Last edited 3 months ago by SergeyBiryukov (previous) (diff)

#3 @dlh
3 months ago

@Howdy_McGee I'm sorry to hear that this change to the Customizer caused an issue on your site. It probably could have used a stronger callout in a 5.5 dev note, and it's on me for not having thought of it.

Still, I'm inclined to agree with Sergey that it's not going to be feasible to try to load the same admin context in the Customizer that get_post_states() ran in previously. Adding the function_exists() check might be the best way to go. A @since note in the filter docs might also be appropriate, as suggested.

#4 @Howdy_McGee
3 months ago

  • Resolution set to wontfix
  • Status changed from new to closed

That's understandable, I wasn't sure if this was intended behavior. Closing as wontfix, thanks everyone!

#5 @SergeyBiryukov
3 months ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Let's keep the ticket open to improve the documentation for the filter :)

#6 @SergeyBiryukov
3 months ago

  • Owner set to SergeyBiryukov
  • Status changed from reopened to accepted

#7 @khag7
3 months ago

@SergeyBiryukov what's the protocol for updating documentation? Happy to help contribute to that, just don't know how!

#8 @garrett-eclipse
3 months ago

@khag7 appreciate the offer, all that's needed to contribute an update to the filter documentation is to supply either a svn diff patch or a git PR with the updated verbiage.

Working with Patches - https://make.wordpress.org/core/handbook/tutorials/working-with-patches/
Submitting a Patch - https://make.wordpress.org/core/handbook/tutorials/trac/submitting-a-patch/
Prefer Github? You can PR - https://make.wordpress.org/core/handbook/contribute/git/github-pull-requests-for-code-review/

Do note the PHP Documentation Standards;
https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/

The existing PHP docblock to look at for this is here;
https://github.com/WordPress/wordpress-develop/blob/5602ba4db7284a4787f0f311008ca9f99a22eab1/src/wp-admin/includes/template.php#L2221-L2230

Hope that helps, there's also the new contributor meetings on wednesdays.
Upcoming WordPress Meetings: https://make.wordpress.org/meetings/

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


3 months ago

@audrasjb
3 months ago

Docs: Properly document 5.5 changes in display_post_states filter doc block

#10 follow-up: @audrasjb
3 months ago

  • Keywords has-patch added; needs-testing removed

For some reason, Trac crashed during the upload so my patch ended up to be renamed as 51081.2.diff :D

This patch is a proposal for the @since change. Not 100% sure about the best phrasing.

#11 in reply to: ↑ 10 @garrett-eclipse
3 months ago

Replying to audrasjb:

This patch is a proposal for the @since change. Not 100% sure about the best phrasing.

Thanks @audrasjb maybe we can go with;
"Now also applied in the Customizer context. If any admin functions are used within the filter their existence should be checked with function_exists before use."

Thoughts

@garrett-eclipse
3 months ago

Updated verbiage; Capitalize Customizer, be extra clear indicate function_exists is the best way to check.

#12 @SergeyBiryukov
3 months ago

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

In 48910:

Docs: Add a @since note to the display_post_states filter to clarify that it is now also applied in the Customizer context.

If any admin functions are used within the filter, their existence should be checked with function_exists() before being used.

Follow-up to [47763], [48620].

Props audrasjb, garrett-eclipse, Howdy_McGee, dlh, khag7, SergeyBiryukov.
Fixes #51081.

#13 @SergeyBiryukov
3 months ago

  • Keywords commit dev-feedback fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backporting to the 5.5 after a second committer's review.

Version 0, edited 3 months ago by SergeyBiryukov (next)

#14 @desrosj
3 months ago

  • Keywords dev-reviewed added; dev-feedback removed

👍🏼 for backport.

#15 @desrosj
3 months ago

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

In 48915:

Docs: Add a @since note to the display_post_states filter to clarify that it is now also applied in the Customizer context.

If any admin functions are used within the filter, their existence should be checked with function_exists() before being used.

Follow-up to [47763], [48620].

Props audrasjb, garrett-eclipse, Howdy_McGee, dlh, khag7, SergeyBiryukov.
Merges [48910] to the 5.5 branch.
Fixes #51081.

Note: See TracTickets for help on using tickets.