Make WordPress Core

Opened 2 years ago

Last modified 6 months ago

#55290 accepted defect (bug)

Not all image edits are applied to all subsizes

Reported by: mitogh's profile mitogh Owned by: joedolson's profile joedolson
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch needs-testing needs-unit-tests needs-refresh
Focuses: administration Cc:

Description

When editing an image using the admin editor, not all edits are applied correctly to all image subsizes. Specifically, the problem occurs when an edit results in a smaller size than the defined image size. Given the following image subsizes, and the attached image.

=> array(6) {
  ["thumbnail"]=>
  array(3) {
    ["width"]=>
    int(150)
    ["height"]=>
    int(150)
    ["crop"]=>
    bool(true)
  }
  ["medium"]=>
  array(3) {
    ["width"]=>
    int(300)
    ["height"]=>
    int(300)
    ["crop"]=>
    bool(false)
  }
  ["medium_large"]=>
  array(3) {
    ["width"]=>
    int(768)
    ["height"]=>
    int(0)
    ["crop"]=>
    bool(false)
  }
  ["large"]=>
  array(3) {
    ["width"]=>
    int(1024)
    ["height"]=>
    int(1024)
    ["crop"]=>
    bool(false)
  }
  ["1536x1536"]=>
  array(3) {
    ["width"]=>
    int(1536)
    ["height"]=>
    int(1536)
    ["crop"]=>
    bool(false)
  }
  ["2048x2048"]=>
  array(3) {
    ["width"]=>
    int(2048)
    ["height"]=>
    int(2048)
    ["crop"]=>
    bool(false)
  }
}

The defined sizes above are the ones coming by default with the setting provided below.

Steps to replicate the problem:

  1. Upload the image into your installation.
  2. After the image is uploaded, click on edit to open the image editor or go to the image editor from within the media library.
  3. Click on Edit image
  4. Click on rotate image (either right or left) only once.
  5. Make sure the changes are applied to all images by checking that the setting Apply changes to: All image sizes is set
  6. Click on save

After the image is saved observe the changes were applied only to the following image sizes:

  • thumbnail
  • medium
  • large
  • full

The only size that was not updated as expected was:

  • medium_large

Setup

### wp-core ###

version: 5.9.1
site_language: en_US
user_language: en_US
timezone: +00:00
permalink: /%year%/%monthnum%/%day%/%postname%/
https_status: true
multisite: false
user_registration: 0
blog_public: 1
default_comment_status: open
environment_type: production
user_count: 1
dotorg_communication: true

### wp-paths-sizes ###

wordpress_path: /app
wordpress_size: 152.90 MB (160327567 bytes)
uploads_path: /app/wp-content/uploads
uploads_size: 22.75 MB (23851112 bytes)
themes_path: /app/wp-content/themes
themes_size: 6.47 MB (6780262 bytes)
plugins_path: /app/wp-content/plugins
plugins_size: 208.42 MB (218539593 bytes)
database_size: 3.96 MB (4149380 bytes)
total_size: 394.49 MB (413647914 bytes)

### wp-active-theme ###

name: Twenty Twenty-Two (twentytwentytwo)
version: 1.0 (latest version: 1.1)
author: the WordPress team
author_website: https://wordpress.org/
parent_theme: none
theme_features: core-block-patterns, post-thumbnails, responsive-embeds, editor-styles, html5, automatic-feed-links, block-templates, widgets-block-editor, wp-block-styles, editor-style
theme_path: /app/wp-content/themes/twentytwentytwo
auto_update: Disabled

### wp-themes-inactive (3) ###

Twenty Nineteen: version: 2.2, author: the WordPress team, Auto-updates disabled
Twenty Twenty: version: 1.9, author: the WordPress team, Auto-updates disabled
Twenty Twenty-One: version: 1.5, author: the WordPress team, Auto-updates disabled


### wp-plugins-inactive (1) ###

Performance Lab: version: 1.0.0-beta.1, author: WordPress Performance Group, Auto-updates disabled

### wp-media ###

image_editor: WP_Image_Editor_Imagick
imagick_module_version: 1691
imagemagick_version: ImageMagick 6.9.11-60 Q16 x86_64 2021-01-25 https://imagemagick.org
imagick_version: 3.7.0
file_uploads: File uploads is turned off
post_max_size: 100M
upload_max_filesize: 100M
max_effective_size: 100 MB
max_file_uploads: 20
imagick_limits: 
	imagick::RESOURCETYPE_AREA: 122 MB
	imagick::RESOURCETYPE_DISK: 1073741824
	imagick::RESOURCETYPE_FILE: 786432
	imagick::RESOURCETYPE_MAP: 512 MB
	imagick::RESOURCETYPE_MEMORY: 256 MB
	imagick::RESOURCETYPE_THREAD: 1
