Client-side media processing: only use media upload provider when not in preview mode#76124
Open
andrewserong wants to merge 2 commits intotrunkfrom
Conversation
andrewserong
requested review from
aaronrobertshaw,
adamsilverstein,
ramonjd,
swissspidy,
talldan and
tellthemachines
andrewserong
added
[Type] Bug
This was referenced Mar 4, 2026
andrewserong
added
the
Backport to WP 7.0 Beta/RC
Member
|
The fix looks fine for now @andrewserong - I guess a separate function makes sense as a broader solution unless you have other ideas to try? |
|
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. |
|
Size Change: +11 B (0%) Total Size: 6.87 MB
ℹ️ View Unchanged
|
github-project-automation
bot
moved this to 🔎 Needs Review
in WordPress 7.0 Editor Tasks
Contributor
Author
Yeah, that's the only thing I could think of — something like I gotta run for the day, but I've kicked off a failing e2e test in case it's just a transient error. If it's real, though, I can dig in further tomorrow. Thanks for the speedy review! |
Member
|
Thanks for the quick fix! On trunk, I couldn't do this on a page or in the site editor: Kapture.2026-03-04.at.17.42.47.mp4On this branch 👍🏻 |
ramonjd
reviewed
Mar 4, 2026
| // | ||
| // Only the top-level (non-preview) provider should do this. | ||
| // Nested preview providers (e.g. from useBlockPreview in | ||
| // core/post-template) inherit settings that already contain |
Member
There was a problem hiding this comment.
Ah, so would not being able to upload images on a "page" post type be due to the template it's using (in TT5)?
ramonjd
approved these changes
Mar 4, 2026
Member
ramonjd
left a comment
ramonjd
left a comment
There was a problem hiding this comment.
Excellent sleuthing! Tested across the editors and this works for me in all contexts.
The e2e fail doesn't look related to this change, but it also fails for me locally.
test/e2e/specs/site-editor/user-global-styles-revisions.spec.js:136:2 › Style Revisions › should access from the site editor sidebar
I've kicked them off again. If they keep failing we can look into it.
Member
Because I had revisions in my test db and forgot to |
talldan
reviewed
Mar 4, 2026
| if ( isClientSideMediaEnabled && _settings?.mediaUpload ) { | ||
| if ( | ||
| isClientSideMediaEnabled && | ||
| _settings?.mediaUpload && |
Contributor
There was a problem hiding this comment.
Part of the existing condition here is settings.mediaUpload needs to be defined for client side media processing to work. I found that curious, is this because client side media processing eventually calls through to the vanilla server version of mediaUpload?
I think an alternative fix might be to make mediaUpload undefined for BlockPreview. Users shouldn't be uploading media in previews anyway. That would avoid the need for extra isPreview checks (though would still need to do something to avoid rendering the provider, it could work the same).
Contributor
There was a problem hiding this comment.
Here's where it could be implemented -
gutenberg/packages/block-editor/src/components/block-preview/index.js
Lines 59 to 66 in d37cda8
Contributor
Author
There was a problem hiding this comment.
I think an alternative fix might be to make mediaUpload undefined for BlockPreview.
Just a drive-by comment before I close my laptop for the day: setting it to undefined for BlockPreview results in the function not being available within upload-media and throws an error (as in this context, it'll wind up updating the shared settings used across the media upload provider). (I tried that approach first, but didn't get very far — I do like the idea of trying to simplify things, though 🤔 )
I think a separate fix would be to untangle the re-use of the same key (i.e. call the "real" upload function something different within upload-media), but that's a bit beyond the scope of the quick fix.
It's a little convoluted this!
Contributor
There was a problem hiding this comment.
Ideally none of the upload-media code executes for block previews at all, so then there would never be an opportunity to throw an error. 😄
Contributor
Author
Hrm, weirdly it does seem to be failing consistently, though. I'll do some more digging tomorrow! |
5 tasks
Member
|
@andrewserong - #76137 is an alternate take based on your suggestion of using separate keys. I'm not certain it fixes your issue. |
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
4 participants
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.
What?
Part of:
Fix a bug where images uploaded to an image block in the site editor would get stuck in an endless loading state without uploading.
Why?
In the site editor when we're editing a template we typically have a Query Loop / Post Template block somewhere in the template rendering a block preview. When a block preview is present, a nested
ExperimentalBlockEditorProvideris rendered, and this causes a problem — because the nested provider's access to its block editor settings uses the overriddenmediaUploadfunction that simply queues items for upload, instead of actually uploading to the server.Because the
MediaUploadProviderdoes not use a sub-registry, when we update the media upload settings, the upload-media store gets stuck in a state where subsequent calls tomediaUploadsimply queue more images for upload without an upload ever occurring. The result is: endlessly stuck in a loading / pending state.It's hard to describe the issue: but effectively the upload-media store gets stuck in a state where it no longer has access to the "real"
mediaUploadthat uploads to the server.This idea in this PR is to do something of a quick fix — skip outputting the
MediaUploadProviderand overridingmediaUploadif we're in a preview state. As a bandaid and to fix the issue in 7.0, I think this is a reasonable fix for now.More broadly, though, I'm wondering if there's a better API for the internals of
upload-mediato avoid the problem that the server-sidemediaUpload(that is, the function that actually submits the media to the server) has the same key as themediaUploadthat adds items to the queue.I.e. possibly we could have a separate function... but that feels like a bit more of a rabbit hole than fixing the task at hand. So I've tried to keep things simple, while adding lots of comments to explain the issue.
If there's a better approach, though, happy to close this out!
How?
mediaUploadand outputtingMediaUploadProviderwhen the block editor settings are in a preview mode state (i.e. when used by BlockPreview)Testing Instructions
On trunk:
With this PR:
Note: there is a separate issue with what happens when an upload completes (it appears like it's an external image). There's a separate fix for this over in: #76121
Screenshots or screencast
Before
You'd just get stuck in this state endlessly:
After
Upload completes successfully:
2026-03-04.17.03.21.mp4