Commit 39d2eddb09e for woocommerce
commit 39d2eddb09e2dce6f6b6f71c442a0b80b1bd8360
Author: Allison Levine <1689238+allilevine@users.noreply.github.com>
Date: Thu Aug 13 10:38:19 2026 -0400
[Email Editor] Fix block background color being rendered twice (#67673)
* fix: render block background color once instead of twice
A block's background was applied both to the wrapping table cell and again
to the inner element, so a translucent palette color composited over itself
and rendered as a visibly darker band inside the cell's padding.
The class cleanup in the Text and Social Links renderers used a literal
str_replace for "has-background", which does not match the preset class
"has-<slug>-background-color" that WordPress adds alongside it, so the
preset class survived on the inner element and the CSS inliner resolved it
there a second time. Replace both copies with a shared helper that compares
whole class names through the HTML API, covering the preset classes as well
as the border classes, and drop a custom background color from the inner
element's inline style for the same reason padding and borders are dropped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019C19aLQBVVc7xG7nUwWxQb
* fix: match the background-color declaration case-insensitively
CSS property names are case-insensitive and a colon may be surrounded by
whitespace, so "BACKGROUND-COLOR : x" is the same declaration as
"background-color:x". The style engine only emits the lowercase, unspaced
form, but block markup is hand-editable, so match both. The lookbehind that
keeps the match anchored to the start of a property name still holds under
the case-insensitive flag.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019C19aLQBVVc7xG7nUwWxQb
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
diff --git a/packages/php/email-editor/changelog/fix-duplicate-preset-background-color b/packages/php/email-editor/changelog/fix-duplicate-preset-background-color
new file mode 100644
index 00000000000..5b7130b2dc8
--- /dev/null
+++ b/packages/php/email-editor/changelog/fix-duplicate-preset-background-color
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Render a block's background color once instead of twice, which made a translucent palette color appear darker than intended.
diff --git a/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-social-links.php b/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-social-links.php
index 03310586059..0d883fc9970 100644
--- a/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-social-links.php
+++ b/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-social-links.php
@@ -9,6 +9,7 @@ declare( strict_types = 1 );
namespace Automattic\WooCommerce\EmailEditor\Integrations\Core\Renderer\Blocks;
use Automattic\WooCommerce\EmailEditor\Engine\Renderer\ContentRenderer\Rendering_Context;
+use Automattic\WooCommerce\EmailEditor\Integrations\Utils\Html_Processing_Helper;
use Automattic\WooCommerce\EmailEditor\Integrations\Utils\Social_Links_Helper;
use Automattic\WooCommerce\EmailEditor\Integrations\Utils\Table_Wrapper_Helper;
/**
@@ -270,12 +271,8 @@ class Social_Links extends Abstract_Block_Renderer {
/** @var string $block_classes */ // phpcs:ignore Generic.Commenting.DocComment.MissingShort -- used for phpstan
$block_classes = $html->get_attribute( 'class' ) ?? '';
$classes .= ' ' . $block_classes;
- // remove has-background to prevent double padding applied for wrapper and inner element.
- $block_classes = str_replace( 'has-background', '', $block_classes );
- // remove border related classes because we handle border on wrapping table cell.
- $block_classes = preg_replace( '/[a-z-]+-border-[a-z-]+/', '', $block_classes );
- /** @var string $block_classes */ // phpcs:ignore Generic.Commenting.DocComment.MissingShort -- used for phpstan
- $html->set_attribute( 'class', trim( $block_classes ) );
+ // Remove the background and border classes because we render both on the wrapping table cell.
+ Html_Processing_Helper::remove_wrapper_handled_classes( $html );
$block_content = $html->get_updated_html();
}
diff --git a/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-text.php b/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-text.php
index fb66c161704..710f69845b7 100644
--- a/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-text.php
+++ b/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-text.php
@@ -9,6 +9,7 @@ declare( strict_types = 1 );
namespace Automattic\WooCommerce\EmailEditor\Integrations\Core\Renderer\Blocks;
use Automattic\WooCommerce\EmailEditor\Engine\Renderer\ContentRenderer\Rendering_Context;
+use Automattic\WooCommerce\EmailEditor\Integrations\Utils\Html_Processing_Helper;
use Automattic\WooCommerce\EmailEditor\Integrations\Utils\Styles_Helper;
use Automattic\WooCommerce\EmailEditor\Integrations\Utils\Table_Wrapper_Helper;
@@ -55,12 +56,8 @@ class Text extends Abstract_Block_Renderer {
$alignment_from_class = 'left';
}
- // remove has-background to prevent double padding applied for wrapper and inner element.
- $block_classes = str_replace( 'has-background', '', $block_classes );
- // remove border related classes because we handle border on wrapping table cell.
- $block_classes = preg_replace( '/[a-z-]+-border-[a-z-]+/', '', $block_classes );
- /** @var string $block_classes */ // phpcs:ignore Generic.Commenting.DocComment.MissingShort -- used for phpstan
- $html->set_attribute( 'class', trim( $block_classes ) );
+ // Remove the background and border classes because we render both on the wrapping table cell.
+ Html_Processing_Helper::remove_wrapper_handled_classes( $html );
$block_content = $html->get_updated_html();
}
@@ -124,6 +121,18 @@ class Text extends Abstract_Block_Renderer {
// Remove border styles. We apply border styles on the wrapping table cell.
$element_style = (string) preg_replace( '/border[^:]*:.?[0-9a-z-()#]+;?/', '', $element_style );
+ // Remove the background color for the same reason we remove the background classes: it is
+ // rendered on the wrapping table cell, and a translucent color left here paints twice.
+ // This assumes the cell gets the same color from the block attributes, which is true for
+ // anything the editor saves. Markup that carries an inline background without the matching
+ // attribute loses it, the same way padding, margin, and border above already behave.
+ // The lookbehind keeps the match anchored to the start of a property name, so a longer
+ // property that merely ends in "background-color" (a custom property, say) is not cut in
+ // half, which would leave its prefix fused to the following declaration. Property names
+ // are case-insensitive in CSS and a colon may be surrounded by whitespace, so both are
+ // matched — unlike the class names above, which the CSS inliner matches case-sensitively.
+ $element_style = (string) preg_replace( '/(?<![a-z-])background-color\s*:\s*[^;]+;?/i', '', $element_style );
+
// We define the font-size on the wrapper element, but we need to keep font-size definition here
// to prevent CSS Inliner from adding a default value and overriding the value set by user, which is on the wrapper element.
// The value provided by WP uses clamp() function which is not supported in many email clients.
diff --git a/packages/php/email-editor/src/Integrations/Utils/class-html-processing-helper.php b/packages/php/email-editor/src/Integrations/Utils/class-html-processing-helper.php
index ddc24c04c47..10c16c9d0af 100644
--- a/packages/php/email-editor/src/Integrations/Utils/class-html-processing-helper.php
+++ b/packages/php/email-editor/src/Integrations/Utils/class-html-processing-helper.php
@@ -58,6 +58,58 @@ class Html_Processing_Helper {
return trim( $classes );
}
+ /**
+ * Remove from an element the class names whose styles the renderer applies to the wrapping table cell.
+ *
+ * Background and border classes are resolved by the CSS inliner wherever they appear. The wrapping
+ * cell keeps the block's original class list *and* receives the same styles inline, so leaving these
+ * classes on the inner element paints them a second time. For an opaque color that is invisible, but
+ * a translucent palette color composites over itself and renders as a visibly darker band inside the
+ * cell's padding.
+ *
+ * @param \WP_HTML_Tag_Processor $html Tag processor positioned on the element to clean.
+ */
+ public static function remove_wrapper_handled_classes( \WP_HTML_Tag_Processor $html ): void {
+ $class_attribute = $html->get_attribute( 'class' );
+ if ( ! is_string( $class_attribute ) ) {
+ return;
+ }
+
+ $class_names = preg_split( '/\s+/', trim( $class_attribute ) );
+ if ( ! is_array( $class_names ) ) {
+ return;
+ }
+
+ // Whole class names are compared and removed, so a class that merely contains one of these
+ // names as a substring is left intact instead of being reduced to a fragment.
+ foreach ( $class_names as $class_name ) {
+ if ( '' !== $class_name && self::is_wrapper_handled_class( $class_name ) ) {
+ $html->remove_class( $class_name );
+ }
+ }
+ }
+
+ /**
+ * Whether a single class name applies a background or border that the wrapping table cell already renders.
+ *
+ * @param string $class_name Class name to test.
+ * @return bool True when the class should not stay on the inner element.
+ */
+ private static function is_wrapper_handled_class( string $class_name ): bool {
+ // `has-background` is added for any background. Preset palette backgrounds add
+ // `has-<slug>-background-color` on top of it, which is why matching the bare name is not enough.
+ if ( 'has-background' === $class_name ) {
+ return true;
+ }
+
+ if ( str_starts_with( $class_name, 'has-' ) && str_ends_with( $class_name, '-background-color' ) ) {
+ return true;
+ }
+
+ // Border classes, e.g. `has-border-color`, `has-<slug>-border-color`.
+ return false !== strpos( $class_name, '-border-' );
+ }
+
/**
* Sanitize CSS value to prevent injection attacks.
*
diff --git a/packages/php/email-editor/tests/integration/Integrations/Core/Renderer/Blocks/Heading_Test.php b/packages/php/email-editor/tests/integration/Integrations/Core/Renderer/Blocks/Heading_Test.php
index 58eaa2b7298..b9114b150b6 100644
--- a/packages/php/email-editor/tests/integration/Integrations/Core/Renderer/Blocks/Heading_Test.php
+++ b/packages/php/email-editor/tests/integration/Integrations/Core/Renderer/Blocks/Heading_Test.php
@@ -134,6 +134,108 @@ class Heading_Test extends \Email_Editor_Integration_Test_Case {
$this->assertStringNotContainsString( 'margin', $table_cell_style );
}
+ /**
+ * The preset background class must not stay on the inner element.
+ *
+ * The wrapping table cell keeps the block's classes and also carries the resolved background
+ * inline. Leaving `has-<slug>-background-color` on the heading made the CSS inliner paint the
+ * same color a second time, which composites to a darker band for a translucent palette color.
+ */
+ public function testItRemovesPresetBackgroundClassFromInnerElement(): void {
+ $content = '<h1 class="wp-block-heading has-vivid-red-background-color has-background">This is Heading 1</h1>';
+ $parsed_heading = $this->parsed_heading;
+ $parsed_heading['innerHTML'] = $content;
+ $parsed_heading['innerContent'] = array( $content );
+
+ $rendered = $this->heading_renderer->render( $content, $parsed_heading, $this->rendering_context );
+
+ $html = new \WP_HTML_Tag_Processor( $rendered );
+ $this->assertTrue( $html->next_tag( array( 'tag_name' => 'h1' ) ) );
+ $heading_classes = (string) $html->get_attribute( 'class' );
+ $this->assertStringNotContainsString( 'has-vivid-red-background-color', $heading_classes );
+ $this->assertStringNotContainsString( 'has-background', $heading_classes );
+ // Unrelated classes are untouched.
+ $this->assertStringContainsString( 'wp-block-heading', $heading_classes );
+
+ // The background is still rendered exactly once, on the wrapping table cell.
+ $html = new \WP_HTML_Tag_Processor( $rendered );
+ $this->assertTrue( $html->next_tag( array( 'tag_name' => 'td' ) ) );
+ $this->assertStringContainsString( 'background-color', (string) $html->get_attribute( 'style' ) );
+ }
+
+ /**
+ * A custom background color set inline must not stay on the inner element either.
+ */
+ public function testItRemovesInlineBackgroundColorFromInnerElement(): void {
+ $content = '<h1 class="wp-block-heading has-background" style="background-color:#c284426b;">This is Heading 1</h1>';
+ $parsed_heading = $this->parsed_heading;
+ $parsed_heading['innerHTML'] = $content;
+ $parsed_heading['innerContent'] = array( $content );
+ $parsed_heading['attrs']['style']['color']['background'] = '#c284426b';
+ unset( $parsed_heading['attrs']['backgroundColor'] );
+
+ $rendered = $this->heading_renderer->render( $content, $parsed_heading, $this->rendering_context );
+
+ $html = new \WP_HTML_Tag_Processor( $rendered );
+ $this->assertTrue( $html->next_tag( array( 'tag_name' => 'h1' ) ) );
+ $this->assertStringNotContainsString( 'background-color', (string) $html->get_attribute( 'style' ) );
+
+ $html = new \WP_HTML_Tag_Processor( $rendered );
+ $this->assertTrue( $html->next_tag( array( 'tag_name' => 'td' ) ) );
+ $this->assertStringContainsString( 'background-color:#c284426b', (string) $html->get_attribute( 'style' ) );
+ }
+
+ /**
+ * Only the background-color declaration itself is removed from the inner element.
+ *
+ * The removal is anchored to the start of a property name. Without that anchor a longer
+ * property ending in "background-color" is cut in half and its prefix is left fused to the
+ * next declaration, turning `--brand-background-color:#fff;color:red` into `--brand-color:red`
+ * — a different, valid-looking declaration rather than a visibly broken one.
+ */
+ public function testItRemovesOnlyTheBackgroundColorDeclarationFromInnerElement(): void {
+ $content = '<h1 class="wp-block-heading" style="--brand-background-color:#ffffff;background-color:#c284426b;color:#ff0000;">This is Heading 1</h1>';
+ $parsed_heading = $this->parsed_heading;
+ $parsed_heading['innerHTML'] = $content;
+ $parsed_heading['innerContent'] = array( $content );
+
+ $rendered = $this->heading_renderer->render( $content, $parsed_heading, $this->rendering_context );
+
+ $html = new \WP_HTML_Tag_Processor( $rendered );
+ $this->assertTrue( $html->next_tag( array( 'tag_name' => 'h1' ) ) );
+ $heading_style = (string) $html->get_attribute( 'style' );
+
+ // The custom property survives intact, and does not get welded onto the next declaration.
+ $this->assertStringContainsString( '--brand-background-color:#ffffff', $heading_style );
+ $this->assertStringNotContainsString( '--brand-color', $heading_style );
+ // The real background is still gone, and the unrelated declaration is untouched.
+ $this->assertStringNotContainsString( '#c284426b', $heading_style );
+ $this->assertStringContainsString( 'color:#ff0000', $heading_style );
+ }
+
+ /**
+ * The background is removed however the declaration is spelled.
+ *
+ * CSS property names are case-insensitive and a colon may be surrounded by whitespace, so
+ * `BACKGROUND-COLOR : x` is the same declaration as `background-color:x`. The editor's style
+ * engine only ever emits the lowercase, unspaced form, but block markup is hand-editable.
+ */
+ public function testItRemovesBackgroundColorRegardlessOfDeclarationSpelling(): void {
+ $content = '<h1 class="wp-block-heading" style="BACKGROUND-COLOR : #c284426b;color:#ff0000;">This is Heading 1</h1>';
+ $parsed_heading = $this->parsed_heading;
+ $parsed_heading['innerHTML'] = $content;
+ $parsed_heading['innerContent'] = array( $content );
+
+ $rendered = $this->heading_renderer->render( $content, $parsed_heading, $this->rendering_context );
+
+ $html = new \WP_HTML_Tag_Processor( $rendered );
+ $this->assertTrue( $html->next_tag( array( 'tag_name' => 'h1' ) ) );
+ $heading_style = (string) $html->get_attribute( 'style' );
+
+ $this->assertStringNotContainsStringIgnoringCase( 'background-color', $heading_style );
+ $this->assertStringContainsString( 'color:#ff0000', $heading_style );
+ }
+
/**
* Test it uses inherited color from email_attrs when no color is specified
*/
diff --git a/packages/php/email-editor/tests/integration/Integrations/Utils/Html_Processing_Helper_Test.php b/packages/php/email-editor/tests/integration/Integrations/Utils/Html_Processing_Helper_Test.php
index 8facb2b27eb..67d14539d1f 100644
--- a/packages/php/email-editor/tests/integration/Integrations/Utils/Html_Processing_Helper_Test.php
+++ b/packages/php/email-editor/tests/integration/Integrations/Utils/Html_Processing_Helper_Test.php
@@ -195,4 +195,119 @@ class Html_Processing_Helper_Test extends \Email_Editor_Integration_Test_Case {
$this->assertStringContainsString( 'data-id="5"', $attributed );
$this->assertStringNotContainsString( 'data-bad', $attributed );
}
+
+ /**
+ * Background and border classes are dropped, everything else on the element is kept.
+ *
+ * @dataProvider wrapper_handled_class_provider
+ * @param string $class_attribute Class attribute to clean.
+ * @param array<string> $expected Class names that must remain, in order.
+ */
+ public function test_remove_wrapper_handled_classes( string $class_attribute, array $expected ): void {
+ $html = new \WP_HTML_Tag_Processor( '<h3 class="' . $class_attribute . '">Heading</h3>' );
+ $this->assertTrue( $html->next_tag() );
+
+ Html_Processing_Helper::remove_wrapper_handled_classes( $html );
+
+ // Read back through the tag processor so the assertion sees the rewritten attribute.
+ $class_names = preg_split( '/\s+/', trim( (string) $html->get_attribute( 'class' ) ) );
+ $remaining = array_values(
+ array_filter(
+ is_array( $class_names ) ? $class_names : array(),
+ function ( $class_name ) {
+ return '' !== $class_name;
+ }
+ )
+ );
+
+ $this->assertSame( $expected, $remaining );
+ }
+
+ /**
+ * Class attributes that are not a plain space-separated list must be handled without warnings.
+ *
+ * The helper reads the attribute rather than the parsed class list, so it has to cope with the
+ * shapes WP_HTML_Tag_Processor can hand back: null when the attribute is absent, and boolean
+ * true for a valueless attribute. Neither is a string, and both would otherwise reach trim().
+ */
+ public function test_remove_wrapper_handled_classes_handles_unusual_class_attributes(): void {
+ // No class attribute at all: nothing to do, and the tag is left exactly as it was.
+ $html = new \WP_HTML_Tag_Processor( '<h3>Heading</h3>' );
+ $this->assertTrue( $html->next_tag() );
+ Html_Processing_Helper::remove_wrapper_handled_classes( $html );
+ $this->assertSame( '<h3>Heading</h3>', $html->get_updated_html() );
+
+ // Valueless attribute: get_attribute() returns boolean true, not a string.
+ $html = new \WP_HTML_Tag_Processor( '<h3 class>Heading</h3>' );
+ $this->assertTrue( $html->next_tag() );
+ Html_Processing_Helper::remove_wrapper_handled_classes( $html );
+ $this->assertSame( '<h3 class>Heading</h3>', $html->get_updated_html() );
+
+ // Empty value: no class names to walk.
+ $html = new \WP_HTML_Tag_Processor( '<h3 class="">Heading</h3>' );
+ $this->assertTrue( $html->next_tag() );
+ Html_Processing_Helper::remove_wrapper_handled_classes( $html );
+ $this->assertStringContainsString( 'Heading', $html->get_updated_html() );
+
+ // Class names may be separated by any whitespace, not just single spaces.
+ $html = new \WP_HTML_Tag_Processor( "<h3 class=\"wp-block-heading\n\thas-background has-tertiary-background-color\">Heading</h3>" );
+ $this->assertTrue( $html->next_tag() );
+ Html_Processing_Helper::remove_wrapper_handled_classes( $html );
+ $remaining = (string) $html->get_attribute( 'class' );
+ $this->assertStringContainsString( 'wp-block-heading', $remaining );
+ $this->assertStringNotContainsString( 'has-background', $remaining );
+ $this->assertStringNotContainsString( 'has-tertiary-background-color', $remaining );
+ }
+
+ /**
+ * Data provider for wrapper-handled class removal.
+ *
+ * @return array<string, array{string, array<string>}>
+ */
+ public function wrapper_handled_class_provider(): array {
+ return array(
+ // The reported bug: the preset background class survived on the inner element because it
+ // does not contain the literal "has-background", so the translucent color painted twice.
+ 'preset background color' => array(
+ 'wp-block-heading has-tertiary-background-color has-background',
+ array( 'wp-block-heading' ),
+ ),
+ 'generic background only' => array(
+ 'wp-block-heading has-background',
+ array( 'wp-block-heading' ),
+ ),
+ 'multi word slug' => array(
+ 'has-vivid-red-background-color has-background',
+ array(),
+ ),
+ 'text color is kept' => array(
+ 'has-pale-cyan-blue-color has-text-color has-vivid-red-background-color has-background',
+ array( 'has-pale-cyan-blue-color', 'has-text-color' ),
+ ),
+ 'alignment class is kept' => array(
+ 'has-text-align-center has-tertiary-background-color',
+ array( 'has-text-align-center' ),
+ ),
+ 'border classes are dropped' => array(
+ 'wp-block-heading has-border-color has-accent-border-color',
+ array( 'wp-block-heading' ),
+ ),
+ // A whole class name is removed rather than a substring of one, so a class that merely
+ // starts with "has-background" is left intact instead of being reduced to a fragment.
+ 'unrelated background prefix' => array(
+ 'has-background-dim wp-block-cover',
+ array( 'has-background-dim', 'wp-block-cover' ),
+ ),
+ 'nothing to remove' => array(
+ 'wp-block-heading has-large-font-size',
+ array( 'wp-block-heading', 'has-large-font-size' ),
+ ),
+ // Matching is case-sensitive, mirroring how the CSS inliner matches these classes
+ // outside quirks mode. WordPress only ever emits them lowercase.
+ 'uppercase is not matched' => array(
+ 'HAS-BACKGROUND wp-block-heading',
+ array( 'HAS-BACKGROUND', 'wp-block-heading' ),
+ ),
+ );
+ }
}