Repository navigation
Media: Enable client-side uploads in the Media Library and media modal - #12585
adamsilverstein wants to merge 88 commits into
Conversation
The client-side media pipeline needs SharedArrayBuffer, which requires a cross-origin isolated context. Core only isolates the block editor screens, so uploads from the Media Library grid cannot use the pipeline. Hook the existing Document-Isolation-Policy output buffer on load-upload.php, gated to grid mode for users who can upload files. The mode is resolved the same way upload.php resolves it later in the request, without updating the saved user option. List mode has no pipeline integration and stays untouched, avoiding isolation side effects on a screen that gets no benefit.
Grid uploads go through wp.Uploader/plupload to async-upload.php, doing all image processing server-side even when the browser could handle it. Add a media-library-upload script that configures the @wordpress/upload-media store and intercepts plupload's FilesAdded at a higher priority, routing each file through the pipeline: REST upload of the original, client-side thumbnails via wasm-vips, then sideload and finalize. The grid UI is preserved by mirroring wp-plupload's placeholder tiles, progress, queue reset, and error sidebar. mediaSideload/mediaFinalize are thin apiFetch wrappers because the @wordpress/media-utils equivalents are private APIs. When the browser is not cross-origin isolated or lacks client-side support, the script no-ops and classic plupload keeps handling uploads, so degraded environments lose nothing.
Assert the Document-Isolation-Policy header is sent on the grid and not in list mode, that a JPEG upload flows through the REST create, sideload, and finalize endpoints with no async-upload.php requests, and that a disallowed file type surfaces in the error sidebar. Playwright's Chromium build ships without Document-Isolation-Policy support, so the upload assertions skip when the context is not cross-origin isolated; the header assertions still run everywhere.
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
The ticket did not exist yet when the tests were written; annotate all new test methods now that it has been filed.
CI's Playwright Chromium now supports Document-Isolation-Policy, so the pipeline E2E test runs for real instead of skipping. The previous asset was the 50x50 phpunit fixture, smaller than every registered sub-size, so the pipeline correctly generated zero thumbnails and the sideload-count assertion failed. Swap in the 640x480 canola.jpg fixture so thumbnail and medium sub-sizes are generated and sideloaded, and update the skip comments that claimed Playwright lacks DIP support. See #65661
The suites gated the isolation callback and enqueue but never proved the callback is actually wired to load-upload.php, that the inline settings match what the server computes, or that the pipeline's output is real: * Assert the default-filters.php hook registration, without which none of the gating logic runs. * Assert the inline settings are exactly the JSON encoding of wp_get_media_library_upload_settings(), and that the upload_mimes, image_strip_meta, and image_max_bit_depth filters flow through to the settings the browser pipeline consumes. * Extend the E2E happy path past request counting: the finalized attachment must carry thumbnail and medium sub-sizes in its metadata, and the sideloaded thumbnail file must actually be servable.
Interrupting a client-side pipeline upload is worse than interrupting a classic plupload one: classic uploads complete server-side once the bytes arrive, but an interrupted pipeline upload loses browser-generated thumbnails that were not sideloaded yet and leaves the attachment unfinalized. Trigger the browser's leave confirmation while the progress map is non-empty; the guard is scoped to the pipeline, so classic uploads behave exactly as before. See #65662
…eline media-new.php was the last admin upload surface without pipeline integration: plupload-handlers creates a raw plupload.Uploader (wp.Uploader never loads there) posting to async-upload.php with all image processing server-side. Extend cross-origin isolation to the screen via a new wp_set_up_media_new_cross_origin_isolation() on load-media-new.php, gated on client-side processing being enabled and the upload_files capability (the screen itself already requires it). Add a new media-new-upload admin script that binds a higher-priority FilesAdded handler on the plupload-handlers uploader instance and routes files through the @wordpress/upload-media store, sharing its settings with the grid integration via wp_get_media_library_upload_settings(). The screen's existing UI helpers are reused rather than replicated: fileQueued() builds the progress item, uploadSuccess() renders the finished attachment row through the existing async-upload.php markup endpoint, itemAjaxError() surfaces per-file errors, and uploadComplete() runs when the queue drains, so the screen looks and behaves unchanged. The same beforeunload guard as the grid warns while pipeline uploads are in flight. When the browser is not cross-origin isolated or lacks client-side support the script no-ops and the classic plupload flow (and the browser-uploader HTML fallback form) keep working unchanged. See #65662
Assert the Document-Isolation-Policy header on media-new.php, the happy-path pipeline upload (create, sideload, and finalize via REST with no file upload through async-upload.php; the fetch=3 markup POST is expected and excluded), and the disallowed-file-type error path. CI's Playwright Chromium supports Document-Isolation-Policy, so the full pipeline is exercised there; the upload assertions still skip in browsers where isolation is unavailable, and the existing media-upload spec keeps covering the classic path as the degradation check. See #65662
The test asserts that a failed SVG upload on media-new.php shows a dismissible error. With the client-side pipeline active (CI's Chromium is cross-origin isolated), the disallowed file is rejected client-side and the error renders through the standard per-file error UI, where the dismiss control is a link, instead of the server-rendered async-upload.php notice, where it is a button. Target the .dismiss control by class so both variants pass; the error text and the dismissal behavior asserted are unchanged. See #65662
…oreunload guards The beforeunload guards from this branch had no test coverage at all, and the media-new.php suites had the same blind spots just closed for the grid on the base branch: * Assert the load-media-new.php hook registration in default-filters.php and that the inline settings are exactly the JSON encoding of wp_get_media_library_upload_settings(). * Add an E2E test per screen for the beforeunload guard: hold sideload requests via routing so the upload is deterministically in flight, then dispatch a synthetic cancelable beforeunload and assert it is prevented while uploading and no longer prevented after completion. * Extend the media-new.php E2E happy path past request counting: the finalized attachment must carry thumbnail and medium sub-sizes and the sideloaded thumbnail file must actually be servable.
There was a problem hiding this comment.
Pull request overview
Extends WordPress’s client-side media processing pipeline beyond the block editor to the remaining admin upload surfaces (Media Library grid and Media > Add New), including cross-origin isolation setup, upload routing via the @wordpress/upload-media store, and new automated coverage to validate the end-to-end pipeline and headers.
Changes:
- Adds Media Library mode resolution and new helpers to enable Document-Isolation-Policy isolation and enqueue upload-routing scripts for
upload.php(grid) andmedia-new.php. - Introduces new admin upload integration scripts (
media-library-upload,media-new-upload) that intercept plupload flows and route files through the REST-based client-side pipeline. - Adds comprehensive PHPUnit + Playwright E2E tests covering header gating, pipeline upload behavior (create/sideload/finalize), beforeunload guards, and error handling.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/phpunit/tests/media/wpMediaNewCrossOriginIsolation.php | Adds PHPUnit coverage for DIP isolation setup on media-new.php. |
| tests/phpunit/tests/media/wpMediaLibraryCrossOriginIsolation.php | Adds PHPUnit coverage for DIP isolation setup and Media Library mode resolution on upload.php grid. |
| tests/phpunit/tests/media/wpEnqueueMediaNewUpload.php | Adds PHPUnit coverage for enqueueing media-new-upload and inline settings. |
| tests/phpunit/tests/media/wpEnqueueMediaLibraryUpload.php | Adds PHPUnit coverage for enqueueing media-library-upload, inline settings, and filter plumbing into settings. |
| tests/e2e/specs/media-upload.test.js | Adjusts an existing E2E assertion to support both server and client-side rejection UIs. |
| tests/e2e/specs/media-new-client-side-upload.test.js | Adds E2E coverage for media-new.php DIP header, pipeline upload, beforeunload guard, and disallowed types. |
| tests/e2e/specs/media-library-client-side-upload.test.js | Adds E2E coverage for upload.php grid DIP header, pipeline upload, beforeunload guard, and disallowed types (plus list-mode header absence). |
| src/wp-includes/script-loader.php | Registers the new admin script handles and their dependencies/translations. |
| src/wp-includes/media.php | Adds mode helper, isolation setup helpers, shared upload settings helper, and enqueue helpers for the two upload surfaces. |
| src/wp-includes/default-filters.php | Wires the new isolation setup callbacks into the relevant load-* hooks. |
| src/wp-admin/upload.php | Enqueues the new grid upload integration script in Media Library grid mode. |
| src/wp-admin/media-new.php | Enqueues the new Media > Add New upload integration script. |
| src/js/_enqueues/admin/media-new-upload.js | Implements media-new.php plupload interception, pipeline routing, UI mirroring, progress sync, and beforeunload guard. |
| src/js/_enqueues/admin/media-library-upload.js | Implements Media Library grid uploader interception, pipeline routing, UI mirroring, progress sync, and beforeunload guard. |
| Gruntfile.js | Adds build mappings for the two new admin scripts. |
Comments suppressed due to low confidence (1)
src/wp-includes/media.php:6846
wp_enqueue_media_new_upload()enqueues the pipeline integration on any secure origin, butwp_start_cross_origin_isolation_output_buffer()(and thereforecrossOriginIsolated) is Chromium 137+ only. On other browsers this script cannot run and will always no-op, so enqueueing it adds avoidable page weight. Consider gating enqueueing onwp_get_chromium_major_version() >= 137to skip loading unused bundles in browsers that can’t be isolated by DIP.
if ( ! wp_is_client_side_media_processing_enabled() ) {
return;
}
wp_enqueue_script( 'media-new-upload' );
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
tests/phpunit/tests/media/wpEnqueueMediaLibraryUpload.php:46
- tear_down() restores HTTP_HOST but not HTTP_USER_AGENT. If set_up() sets a Chromium UA for deterministic enqueue behavior, tear_down() should restore/unset it to avoid leaking the UA into later tests.
if ( null === $this->original_http_host ) {
unset( $_SERVER['HTTP_HOST'] );
} else {
$_SERVER['HTTP_HOST'] = $this->original_http_host;
}
…ient-side-uploads # Conflicts: # src/wp-includes/media.php
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Combines the enqueue guards into one condition and sets the flag through a declared global, documented with @global, in both the function that sets it and the one that reads it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDLDCCiR2MuQ6MeizyiAgi
westonruter
left a comment
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5
A code review of the branch (medium effort) turned up the findings below, attached inline and ordered roughly by severity. They have not been independently verified, so each should be confirmed before acting on it.
The ones most likely to regress behavior that works today:
- Arbitrary plupload params are forwarded to
POST /wp/v2/mediaas schema fields. - Document-Isolation-Policy is sent on
upload.php(grid) andmedia-new.phpwith no opt-out. - Plupload's client-side
resizesetting is bypassed. - A second
MediaUploadProvidercan race the editor's on block editor screens. wp_max_upload_size()now coerces theupload_size_limitfilter result.
The rest are parity gaps (no progress callback) and cleanups (duplicated settings logic, repeated enqueue work, global isolation state).
Casting and clamping the `upload_size_limit` filter result changed a widely used public function only to satisfy a type annotation: a filter returning shorthand such as '64M' became 64 bytes. Clamp the value in the pipeline settings instead, where a non-negative integer is required. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDLDCCiR2MuQ6MeizyiAgi
A plugin calling wp_enqueue_media() on the "Add New Media File" screen enqueues both screen scripts, and each appended another copy of the settings blob, with every image size and mime type, to the shared pipeline script. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDLDCCiR2MuQ6MeizyiAgi
The media frame script configured the store when it loaded and, when the editor had not done so yet, rendered its own MediaUploadProvider into the same store. Whichever provider committed last won, so this one could replace the editor's mediaUpload setting. On block editor screens it now only waits for the editor's settings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDLDCCiR2MuQ6MeizyiAgi
wp-plupload.js calls it on every UploadProgress, and views or plugins passing their own `progress` callback rely on it. Pipeline uploads updated the tile's percent but never called it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDLDCCiR2MuQ6MeizyiAgi
One audio file in a batch kept every file in it on the classic path, so a HEIC dropped together with an MP3 failed on a server that cannot convert HEIC, although the browser could have. Eligible files now go through the pipeline and the rest are re-queued on plupload, the way a HEIC the browser cannot decode already was. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDLDCCiR2MuQ6MeizyiAgi
The pipeline uploaded the original file, so plupload's client-side resize (the "Scale images" setting, or a `resize` passed through `plupload_init`) was silently skipped. Those images now take the classic path whenever resizing is enabled. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDLDCCiR2MuQ6MeizyiAgi
The Media Library pipeline repeated the filters the REST index applies for the block editor, and the copies had already drifted: it never passed `wp_generate_animated_image_subsizes`, which upload-media reads to decide whether animated sub-sizes keep their animation. Both now read one private helper. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDLDCCiR2MuQ6MeizyiAgi
|
I have addressed all of the outstanding feedback. |
westonruter
left a comment
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5
A second code review of the branch (medium effort) turned up the findings below, attached inline and ordered roughly by severity. None of them repeats the earlier review. They have not been verified by running them, so each should be confirmed before acting on it.
Possible bugs:
get_current_screen()is called in a path that can run on the front end, which is a fatal error.- On
media-new.php,uploadComplete()can fire while uploads are still running. - Pipeline entries leak when upload-media finishes or drops an item without calling
onSuccessoronError. - A failed finalize leaves an orphaned attachment on the server.
- The frame passes an unchecked
post_idto REST, so uploads fail where the classic uploader would upload unattached.
Cleanups: the batch flow is duplicated between the two screens, the run-once guard searches inline script text, and isolated screens are hardcoded in conditionals. Bugs 3 and 4 start in @wordpress/upload-media's own behavior and only show up through this integration.
| } | ||
|
|
||
| $settings = wp_get_media_library_upload_settings(); | ||
| $screen = get_current_screen(); |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5
_wp_enqueue_media_upload_pipeline_script() calls get_current_screen(), which only exists in wp-admin, so it fatals when wp_enqueue_media() runs on the front end in a cross-origin isolated response.
Failure scenario: A front-end editing plugin sends the Document-Isolation-Policy header itself (wp_is_cross_origin_isolated_request() deliberately counts a header a plugin queued), the visitor uses Chromium 137+ with client-side processing enabled, and the plugin calls wp_enqueue_media() on the front end. The three guards pass, then get_current_screen() is an undefined function and the request fails with a PHP fatal error.
Guarding with function_exists( 'get_current_screen' ), or computing isBlockEditor some other way, would avoid it.
There was a problem hiding this comment.
Good catch, fixed in f7a2aaf. It now falls back to no screen when get_current_screen() isn't loaded.
| * The built-in UploadComplete binding never fires for pipeline uploads | ||
| * because every file is removed from plupload before its queue starts. | ||
| */ | ||
| function finishUpload() { |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5
finishUpload() fires uploadComplete() whenever the in-flight counter reaches 0 during the batch loop, and it ignores files handed back to plupload, so "upload complete" fires too early.
Failure scenario: The user drops [bad.exe, a.png, b.webp]. addItems() rejects bad.exe synchronously inside dispatch (validateMimeTypeForUser runs before any await), so onError runs, the counter goes 1→0, and uploadComplete() fires before a.png is even queued. The same happens when a batch's audio or JPEG share, or a HEIC hand-off, is still uploading through plupload. #insert-gallery is re-enabled and the queue is reported finished while uploads are still running.
There was a problem hiding this comment.
Confirmed, and fixed in 11e485c.
Claude reproduced it before fixing, notes below:
Reproduced with an image plus a WAV in one batch while the WAV's classic upload was held back:
uploadComplete()ran with plupload stillSTARTED. The other direction was wrong too - plupload's ownUploadCompletewould fire for the classic share while pipeline uploads were still running.The batch is now counted before any file is queued, so a synchronous
addItems()rejection can't drain the count mid-batch.finishUpload()only fires when plupload isn'tSTARTED, and a priority-100UploadCompletehandler holds back the built-in one while pipeline uploads are in flight. A new e2e test covers the mixed batch.
| /** | ||
| * @param {PipelineAttachment[]} attachments The uploaded attachments. | ||
| */ | ||
| function onSuccess( attachments ) { |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5
An entry is deleted only when onSuccess or onError runs, but upload-media can finish or drop a top-level item without calling either, which leaks the entry and leaves the beforeunload prompt on for good.
Failure scenario: processItem() calls onSuccess only if ( attachment ), then removeItem() runs regardless. cancelItem( id, err, true ) (silent) skips onError. If either path is hit, the entry stays in entries. Every navigation then shows the "leave site?" prompt, the frame tile stays at 99% with uploading: true (the queue never resets), and on media-new.php the in-flight counter never drains.
There was a problem hiding this comment.
Fixed in 11e485c, though neither path seems reachable with the bundled package today.
Claude went through the upload-media bundle for this one:
In the bundled build the only silent
cancelItem( ..., true )calls are for the GIF companion video, which is a child item, and a top-level item always carries an attachment (at least the blob URL placeholder) by the timeprocessItem()runs out of operations. So no current path leaks an entry.It's cheap to guard anyway:
onStoreChange()now marks each entry once its item shows up in the store, and an entry whose item has disappeared without either callback running is failed with a generic error. That releases the unload guard, the frame tile, and the media-new counter.
| * @param {UploadError} error The upload error. | ||
| * @param {File} nativeFile The original file (for the error label). | ||
| */ | ||
| function handleError( wpUploader, model, error, nativeFile ) { |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5
When finalize fails, the tile is removed and an upload error is shown, but the attachment already exists on the server and is never deleted.
Failure scenario: The upload and sideloads succeed, then /finalize returns a 500. upload-media cancels with MEDIA_FINALIZE_ERROR and does not call mediaDelete. handleError() destroys the model and lists the file as failed to upload. The user retries, which creates a duplicate attachment, and the first upload (without finalized metadata) shows up in the library on the next load.
There was a problem hiding this comment.
Confirmed, fixed in 11e485c.
Claude checked the store and wrote the fix:
finalizeItem()cancels withMEDIA_FINALIZE_ERRORand the store only callsmediaDeletewhen every sub-size fails, so the attachment stayed behind. The pipeline now tracks the attachment ID once the original has uploaded, and deletes it when a top-level upload fails after that point. A new e2e test fails/finalizewith a 500 and asserts the library is empty afterwards.The same gap exists for the block editor, so the better long-term home is
cancelItem()in@wordpress/upload-media.
I'll open a Gutenberg issue for the package-side fix.
| // async-upload.php; forward them so plugins reading $_POST see the | ||
| // same fields, with `post_id` spelled `post` for the REST API. | ||
| const params = ( up.settings && up.settings.multipart_params ) || {}; | ||
| const additionalData = pipeline.additionalDataFromParams( |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5
The frame passes the raw, unvalidated multipart post_id to REST as post. REST rejects a post the user can't edit, where async-upload.php would silently upload the file unattached.
Failure scenario: A plugin opens wp.media with uploader params post_id=N for a post the user cannot edit, or one that was deleted. The classic upload succeeded unattached. The pipeline's REST create returns rest_cannot_edit or rest_invalid_param, so the upload fails outright. media-new.php validates the ID through #post_id; the frame does not.
There was a problem hiding this comment.
I don't think this one holds up - the classic frame path rejects the same posts.
Claude tried a fix first, then tested the classic path:
Media frames post to
async-upload.phpwithaction=upload-attachment, which goes towp_ajax_upload_attachment()rather than themedia_handle_upload()branch that unattaches. That handler checkscurrent_user_can( 'edit_post', $post_id )and fails with "Sorry, you are not allowed to attach files to this post." For a deleted post,edit_postmaps todo_not_allow, so that fails too.A test that set
post_idto a nonexistent post on the grid uploader got exactly that error on the classic path, so the REST rejection matches what happens today. The check and the test were dropped. Only media-new.php unattaches silently, and that screen already reads the validated#post_id.
| * @param {plupload.File[]} files Files added to the queue. | ||
| * @return {boolean|undefined} False to suppress the built-in handler. | ||
| */ | ||
| function handleFilesAdded( up, files ) { |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5
handleFilesAdded() repeats the frame's batch flow nearly line for line, and the same goes for its onError hand-off branch.
Both screens run the same sequence: isReady, partitionBatch, read the multipart params, additionalDataFromParams, getNative, removeFile, queueFile, handOffToClassic, handOffBatchToClassic, refresh, return false (media-frame-upload.js lines 175–262). A fix to one copy, such as the counter or entry-leak bugs, has to be made twice. A pipeline helper like routeBatch( up, files, postId, { onQueued, onSuccess, onError, onProgress } ) could hold the shared part.
There was a problem hiding this comment.
Agreed, done in 11e485c.
Claude did the refactor, here is the shape of it:
The shared flow is now
wp.mediaUploadPipeline.routeBatch( up, files, { postId, onStart, onQueue } ). It coversisReady,partitionBatch, the multipart params,getNative/removeFile/queueFile, the HEIC hand-off and the classic share.onQueuebuilds the screen's UI for a file and returns itsonSuccess/onError/onHandOff/onProgresscallbacks. The fixes above landed once, in the shared copy.wp.mediaUploadPipelinenow exposes onlyisSupported,configure,isReady,routeBatchandgetErrorText.
| wp_enqueue_script( $handle ); | ||
|
|
||
| // Both screen scripts share the pipeline script, so add its settings once. | ||
| if ( str_contains( wp_scripts()->get_inline_script_data( 'media-upload-pipeline', 'before' ), '_wpMediaUploadPipelineSettings' ) ) { |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5
The settings are added only once by searching the printed inline script text for a substring, which is fragile.
Every call scans the inline before data for _wpMediaUploadPipelineSettings. Renaming the global, or a plugin adding an inline script that mentions it, silently drops the settings or adds them twice. A static flag in the function would do the same job reliably.
There was a problem hiding this comment.
Changed in cd98d82, with one tweak on the static flag.
Claude explains why it isn't a static:
A static in the function outlives the
WP_Scriptsinstance, so it leaks across PHPUnit tests that reset$wp_scripts(that's why the inline-text check was used before). The guard is nowwp_scripts()->add_data( 'media-upload-pipeline', 'settings_added', true )andget_data(). It lives on the registered script, so it resets along with the inline data and no longer depends on the global's name.
| } | ||
|
|
||
| if ( ! $screen->is_block_editor() && 'site-editor' !== $screen->id && ! ( 'widgets' === $screen->id && wp_use_widgets_block_editor() ) ) { | ||
| $is_media_screen = 'media' === $screen->id || ( 'upload' === $screen->id && 'grid' === wp_get_media_library_mode() ); |
There was a problem hiding this comment.
🤖 Comment by Claude Opus 5.5
Each new screen adds another special case to wp_set_up_cross_origin_isolation(), plus a matching exemption from the page-builder action guard, instead of letting screens opt in through a filter or list.
$is_media_screen now has to be threaded through both the screen check and the action guard. The next screen that wants isolation (the classic editor, a plugin screen) means editing core conditionals again. A filterable list of screen IDs with a per-screen predicate would avoid that and let plugins opt in. This is related to the missing opt-out raised in the previous review.
There was a problem hiding this comment.
I agree this would be nicer, but I'd like to keep it out of this PR.
Claude's reasoning for holding off:
A filterable list of screens, each with its own predicate, would be a new public API. It should be designed together with the opt-out question from the last round, and it would touch the existing block editor, site editor and widgets cases, not just the media screens added here. Keeping it separate leaves this PR to route uploads only, and gives the API its own ticket where the shape (screen IDs, per-screen callbacks, an opt-out) can be decided on its own.
Want me to open that ticket?
…t end. wp_enqueue_media() can run on the front end of a cross-origin isolated response, where get_current_screen() is not loaded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDLDCCiR2MuQ6MeizyiAgi
Searching the inline script text for the global's name breaks if it is renamed or another script mentions it. The flag lives on the registered script, so it resets with WP_Scripts the way the inline data does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDLDCCiR2MuQ6MeizyiAgi
Both screens ran the same batch flow, so it now lives in wp.mediaUploadPipeline.routeBatch() with per-screen hooks, and the fixes below land once: - Delete the attachment when an upload fails after the original reached the server (a failed finalize, say), so a retry does not leave a duplicate beside unfinished metadata. - Fail an upload whose item left the store without calling either callback, so it cannot hold the unload guard forever. - On Add New Media File, count a batch before queueing it and fire uploadComplete() only once plupload is done too, holding back plupload's own UploadComplete while pipeline uploads run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDLDCCiR2MuQ6MeizyiAgi
Claude drafted this description, folding in the media modal work from #13875:
Trac ticket: https://core.trac.wordpress.org/ticket/65661
This now also includes the media modal work that was in #13875 (Trac https://core.trac.wordpress.org/ticket/66220), so one PR covers every place core uploads media through
wp.Uploader.Description
Client-side media processing currently only works for files dropped straight on a block in the editor - uploads from the Media Library grid, the Add New Media File screen and the media modal (the
wp.mediaframe behind an Image block's "Media Library" button, the featured image panel and so on) still send the original file toasync-upload.phpand generate every sub-size on the server. That is why a HEIC converts and uploads when dropped on an Image block but fails in the modal on a server with no HEIC support. This PR routes uploads on all three through the same client-side pipeline the editor uses, so the browser resizes the image and generates the thumbnails.Why this matters: server-side image processing is a common source of timeouts and memory errors on large images, and results vary depending on what image libraries the host has installed. Processing in the browser avoids both problems, and users get the same upload experience everywhere media is uploaded, not just in the editor.
How it works:
Both screens send the
Document-Isolation-Policyheader so the page is cross-origin isolated, which the wasm-vips image library requires. This reuseswp_set_up_cross_origin_isolation(), extended to cover the grid and the Add New Media File screen. A sharedmedia-upload-pipelinescript configures the@wordpress/upload-mediastore, queues files, tracks progress, and builds error text; a thin script on each screen intercepts files as they are added to the existing uploader and hands them to that pipeline instead: the original is uploaded via the REST API, the browser generates the sub-sizes and sideloads them, then the attachment is finalized. The existing UI - progress bars, grid tiles, error notices - is reused, so the screens look and behave unchanged. While an upload is in flight the page warns before you navigate away, since an interrupted client-side upload would lose thumbnails that were not sideloaded yet.If the browser does not support cross-origin isolation (currently Chromium 137+ on a secure origin, exposed as
wp_is_document_isolation_policy_supported()) or client-side processing is disabled, the scripts do nothing and uploads fall back to the classic flow unchanged. Audio files also stay on the classic path, sincemedia_handle_upload()derives their title and description from ID3 tags and the REST endpoint does not. Any extra multipart params a plugin adds throughplupload_default_paramsorwp.Uploader.param()are forwarded to the REST request so they still arrive in$_POST. A HEIC file the browser cannot decode (Linux, and Windows without the HEVC extension) is handed back to the classic uploader when the server's image editor supports HEIC, so the server converts it instead of the upload failing; when neither can convert it, the pipeline's error is shown.Error messages come straight from the pipeline, so the platform-specific HEIC wording from WordPress/gutenberg#81130 shows up in the grid's error sidebar and in the Add New Media File error rows the same way it does in the editor. Progress is estimated from the pipeline's remaining operations and sideloaded sub-sizes, since the store does not report a percentage of its own.
The media modal
The block editor screens are already cross-origin isolated, so the modal mostly needed wiring:
media-frame-uploadand is enqueued fromwp_enqueue_media(), so it loads on every screen that opens a media frame, the grid included. It only loads on requests that are actually isolated, checked by a newwp_is_cross_origin_isolated_request(), which also honors aDocument-Isolation-Policyheader a plugin sent itself (the Gutenberg plugin does). The Customizer, the Site Icon picker and the classic editor open the frame without isolation and keep classic uploads, without downloading thewp-upload-mediachain.async-upload.phpwith theupload-attachmentaction) are intercepted, so a plugin's ownwp.Uploaderpointed somewhere else is left alone.This replaces the plugin-side workaround in WordPress/gutenberg#82473 and resolves WordPress/gutenberg#82409. Still not covered, worth their own tickets: isolating the Customizer, the Site Icon picker and the classic editor's modal.
Testing Instructions
Test in Chrome 137 or newer on a secure origin (https or localhost).
Check that client-side processing is enabled:
crossOriginIsolatedin the console - it should returntrue. You can also confirm the page response includes theDocument-Isolation-Policy: isolate-and-credentiallessheader in the Network tab. Client-side processing is on by default in a secure context; thewp_client_side_media_processing_enabledfilter can turn it off.Verify uploads go through the client-side pipeline:
wp/v2/mediacreating the attachment, followed by sideload and finalize requests - and no file POST toasync-upload.php. That is how you can tell the client-side pipeline handled the upload.Error handling:
.xyz): the grid's error sidebar shows the file name and a readable reason, and the placeholder tile is removed rather than left spinning. On Add New Media File the error renders the same notice a server-side failure does, with a Dismiss button that announces the error and returns focus to the browse button.wp/v2/mediarequest in DevTools (Network > right-click > Block request URL) and upload again: the server's message appears in the sidebar under the file name.Media modal:
async-upload.php. The tile's progress bar advances, and "Select" inserts the finished image into the block.wp/v2/mediarequest and upload from the modal: the server's message shows in the modal's error list.async-upload.phpandmedia-frame-upload.jsis not loaded.Fallback:
?browser-uploaderon media-new.php) - files should upload throughasync-upload.phpas before. In list mode (Media > Library, list view) theDocument-Isolation-Policyheader is not sent and the grid script is not loaded.async-upload.php, and the attachment title comes from the file's tags as before.?post_id=set to a post that does not exist and upload an image: it uploads unattached, with no error, matching the classic behavior.Formats:
Automated coverage:
npm run test:e2e -- media-modal-client-side-upload media-library-client-side-upload media-new-client-side-uploadruns 33 tests on Chromium (which supports DIP as of the bundled Chrome 149), covering the headers, the pipeline requests, the classic fallback when the page is not isolated, multi-file uploads with duplicates, mid-upload progress, the unload guard, audio staying classic, forwarded upload params, an invalidpost_id, the error message, file name, and accessible Dismiss button for rejected and failed uploads, the readiness gate before the store's settings land, the HEIC hand-off to a server that can convert it, aPostInithandler another script placed inwpUploaderInit, and in the media modal: the script loading in the block editor, an upload going create, sideload, finalize and landing in the block, a server error reaching the modal's error list, an uploader with another action left to plupload, and the classic fallback without isolation.npm run test:php -- --filter 'wpEnqueueMediaFrameUpload|wpEnqueueMediaNewUpload|CrossOriginIsolation|wpIsDocumentIsolationPolicySupported'runs 71 PHPUnit tests. All green locally.Screencasts
1-grid-client-side-upload.mp4
2-add-new-media-file-and-multi-upload.mp4
3-error-handling.mp4
4-media-modal-and-site-icon.mp4
5-fallbacks-audio-invalid-post.mp4
6-all-formats.mp4
AI Use
Code and description both came out of a long back and forth with 🤖 Claude Code. I will review and test.