Skip to content

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

New issue
New issue
Closed
#7286
Closed
Incorrect assumption of hex input by ReaderThemeSupportFeatures::get_relative_luminance_from_hex()#7285
#7286
Labels
BugSomething isn't workingChangeloggedWhether the issue/PR has been added to release notes.
Milestone
v2.4
@westonruter

Description

@westonruter
westonruter
opened on Oct 4, 2022

Bug Description

See support forum topic.

The ReaderThemeSupportFeatures::get_relative_luminance_from_hex() method incorrectly assumes hex input:

amp-wp/src/ReaderThemeSupportFeatures.php

Lines 364 to 383 in a373ac8

public function get_relative_luminance_from_hex( $hex ) {
// Remove the "#" symbol from the beginning of the color.
$hex = ltrim( $hex, '#' );
// Make sure there are 6 digits for the below calculations.
if ( 3 === strlen( $hex ) ) {
$hex = $hex[0] . $hex[0] . $hex[1] . $hex[1] . $hex[2] . $hex[2];
}
// Get red, green, blue.
$red = hexdec( substr( $hex, 0, 2 ) );
$green = hexdec( substr( $hex, 2, 2 ) );
$blue = hexdec( substr( $hex, 4, 2 ) );
// Calculate the luminance.
$lum = ( 0.2126 * $red ) + ( 0.7152 * $green ) + ( 0.0722 * $blue );
return (int) round( $lum );
}
}

It turns out that in some cases, a theme may be passing a CSS variable like var(--base-2), or presumably rgba(...) could also be passed.

To account for this, the print_editor_color_palette_styles method should skip passing the color value into get_relative_luminance_from_hex when it is not a valid hex.

This could be achieved by adding:

if ( ! preg_match( '/^[0-9a-f]{3}[0-9a-f]{3}?$/i', ltrim( $color_option[ self::KEY_COLOR ], '#' ) ) ) {
    continue;
}

After the following condition (or as part of it):

amp-wp/src/ReaderThemeSupportFeatures.php

Lines 281 to 286 in a373ac8

private function print_editor_color_palette_styles( array $color_palette ) {
echo '<style id="amp-wp-theme-support-editor-color-palette">';
foreach ( $color_palette as $color_option ) {
if ( ! $this->has_required_feature_props( self::FEATURE_EDITOR_COLOR_PALETTE, $color_option ) ) {
continue;
}

Expected Behaviour

If a non-hex color is used, no deprecated error should be raised from hexdec() here:

amp-wp/src/ReaderThemeSupportFeatures.php

Lines 375 to 377 in a373ac8

$red = hexdec( substr( $hex, 0, 2 ) );
$green = hexdec( substr( $hex, 2, 2 ) );
$blue = hexdec( substr( $hex, 4, 2 ) );

Screenshots

No response

PHP Version

No response

Plugin Version

2.3.0

AMP plugin template mode

Reader

WordPress Version

No response

Site Health

No response

Gutenberg Version

No response

OS(s) Affected

No response

Browser(s) Affected

No response

Device(s) Affected

No response

Acceptance Criteria

No response

Implementation Brief

No response

QA Testing Instructions

No response

Demo

No response

Changelog Entry

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    BugSomething isn't workingChangeloggedWhether the issue/PR has been added to release notes.

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions