Fix: post saving should be locked during media uploads#76973
Open
adamsilverstein wants to merge 7 commits intotrunkfrom
Open
Fix: post saving should be locked during media uploads#76973adamsilverstein wants to merge 7 commits intotrunkfrom
adamsilverstein wants to merge 7 commits intotrunkfrom
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: -15 B (0%) Total Size: 7.73 MB
ℹ️ 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
adamsilverstein
force-pushed
the
fix-saving-should-be-locked-during-uploads
branch
from
90e62c0 to
8ac1629
Compare
adamsilverstein
added
[Type] Bug
- 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
adamsilverstein
changed the title
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 8f170a2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23884947940
|
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