Media: Add hooks and extension points for client-side media processing#74913
Media: Add hooks and extension points for client-side media processing#74913adamsilverstein merged 16 commits intotrunkfrom
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
f71a12f to
d801ac7
Compare
|
Size Change: +165 B (0%) Total Size: 8.75 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 499dd0c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23159426628
|
Image sizes are already filterable server-side via intermediate_image_sizes_advanced and related hooks.
Verifies the filter is called with the default quality value and correct context during resizeCropItem, and that it is skipped when no resize args are provided.
The test file imports @wordpress/hooks, so the tsconfig needs a project reference to satisfy lint:tsconfig.
Resolve conflicts in upload-media test file: keep both resizeCropItem tests (imageQuality filter) and trunk's generateThumbnails tests, merge expanded mock setup. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Links the finalize endpoint core backport PR to Gutenberg PR #74913 for client-side media processing.
There was a problem hiding this comment.
Thanks for the ping! Adding the finalize step makes sense to me. I'd suggest possibly splitting this PR out into two: while the finalize step makes good sense and hooks into existing hooks, the imageQuality filter is proposing a new API, and I wondered if it's the right fit for the job (the JS extensions might be something to pursue for 7.1 instead of 7.0?).
It could be that we might want a user preference or site preference to handle this (i.e. block editor preferences, etc), or even be able to set this value in PHP somewhere, so I wondered if it's worth thinking through the options a bit more before proposing that as an API?
What do you think?
| * Allows plugins to control the quality (0-1) used when resizing images. | ||
| * Note: Quality is not yet wired through to the vips worker but will | ||
| * be in a future update. This hook is provided as an extension point. |
There was a problem hiding this comment.
If it isn't wired up yet, is there any point in adding the filter just yet? I'm also wondering if a filter is the right way to go about it.
I.e. where else might image quality settings live? Would they be set site-wide? If so, then would it be editor settings of some kind, site settings, or preferences based?
I guess I'm slightly wary of adding a JS filter just yet. @Mamaduka made the point elsewhere just recently that once added they can be hard to remove!
From my perspective, I think setting a default quality option somewhere is a great idea, I'm just not quite sure about where it should live in terms of extensibility, but I'm wondering if it should live in a similar place to where we get the data for image sizes 🤔
There was a problem hiding this comment.
Also wanted to add that this is better handled through a store, like I did in https://github.com/swissspidy/media-experiments.
applyFilters is more expensive to run (potentially dozens of times per upload) vs one-of updateSettings()/getSettings() or registry .select( preferencesStore ) .get( PREFERENCES_NAME, 'default_quality' )
| * @param string $context Context: 'create' or 'update'. | ||
| */ | ||
| // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound | ||
| $metadata = apply_filters( 'wp_generate_attachment_metadata', $metadata, $attachment_id, 'update' ); |
There was a problem hiding this comment.
Just a question, but I was wondering if we want finalize to be only for the filter, or if we'd want to call wp_update_image_subsizes as a catch all?
I'm very possibly overthinking this, but if for some reason the client-side processing didn't generate everything it needed to, I was wondering if the wp_update_image_subsizes function could cover things off on the server-side.
That said, I'm suspicious of my own comment here, so mostly just wanted to capture the thought in case there's anything there.
There was a problem hiding this comment.
The intention here is that wp_generate_attachment_metadata gets triggered only when all of the image processing and uploading is completed successfully. We shouldn't need to generate additional sizes once this is called.
If client side generation fails to generate all of the sub sizes for any reason, we will continue that process at the next possible opportunity (when the editor is open & connected on the same post).
The applyFilters hook runs per-upload and is hard to remove once added. Use the existing updateSettings/getSettings store pattern instead, which is a one-time operation and consistent with how other image settings are managed.
The upload-media package is implementation-agnostic and should not make direct REST API calls. This moves the apiFetch call to a new mediaFinalize utility in the editor package and injects it via the existing settings pattern, matching how mediaUpload and mediaSideload already work.
Verify the injected callback pattern works correctly: finalizeItem calls the setting, handles missing callbacks, and recovers from errors. mediaFinalize hits the correct REST endpoint.
|
@swissspidy this is ready for another review. We landed the Core PR: WordPress/wordpress-develop#11168 |
westonruter
left a comment
There was a problem hiding this comment.
This looks correct to me, aside from the need to sync the REST API endpoint implementation from core.
andrewserong
left a comment
There was a problem hiding this comment.
I see the backport PR has been approved for the finalize endpoint, so that side of things all looks good 👍
This is testing well for me manually, with the finalize requests being fired off correctly after upload. I like the move to having the image quality in the upload-media store's settings, that feels like a good place and opens up being able to add block editor settings to update it further down the track if need be, too
Also, the addition of passing down the finalizeUpload function makes sense to me given that we also have mediaUpload and mediaSideload. Just left a question about naming: if we should make the setting mediaFinalize for consistency.
But otherwise, I think this is in good shape!
| : undefined, | ||
| mediaUpload: hasUploadPermissions ? mediaUpload : undefined, | ||
| mediaSideload: hasUploadPermissions ? mediaSideload : undefined, | ||
| finalizeUpload: hasUploadPermissions ? mediaFinalize : undefined, |
There was a problem hiding this comment.
Minor naming thing, but given the other two properties are mediaUpload and mediaSideload, would it make sense for the property to also be mediaFinalize?
There was a problem hiding this comment.
Yes! Thank you for the suggestion. I will make it so.
| if ( sizesToGenerate.length === 0 ) { | ||
| dispatch.finishOperation( id, {} ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Tiny nit: but it looks like in this block attachment.missing_image_sizes will always be greater than zero so this will never be reached. Was this handling meant to be in an else statement somewhere, or is it no longer relevant?
| OperationType.Finalize | ||
| ); | ||
| } else { | ||
| operations.push( OperationType.Upload ); |
There was a problem hiding this comment.
When not dealing with an image I assume we don't need to also fire the Finalize operation?
There was a problem hiding this comment.
Right! Finalize is only needed for images since it triggers wp_generate_attachment_metadata after client-side processing completes. For non-images, the server handles everything
during upload via generate_sub_sizes: true (line 774-782), so there's no client-side work to finalize.
Co-authored-by: Weston Ruter <[email protected]>
Co-authored-by: Weston Ruter <[email protected]>
|
Thanks for the reviews, I will address the final points and get this merged. |
The outer condition already guarantees missing_image_sizes.length > 0, making the sizesToGenerate.length === 0 block dead code.
Aligns with the existing mediaUpload and mediaSideload naming convention used in block editor settings.
andrewserong
left a comment
There was a problem hiding this comment.
Thanks for the updates, this is still testing well for me! Naming looks consistent now, too 👍
🚀
|
I just cherry-picked this PR to the wp/7.0 branch to get it included in the next release: 8e7b214 |
What?
Adds hooks and extension points to ensure existing media hooks continue to run when client-side media processing is active.
Closes #74358.
Why?
When client-side media processing handles image uploads (resizing, thumbnail generation, format conversion), server-side WordPress hooks like
wp_generate_attachment_metadataare bypassed. Plugins that rely on these hooks (e.g., for watermarking, CDN sync, custom sizes) stop working. This PR adds the necessary extension points.There is significant usage of the
wp_generate_attachment_metadatafilter by plugins.Changes
PHP: Finalize REST Endpoint
POST /wp/v2/media/{id}/finalize— New endpoint that triggerswp_generate_attachment_metadataafter all client-side operations complete, so server-side plugins can post-process attachments.JS: Store Setting
imageQuality— Store setting allowing control of image quality (0-1) for resize/crop operations, replacing the previous JS filter hook approach. Set viaupdateSettings().JS: Finalize Pipeline
OperationType.Finalize— New operation type appended to the image upload pipeline after thumbnail generation.finalizeItem()— Uses the injectedfinalizeUploadsetting callback to call the finalize endpoint, following the same pattern asmediaUploadandmediaSideload.finalizeUploadsetting — New optional setting in@wordpress/upload-mediathat accepts a callback(id: number) => Promise<void>. The actual REST API call (POST /wp/v2/media/{id}/finalize) lives in@wordpress/editorasmediaFinalize, keeping@wordpress/upload-mediaimplementation-agnostic.Architecture: Injected Callback Pattern
The
finalizeItemfunction no longer makes a directapiFetchcall. Instead, it uses thefinalizeUploadsetting injected via the block editor settings, matching the existing pattern used bymediaUploadandmediaSideload. This keeps@wordpress/upload-mediafree of direct REST API dependencies (the@wordpress/api-fetchdependency has been removed from the package).The callback flows through:
@wordpress/editor— definesmediaFinalizeutility (makes the actualapiFetchcall)use-block-editor-settings.js— wiresfinalizeUpload: mediaFinalizeinto settings@wordpress/block-editor— passesfinalizeUploadthroughuseMediaUploadSettings@wordpress/upload-media— consumesfinalizeUploadfrom settings infinalizeItem()Hook Usage Examples
POST /wp/v2/media/{id}/finalizeEndpointServer-side: Hook into
wp_generate_attachment_metadatato process attachments after client-side uploads complete:OperationType.FinalizePipelineThis is an internal pipeline operation, not a public hook. When all child sideloads (thumbnails) for an uploaded image have completed, the Finalize operation fires automatically, calling the
POST /wp/v2/media/{id}/finalizeendpoint to re-triggerwp_generate_attachment_metadataon the server. Plugin developers do not need to interact with this directly — they should use the PHP filter above.Backport Plan (WordPress Core)
What needs backporting (PHP only)
register_routes()entry andfinalize_item()/finalize_item_permissions_check()methods to Core'sWP_REST_Attachments_Controllerclass (wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php).wp_generate_attachment_metadatafilter already exists in Core. The finalize endpoint simply re-triggers it with context'update'.What does NOT need backporting
imageQuality,finalizeUpload) — These live in the@wordpress/upload-mediapackage which ships with the Gutenberg plugin, not Core.OperationType.Finalize— JS pipeline code, ships with Gutenberg.Backport PR structure
wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php— Addfinalizeroute registration, permission check, and handlertests/phpunit/tests/rest-api/rest-attachments-controller.php— Add tests for the new endpointFiles Changed
lib/media/class-gutenberg-rest-attachments-controller.phppackages/upload-media/package.json@wordpress/api-fetchdependencypackages/upload-media/src/store/types.tsFinalizetoOperationTypeenum, addfinalizeUploadsettingpackages/upload-media/src/store/private-actions.tsfinalizeUploadcallback instead of directapiFetchpackages/upload-media/src/store/test/private-actions.jsfinalizeItemwith injected callbackpackages/editor/src/utils/media-finalize/index.jsmediaFinalizeutility (REST API call)packages/editor/src/utils/media-finalize/test/index.jsmediaFinalizepackages/editor/src/components/provider/use-block-editor-settings.jsfinalizeUploadinto settingspackages/block-editor/src/components/provider/use-media-upload-settings.jsfinalizeUploadthroughTesting Instructions
POST .../finalize)wp_generate_attachment_metadataand verify it fires after upload completesnpm run test:unit -- --testPathPattern='packages/upload-media/src/store/test/private-actions'npm run test:unit -- --testPathPattern='packages/editor/src/utils/media-finalize/test'🤖 Generated with Claude Code