[wp-trac] [WordPress Trac] #57583: Add support for editing block style variations in the global styles

WordPress Trac noreply at wordpress.org
Mon Feb 13 16:46:17 UTC 2023


#57583: Add support for editing block style variations in the global styles
-------------------------------------------------+-------------------------
 Reporter:  isabel_brison                        |       Owner:
                                                 |  hellofromTonya
     Type:  task (blessed)                       |      Status:  reopened
 Priority:  normal                               |   Milestone:  6.2
Component:  Editor                               |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch gutenberg-merge has-       |     Focuses:
  testing-info has-unit-tests                    |
-------------------------------------------------+-------------------------

Comment (by hellofromTonya):

 Replying to [comment:11 SergeyBiryukov]:
 > This appears to have introduced a PHP warning when running the test
 suite, as seen in the [https://github.com/WordPress/wordpress-
 develop/actions/runs/4064981346/jobs/6999076699#step:17:208 recent test
 runs]:
 > {{{
 > Warning: foreach() argument must be of type array|object, string given
 in wp-includes/class-wp-theme-json.php on line 835
 > }}}

 The PHP warning is coming from the a block that is registered with a
 string as its `'styles'` https://github.com/WordPress/wordpress-
 develop/blob/trunk/tests/phpunit/tests/rest-api/rest-block-type-
 controller.php#L226.

 Should the `'styles'` setting for a block **always** be of an `array` data
 type? Yes, the data type is documented as:

 * `register_block_type()`: an `array[]`
 https://developer.wordpress.org/reference/functions/register_block_type/
 * `WP_Block_Types`: an `array`
 https://developer.wordpress.org/reference/classes/wp_block_type/

 `register_block_type()` uses `WP_Block_Type()`, which sets the passed
 `$args` to its properties via `WP_Block_Type::set_props()`. But notice,
 there are no data type checks in `set_props()`. So the properties could be
 set any data type and/or value. That's problematic as it could result in
 unexpected behavior or errors such as seen in the test suites with the PHP
 warning.

 Guarding is one way to go for the code introduced in [55172] to ensure it
 only processes as array. I'm okay with doing so.

 However, the root cause is not in [55172]. Rather it's the lack of
 validation before setting the block type properties that concerns me.
 Without such validation, more and more type checks will be required in the
 code that is processing these properties/settings.

 A quick fix for the test suite is to eliminate the `'invalid_styles'`
 value from test that is causing the PHP Warning, as that string value is
 not being used.

 I'll follow-up by opening an issue in Gutenberg to further explore
 validation before setting properties in `WP_Block_Type`.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/57583#comment:19>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list