Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/wp-includes/class-wp-block-supports.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,9 @@ function get_block_wrapper_attributes( $extra_attributes = array() ) {

// This is hardcoded on purpose.
// We only support a fixed list of attributes.
$attributes_to_merge = array( 'style', 'class', 'id', 'aria-label' );
$attributes = array();
$attributes_to_merge = array( 'style', 'class', 'id', 'aria-label' );
Comment thread
t-hamano marked this conversation as resolved.
Outdated
$attributes_to_override = array( 'id', 'aria-label' );
$attributes = array();
foreach ( $attributes_to_merge as $attribute_name ) {
if ( empty( $new_attributes[ $attribute_name ] ) && empty( $extra_attributes[ $attribute_name ] ) ) {
continue;
Expand All @@ -198,6 +199,11 @@ function get_block_wrapper_attributes( $extra_attributes = array() ) {
continue;
}

if ( in_array( $attribute_name, $attributes_to_override, true ) ) {
$attributes[ $attribute_name ] = $extra_attributes[ $attribute_name ];
continue;
}

$attributes[ $attribute_name ] = $extra_attributes[ $attribute_name ] . ' ' . $new_attributes[ $attribute_name ];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of merging style, should special handling be added to ensure that $extra_attributes[ $attribute_name ] ends in a semicolon? The current empty space is only really appropriate for class.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the entire way this function works needs to be completely rethought.

  • What happens if a non-array is passed as a parameter?
  • What if the array value is not a string?
  • What if there is already a semicolon at the end of the style?

However, I'm not sure if all of these fixes should be included in the 7.0 release. The original scope of this PR was to fix the unintended merging of ID attributes. WordPress/gutenberg#75251 (comment)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I guess if there isn't a problem right now then it doesn't have to be fixed. The hardening you identified would be a nice-to-have.

}

Expand Down
123 changes: 97 additions & 26 deletions tests/phpunit/tests/blocks/supportedStyles.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,37 +90,17 @@ private function get_content_from_block( $block ) {
/**
* Returns the rendered output for the current block.
*
* @param array $block Block to render.
* @param array $block Block to render.
* @param array $extra_attributes Extra attributes to pass to get_block_wrapper_attributes().
* @return string Rendered output for the current block.
*/
private function render_example_block( $block ) {
private function render_example_block( $block, $extra_attributes = array() ) {
WP_Block_Supports::init();
WP_Block_Supports::$block_to_render = $block;
$wrapper_attributes = get_block_wrapper_attributes(
array(
'class' => 'foo-bar-class',
'style' => 'test: style;',
)
);
$wrapper_attributes = get_block_wrapper_attributes( $extra_attributes );
return '<div ' . $wrapper_attributes . '>' . self::BLOCK_CONTENT . '</div>';
}

/**
* Runs assertions that the rendered output has expected class/style attrs.
*
* @param array $block Block to render.
* @param string $expected_classes Expected output class attr string.
* @param string $expected_styles Expected output styles attr string.
*/
private function assert_styles_and_classes_match( $block, $expected_classes, $expected_styles ) {
Comment thread
t-hamano marked this conversation as resolved.
$styled_block = $this->render_example_block( $block );
$class_list = $this->get_attribute_from_block( 'class', $styled_block );
$style_list = $this->get_attribute_from_block( 'style', $styled_block );

$this->assertSame( $expected_classes, $class_list, 'Class list does not match expected classes' );
$this->assertSame( $expected_styles, $style_list, 'Style list does not match expected styles' );
}

/**
* Runs assertions that the rendered output has expected content and class/style attrs.
*
Expand All @@ -129,7 +109,13 @@ private function assert_styles_and_classes_match( $block, $expected_classes, $ex
* @param string $expected_styles Expected output styles attr string.
*/
private function assert_content_and_styles_and_classes_match( $block, $expected_classes, $expected_styles ) {
$styled_block = $this->render_example_block( $block );
$styled_block = $this->render_example_block(
$block,
array(
'class' => 'foo-bar-class',
'style' => 'test: style;',
)
);

// Ensure blocks to not add extra whitespace.
$this->assertSame( $styled_block, trim( $styled_block ) );
Expand Down Expand Up @@ -158,7 +144,13 @@ private function assert_content_and_styles_and_classes_match( $block, $expected_
* @param string $expected_aria_label Expected output aria-label attr string.
*/
private function assert_content_and_aria_label_match( $block, $expected_aria_label ) {
$styled_block = $this->render_example_block( $block );
$styled_block = $this->render_example_block(
$block,
array(
'class' => 'foo-bar-class',
'style' => 'test: style;',
)
);
$content = $this->get_content_from_block( $styled_block );

$this->assertSame( self::BLOCK_CONTENT, $content, 'Block content does not match expected content' );
Expand All @@ -169,6 +161,7 @@ private function assert_content_and_aria_label_match( $block, $expected_aria_lab
);
}


Comment thread
t-hamano marked this conversation as resolved.
Outdated
/**
* Tests color support for named color support for named colors.
*/
Expand Down Expand Up @@ -201,6 +194,40 @@ public function test_named_color_support() {
$this->assert_content_and_styles_and_classes_match( $block, $expected_classes, $expected_styles );
}

/**
* Ensures that class and style attributes from extra attributes and block supports are merged.
Comment thread
t-hamano marked this conversation as resolved.
*/
public function test_block_wrapper_attributes_merge() {
$block_type_settings = array(
'attributes' => array(),
'supports' => array(
'color' => true,
),
'render_callback' => true,
);
$this->register_block_type( 'core/example-merge', $block_type_settings );

$block = array(
'blockName' => 'core/example-merge',
'attrs' => array(
'style' => array(
'color' => array(
'text' => '#000',
'background' => '#fff',
),
),
),
'innerBlock' => array(),
'innerContent' => array(),
'innerHTML' => array(),
);

$expected_styles = 'test: style;color:#000;background-color:#fff;';
$expected_classes = 'foo-bar-class wp-block-example-merge has-text-color has-background';

$this->assert_content_and_styles_and_classes_match( $block, $expected_classes, $expected_styles );
}

/**
* Tests color support for custom colors.
*/
Expand Down Expand Up @@ -728,6 +755,50 @@ public function test_aria_label_support() {
$this->assert_content_and_aria_label_match( $block, 'Label' );
}

/**
* Ensures that user-provided attributes override the attributes generated by block supports.
Comment thread
t-hamano marked this conversation as resolved.
*/
public function test_block_wrapper_attributes_prefer_user_id_and_aria_label_over_generated() {
$block_type_settings = array(
'attributes' => array(),
'supports' => array(
'ariaLabel' => true,
'anchor' => true,
),
);
$this->register_block_type( 'core/example-override', $block_type_settings );

$block = array(
'blockName' => 'core/example-override',
'attrs' => array(
'ariaLabel' => 'Generated label',
'anchor' => 'generated-id',
),
'innerBlock' => array(),
'innerContent' => array(),
'innerHTML' => array(),
);

$styled_block = $this->render_example_block(
$block,
array(
'aria-label' => 'User label',
'id' => 'user-id',
)
);

$this->assertSame(
'User label',
$this->get_attribute_from_block( 'aria-label', $styled_block ),
'Aria-label should prefer user-provided value over generated one'
);
$this->assertSame(
'user-id',
$this->get_attribute_from_block( 'id', $styled_block ),
'Id should prefer user-provided value over generated one'
);
}

/**
* Ensures libxml_internal_errors is being used instead of @ warning suppression
*/
Expand Down
Loading