Fix: post saving should be locked during media uploads#76973
Merged
adamsilverstein merged 12 commits intoApr 21, 2026
Conversation
…ide support When window.__clientSideMediaProcessing is true but the browser lacks required APIs (WebAssembly, SharedArrayBuffer), the block-editor falls back to legacy server-side uploads. However, the legacy media upload function skipped its save-lock logic because the flag was set, and useUploadSaveLock watched an upload store that nothing was using. This left a gap where no save locking occurred. Fix by: 1. Removing the flag guard from useUploadSaveLock so it always watches the upload store (harmless when empty). 2. Using isClientSideMediaSupported() in the legacy upload path to only skip locking when client-side processing is truly active. Fixes #76971
|
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. |
The PostSavedState component (Save draft button) did not check isPostSavingLocked, so it remained enabled during media uploads even though locks were being set. The Publish button and Ctrl+S shortcut already checked this lock, but the Save draft button did not. Fixes #76971
|
Size Change: +29 B (0%) Total Size: 7.75 MB 📦 View Changed
ℹ️ View Unchanged
|
The legacy (non-client-side) media upload lock logic toggled the save lock on each onFileChange callback, which fires multiple times for multi-file uploads. This caused the lock to end up in the wrong state depending on the total number of callbacks. Fix by locking once at the start of the upload and only unlocking when all files in the array have been uploaded (all have IDs). Fixes #76971
Verify that the Save draft button is disabled while a media upload is in progress and re-enabled once the upload completes or fails. Uses route interception to control upload timing. Ref #76971
90e62c0 to
8ac1629
Compare
- Update publishing.spec.js to expect Save draft button to be disabled when post saving is locked (matches new behavior) - Use expect.poll() with timeouts for lock state checks in upload tests to handle async client-side media processing - Remove fragile uploadCount assertion from gallery test Ref #76971
Use a single poll that checks both the lock state and the button disabled state atomically, avoiding race conditions where the upload completes between the two separate assertions. Ref #76971
andrewserong
reviewed
Apr 2, 2026
Contributor
There was a problem hiding this comment.
Cool idea! I think it makes sense to disable here as well 👍
Not sure if this is the cause of the failing e2e tests, but I notice that the Publish button that opens the pre-publish panel isn't disabled during upload, only the save draft button. Shall we add that in, too? Or do we want users to be able to open that panel 🤔
2026-04-02.15.48.20.mp4
The Publish button's disabled state with isPostSavingLocked is already covered by publishing.spec.js. This test was flaky in CI due to the upload completing before the lock could be observed. Ref #76971
|
Flaky tests detected in 9a7bf02. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24697651407
|
Member
Author
I wasn't sure about the desired interaction here either. If you do click the publish button, the 2nd publish button in the prepublish panel will be disabled. I thought maybe that was sufficient. Testing this again however, I realized there is a checkbox users can uncheck to skip the prepublish panel entirely, so I think we do need to disable this publish button as well. We don't want users publishing a post mid-image upload. Will correct. |
Users can skip the pre-publish panel via a preference, which means the Publish toggle in the top bar can directly publish a post. Add isPostSavingLocked to isToggleDisabled so the toggle is disabled while uploads are in progress, matching the behavior of the actual Publish button inside the panel. Also adds an E2E test verifying the Publish button disables during upload.
Open the publish panel before calling lockPostSaving so the top-bar Publish toggle is still clickable. The previous order caused a timeout because the toggle is now disabled when saving is locked.
andrewserong
approved these changes
Apr 7, 2026
andrewserong
left a comment
Contributor
There was a problem hiding this comment.
Nice, thanks for the updates, this is testing well for me:
✅ Uploading images to a post that has never been saved or published
✅ Uploading images to a post that has already been saved or published
✅ The above cases when client-side media processing is disabled
Looking good, along with the e2e coverage. I half-wondered if the test to check the keyboard shortcut could be rolled into one of the other tests as it seemed a little duplicative, but that's very nitpicky.
Looks good to merge to me! 
Comment on lines
+65
to
+73
| // Hold the upload request so we can verify the lock state. | ||
| let resolveUpload; | ||
| const uploadPromise = new Promise( ( resolve ) => { | ||
| resolveUpload = resolve; | ||
| } ); | ||
| await page.route( '**/wp/v2/media', async ( route ) => { | ||
| await uploadPromise; | ||
| await route.continue(); | ||
| } ); |
Contributor
There was a problem hiding this comment.
Oh, clever, I like this 👍
…oads When media uploads create entity records, hasNonPostEntityChanges becomes true and the inner Publish button in the prepublish panel switches its label to "Save". Gate the hasNonPostEntityChanges label check on isPostSavingLocked so the label stays as "Publish" during uploads.
Member
Author
Thanks for reviewing @andrewserong! I was testing this again today and noticed some edge cases I am working to address:
inner.button.turns.to.save.mp4
save.with.partial.possible.mp4reloading the editor fixed the issue. works.as.expected.after.reload.mp4 |
Member
Author
Hmm, this seems like an existing bug - I can reproduce in core trunk without Gutenberg:
the button changes from publish to save. I guess its fine to fix that here along with the other changes. I wonder if there is an existing open issue. |
After publishing a post, uploading media creates non-post entity changes which caused hasNonPostEntityChanges to override the isPostSavingLocked check via the && clause. Move isPostSavingLocked to a top-level OR so it unconditionally disables the button regardless of entity change state.
andrewserong
approved these changes
Apr 17, 2026
andrewserong
left a comment
Contributor
There was a problem hiding this comment.
Good catches, thanks for updating the logic and fixing those edge cases. This is smoke testing well for me locally 👍
Member
Author
|
Resolving conflicts and doing some final testing before merging. |
Resolve conflict in use-upload-save-lock.js: keep the PR's approach of always watching the upload store rather than gating on a window flag. Trunk added `|| window.__heicUploadSupport` to the flag guard as part of the HEIC support commit (1125ef2). The PR's broader fix drops the guard entirely — HEIC uploads go through the same upload store, so the flag check is unnecessary once the hook always watches.
Member
Author
|
This worked well in my final testing, merging! |
This was referenced Apr 21, 2026
adamsilverstein
added a commit
that referenced
this pull request
Apr 28, 2026
The feature shipping target moved from 7.0 to 7.1 (per #76756). Bring the architecture explanation and how-to guide up to date with the post-7.0-cycle state of the code: - Reposition introductions around 7.1 with 7.0 as the groundwork cycle. - HEIC/HEIF: document the canvas-based fallback path (createImageBitmap, HTMLImageElement+OffscreenCanvas, HEIC container parsing + WebCodecs VideoDecoder), JPEG companion file, and isHeicCanvasSupported() gate. - AVIF: note the wp_prevent_unsupported_mime_type_uploads bypass when generate_sub_sizes=false (#76371). - Batch thumbnail generation via image.copyMemory() / thumbnailImage() (#76979). - Sub-size deduplication via image_size: string|string[] on the sideload route (#77036). - Single VIPS instance via promise caching (#76780). - Build-output trimming: remove vips-jxl.wasm (#76639), skip non-min worker (#76615), skip WASM-inlined sourcemaps (#75993). - COI: <img> excluded from cross-origin attribute injection (#76618). - Loosened feature-detection thresholds: 2 cores, 3g (#76616). - Post-saving lock fix: Save Draft now respects the lock (#76973). - convert_format declared as boolean on sideload route (#77565). - Fix incorrect REST index settings list (only image_sizes and image_size_threshold are exposed). - Remove references to client_side_supported_mime_types filter (PR #76549 was closed unmerged); state the supported MIME set is fixed at CLIENT_SIDE_SUPPORTED_MIME_TYPES. Refs #75111.
adamsilverstein
added a commit
that referenced
this pull request
May 7, 2026
…client-side media docs The filter actually fires twice during a client-side upload — once with context 'create' during the initial upload (WP core's wp_generate_attachment_metadata() runs even when intermediate_image_sizes_advanced is filtered to empty) and again with 'update' at finalize. Earlier wording suggested the filter was bypassed initially and only ran at finalize, which is wrong. The double-fire pattern matches how WordPress already handles big-image uploads on the server, where sub-size generation is deferred and triggers a second 'update' pass. Frame the contract that way across all three docs and tell plugin authors to be idempotent — there's nothing new for them to handle that they haven't already accommodated for big-image uploads. Other corrections in the same pass: - Soften the "security model is unchanged" FAQ in the make/core post: AVIF uploads from the client-side path do bypass wp_prevent_unsupported_mime_type_uploads when generate_sub_sizes is false, which is a deliberate exception worth surfacing. - Fix HEVC fallback wording in the how-to guide (Chrome 107+ on macOS → Chromium 107+ via the platform codec, covering Windows HEVC Video Extension as well). - Note Safari's HEIC canvas fallback in browser-support tables across all three docs (architecture and how-to guide both said flat "Not supported"). - Add packages/vips/src/worker.ts to the source-files list — it's the actual entry point loader.ts dynamically imports. - Align image quality figure to "0–1, default 0.82" everywhere (architecture doc had "82%"). - Drop residual historical PR link in the how-to guide troubleshooting table (#76973 Save Draft fix). - Drop the "graduated from a Gutenberg experiment" version-history framing from the architecture doc and make/core post. - Drop the historical 7.0 iteration link from the make/core post related-tracking-issues list.
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
2 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.
Summary
isPostSavingLocked— it was the only save mechanism that didn't check the lock, so it remained enabled during media uploads even though locks were being set correctlyonFileChangecallback, which broke for multi-file uploads (the lock ended up in the wrong state). Now locks once at upload start and unlocks only when all files are done.window.__clientSideMediaProcessingflag guard fromuseUploadSaveLockso it always watches the upload store (harmless when queue is empty)isClientSideMediaSupported()in the legacy upload path to correctly determine when to skip its own locking, closing a gap when the flag is enabled but browser lacks required APIsRoot Cause
Three issues combined:
The "Save draft" button (
PostSavedState) never checkedisPostSavingLocked. The Publish button and Ctrl+S shortcut did, but Save draft did not.A gap in the lock mechanism: After Implement WebAssembly support detection and fallbacks #74827 added browser capability detection to
shouldEnableClientSideMediaProcessing(), when the flag is on but browser support is missing, neither locking path was active.Legacy lock toggling bug: The
onFileChangecallback toggled the lock each time it fired. For multi-file uploads, the final lock state depended on whether the total number of callbacks was even or odd.Test plan
npm run test:e2e -- test/e2e/specs/editor/various/upload-save-lock.spec.jsFixes #76971