Indirect modification of overloaded property WP_Block_Type::$variations has no effect

[thekt12]

Original bug reported here - ​https://github.com/WordPress/wordpress-develop/pull/5718#issuecomment-1901940214

This bug was introduced after the merging of https://core.trac.wordpress.org/ticket/59969

The issue is

The setting of WP_Block_Type::$variations from the outside in a normal way like
$block_type->variations = array( array(...), ... ) works fine.
However if we try to set it especially $block_type->variations[] = array( ... ); it throws
Indirect modification of overloaded property WP_Block_Type::$variations has no effect

The reason is WP_Block_Type::$variations is only accessible via the magic __get, __set function. __get always returns a copy of the original attribute variable and not the reference of the actual attribute variable.

So, when we do $block_type->variations = array( array(...), ... ) we are only setting the temporary variable which is then processed by __set to set the original attribute.

But this fails when we try to set the variable in a way where it's original state is checked like in ->
$block_type->variations[] = array( ... );; Here, we try to add to the original variable array.

The way to fix this is by trying to get the reference of the original variable from __get instead of copy of variable

Trac ticket: [](https://core.trac.wordpress.org/ticket/60309#ticket)

This ticket fixes - ​https://github.com/WordPress/wordpress-develop/pull/5718#issuecomment-1901940214

[ellatrix]

Either we'll need to get this fix in ASAP, or we'll need to revert #59969. Currently all Gutenberg PRs are blocked from merging.

[Mamaduka]

I believe the failing test case is phpunit/blocks/block-navigation-link-variations-test.php. We can confirm that the solution works by running the same test on the trunk.

Drafted due to test failure.

[joemcgill]

The PHPUnit failures in the Gutenberg repo have been resolved via ​https://github.com/WordPress/gutenberg/pull/58090. I'm going to close this ticket and we can continue any other back-compat concerns with the changes to block variations in #59969 so everything stays in one place.

@thekt12 I think ​https://github.com/WordPress/wordpress-develop/pull/5915 still has merit to explore further, do you mind updating the PR description to point to #59969 instead? I'm still not confident that we want to support this type of direct modification of overloaded properties, but if we can find an easy solution that makes the class more robust, I'm not opposed.

there are few bottoms outreached by cafnao
​https://eventmanagementexpert.com/
​https://eventplannertoronto.ca/

moshiajonta

Please don't spam here.

Flagging this as spam, and even reported this to GitHub

there are few bottoms outreached by cafnao
​https://eventmanagementexpert.com/
​https://eventplannertoronto.ca/