Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#36345 closed defect (bug) (fixed)

We shouldn't use "full" as image size name in wp_calculate_image_srcset()

Reported by: jaspermdegroot's profile jaspermdegroot Owned by: joemcgill's profile joemcgill
Milestone: 4.6 Priority: normal
Severity: normal Version: 4.4
Component: Media Keywords: has-patch commit
Focuses: Cc:


In wp_calculate_image_srcset() we use "full" as key when we add the full size image to $image_sizes, but "full" isn't on the list of reserved names in the add_image_size() documentation so we might be removing someone's intermediate size there.
Since we don't use the key name in any way we can just not use a name at all.

Attachments (1)

36345.patch (678 bytes) - added by jaspermdegroot 7 years ago.

Download all attachments as: .zip

Change History (9)

#1 @jaspermdegroot
7 years ago

In 36345.patch: Removed "full" as key name.

This ticket was mentioned in Slack in #core-images by jaspermdegroot. View the logs.

7 years ago

#3 @ocean90
7 years ago

  • Keywords has-patch added
  • Version changed from trunk to 4.4

Introduced in [34855], changed in [35412] and [35561].

#4 @azaozz
7 years ago

It is true that full is not a "reserved name" like thumbnail, medium, and large, but so was the newly introduced medium-large in 4.4. Also full is used quite a lot in the UI for naming the original image.

Perhaps we should "reserve" full and even original as size names that may be used by core, similarly to medium-large.

#5 @joemcgill
7 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Owner set to joemcgill
  • Status changed from new to accepted

I agree. There's no reason to use a key here since we don't ever use the key name anywhere else, nor do we pass it to any filters. Let's look at this early next cycle.

#6 @joemcgill
7 years ago

  • Milestone changed from Future Release to 4.6

Replying to azaozz:

Perhaps we should "reserve" full and even original as size names that may be used by core, similarly to medium-large.

I like the idea of reserving both the original and full size names. In the future, I could even see a use case for regenerating a full size image from the original that is the same dimensions if someone uploads a file that isn't optimized for the web. This is also another good reason for not automatically adding the original file to the $image_sizes array using full as the key name.

#7 @joemcgill
7 years ago

  • Keywords commit added

The original patch 36345.patch still applies and seems like a good idea. We should commit and then look into reserving additional size names as @azaozz suggests above.

#8 @joemcgill
7 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 37986:

Media: Don't use 'full' as array key in wp_calculate_image_srcset().

In wp_calculate_image_srcset() we get an array of image sizes
associated with an attachment and then add the original image's
information to the array before processing the srcset. In doing
so, we set the original data to a $image_sizes['full'] key, which
could stomp on any custom image sizes using full as a size name.

This avoid the issues by adding the original data without a named
key, which is never referenced anyway.

Props jaspermdegroot.
Fixes #36345.

Note: See TracTickets for help on using tickets.