WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 3 months ago

#47713 reviewing defect (bug)

image_get_intermediate_size does not return size if it equals original size

Reported by: mahnunchik Owned by: SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: major Version: 5.2.2
Component: Media Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

Function image_get_intermediate_size does not return size if it equal original size.

  1. WordPress itself doesn't generate sizes metadata item for size equal original image size.
  2. image_get_intermediate_size looks throught sizes metadata and doesn't see necessary size.
  3. It doesn't check original image size.

More details here: https://github.com/Yoast/wordpress-seo/issues/10863

Attachments (2)

47713.diff (1.2 KB) - added by mahnunchik 17 months ago.
Select full size image if it is exact match to intermediate image fixes #47713 and #21714
47713.2.diff (1.4 KB) - added by mahnunchik 16 months ago.
correct patch

Download all attachments as: .zip

Change History (18)

@mahnunchik
17 months ago

Select full size image if it is exact match to intermediate image fixes #47713 and #21714

#2 @mahnunchik
17 months ago

I've fixed it! And another 7 years old ticket #21714

Last edited 11 months ago by SergeyBiryukov (previous) (diff)

#3 @mahnunchik
16 months ago

The correct version of patch attached.

@mahnunchik
16 months ago

correct patch

#4 follow-up: @mahnunchik
8 months ago

Any news?

#5 @markparnell
8 months ago

  • Keywords has-patch added; needs-patch removed

#6 in reply to: ↑ 4 @SergeyBiryukov
8 months ago

  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release

Replying to mahnunchik:

Any news?

Thanks for the patch, sorry it took so long for someone to get back to you.

Just a quick note that I've started reviewing the patch a while ago, but didn't get a chance to finish the review yet. I'll try to follow up in near future.

Some unit tests would be helpful here.

#7 follow-up: @SergeyBiryukov
8 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#8 in reply to: ↑ 7 ; follow-up: @mahnunchik
8 months ago

Replying to SergeyBiryukov:

Hi, I'm happy to hear it.

Could you give me some useful links for documentation about testing such features. Links to examples of tests will be useful too.

#9 in reply to: ↑ 8 @skorasaurus
8 months ago

Replying to mahnunchik:

Replying to SergeyBiryukov:

Hi, I'm happy to hear it.

Could you give me some useful links for documentation about testing such features. Links to examples of tests will be useful too.

https://make.wordpress.org/core/handbook/testing/automated-testing/phpunit/
https://carlalexander.ca/introduction-wordpress-unit-testing/

I hope that helps :)

#10 @Mista-Flo
3 months ago

I am not sure to understand the issue here. Can you please add exact steps to reproduce, and screenshots with and without the patch applied? I am not sure I'm looking into the good scneario here.

@mahnunchik

Here's what I tried:

<?php
                add_image_size( 'test-size', 500, 500, true );

                $file = DIR_TESTDATA . '/images/test_500x500.png';
                $id   = $this->_make_attachment( $file, 0 );

                // Look for a size by name.
                $image = image_get_intermediate_size( $id, 'test-size' );
                var_dump( $image );

                $image = image_get_intermediate_size( $id, [500, 500] );
                var_dump( $image );

It always output the same thing, with trunk version, with your first patch applied or with your second one.

array(6) {
  ["file"]=>
  string(24) "test_500x500-500x500.png"
  ["width"]=>
  int(500)
  ["height"]=>
  int(500)
  ["mime-type"]=>
  string(9) "image/png"
  ["path"]=>
  string(32) "2020/09/test_500x500-500x500.png"
  ["url"]=>
  string(70) "http://example.org/wp-content/uploads/2020/09/test_500x500-500x500.png"
}
Last edited 3 months ago by Mista-Flo (previous) (diff)

#11 @Mista-Flo
3 months ago

Last edited 3 months ago by Mista-Flo (previous) (diff)

#12 @mahnunchik
3 months ago

Hi @Mista-Flo

  1. Image with dimensions 100px x 200px has been uploaded to WP
  2. You call function image_get_intermediate_size($id, array(100, 200))

Actual behaviour:

Returned false

Expected behaviour:

Returned image object with original 100px x 200px dimensions.

#13 @Mista-Flo
3 months ago

Hum that's weird, I have updated my last comment to show you what I tried. Can you test on last trunk version of the Core please to see if you still replicate the issue (with a default WordPress, no plugin please)

Replying to mahnunchik:

Function image_get_intermediate_size does not return size if it equal original size.

  1. WordPress itself doesn't generate sizes metadata item for size equal original image size.
  2. image_get_intermediate_size looks throught sizes metadata and doesn't see necessary size.
  3. It doesn't check original image size.
  1. For me it does generate metadata size even if it's equal to original image size.
  2. It does not return false for me, but it returns the image size generated by WordPress instead of the original size.
Last edited 3 months ago by Mista-Flo (previous) (diff)

#14 @mahnunchik
3 months ago

@Mista-Flo I've tested on trunk, the same incorrect behaviour.

According to your test it seems you are used image bigger than 500x500 for testing images/test_500x500.png.

WP doesn't generate additional file if size is equal to original image, so name of produced file should be test_500x500.png not test_500x500-500x500.png.

Please test on jpg file with exactly the same dimensions as requested. For example this image https://user-images.githubusercontent.com/11629607/44900132-13e43f00-ad04-11e8-86f6-fe830e462abc.jpg and 2000 x 1200 px dimensions.

#15 @Mista-Flo
3 months ago

I tried with your image, renamed it test-2000x1200.jpg

<?php
                add_image_size( 'test-size', 2000, 1200, true );

                $file = DIR_TESTDATA . '/images/test-2000x1200.jpg';
                $id   = $this->_make_attachment( $file, 0 );

                // Look for a size by name.
                $image = image_get_intermediate_size( $id, 'test-size' );
                var_dump( $image );

                $image = image_get_intermediate_size( $id, [2000, 1200] );
                var_dump( $image );

Here's my var_dump output

array(6) {
  ["file"]=>
  string(30) "test-2000x1200-1-2000x1200.jpg"
  ["width"]=>
  int(2000)
  ["height"]=>
  int(1200)
  ["mime-type"]=>
  string(10) "image/jpeg"
  ["path"]=>
  string(38) "2020/09/test-2000x1200-1-2000x1200.jpg"
  ["url"]=>
  string(76) "http://example.org/wp-content/uploads/2020/09/test-2000x1200-1-2000x1200.jpg"
}
array(6) {
  ["file"]=>
  string(30) "test-2000x1200-1-2000x1200.jpg"
  ["width"]=>
  int(2000)
  ["height"]=>
  int(1200)
  ["mime-type"]=>
  string(10) "image/jpeg"
  ["path"]=>
  string(38) "2020/09/test-2000x1200-1-2000x1200.jpg"
  ["url"]=>
  string(76) "http://example.org/wp-content/uploads/2020/09/test-2000x1200-1-2000x1200.jpg"
}

What's wrong with my test please, I don't get it.

This ticket was mentioned in Slack in #core-media by florian-tiar. View the logs.


3 months ago

Note: See TracTickets for help on using tickets.