Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Incorrect assumption of hex input by ReaderThemeSupportFeatures::get_relative_luminance_from_hex() #7286

Merged
merged 4 commits into from
Oct 6, 2022
Merged

Fix Incorrect assumption of hex input by ReaderThemeSupportFeatures::get_relative_luminance_from_hex() #7286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7cc3944
Add guard to skip usage of hexdec function if a valid hex is not passed
thelovekesh Oct 4, 2022
ac66752
Update test cases for ReaderThemeSupportFeatures::print_editor_color_…
thelovekesh Oct 4, 2022
bd7c22d
Fix phpcs errors
thelovekesh Oct 4, 2022
b321f60
Merge branch 'develop' into fix/hexdec-errors
thelovekesh Oct 5, 2022
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
5 changes: 5 additions & 0 deletions src/ReaderThemeSupportFeatures.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
16 changes: 16 additions & 0 deletions tests/php/src/ReaderThemeSupportFeaturesTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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 */
Expand Down