Opened 5 weeks ago
Closed 4 weeks ago
#65662 closed defect (bug) (worksforme)
Media: Client-side uploads for the media-new.php uploader and a guard for in-flight uploads
| Reported by: | adamsilverstein | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | Media | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: |
Description
#65661 extends WordPress 7.1's client-side media processing pipeline (wasm-vips) to the Media Library grid at upload.php. Two follow-ups were explicitly left out of scope there:
- The "Add New Media File" screen (
media-new.php). This screen uses a separate uploader path:plupload-handlerscreates a rawplupload.Uploader(notwp.Uploader), with its own progress list UI (#media-items) and its own error surfaces. Uploads go toasync-upload.phpwith all image processing server-side. It is the last remaining admin upload surface without pipeline integration.
- No warning when leaving during an in-flight upload. With the client-side pipeline, closing the tab mid-upload is worse than the classic flow: thumbnails that have not been sideloaded yet are lost and the attachment is left unfinalized (no
finalizerequest), whereas classic per-file uploads complete server-side once the bytes arrive. Abeforeunloadguard should warn while pipeline uploads are in flight.
Proposed change
media-new.php pipeline integration:
- Extend cross-origin isolation to
media-new.phpby hooking the existing Document-Isolation-Policy output buffer (wp_start_cross_origin_isolation_output_buffer()) onload-media-new.php, gated to users withupload_files(the screen already dies without that capability). No mode gating applies here. - Add a
media-new-uploadadmin script that binds a higher-priorityFilesAddedhandler on theplupload-handlersuploader instance and routes files through the@wordpress/upload-mediastore (REST upload of the original, client-side thumbnails, sideload, finalize). The existing screen UI is reused rather than replicated: the script calls the globalfileQueued(),uploadSuccess(),itemAjaxError(), anduploadComplete()helpers fromplupload-handlers, so progress items, the rendered attachment row (via the existingasync-upload.php?fetch=3markup endpoint), and the error styling all work unchanged. - Settings are shared with the grid integration via
wp_get_media_library_upload_settings()(introduced in #65661). - Graceful degradation, as on the grid: 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-uploaderHTML fallback form) keep working unchanged.
beforeunload guard:
- Both integration scripts (grid, from #65661, and the new
media-new.phpone) register abeforeunloadhandler that triggers the browser's leave-confirmation while pipeline uploads are in flight, and unregisters when the queue drains. Scope is intentionally limited to the client-side pipeline; the classic plupload flow is unchanged.
Testing instructions
- Use Chrome 137+ on a secure origin (HTTPS or localhost).
- Go to Media > Add New Media File (
media-new.php) and upload a JPEG via drag-and-drop or "Select Files". - In DevTools > Network, observe a POST to
/wp/v2/media, one or more POSTs to/wp/v2/media/<id>/sideload, one POST to/wp/v2/media/<id>/finalize, and no upload POST toasync-upload.php(a GET withfetch=3for the item markup is expected). - Confirm the progress bar item resolves to the normal attachment row with the Edit and Copy URL links.
- While a large upload is in flight (either on the grid or on
media-new.php), try closing the tab: the browser should ask for confirmation. After the upload completes, closing should be silent. - In Firefox/Safari, or with
?browser-uploaderonmedia-new.php, confirm uploads still work via the classic path.
Depends on / builds on #65661 (the PR is stacked on its branch).
Change History (3)
This ticket was mentioned in PR #12586 on WordPress/wordpress-develop by @adamsilverstein.
5 weeks ago
#1
- Keywords has-patch has-unit-tests added
@adamsilverstein commented on PR #12586:
4 weeks ago
#2
Consolidated into #12585. This branch was stacked on that one and the two changes share their PHP infrastructure and settings plumbing (and this PR's diff already showed the full combined change since its base was trunk), so a single PR is simpler to review and maintain. All five commits from this branch are now part of #12585 unchanged; Trac #65662 is being closed as a duplicate of #65661.
#3
@
4 weeks ago
- Milestone Awaiting Review
- Resolution → worksforme
- Status new → closed
I wound up folding this into https://core.trac.wordpress.org/ticket/65661
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
## What
Two follow-ups that #12585 explicitly left out of scope:
media-new.php). It uses a separate uploader path:plupload-handlerscreates a rawplupload.Uploader(wp.Uploadernever loads there) posting toasync-upload.phpwith all image processing server-side. It was the last remaining admin upload surface without client-side pipeline integration.beforeunloadguard while pipeline uploads are in flight. Interrupting a pipeline upload is worse than interrupting a classic 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.## How
Commit 1 - beforeunload guard for the grid.
media-library-upload.jstriggers the browser's leave confirmation while its progress map is non-empty (models stay there from interception until success or error). Scoped to the pipeline; classic uploads behave exactly as before.Commit 2 - media-new.php pipeline routing. A new
wp_set_up_media_new_cross_origin_isolation()onload-media-new.phpextends core's Document-Isolation-Policy output buffer to the screen, gated on client-side processing being enabled andupload_files(the screen itself already requires it; no mode gating applies). A newmedia-new-uploadadmin script binds a higher-priorityFilesAddedhandler on theplupload-handlersuploader instance and routes files through the@wordpress/upload-mediastore, sharing settings with the grid integration viawp_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 existingasync-upload.php?fetch=3markup endpoint,itemAjaxError()surfaces per-file errors, anduploadComplete()runs when the queue drains - so the screen looks and behaves unchanged. Store progress is mirrored onto the screen's progress bars, and the same beforeunload guard applies. 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-uploaderHTML fallback form) keep working unchanged - degradation, never data loss.The
mediaSideload/mediaFinalizeapiFetch wrappers are intentionally duplicated from the grid script rather than extracted into a shared handle: they are thin, and the two screens share no other loadable script (the grid script requireswp.Uploader/media-views, whichmedia-new.phpnever loads).Commit 3 - E2E coverage. New spec asserting the DIP header on
media-new.php, the happy-path upload (create + sideload + finalize via REST, no file upload throughasync-upload.php; thefetch=3markup POST is expected and excluded), and the disallowed-file-type error path. As in #12585, Playwright's Chromium lacks Document-Isolation-Policy support, so upload assertions skip when the context is not isolated; the header assertion always runs.One deliberate scope choice: the built-in
FilesAddedhandler blocks WebP/AVIF (and warns for HEIC) when the server cannot edit them; the pipeline handler bypasses those checks because conversion happens client-side - the same behavior the grid integration has.## Testing
wpMediaNewCrossOriginIsolation.php,wpEnqueueMediaNewUpload.php) pass locally, as do the two suites from #12585.async-upload.php; the progress item resolves to the normal row with Edit and Copy URL links; closing the tab mid-upload prompts for confirmation; Firefox/Safari and?browser-uploaderkeep the classic path.