Skip to content

Avoid potentially adding invalid attributes or duplicates for dominant color images #578

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

Merged
mukeshpanchal27 merged 15 commits into from
Nov 17, 2022
Merged

Avoid potentially adding invalid attributes or duplicates for dominant color images #578

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9b5c6cd
add test for style before adding and add just CSS if found
pbearne Nov 7, 2022
844d17b
changes from code review
pbearne Nov 8, 2022
f9b4ce7
changes from code review
pbearne Nov 8, 2022
61879e2
Update modules/images/dominant-color/load.php
pbearne Nov 9, 2022
8859196
Update modules/images/dominant-color/load.php
pbearne Nov 9, 2022
b3adffe
Merge branch 'trunk' into 577-revise-dominant-color-styling-and-inlin…
pbearne Nov 9, 2022
eec7c37
Add check for src=" and bail if not found
pbearne Nov 9, 2022
77de817
Remove unnecessary leftover CSS.
Nov 15, 2022
a258c52
Update quote check to use str_contains, add test coverage.
Nov 16, 2022
c829b21
Merge branch 'trunk' into 577-revise-dominant-color-styling-and-inlin…
Nov 16, 2022
6d28ef2
Add check to not update img tag if the logic already ran before on th…
Nov 16, 2022
c4c1a40
Fix test error.
Nov 16, 2022
e100c6f
Merge branch 'release/1.7.0' into 577-revise-dominant-color-styling-a…
Nov 16, 2022
6d7501a
Use data provider for test (props @costdev).
Nov 17, 2022
2d246a1
Use assertStringContainsString for more helpful failure messages.
Nov 17, 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
Prev Previous commit
Next Next commit
Merge branch 'trunk' into 577-revise-dominant-color-styling-and-inlin… 
…e-styling
  • Loading branch information
Felix Arntz
Felix Arntz committed Nov 16, 2022
commit c829b21ccfc36af1061038b9309c2cd9d7c77357
17 changes: 8 additions & 9 deletions modules/images/dominant-color/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,12 @@ function dominant_color_img_tag_add_dominant_color( $filtered_image, $context, $

Comment thread
felixarntz marked this conversation as resolved.
if ( ! empty( $image_meta['dominant_color'] ) ) {
$data .= sprintf( 'data-dominant-color="%s" ', esc_attr( $image_meta['dominant_color'] ) );
$style = '--dominant-color: #' . esc_attr( $image_meta['dominant_color'] ) . '; ';

if ( str_contains( $filtered_image, ' style="' ) ) {
$filtered_image = str_replace( ' style="', ' style="--dominant-color: #' . esc_attr( $image_meta['dominant_color'] ) . '; ', $filtered_image );
} else {
$filtered_image = str_replace( '<img ', '<img style="--dominant-color: #' . esc_attr( $image_meta['dominant_color'] ) . ';" ', $filtered_image );
}
}

if ( isset( $image_meta['has_transparency'] ) ) {
Expand All @@ -131,14 +136,8 @@ function dominant_color_img_tag_add_dominant_color( $filtered_image, $context, $
$extra_class = $image_meta['has_transparency'] ? 'has-transparency' : 'not-transparent';
}

if ( ! empty( $data ) || ! empty( $style ) || ! str_contains( $data, $filtered_image ) ) {
if ( str_contains( $filtered_image, ' style=' ) ) {
$filtered_image = str_replace( ' style="', ' ' . $data . 'style="' . $style, $filtered_image );
} elseif ( ! empty( $style ) ) {
$filtered_image = str_replace( '<img ', '<img ' . $data . 'style="' . $style . '" ', $filtered_image );
} else {
$filtered_image = str_replace( '<img ', '<img ' . $data, $filtered_image );
}
if ( ! empty( $data ) ) {
$filtered_image = str_replace( '<img ', '<img ' . $data, $filtered_image );
}

if ( ! empty( $extra_class ) ) {
Expand Down
42 changes: 42 additions & 0 deletions tests/modules/images/dominant-color/dominant-color-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,48 @@ public function test_dominant_color_img_tag_add_dominant_color_requires_proper_q
$this->assertStringNotContainsString( ' data-dominant-color=', dominant_color_img_tag_add_dominant_color( $img_with_escaped_quotes ) );
}

/**
* Tests that dominant_color_img_tag_add_dominant_color() does not replace existing inline styles.
*
* @dataProvider data_provider_dominant_color_check_inline_style
*
* @covers ::dominant_color_img_tag_add_dominant_color
*
* @param string $filtered_image The filtered image markup.
* Must include `src="%s" width="%d" height="%d"`.
* @param string $expected The expected style attribute and value.
*/
public function test_dominant_color_img_tag_add_dominant_color_should_add_dominant_color_inline_style( $filtered_image, $expected ) {
$attachment_id = self::factory()->attachment->create_upload_object( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/dominant-color/red.jpg' );
wp_maybe_generate_attachment_metadata( get_post( $attachment_id ) );

list( $src, $width, $height ) = wp_get_attachment_image_src( $attachment_id );

$filtered_image = sprintf( $filtered_image, $src, $width, $height );

$this->assertStringContainsString(
$expected,
dominant_color_img_tag_add_dominant_color( $filtered_image, 'the_content', $attachment_id )
);
}

/**
* Data provider for test_dominant_color_img_tag_add_dominant_color_should_add_dominant_color_inline_style().
*
* @return array[]
*/
public function data_provider_dominant_color_check_inline_style() {
return array(
'no existing inline styles' => array(
'filtered_image' => '<img src="%s" width="%d" height="%d" />',
'expected' => 'style="--dominant-color: #fe0000;"',
),
'existing inline styles' => array(
'filtered_image' => '<img style="color: #ffffff;" src="%s" width="%d" height="%d" />',
'expected' => 'style="--dominant-color: #fe0000; color: #ffffff;"',
),
);
}

/**
* Tests dominant_color_set_image_editors().
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.