imagemagick_file_formats: 3FR, 3G2, 3GP, AAI, AI, APNG, ART, ARW, AVI, AVIF, AVS, BGR, BGRA, BGRO, BIE, BMP, BMP2, BMP3, BRF, CAL, CALS, CANVAS, CAPTION, CIN, CIP, CLIP, CMYK, CMYKA, CR2, CR3, CRW, CUR, CUT, DATA, DCM, DCR, DCX, DDS, DFONT, DJVU, DNG, DOT, DPX, DXT1, DXT5, EPDF, EPI, EPS, EPS2, EPS3, EPSF, EPSI, EPT, EPT2, EPT3, ERF, EXR, FAX, FILE, FITS, FRACTAL, FTP, FTS, G3, G4, GIF, GIF87, GRADIENT, GRAY, GRAYA, GROUP4, GV, H, HALD, HDR, HEIC, HISTOGRAM, HRZ, HTM, HTML, HTTP, HTTPS, ICB, ICO, ICON, IIQ, INFO, INLINE, IPL, ISOBRL, ISOBRL6, J2C, J2K, JBG, JBIG, JNG, JNX, JP2, JPC, JPE, JPEG, JPG, JPM, JPS, JPT, JSON, K25, KDC, LABEL, M2V, M4V, MAC, MAGICK, MAP, MASK, MAT, MATTE, MEF, MIFF, MKV, MNG, MONO, MOV, MP4, MPC, MPG, MRW, MSL, MSVG, MTV, MVG, NEF, NRW, NULL, ORF, OTB, OTF, PAL, PALM, PAM, PANGO, PATTERN, PBM, PCD, PCDS, PCL, PCT, PCX, PDB, PDF, PDFA, PEF, PES, PFA, PFB, PFM, PGM, PGX, PICON, PICT, PIX, PJPEG, PLASMA, PNG, PNG00, PNG24, PNG32, PNG48, PNG64, PNG8, PNM, POCKETMOD, PPM, PREVIEW, PS, PS2, PS3, PSB, PSD, PTIF, PWP, RADIAL-GRADIENT, RAF, RAS, RAW, RGB, RGBA, RGBO, RGF, RLA, RLE, RMF, RW2, SCR, SCT, SFW, SGI, SHTML, SIX, SIXEL, SPARSE-COLOR, SR2, SRF, STEGANO, SUN, SVG, SVGZ, TEXT, TGA, THUMBNAIL, TIFF, TIFF64, TILE, TIM, TTC, TTF, TXT, UBRL, UBRL6, UIL, UYVY, VDA, VICAR, VID, VIDEO, VIFF, VIPS, VST, WBMP, WEBM, WEBP, WMF, WMV, WMZ, WPG, X, X3F, XBM, XC, XCF, XPM, XPS, XV, XWD, YCbCr, YCbCrA, YUV
gd_version: bundled (2.1.0 compatible)
gd_formats: GIF, JPEG, PNG, WebP, BMP
ghostscript_version: 9.53.3

### wp-server ###

server_architecture: Linux 5.15.12-1-MANJARO x86_64
httpd_software: nginx/1.17.10
php_version: 7.4.28 64bit
php_sapi: fpm-fcgi
max_input_variables: 10000
time_limit: 3
memory_limit: 1G
max_input_time: 900
upload_max_filesize: 100M
php_post_max_size: 100M
curl_version: 7.74.0 OpenSSL/1.1.1k
suhosin: false
imagick_availability: true
pretty_permalinks: true

### wp-database ###

extension: mysqli
server_version: 10.3.27-MariaDB
client_version: mysqlnd 7.4.28
max_allowed_packet: 33554432
max_connections: 151

### wp-constants ###

WP_HOME: undefined
WP_SITEURL: undefined
WP_CONTENT_DIR: /app/wp-content
WP_PLUGIN_DIR: /app/wp-content/plugins
WP_MEMORY_LIMIT: 40M
WP_MAX_MEMORY_LIMIT: 1G
WP_DEBUG: true
WP_DEBUG_DISPLAY: true
WP_DEBUG_LOG: true
SCRIPT_DEBUG: false
WP_CACHE: false
CONCATENATE_SCRIPTS: undefined
COMPRESS_SCRIPTS: undefined
COMPRESS_CSS: undefined
WP_ENVIRONMENT_TYPE: Undefined
DB_CHARSET: utf8mb4
DB_COLLATE: undefined

