Merge pull request #7286 from ampproject/fix/hexdec-errors

Fix Incorrect assumption of hex input by `ReaderThemeSupportFeatures::get_relative_luminance_from_hex()`
ampproject · Oct 6, 2022 · 5a45909 · 5a45909
2 parents 20b69ee + b321f60
commit 5a45909
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 0 deletions.
diff --git a/src/ReaderThemeSupportFeatures.php b/src/ReaderThemeSupportFeatures.php
@@ -285,6 +285,11 @@ private function print_editor_color_palette_styles( array $color_palette ) {
 				continue;
 			}
 
+			// If not a valid hex color, skip it.
+			if ( ! preg_match( '/^[0-9a-f]{3}[0-9a-f]{3}?$/i', ltrim( $color_option[ self::KEY_COLOR ], '#' ) ) ) {
+				continue;
+			}
+
 			// There is no standard way to retrieve or derive the `color` style property when the editor color is being used
 			// for the background, so the best alternative at the moment is to guess a good default value based on the
 			// luminance of the editor color.

diff --git a/tests/php/src/ReaderThemeSupportFeaturesTest.php b/tests/php/src/ReaderThemeSupportFeaturesTest.php
@@ -58,6 +58,16 @@ final class ReaderThemeSupportFeaturesTest extends DependencyInjectedTestCase {
 			'slug'  => 'white',
 			'color' => '#FFFFFF',
 		],
+		[
+			'name'  => 'Purple',
+			'slug'  => 'purple',
+			'color' => 'var(--base-purple)',
+		],
+		[
+			'name'  => 'Yellow',
+			'slug'  => 'yellow',
+			'color' => 'rgb(255, 255, 0)',
+		],
 	];
 
 	const TEST_ALL_THEME_SUPPORTS = [
@@ -387,6 +397,12 @@ public function test_print_theme_support_styles_reader( $is_legacy ) {
 		$this->assertStringContainsString( ':root .is-gigantic-text, :root .has-gigantic-font-size { font-size: 144px; }', $output );
 		$this->assertStringContainsString( '<style id="amp-wp-theme-support-editor-gradient-presets">', $output );
 		$this->assertStringContainsString( '.has-yellow-to-purple-gradient-background { background: linear-gradient(160deg, #EEEADD 0%, #D1D1E4 100%); }', $output );
+
+		// assert output string does not contain background color if it is not hexdec compatible but contains color.
+		$this->assertStringNotContainsString( '.has-purple-background-color { background-color: var(--base-purple); color: #000; }', $output );
+		$this->assertStringNotContainsString( '.has-yellow-background-color { background-color: rgb(255, 255, 0); color: #000; }', $output );
+		$this->assertStringContainsString( '.has-purple-color { color: var(--base-purple); }', $output );
+		$this->assertStringContainsString( '.has-yellow-color { color: rgb(255, 255, 0); }', $output );
 	}
 
 	/** @return array */