WordPress.org

Make WordPress Core

Opened 3 weeks ago

Last modified 3 days ago

#48599 accepted defect (bug)

Warning: count(): Parameter must be an array or an object that implements Countable in ../wp-admin/includes/plugin.php on line 1392

Reported by: ispreview Owned by: adamsilverstein
Milestone: 5.3.1 Priority: normal
Severity: normal Version: 5.3
Component: Menus Keywords: has-patch has-unit-tests commit
Focuses: ui Cc:
PR Number:

Description (last modified by SergeyBiryukov)

Myself and several others are seeing this through our WP Admin pages (at the top) after upgrading to WP5.3. I've noted that it shows on servers using PHP7.3 but is gone on the latest RC for PHP7.4.

https://wordpress.org/support/topic/error-after-5-3-update/

Possibly related to this: #39776

Attachments (5)

48599.patch (710 bytes) - added by ayeshrajans 3 weeks ago.
These count() warnings can pop up at unexpected places. The attached patch checks if the variable is countable (WordPress provides a polyfill for is_countable) before attempting to actually call count().
48599.1.diff (644 bytes) - added by garrett-eclipse 3 weeks ago.
Patch to test the theory it's the parent and menu slugs being identical causing $submenu[ $parent_slug ] to be undefined
48599.diff (3.5 KB) - added by welcher 2 weeks ago.
Patch with unit tests
48599.2.diff (3.6 KB) - added by garrett-eclipse 9 days ago.
Minor CS Update and Updated Comment
48599.3.diff (3.7 KB) - added by adamsilverstein 3 days ago.

Download all attachments as: .zip

Change History (31)

#1 @SergeyBiryukov
3 weeks ago

  • Description modified (diff)
  • Milestone changed from Awaiting Review to 5.3.1

#2 @hummelmose
3 weeks ago

I have the same error - WordPress 5.3 - NewsMag theme - PHP 7.2.19.

It's quite annoying.

#3 @marcusraitner
3 weeks ago

Same issue here. Really annoying.

@ayeshrajans
3 weeks ago

These count() warnings can pop up at unexpected places. The attached patch checks if the variable is countable (WordPress provides a polyfill for is_countable) before attempting to actually call count().

#4 @ispreview
3 weeks ago

Perhaps I'm doing it wrong but if I apply the line change in @ayeshrajans patch it returns two new errors on the admin dashboard:

Warning: array_unshift() expects parameter 1 to be array, null given in ../wp-admin/includes/plugin.php on line 1400

Warning: ksort() expects parameter 1 to be array, null given in ../wp-admin/includes/plugin.php on line 1414

#5 @ayeshrajans
3 weeks ago

