Media: Route media modal uploads through the client-side pipeline - #13875
Open
adamsilverstein wants to merge 42 commits into
Open
adamsilverstein wants to merge 42 commits into
adamsilverstein wants to merge 42 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 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.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
wp_enqueue_media_library_upload() returns early unless the request comes from Chromium 137+, the only engine that honors Document-Isolation-Policy. The PHPUnit bootstrap defines no HTTP_USER_AGENT at all, so every test that expected the script to be enqueued was asserting against a bail-out and failing on every CI matrix leg. Set a Chromium 137 User-Agent in set_up() (restoring the original in tear_down(), as the cross-origin isolation tests already do) and cover the gate itself so a Firefox or pre-137 request is proven not to enqueue.
wp_get_media_library_mode() collapsed any saved media_library_mode outside 'grid'/'list' to 'grid', while upload.php treats every truthy saved value as the mode and renders grid only for the exact string 'grid'. A value a plugin stored therefore rendered list mode on screen while the helper reported grid, and wp_set_up_media_library_cross_origin_isolation() sent Document-Isolation-Policy for a page with no client-side pipeline - isolating a screen that has nothing to gain from it and can break same-origin iframes there. Return the saved value verbatim so the two agree; only a missing or empty option falls back to 'grid'. The query-string branch is unchanged and still accepts the two known modes only.
wp_enqueue_media_library_upload() skips the enqueue when the request cannot be cross-origin isolated, but its media-new.php sibling had no such check. Firefox and Safari therefore downloaded media-new-upload.js and the whole wp-upload-media / wp-media-utils / wp-element chain on a screen where the script can only detect the missing isolation and no-op. Apply the same Chromium 137 gate that wp_start_cross_origin_isolation_output_buffer() already uses to decide whether to send Document-Isolation-Policy at all, and cover it, along with the missing User-Agent that made the existing enqueue assertions test a bail-out rather than the enqueue.
wp.uploadMedia.getErrorMessage() takes an error *code* and a file name and
returns a { title, description, action } object. Both integrations passed it
the Error instance with no file name and used the result as a display string,
so every failed client-side upload rendered as "[object Object]" - in the
grid's error sidebar and, on media-new.php, inside the item's error div. The
`|| error.message || __( … )` fallbacks never ran, because the returned object
is always truthy.
Look the message up by error.code, and only for codes the helper actually
maps: its fallback and its GENERAL entry say no more than the error's own
message, so preferring the message there keeps a server-supplied reason
rather than replacing it with "Please try again."
itemAjaxError() writes its argument straight into the item's HTML and the
message now carries the file name, so escape it before handing it over.
…tions Five gaps between the client-side pipeline and the flows it replaces, all in the two upload integrations: - Uploads from media-new.php?post_id=N landed unattached. The classic flow posts `post_id` to async-upload.php; addItems() sent no equivalent, so the attachment got post_parent 0 even though the screen tagged its item `child-of-N` and counted it against the post. Send the REST spelling, `post`, taken from the uploader's own multipart_params so a plugin filtering `upload_post_params` is still honored. The grid does the same for the post its uploader was opened for. - Neither integration passed `mediaDelete`, so when every sub-size sideload failed the queue could not clean up: the tile vanished with an error while the original file stayed in the Media Library as a metadata-less attachment. The block editor passes it for exactly this case. - `window.__clientSideMediaProcessing` was never set, so @wordpress/media-utils took its non-pipeline branch and created, emitted, and revoked a throwaway blob URL per file. - plupload's getNative() returns null for sources it cannot expose as a File. fileKey( null ) then threw mid-forEach, before `return false`, leaving earlier tiles stuck at "uploading" and the rest of the batch neither uploaded nor handed back. Check the batch up front and defer all of it to classic plupload instead. - The grid never called wp.Uploader's `success` and `error` callbacks, which wp-plupload.js exposes so other code can react to a finished or failed upload.
…ient-side-uploads
The upload-media store never dispatches a numeric progress value, so the grid tiles and the Add New Media File progress bars stayed at zero until an upload finished. Derive progress from the queue item's remaining operations and the sub-sizes sideloaded so far, still preferring `item.progress` whenever the store provides one. Claude-Session: https://claude.ai/code/session_018sL2Far7Hyjc1mJp2RKAcd
Cover the classic fallback when the page is not isolated, multi-file uploads including duplicates, mid-pipeline progress, and the error message and file name surfaced for a failed upload, on both the grid and the Add New Media File screen. Fix the disallowed-file-type assertion, which looked for the file name in the message span when the sidebar renders it in its own span. Claude-Session: https://claude.ai/code/session_018sL2Far7Hyjc1mJp2RKAcd
…reens. Move everything the grid and Add New Media File scripts had in common into a new media-upload-pipeline script: feature detection, configuring the upload-media store, the REST sideload/finalize/delete helpers, queueing a file with progress tracking, the error text, and the unload guard. The two screen scripts become thin adapters for their own UI. While there, fix what the review turned up in that shared code: - Read the post to attach to from the screen's validated post_id rather than plupload's raw multipart params, which the REST API rejects for a deleted or unpermitted post where async-upload.php silently uploads unattached. - Forward the remaining multipart params to the REST request so plugins that add fields through plupload_default_params or wp.Uploader.param() still see them in $_POST. - Leave audio files to the classic uploader, which derives the title and description from ID3 tags where the REST endpoint does not. - Track queue items by id after the first match, since HEIC conversion swaps the item's sourceFile. - Subscribe only to the upload-media store and skip sub-size children. - Render failed uploads on media-new.php with the same notice async-upload.php returns: a real Dismiss button described by the notice, a screen reader announcement, and focus returned to the browse button, and restore the e2e assertion that relied on it. Add wp_is_document_isolation_policy_supported() so the Chromium 137 check lives in one place. Claude-Session: https://claude.ai/code/session_018sL2Far7Hyjc1mJp2RKAcd
…isolation(). The two screen-specific wrappers duplicated the enabled, capability, and output-buffer tail of the block editor's isolation callback. Extend its screen check to the Media Library grid and the Add New Media File screen instead and hook it on their load actions, keeping the page builder guard scoped to the editor screens. Claude-Session: https://claude.ai/code/session_018sL2Far7Hyjc1mJp2RKAcd
…ode(). upload.php falls back to grid mode for any falsey saved option and renders list mode for a truthy value that is not exactly 'grid'. The helper treated a saved '0' as a non-grid mode and collapsed a non-string value to grid, so the isolation decision could disagree with the mode the page then rendered. Claude-Session: https://claude.ai/code/session_01KbT1XjMNEosC6ueLGxbspA
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Add the three new admin upload scripts to the TypeScript project so their types are checked from the start rather than fixed up later, with typings for the plupload globals they rely on. Finish the const/let conversion, and replace the under-specific `Object` and `Array` JSDoc types with real shapes: the FilesAdded arrays hold plupload file objects, not strings. Also extend the placeholder tile's early mime scan to the formats the client-side pipeline accepts, so a WebP or AVIF drop gets the same `type-image` placeholder a JPEG does.
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Tighten wp_get_media_library_mode() so it only ever returns 'grid' or 'list', matching what upload.php renders: nothing saved renders the grid, the two known values render themselves, and any other saved value renders the list. Document the narrowed return type for PHPStan and drop the trailing whitespace the multi-line conditional picked up.
jshint rejects a ternary broken before its `?`, which failed the JavaScript coding standards check. Hoist the lower-cased subtype so the ternary fits on one line.
The shapes the three client-side upload scripts pass between each other lived as JSDoc typedefs in whichever script happened to declare them, with a comment in the others noting where to look. Declare them once in the typings the scripts already use, extract the queueFile() callbacks into a named type, and describe an attachment model as a Backbone.Model via @types/backbone rather than a hand-written subset. Backbone is no longer `any` for type-checked scripts as a result.
The script only used jQuery for DOM building and for a ready callback that had to run after the one in plupload-handlers.js created the uploader. Build the error notice and progress updates with plain DOM APIs, and bind the FilesAdded interceptor from a PostInit handler in the `wpUploaderInit` settings' `init` map, which plupload binds while the uploader initializes. That reaches the uploader without depending on ready-callback ordering.
…ient-side-uploads
The pipeline's readiness gate checked the store for a `mediaUpload` setting, but the upload-media store's default state already carries a no-op `mediaUpload`, so the gate passed before the provider had delivered the real settings. A file added in that window was removed from plupload and queued into a store that called the no-op: the tile stayed uploading forever and the unload prompt never cleared. Check for the pipeline's own `mediaSideload` instead, which only the provider sets, so an early file is left to classic plupload as the gate always intended. The pipeline converts HEIC in the browser and fails outright where the platform has no HEIC decoder, even on a server whose image editor would have converted the file on the classic path. When the store reports `HEIC_DECODE_ERROR` and plupload carries no `heic_upload_error` setting (the server supports HEIC), hand the file back to plupload so its own FilesAdded handler uploads it and the server converts it. Files handed back are remembered so the interceptors let them through. When neither side can convert the file, the pipeline's message still shows. On media-new.php, hooking `PostInit` into the `wpUploaderInit.init` map replaced any handler another script had already placed there. Chain it the way the function form already did. Five e2e tests cover the three cases. The readiness test stops the provider from rendering to reproduce the not-yet-ready window, and the HEIC tests use a file no browser can decode with `heic_upload_error` adjusted either way.
Rename the Media Library grid integration to media-frame-upload and enqueue it from wp_enqueue_media() on every screen wp_set_up_cross_origin_isolation() isolated, so the modal the block editor, the site editor and the block widgets screen open uploads through the same pipeline as the grid. Screens that were not isolated, such as the Customizer, keep their classic uploads and do not download the pipeline bundles. Let the shared pipeline script adopt the upload-media store the block editor has already configured instead of rendering a second provider into it, and only intercept uploaders that post attachments so a plugin's own wp.Uploader is left alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015q272S5jKgWCuQYrbz2SKb
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trac ticket: https://core.trac.wordpress.org/ticket/66220
Stacked on #12585 - only the last commit is new here, everything before it is that PR. Once #12585 lands this rebases down to the one commit.
Description
#12585 routes uploads on the Media Library grid and the "Add New Media File" screen through the client-side media processing pipeline. The media modal - the
wp.mediaframe the block editor, the site editor and the block widgets screen open from an Image block's "Media Library" button, the featured image panel and so on - still uploads throughwp.Uploader/plupload toasync-upload.php. That is why a HEIC file converts and uploads when dropped on an Image block but fails in the modal on a server with no HEIC support, and why anything uploaded through the modal skips browser-generated sub-sizes, the big image threshold and animated GIF handling. Gutenberg currently works around this in the plugin (WordPress/gutenberg#82473); this PR gives it a home in core.The block editor screens are already cross-origin isolated by
wp_set_up_cross_origin_isolation(), and the grid script from #12585 already binds to everywp.Uploaderinstance, so the delta is mostly wiring:media-library-uploadtomedia-frame-uploadand enqueue it fromwp_enqueue_media(), so it loads on every screen that opens a media frame instead of only onupload.php. The grid keeps working through the same call.wp_set_up_cross_origin_isolation()actually isolated, via a newwp_is_cross_origin_isolated_request(). Screens that are not isolated - the Customizer, the Site Icon picker on Settings > General, the classic editor - load the media frame too, but they would download the wholewp-upload-mediachain for a script that can only no-op there. Their uploads stay on the classic path for now.mediaSideloadandmediaFinalizehave no defaults) instead of "this script's own settings landed", since the block editor's provider can render after ours and replace them with equivalent ones.async-upload.phpwith theupload-attachmentaction - so a plugin's ownwp.Uploaderpointed at another endpoint is left alone. This mirrors a review finding from @andrewserong on the Gutenberg PR.Not covered here, worth their own tickets once this lands: isolating the Customizer (its preview iframe needs same-origin access that Document-Isolation-Policy would block), the Site Icon picker, and the classic editor's media modal.
Testing Instructions
Test in Chrome 137 or newer on a secure origin (https or localhost).
wp/v2/media, then sideload and finalize requests, and no POST toasync-upload.php. The tile's progress bar advances, the finished tile is selectable, and "Select" inserts it into the block as usual.Document-Isolation-Policyheader blocked in DevTools: the upload goes throughasync-upload.phpas before.async-upload.php, andmedia-frame-upload.jsis not loaded on the page.Automated coverage, all green locally:
npm run test:php -- --filter 'wpEnqueueMediaFrameUpload|wpEnqueueMediaNewUpload|CrossOriginIsolation'- 76 tests, including four new ones for the isolated-request gate and thewp_enqueue_media()enqueue.npm run test:e2e -- media-modal-client-side-upload media-library-client-side-upload media-new-client-side-upload- 31 tests. The new modal spec checks the script loads in the block editor, that an upload from the modal goes create, sideload, finalize with nothing hittingasync-upload.phpand the finished image lands in the block, and that stripping the isolation header falls back to the classic uploader.AI Use
Claude Code did the typing here, I did the asking. I will review and test.
🤖 Generated with Claude Code
https://claude.ai/code/session_015q272S5jKgWCuQYrbz2SKb