### wp-filesystem ###

wordpress: writable
wp-content: writable
uploads: writable
plugins: writable
themes: writable
mu-plugins: writable

Attachments (3)

leafs.jpg (329.0 KB) - added by mitogh 2 years ago.
image-edits.patch (1.3 KB) - added by mitogh 2 years ago.
55290.2.diff (1.8 KB) - added by joedolson 22 months ago.
Refresh patch with minor changes

Download all attachments as: .zip

Change History (46)

@mitogh
2 years ago

#1 @adamsilverstein
2 years ago

Good catch @mitogh! Looks like some conditional checks for in image-edit.php aren't working as expected.

In my testing this happened when the i rotated to the vertical position. rotating again and saving my medium_large size was correctly edited. flipping images always seemed to work. cropping worked until i tried a vertical thin crop, then some sizes were saved and others not.

#2 @mitogh
2 years ago

Right, it seems like the issue happens for rotation specifically on images that do not have dimensions like a landscape or portrait image if is rotated the dimensions are different, flip works normally on my end due to the image dimensions are not modified from the original image.

Let me know in case you have any additional questions. @adamsilverstein, thanks for taking a look and replicating the steps for this issue.

If time allows I can try to find a patch for this one as well.

#3 @SergeyBiryukov
2 years ago

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

Hi there, welcome back to WordPress Trac! Thanks for the ticket.

Just noting that I was able to reproduce the issue with the steps described. I was also able to confirm comment:1.

@mitogh
2 years ago

#4 @mitogh
2 years ago

  • Keywords has-patch added; needs-patch removed

I've added a patch with a suggested fix. The suggested fix follows the same approach as WordPress follows when the image is uploaded for the first time, this approach is basically that when an uploaded image is smaller in dimensions than an image size the image size is not created for the image, which was the case for this image when the image Was rotated it was smaller in dimensions that the failed size creating an error and not saving the edit.

As described this approach would not save any image that was errored out by the editors, images would remain in the backup sizes and can be restored at any point in time, by rolling back an existing size.

The function wp_get_additional_image_sizes was replaced with wp_get_registered_image_subsizes in order to have a more accurate access to the defined image sizes by the user.

Cast of values to int was introduced as well in order to make sure settings (as strings) were converted to integers.

#5 @SergeyBiryukov
2 years ago

  • Milestone changed from Future Release to 6.0

This ticket was mentioned in Slack in #core by costdev. View the logs.


2 years ago

#7 @costdev
2 years ago

  • Keywords needs-testing added
  • Milestone changed from 6.0 to 6.1

Per the discussion in the bug scrub, this still needs-testing, needs-unit-tests and a review to ensure no BC breaks are introduced. Moving to the 6.1 milestone.

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


2 years ago

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


2 years ago

#10 @adamsilverstein
2 years ago

I've added a patch with a suggested fix

@mitogh is the patch you uploaded complete?

#11 @mitogh
2 years ago

YEs @adamsilverstein the patch work is complete let me know if you have any additional questions.

#12 @alaca
2 years ago

The provided patch does not fix the issue with missing image, in this case for medium_large size.

As you already wrote, @mitogh, the new image will not be created when the image dimensions are smaller than the image size, e.g. this will happen when rotating the image to a vertical position.

However, I still found this patch useful as it adds a few small improvements.
Replacing wp_get_additional_image_sizes with the wp_get_registered_image_subsizes function returns a more accurate list of defined image sizes, and casting options settings to an integer is what was missing in the first place.

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


22 months ago

#14 @joedolson
22 months ago

  • Owner set to joedolson
  • Status changed from new to accepted

@joedolson
22 months ago

Refresh patch with minor changes

#15 @joedolson
22 months ago

Refreshed patch also changes the variable name from $_wp_additional_image_sizes to $_wp_registered_image_sizes, so that the variable matches the data source. While the results of wp_additional_image_sizes and wp_registered_image_sizes should be very similar, it will make the code easier to follow for these to match.

@alaca In my tests, these changes do fix the missing file in the case you described: rotating a horizontal image to a vertical position produces all the expected image edits. Are you getting a different result?

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


22 months ago

#17 @adamsilverstein
22 months ago

@joedolson this ticket came up in the performance team bug scrub today. Is this still a candidate for 6.1 given that we are waiting for more testing/reviewer feedback? Or can we punt this to 6.2?

#18 @joedolson
22 months ago

I think we should still get this into 6.1. I can't identify the additional issue cited by @alaca, but I think that even if this patch is incomplete in some unidentified way, it does fix a significant image editing error. If there's a separate issue, that can be a new ticket.