@ispreview - I'm sorry the patch could be wrong, I didn't manually test it, although tests do pass (https://travis-ci.org/Ayesh/wordpress-develop/builds/611460877). Please revert the patch.

#6 @ispreview
3 weeks ago

It's been found that disabling the "NextScripts: Social Networks Auto-Poster" plugin stops this warning. According to the author of that plugin: "Interesting enough, it’s not our bug. WordPress broke something in the add_submenu_page() function. It gives this warning if the last optional parameter is provided."

Apparently a temporary fix will be introduced in the next plugin release, although WordPress still needs to fix the underlying issue.

#7 @NextScripts
3 weeks ago

Since we been mentioned here. Bug is really in WP itself.

You can install a clean WP 5.3 without any plugins and then install and activate this simple plugin that just creates one menu item with two submenus:

<?php
/*
Plugin Name: Test Plugin
Version: 1.0.0
*/

add_action( 'admin_menu', 'testPlg_adminMenu' );   

function testPlg_adminMenu() {
    add_menu_page( 'Test Plugin', 'Test|Plugin','manage_options','testPlg','testPlg_shoPage1');
    add_submenu_page( 
      'testPlg','SubMenu 1', 
      'SubMenu 1', 'manage_options', 
      'testPlg','testPlg_shoPage1', 
      1  
    );          
    add_submenu_page( 
       'testPlg','SubMenu 1', 
       'SubMenu 1', 'manage_options', 
       'testPlg-page1', 
       'testPlg_shoPage2', 
       2  
    );          
}
?>

You will get “Warning: count(): Parameter must be an array or an object that implements Countable in /home/oradell/public_html/wp-admin/includes/plugin.php on line 1392”

Now edit the code and remove last parameters from add_submenu_page

<?php
/*
Plugin Name: Test Plugin
Version: 1.0.0
*/

add_action( 'admin_menu', 'testPlg_adminMenu' );   

function testPlg_adminMenu() {
    add_menu_page( 'Test Plugin', 'Test|Plugin','manage_options','testPlg','testPlg_shoPage1');
    add_submenu_page( 
      'testPlg','SubMenu 1', 
      'SubMenu 1', 'manage_options', 
      'testPlg','testPlg_shoPage1'
    );          
    add_submenu_page( 
       'testPlg','SubMenu 1', 
       'SubMenu 1', 'manage_options', 
       'testPlg-page1', 
       'testPlg_shoPage2'
    );          
}
?>

Everything is fine now.

According to https://developer.wordpress.org/reference/functions/add_submenu_page/ that last parameter is int and it should work.

$position
(int) (Optional) The position in the menu order this item should appear.

Default value: null

It worked ok before 5.3

#8 follow-up: @adamsilverstein
3 weeks ago

I'm not sure this is a WordPress bug, it looks like this is happening when plugins/themes are mistakenly passing a non-countable 7th variable. The add_submenu_page function did not support the 7th variable until we added it for position (as an int) in WordPress 5.3. Passing anything there was unexpected and unsupported.

Perhaps if a non countable value is passed here, we should throw a _doing_it_wrong and use the default priority? What do you think @welcher?

I see this was fixed upstream in the NextScripts: Social Networks Auto-Poster plugin: https://plugins.trac.wordpress.org/changeset/2192461/estimate-delivery-date-for-woocommerce/trunk/admin/class-pi-edd-menu.php

#9 in reply to: ↑ 8 @NextScripts
3 weeks ago

Please see my example above. I am passing clear integer values 1 and 2 as parameters. Getting error. Integer is "countable", right?

Replying to adamsilverstein:

I'm not sure this is a WordPress bug, it looks like this is happening when plugins/themes are mistakenly passing a non-countable 7th variable. The add_submenu_page function did not support the 7th variable until we added it for position (as an int) in WordPress 5.3. Passing anything there was unexpected and unsupported.

Perhaps if a non countable value is passed here, we should throw a _doing_it_wrong and use the default priority? What do you think @welcher?

I see this was fixed upstream in the NextScripts: Social Networks Auto-Poster plugin: https://plugins.trac.wordpress.org/changeset/2192461/estimate-delivery-date-for-woocommerce/trunk/admin/class-pi-edd-menu.php

#10 follow-up: @adamsilverstein
3 weeks ago

hmmm, @NextScripts appologies, we cross posted. Your sample code should work, can you give any more information about your install, what version of PHP are you running? I would expect our unit tests to fail if this wasn't working as expected.

#11 in reply to: ↑ 10 @NextScripts
3 weeks ago

PHP Version: 7.3.11;
I just tried on fresh new install of WP 5.3 without any plugins with default "Twenty Twenty" theme.

Replying to adamsilverstein:

hmmm, @NextScripts appologies, we cross posted. Your sample code should work, can you give any more information about your install, what version of PHP are you running? I would expect our unit tests to fail if this wasn't working as expected.

#12 @NextScripts
3 weeks ago

If it helps.
I switched to PHP 5.6.40. Warning was replaced with

<b>Notice</b>:  Undefined index: testPlg in <b>/home/mapcom/public_html/wp-admin/includes/plugin.php</b> on line <b>1392</b><br />

Regardles..

If for the first subMenu I replace testPlg in 5th parameter to testPlg-page2 -

<?php
 add_submenu_page( 
      'testPlg','SubMenu 1', 
      'SubMenu 1', 'manage_options', 
      'testPlg-page2','testPlg_shoPage1', 
      1  
    );

no warnings/notices, but I get duplicate menu item.

Test|Plugin
--Test|Plugin
--Submenu 1
--Submenu 2

Last edited 3 weeks ago by NextScripts (previous) (diff)

#13 @adamsilverstein
3 weeks ago

@NextScripts Confirming I am able to reproduce the issue, need to dig in further to see why. Thanks for all the testing details.

Maybe @welcher has an idea whats off as he worked on the original code.

#14 @adamsilverstein
3 weeks ago

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

#15 @adamsilverstein
3 weeks ago

  • Keywords dev-feedback added

@garrett-eclipse
3 weeks ago

Patch to test the theory it's the parent and menu slugs being identical causing $submenu[ $parent_slug ] to be undefined

#16 follow-up: @garrett-eclipse
3 weeks ago

  • Keywords has-patch needs-testing added; needs-patch removed

Hi @adamsilverstein & @NextScripts

Taking a quick glance I found the issue was less to do with the 7th $position parameter of the add_submenu_page but more that the same slug testPlg was used as both the $parent_slug and $menu_slug which caused $submenu[ $parent_slug ] to not be set leaving it undefined and as such non-countable.

Speaking specifically about this code;
if ( ! isset( $submenu[ $parent_slug ] ) && $menu_slug != $parent_slug ) {
Reference - https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/plugin.php#L1363-L1375

Two workarounds;

  1. Having the $parent_slug and $menu_slug for the add_submenu_page as unique avoids $submenu[ $parent_slug ] being undefined.
  2. Not providing the $position on the add_submenu_page call also avoids this as it avoids the reordering code where count is called here - https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/plugin.php#L1393-L1412

I've added https://core.trac.wordpress.org/attachment/ticket/48599/48599.1.diff to test this theory but would love someone with more experience with menus to review. In essence if it slips through the previous check because parent and menu slugs match then it checks if the $submenu[ $parent_slug ] is set.

Thanks

Last edited 3 weeks ago by garrett-eclipse (previous) (diff)

#17 @123host
3 weeks ago

Without this sounding like a me too I have found that some sites throw and error when the position integer is in quotes and others thrown the error when it isn't in quotes.

Read a more comprehensive post I made here https://core.trac.wordpress.org/ticket/48249#comment:21 before being referred to this issue.

Last edited 3 weeks ago by 123host (previous) (diff)

#18 in reply to: ↑ 16 @123host
3 weeks ago

Replying to garrett-eclipse:

Two workarounds;

  1. Having the $parent_slug and $menu_slug for the add_submenu_page as unique avoids $submenu[ $parent_slug ] being undefined.
  2. Not providing the $position on the add_submenu_page call also avoids this as it avoids the reordering code where count is called here - https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/plugin.php#L1393-L1412

Number 2 seems to solve this for now. Thanks

#19 @garrett-eclipse
3 weeks ago

Hello @123host thanks for the report and confirmation about the workaround solving your issue for now.

I took a look at your more comprehensive post, and see it has the same root cause as what I'm hoping to address in my patch (48599.1.diff).
That being the same menu_slug and parent_slug is used in the add_submenu_page call.
So to make workaround #1 work in your case if you do want to use $postion, then setting menu_slug to dk_speakout_child should also work around the issue.

As to your other comment;
'I have found that some sites throw and error when the position integer is in quotes and others thrown the error when it isn't in quotes.'
The error should be slightly different when you supply a quoted $postion, as it's not an integer in that case it should trigger this _doing_it_wrong;
https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/plugin.php#L1378-L1389

#20 @welcher
2 weeks ago

  • Keywords needs-unit-tests added

Sorry for the late response on this and great work so far everyone! I'd like to get some unit tests in place for this specific issue as part of whatever the final fix ends up being.

#21 @welcher
2 weeks ago

  • Keywords has-unit-tests added; needs-unit-tests removed

@garrett-eclipse your patch works great with my local tests. I have also added some unit tests to confirm that these errors are not produced and to ensure that an incorrect priority value fires the _doing_it_wrong.

@welcher
2 weeks ago

Patch with unit tests

@garrett-eclipse
9 days ago

Minor CS Update and Updated Comment

#22 @garrett-eclipse
9 days ago

  • Keywords commit added; dev-feedback needs-testing removed

Thanks @welcher I appreciate the unit tests.

I ran them with the change removed to trigger their failure and then tested with the fix and everything passed nicely.

This should be pretty good to go so marking for commit to get a final review from @adamsilverstein.

My latest patch doesn't change anything aside from CS and expanded on the comment before my change so it's more explicit as to what's happening. Patch 48599.2.diff

#23 @welcher
3 days ago

  • Component changed from Administration to Menus

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


3 days ago

#25 @adamsilverstein
3 days ago

Looks good, I'll get this committed.

#26 @adamsilverstein
3 days ago

48599.3.diff includes fixes from running phpcbf (fixing failing Travis run, see https://github.com/WordPress/wordpress-develop/pull/122)

Last edited 3 days ago by adamsilverstein (previous) (diff)
Note: See TracTickets for help on using tickets.