Opened 10 years ago
Last modified 5 months ago
#34981 assigned defect (bug)
Usage of `image_size_names_choose` breaks JS attachment model attributes
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Future Release | Priority: | normal |
| Severity: | normal | Version: | 4.3.1 |
| Component: | Media | Keywords: | has-patch needs-unit-tests has-test-info |
| Focuses: | performance | Cc: |
Description
Here's a small use case that shows how the issue arise.
I want to limit the options of image sizes in the default media modal to so I use the filter image_size_names_choose provided in wp-admin/includes/media.php and remove the sizes thumbnail and as well as medium. This brings me the expected result:
With this I have now the problem, that for every thumbnail image in the media library (mode: grid) the large image source is used which is a performance issue if there are more than a couple of images in the library.
The reason is, that the same filter image_size_names_choose is also applied to each image in wp_prepare_attachment_for_js() and therefore the attachment model does not reflect all available image sizes (console.log( attachment.sizes ):
Here's a plugin that reproduces the issue in a clean WordPress (4.4) install:
wp-content/plugins/image_sizes_issue/image_sizes_issue.php:
<?php /** * Plugin Name: Image sizes issue */ add_filter( 'image_size_names_choose', function( $sizes ) { unset( $sizes[ 'medium' ] ); unset( $sizes[ 'thumbnail' ] ); return $sizes; } ); add_action( 'admin_enqueue_scripts', function() { wp_enqueue_media(); wp_enqueue_script( 'image_size_test', plugins_url( '/image_sizes_issue.js', __FILE__ ), [ 'jquery' ] ); } );
wp-content/plugins/image_sizes_issue/image_sizes_issue.js:
( function( $, wp ) {
$( function() {
var image = wp.media.model.Attachment.get( '4' );
var promise = image.sync( 'read', image );
promise.then( function( results ) {
console.log( results.sizes );
} );
} );
} )(
jQuery,
window.wp = window.wp || {}
);
For what I see, the filter image_size_names_choose is meant to affect the default UI for customization purpose. At least the name of the filter and the name of the function image_size_input_fields() (introduced in 2.7) suggests that.
In contrast to that, wp_prepare_attachment_for_js() prepares the data structure for the Javascript API. I'm sure there's some reason to apply the filter there, but I have no idea what this reason could be.
A possible, backward compatible solution could be, to pass a second argument to the filter to specify the context, the filter is applied in.
I detected this behaviour in 4.3.1 but I think it go back to 3.5 when wp_prepare_attachments_for_js() was introduced.
Attachments (5)
Change History (28)
#2
@
9 years ago
- Keywords has-patch needs-docs added
- Milestone changed from Awaiting Review to 4.7
34981.diff adds context
#3
@
9 years ago
Using function and method names for a context isn't a good idea IMO. It ties the filter (and thus the filtering code) to where it is actually called. That makes refactorings of the code almost impossible in a sane way. If you want to refactor Custom_Background::wp_set_background_image() later, you'll have to keep the second parameter to the value 'Custom_Background::wp_set_background_image'.
The method itself is marked as deprecated so a refactoring is likely:
/**
*
* @since 3.4.0
* @deprecated 3.5.0
*/
I think we need to declare concrete contexts and name them independently of where the contexts apply. A comparable situation is the context parameter or sanitize_post().
I see actually only two contexts here:
- the select form elements (that one, the filter was originally meant for) in WordPress core's media modal views (another differentiation of these could also be done via JavaScript).
- the data sanitizing for JS APIs in
wp_prepare_attachment_for_js().
(However, all this is just a hack-ish workaround for backward compatibility. I see no reason why wp_prepare_attachment_for_js() should apply this filter anyway.)
#4
@
9 years ago
- Keywords needs-patch added; has-patch removed
Thought for a second that maybe the image_size_names_choose filter could just be removed, but that would reintroduce #22598.
I think context is going to be the key, but I agree with @dnaber-de that it shouldn't be tied to the name of the function, but to the actual context.
#5
@
9 years ago
I don't think that removing the image_size_names_choose filter would reintroduce #22598 as the main issue there is that people might be using the image_downsize filter to short circuit WP's native image resizing implementation.
Besides wp_prepare_attachment_for_js(), the image_size_names_choose filter is applied in three other places:
image_size_input_fields()– only called from legacy media upload forms.Custom_Background::wp_set_background_image()– deprecated in 3.5wp_print_media_templates()– three times, and always used to populate size options for a select element.
If wp_prepare_attachment_for_js() is going to continue to be used as a standard way to create generalized attachment models to return in AJAX requests, then we should probably consider moving this filter to the front-end where interfaces are being populated by size options, rather than doing so when the model is built.
If that approach turns out to be impossible due to backwards compatibility issues, it might be better pass a second parameter to wp_prepare_attachment_for_js() which would give requests an opportunity to bypass this filter and return all available image sizes.
This ticket was mentioned in Slack in #core by stevenkword. View the logs.
9 years ago
This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.
9 years ago
#9
@
9 years ago
I also think the use of image_size_names_choose in wp_prepare_attachment_for_js() is wrong. In this case it controls what image sizes are outputted to JS, i.e. has little to do with the original purpose of only filtering what is displayed in the UI.
However removing it may introduce other errors. For example if a plugin adds more sizes, they will not be available in JS.
We already always add (hard-code) the full size. A simple workaround would be to ensure that the thumbnail size is always present so it can be used in the media modal and library to display the image thumbnails. See 34981.3.diff.
A better way to fix this would be to add srcset to these thumbnails :)
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
9 years ago
#11
@
9 years ago
Seems like this needs some cursory research on what people are doing in the wild with image_size_names_choose and what other filters people use or could use to add/remove image sizes. If people are using this filter and actually counting on it removing those sizes from the attachment model then we should know that first. In a vacuum, it seems like it shouldn't apply to wp_prepare_attachment_for_js() at all.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
9 years ago
This ticket was mentioned in Slack in #core-images by mike. View the logs.
9 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
9 years ago
#16
@
9 years ago
There are ~60 plugins in the .org repo that are filtering on image_size_names_choose — overwhelmingly to add additional image sizes to the insert image dialogue, which is already applying the image_size_names_choose filter when the attachment-display-settings template is created in wp_print_media_templates(). I think we should be fine removing the filter from wp_prepare_attachment_for_js().
In 34981.4.diff, I've skipped the image_size_names_choose filter whenever we're returning sizes from the attachment metadata and am now only using it when we're looking to see if someone is setting sizes by filtering image_downsize. This allows all available image sizes to be returned by wp_prepare_attachment_for_js(). All Media, AJAX, and Customize tests are still passing.
Regarding the unintended consequences in the media library whenever someone removes appropriate sizes, I agree that we should look into using srcset here, but we should also look at converting the media library to use the new JSON REST API endpoints instead of using admin-ajax.
#17
@
9 years ago
- Keywords 4.8-early added
- Milestone changed from 4.7 to Future Release
Let's pick this up early in the 4.8 cycle.
#19
@
3 years ago
- Milestone set to Future Release
Came across this one because it was missing a milestone.
I've refreshed the most recent patch in 34981.5.diff and done some basic testing, and nothing seems to have broken.
I haven't taken the deep dive into the code to make sure this still makes sense, but I did perform a new search of the plugin directory. It looks like there are now 471 occurrences of image_size_names_choose in the directory in 297 different plugins. However, a lot of these are false positives (plugins calling apply_filters( 'image_size_names_choose' ) manually, and a good handful are bundling an older version of Advanced Custom Fields which references the filter and actually calls out oddities mentioned here.
I also checked the Gutenberg code base, and while the filter is not used in the source code, it looks like it's used within end to end tests.
I'm resetting the Owned by field because it has been 6+ years without any progress. Sometimes contributors shy away from tickets with owners.
If @joemcgill or any past participants feel like running with this one, please feel free to self assign again.
#20
@
3 years ago
- Owner joemcgill deleted
Thanks @desrosj! Happy to have someone else take this over as clearly it's not something I'm actively working on.
This ticket was mentioned in PR #9119 on WordPress/wordpress-develop by @SirLouen.
5 months ago
#21
Refreshing again patch 34981.5.diff
Testing information below (in Trac)
Trac ticket: https://core.trac.wordpress.org/ticket/34981
#22
@
5 months ago
- Keywords needs-unit-tests has-test-info added; needs-testing removed
Test Report
Description
✅ This report validates that the indicated patch works as expected.
I've had a good time trying to understand this issue with almost everything broken in the report.
The conclusion for a layman tester is:
- The hook
image_size_names_chooseis only meant to limit certain sizes in the editor, but not JS-wide(*) - After using the hook, the images that have been restricted can't be used JS-wide
- A patch aims to "unlock" all the images JS-wide, as it's supposed to be.
(*) When I say "JS-wide" I mean in places like the JavaScript generated grids of Media Galleries.
Patch tested: https://github.com/WordPress/wordpress-develop/pull/9119.diff
Setup Instructions
- Add the code like a plugin, both the PHP and the JS, put both in the same file, as any other regular plugin
- Be wary that the provided JS is not working any more (at least not for me). Here is a modified working version of the JS:
( function( $, wp ) {
$( function() {
var attachments = wp.media.query( { order: 'DESC', orderby: 'date', type: 'image', posts_per_page: 1 } );
attachments.more().then( function() {
var image = attachments.first();
console.log( 'Sizes:', image.get( 'sizes' ) );
} );
} );
} )(
jQuery,
window.wp = window.wp || {}
);
- Enable the plugin
- After enabling the plugin, add a couple of big images (say >2Mb images), 10 or so would be ok. You can basically upload the same over and over again.
Expected Results Before the Patch
- Adding the hook provided, we are limiting uploads to large and full.
- In the media grid, check the HTML code of any of the thumbnails. You will notice that all the thumbnails are picking the large image, which according to the reporter, it may cause performance issues (although large files are around 250kb and they are paginated), so I'm not sure how much of a performance issue this may provoke, but ultimate is something wrong. I wondered if maybe there were not paginated in the past, and it tried to load like 1K large images?
- If you add an image in a Post, you will see that only Large and Full are the only two sizes enabled to use (this is expected because we are using the hook).
- If you added my JS code, you will see something like this before the patch in the Browser Console:
{
"large": {
"height": 438,
"width": 660,
"url": "http://localhost:8889/wp-content/uploads/2025/06/repeat_10-2-1024x680.jpg",
"orientation": "landscape"
},
"full": {
"url": "http://localhost:8889/wp-content/uploads/2025/06/repeat_10-2-scaled.jpg",
"height": 1700,
"width": 2560,
"orientation": "landscape"
}
}
- 🐞 All the image sizes are missing!!
Expected Results After the patch
- ✅ Issue is solved after the patch
This is what you might see in the JS Browser console if everything went well
{
"medium": {
"height": 199,
"width": 300,
"url": "http://localhost:8889/wp-content/uploads/2025/06/repeat_10-2-300x199.jpg",
"orientation": "landscape"
},
"large": {
"height": 438,
"width": 660,
"url": "http://localhost:8889/wp-content/uploads/2025/06/repeat_10-2-1024x680.jpg",
"orientation": "landscape"
},
"thumbnail": {
"height": 150,
"width": 150,
"url": "http://localhost:8889/wp-content/uploads/2025/06/repeat_10-2-150x150.jpg",
"orientation": "landscape"
},
"medium_large": {
"height": 438,
"width": 660,
"url": "http://localhost:8889/wp-content/uploads/2025/06/repeat_10-2-768x510.jpg",
"orientation": "landscape"
},
"1536x1536": {
"height": 438,
"width": 660,
"url": "http://localhost:8889/wp-content/uploads/2025/06/repeat_10-2-1536x1020.jpg",
"orientation": "landscape"
},
"2048x2048": {
"height": 438,
"width": 660,
"url": "http://localhost:8889/wp-content/uploads/2025/06/repeat_10-2-2048x1360.jpg",
"orientation": "landscape"
},
"post-thumbnail": {
"height": 408,
"width": 660,
"url": "http://localhost:8889/wp-content/uploads/2025/06/repeat_10-2-825x510.jpg",
"orientation": "landscape"
},
"full": {
"url": "http://localhost:8889/wp-content/uploads/2025/06/repeat_10-2-scaled.jpg",
"height": 1700,
"width": 2560,
"orientation": "landscape"
}
}
Additional Notes
- I believe that some Unit Tests are required to pass. Wondering why this report got abandoned.


The filter was added in [22850], #30150 suggests to add a context to each filter call.