#19 @pushpak.pop
22 months ago

The latest patch does fix issues with edits not applying to some sizes.

I did not encounter any specific issues with the testing I did.

This can go as is to fix the main issue and any related issues can be a new separate ticket

#20 @joedolson
22 months ago

  • Milestone changed from 6.1 to 6.2

Punting to 6.2. Still need some work to get the unit tests together for this fix; perhaps we'll get feedback about the potential missing fix.

#21 @joedolson
22 months ago

  • Keywords needs-unit-tests added

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


22 months ago

#23 @mukesh27
19 months ago

@joedolson this ticket came up in the performance team bug scrub today. Are you working on unit tests?

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


19 months ago

#25 @adeltahri
18 months ago

I just tested it on WordPress trunk 6.2.0 with and without the latest patch, and it works. I didn't encounter any issues during my testing.

#26 @adamsilverstein
18 months ago

I just tested it on WordPress trunk 6.2.0 with and without the latest patch, and it works. I didn't encounter any issues during my testing.

I tested this with the latest patch applied and still see the issue. The medium_large rotated image is not created.

  • note: use the original image attached to this ticket for testing. use the default wp theme with no plugins active.
  • after uploading, check the uploads folder and see which image seizes were created.
  • edit the image, rotate 90 degrees and save
  • note the newly created rotated versions - there is one fewer version

the image is excluded because it violates the out of bounds check in image_resize_dimensions here. As far as I can tell the patches don't address this, so it needs more testing. Also, this definitely needs unit tests that validate the issue and the fix.

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


18 months ago

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


17 months ago

#29 @flixos90
17 months ago

  • Version 5.9.1 deleted

@joedolson Is there a recent PR or patch for this ticket? I'm conscious that there haven't been any code updates for this ticket in the past few months and we're getting close to 6.2-RC phase.

Is this still reasonable to land at this time, or should we punt it? I acknowledge it is a bug, but it appears to be a rather complex one, and one that's not introduced by any recent WordPress core version, but has been around for a while.

On that note, I'm gonna remove the 5.9.1 version annotation here, as that field should be used for the version where this bug was introduced, not which version it was tested with. I'm not sure which version it was introduced in, but it has there been longer than 5.9.1.

#30 @joedolson
17 months ago

  • Milestone changed from 6.2 to 6.3

Yes, I think this should be punted to 6.3. It does seem to be a pretty fussy bug, and it's definitely not new.

I haven't done the archaeology to figure out where this comes from, but I agree that it's considerably older than 5.9.1.

#31 @agnieszkaszuba
16 months ago

I tried to test this but default image size medium_large doesn't exist in the latest WordPress version and theme.
I can confirm that the edit was applied to all the existing default sizes (thumbnail, medium, large, fullsize) though.

  • OS: macOS 13.0.1
  • PHP: 7.4.24
  • WordPress: 6.3-alpha-55505-src
  • Browser: Chrome 109.0.5414.119 (Official Build) (x86_64)
  • Theme: Twenty Twenty-Three
  • Active Plugins: -

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


13 months ago

#33 @antpb
13 months ago

  • Milestone changed from 6.3 to 6.4

Moving 6.4 to keep focus on the higher priority issues in 6.3

#34 @mukesh27
12 months ago

  • Focuses performance removed

This ticket was discussed during the Performance bug scrub.

This ticket is currently being actively worked on by @joedolson for the 6.4 release cycle. It was agreed to remove the performance focus.

Additional props to: @joemacgill @flixos90 @spacedmonkey

This ticket was mentioned in Slack in #core by oglekler. View the logs.


11 months ago

#36 @oglekler
11 months ago

Patch needs to be tested, it is possible that it will need refresh, but someone needs to try it to check.

This ticket was mentioned in Slack in #core-test by oglekler. View the logs.


11 months ago

#38 @huzaifaalmesbah
11 months ago

  • Keywords needs-refresh added

@oglekler I tried to test. its needs refresh.

#39 @nicolefurlan
10 months ago

@joedolson just checking on this one, are you able to refresh the patch?

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


10 months ago

#41 @antpb
10 months ago

  • Milestone changed from 6.4 to 6.5

With 6.4 RC1 fast approaching this should move to 6.5. Confirmed okay with @joedolson .

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


6 months ago

#43 @audrasjb
6 months ago

  • Milestone changed from 6.5 to Future Release

As per today's bug scrub, we decided to move this ticket to Future Release to give it more time to get a patch.

Note: See TracTickets for help on using tickets.