Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#50019 closed defect (bug) (fixed)

remove_meta_box triggers warning in PHP 7.4

Reported by: coolmann's profile coolmann Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.5 Priority: normal
Severity: major Version: 5.4
Component: Administration Keywords: has-patch
Focuses: ui Cc:

Description (last modified by SergeyBiryukov)

After updating my local sandbox to PHP 7.4, I started seeing errors on my Edit Page screen for my custom post type:

Notice: Trying to access array offset on value of type bool in /../wp-admin/includes/template.php on line 1079, 1080, 1081

After some research, I narrowed down the issue to a call in one of my plugin, which removes one of the metaboxes on that screen:

remove_meta_box( 'profile_rolediv', 'people', 'normal' );

Everything had been working fine in earlier versions. Looking at the code in template.php, it looks like remove_meta_box just sets values to false, instead of removing the entry:

function remove_meta_box( $id, $screen, $context ) {
  .
  .
  $wp_meta_boxes[ $page ][ $context ][ $priority ][ $id ] = false;
  .
  .
}

which then causes this line in template.php to fail:

$title = $wp_meta_boxes[ $page ][ $a_context ][ $a_priority ][ $id ]['title'];

Changing line 1078 in wp-admin/includes/template.php fixes the issue:

} elseif ( 'sorted' == $priority && is_array( $wp_meta_boxes[ $page ][ $a_context ][ $a_priority ][ $id ] ) ) {

Thank you,
Jason

Attachments (1)

template.php (88.5 KB) - added by coolmann 5 years ago.
edited template.php

Download all attachments as: .zip

Change History (13)

@coolmann
5 years ago

edited template.php

#1 @coolmann
5 years ago

Alternatively, instead of changing line 1078 as suggested above, line 1050 in template.php could be changed from

if ( ! isset( $wp_meta_boxes[ $page ][ $a_context ][ $a_priority ][ $id ] ) ) {

to

if ( empty( $wp_meta_boxes[ $page ][ $a_context ][ $a_priority ][ $id ] ) ) {

This ticket was mentioned in PR #240 on WordPress/wordpress-develop by slimstat.


5 years ago
#2

After updating my local sandbox to PHP 7.4, I started seeing errors on my Edit Page screen for my custom post type:

Notice: Trying to access array offset on value of type bool in /../wp-admin/includes/template.php on line 1079, 1080, 1081

After some research, I narrowed down the issue to a call in one of my plugin, which removes one of the metaboxes on that screen:

remove_meta_box( 'profile_rolediv', 'people', 'normal' );

Everything had been working fine in earlier versions. Looking at the code in template.php, it looks like remove_meta_box just sets values to false, instead of removing the entry:

function remove_meta_box( $id, $screen, $context ) {
.
$wp_meta_boxes[ $page ][ $context ][ $priority ][ $id ] = false;
.
}

which then causes this line in template.php to fail:

$title = $wp_meta_boxes[ $page ][ $a_context ][ $a_priority ][ $id ]title?;

Changing line 1050 in wp-admin/includes/template.php fixes the issue (replace isset with empty):

if ( empty( $wp_meta_boxes[ $page ][ $a_context ][ $a_priority ][ $id ] ) ) {

Thank you,
Jason

Trac ticket: https://core.trac.wordpress.org/ticket/50019

#3 @SergeyBiryukov
5 years ago

  • Description modified (diff)

This ticket was mentioned in PR #241 on WordPress/wordpress-develop by slimstat.


5 years ago
#4

After updating my local sandbox to PHP 7.4, I started seeing errors on my Edit Page screen for my custom post type:

Notice: Trying to access array offset on value of type bool in /../wp-admin/includes/template.php on line 1079, 1080, 1081

After some research, I narrowed down the issue to a call in one of my plugin, which removes one of the metaboxes on that screen:

remove_meta_box( 'profile_rolediv', 'people', 'normal' );

Everything had been working fine in earlier versions. Looking at the code in template.php, it looks like remove_meta_box just sets values to false, instead of removing the entry:

function remove_meta_box( $id, $screen, $context ) {
.
$wp_meta_boxes[ $page ][ $context ][ $priority ][ $id ] = false;
.
}

which then causes this line in template.php to fail:

$title = $wp_meta_boxes[ $page ][ $a_context ][ $a_priority ][ $id ]title?;

Changing line 1050 in wp-admin/includes/template.php fixes the issue (replace isset with empty):

if ( empty( $wp_meta_boxes[ $page ][ $a_context ][ $a_priority ][ $id ] ) ) {

Thank you,
Jason

Trac ticket: https://core.trac.wordpress.org/ticket/50019

<!--
Hi there! Thanks for contributing to WordPress!

Pull Requests in this GitHub repository must be linked to a ticket in the WordPress Core Trac instance (https://core.trac.wordpress.org), and are only used for code review. No pull requests will be merged on GitHub.

See the WordPress Handbook page on using PRs for Code Review more information: https://make.wordpress.org/core/handbook/contribute/git/github-pull-requests-for-code-review/

If this is your first time contributing, you may also find reviewing these guides first to be helpful:

-->

Trac ticket:

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


5 years ago

#6 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 5.5

#7 @SergeyBiryukov
5 years ago

#50122 was marked as a duplicate.

#8 @coolmann
5 years ago

  • Keywords has-patch added; needs-patch removed

#9 @SergeyBiryukov
5 years ago

  • Component changed from Posts, Post Types to Administration

Thanks for the patch!

Some findings from my testing:

  • The issue can only be reproduced if you've previously manually reordered the meta boxes in your own way, and the preference is saved in the meta-box-order_$page user option.
  • If that's the case, do_meta_boxes() pulls the boxes out of their previous priority into the one the user chose, calling add_meta_box() with the sorted priority. This was added in [8682].
  • There is some logic in add_meta_box() to skip previously removed boxes. This was added earlier in [7930], and it only applies to the core priority. This is where the issue is, it should also apply to the sorted priority.

#10 @SergeyBiryukov
5 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 47777:

Administration: Avoid a PHP 7.4 notice in add_meta_box() when attempting to re-add a previously removed box.

The logic for skipping previously removed meta boxes with the core priority should also apply to the sorted priority that is used when the boxes were manually reordered.

Add a unit test.

Props coolmann, franzarmas, SergeyBiryukov.
Fixes #50019.

#11 @coolmann
5 years ago

@SergeyBiryukov thank you for looking into this and for identifying the issue. I'm going to test the patch you linked (47777) and report back. Do you know when is that patch going to be released?

#12 @coolmann
5 years ago

@SergeyBiryukov this patch worked as expected! Thank you so much.

Note: See TracTickets for help on using tickets.