Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 5 years ago

#8163 closed defect (bug) (fixed)

WP 5.3 use of the spread operator, our PHP required version (5.3) & BP_Walker_Nav_Menu

Reported by: imath's profile imath Owned by: imath's profile imath
Milestone: 5.1.0 Priority: high
Severity: major Version: 1.7
Component: Core Keywords: has-patch 2nd-opinion
Cc:

Description

For the context, WordPress introduced the usage of the spread operator ... in various. As we don't use internally the BP_Walker_Nav_Menu nor bp_nav_menu() we haven't seen we were concerned by this evolution. @johnjamesjacoby committed [12497] to fix the problem in trunk (here and on the plugins SVN repo) using this spread operator. As a result Unit Tests are failing when PHP is 5.3.

We had some discussions on Slack about whether we should increase our PHP required version to 5.6 for our next major version.

I believe (after a good night of sleep), as WordPress 5.3 has been released and even if the use of BP_Walker_Nav_Menu or bp_nav_menu() might be an edge case, we should fix this issue in 5.1.0.

@timothybjacobs shared the way WPCLI faced this trouble on our Slack channel (many thanks to him). So I suggest we use a similar way.

See attached patch.

Attachments (3)

8163.patch (5.1 KB) - added by imath 5 years ago.
8163.2.patch (5.7 KB) - added by imath 5 years ago.
8163.2.alt.patch (5.8 KB) - added by imath 5 years ago.

Download all attachments as: .zip

Change History (11)

@imath
5 years ago

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


5 years ago

#2 @maccas83
5 years ago

Hey guys, can you confirm when this will be rolled out in an update? These errors are showing on the front end to my users and it's killing my CPU making it impossible to update my site or put out posts. I can't delete BuddPress as I've built my login and community around it.

#3 @boonebgorges
5 years ago

+1 to fixing in 5.1.0, and +1 to the approach in @imath's patch. Increasing PHP requirements is probably OK for BP 6.0 or beyond, but let's make this fix in the interim.

@maccas83 As long as your server is running PHP 5.4 or greater, you can manually apply https://buddypress.trac.wordpress.org/changeset/12497 to your installation and it should clear up the errors.

@imath
5 years ago

#4 @imath
5 years ago

Hi @boonebgorges

Thanks a lot for your feedback. After running our PHPCS Compat task I realized "trait" was introduced in PHP 5.4.

So I guess the next possibility is to dance a bit more renaming the class that extends Walker_Nav_Menu and using the old name to extend the newly named class to deal with the ... operator.

I believe we should also check for WordPress 5.3 before doing this dance as extending Walker_Nav_Menu in PHP 5.6 & UP should generate a notice error in WordPress < 5.3.

So I'll run some more tests asap and eventually update the patch.

Last edited 5 years ago by imath (previous) (diff)

#5 @imath
5 years ago

Well actually 8163.2.patch is fine with WP 5.2 too. But just in case, as the requirement for PHP 5.6 is only needed when BuddyPress is used inside WP 5.3 and UP, here's an alternative patch :

8163.2.alt.patch

@imath
5 years ago

#6 @imath
5 years ago

In 12498:

Compat classes to handle Walker_Nav_Menu::walk() changes in WP 5.3

WordPress 5.3 modified the Walker_Nav_Menu::walk() signature adding a third parameter to the method that is using the spread operator. johnjamesjacoby quickly fixed the issue in [12497] when PHP >=5.6.

As our current PHP required version is 5.3, we need to provide a temporary compatibility mechanism to make sure our BP_Walker_Nav_Menu will not generate errors when the PHP version is lower than 5.6 before raising the PHP required version in our next major release.

props timothybjacobs, boonebgorges, imath

See #8163 (Trunk)

#7 @imath
5 years ago

In 12499:

Update phplint Grunt task excluding src/bp-core/compat/php56/*.php

This should fix the failing Travis job (PHP: 5.3 WP_VERSION=4.8 BP_TRAVISCI=travis:phpunit)

See #8163 (Trunk)

#8 @imath
5 years ago

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

In 12500:

Compat classes to handle Walker_Nav_Menu::walk() changes in WP 5.3

WordPress 5.3 modified the Walker_Nav_Menu::walk() signature adding a third parameter to the method that is using the spread operator. johnjamesjacoby quickly fixed the issue in [12497] when PHP >=5.6.

As our current PHP required version is 5.3, we need to provide a temporary compatibility mechanism to make sure our BP_Walker_Nav_Menu will not generate errors when the PHP version is lower than 5.6 before raising the PHP required version in our next major release.

props timothybjacobs, boonebgorges, imath

Fixes #8163 (5.0 branch)

Note: See TracTickets for help on using tickets.