Get WordPress

Make WordPress Core

Opened 3 months ago

Last modified 5 weeks ago

#61154 reviewing defect (bug)

Fix the 'attributes' dynamic property in WP_Block

Reported by: antonvlasenko's profile antonvlasenko Owned by: hellofromtonya's profile hellofromTonya
Milestone: 6.7 Priority: normal
Severity: normal Version: 6.6
Component: Editor Keywords: has-patch has-unit-tests needs-testing php82 has-testing-info
Focuses: php-compatibility Cc:

Description

The WP_Block class employs the __get magic method to compute the attributes class property.
However, since PHP 8.2 does not support dynamic properties, it is better to eliminate this approach and explicitly declare the WP_Block::$attributes class property to store the block's attributes instead.

Change History (9)

#1 @antonvlasenko
3 months ago

I'm working on a patch for this ticket.

This ticket was mentioned in ​PR #6500 on ​WordPress/wordpress-develop by ​@antonvlasenko.
3 months ago #2

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

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

#3 @antonvlasenko
3 months ago

  • Keywords needs-testing added

I've just submitted ​https://github.com/WordPress/wordpress-develop/pull/6500
This PR:

  1. Fixes the WP_Block::$attributes dynamic property by utilizing strategy proposed by @hellofromTonya in https://core.trac.wordpress.org/ticket/60875. Props @hellofromTonya.
  2. Implements this suggestion.
  3. Adds unit tests to check how the newly declared magic methods on the WP_Block class handle the WP_Block::$attributes property and other declared public properties.
Last edited 3 months ago by antonvlasenko (previous) (diff)

#4 @ironprogrammer
3 months ago

  • Keywords php82 added
  • Milestone changed from Awaiting Review to 6.6

Thank you for the PR, @antonvlasenko! 🙌🏻 I've tagged this for ​PHP 8.2 compat, and moved it to the 6.6 milestone to get it on the radar.

This ticket was mentioned in ​Slack in #core-php by antonvlasenko. ​View the logs.
3 months ago

#6 @faisal03
2 months ago

== Test Report

Please ignore, it was meant for https://core.trac.wordpress.org/ticket/60236

Last edited 2 months ago by faisal03 (previous) (diff)

#7 follow-up: @hmbashar
7 weeks ago

How can I test the issue?

#8 @hellofromTonya
6 weeks ago

  • Milestone changed from 6.6 to 6.7
  • Owner set to hellofromTonya
  • Status changed from new to reviewing

Related #60875.

This ticket is part of the epic #60875 for handling late initialization and setting of dynamic properties.

As this effort is now slated for 6.7, moving this ticket too.

#9 in reply to: ↑ 7 @antonvlasenko
5 weeks ago

  • Keywords has-testing-info added

Replying to hmbashar:

How can I test the issue?

That's an excellent question, @hmbashar.

It is somewhat challenging to outline precise steps for testing this, as it primarily involves refactoring.
Ideally, PHPUnit tests that have been added in my PR should ensure that the WP_Block class functions correctly.

Here's my best approach for testing this PR:

  1. Review the code changes and assess whether the architecture is fine.
  2. Ensure GitHub CI jobs pass.
  3. Use the Site Editor. Test adding, updating and deleting blocks, ensuring there are no WP_Block related errors in the error log.
Note: See TracTickets for help on using tickets.