Media: Stop claiming "Upload complete" when every upload failed

[adamsilverstein]

What?

Closes #81132
Closes #81708

Stops the upload progress snackbar from saying "Upload complete" for a batch in which nothing was uploaded.

Why?

UploadProgressSnackbar decided an upload had finished by watching the queue drain: when the count of in-flight items went from above zero to zero, it declared completion. Cancelled items are removed from the queue exactly like successful ones, so a batch where everything failed looked identical to a batch where everything succeeded - and you wound up with a completion checkmark sitting right next to the error explaining that the upload failed.

How?

The queue itself can't answer the question after the fact, so both upload paths now keep a tally of what failed:

• @wordpress/upload-media counts cancelled top-level items in its store state, exposed as a new private getFailureCount selector. Sub-size cancellations are excluded - a sub-size that fails on its own does not stop the attachment from uploading, and one bad enough to sink the attachment cancels the parent, which is what gets counted.
• The editor's non-CSM tracker gets advanceFailed, called by the mediaUpload wrapper's onError handler where it previously called plain advance.

Both tallies only ever grow, so the snackbar records their sum when a batch starts and subtracts that when the queue drains. That gives it the number of files that failed in this batch:

• Nothing uploaded: no completion snackbar. The per-file errors already say what happened, and the progress notice is removed rather than replaced (otherwise "Uploading — photo.heic" would sit there forever).
• Some uploaded: "Uploaded 3 of 5", with no checkmark - a checkmark would overstate it.
• Everything uploaded: "Upload complete" with the checkmark, unchanged.

The speak() announcements follow the same three cases.

Testing Instructions

The easiest failure to trigger is a HEIC file in a browser without the platform HEVC codec (Firefox on any OS, or Chrome on Linux). Client-side media processing needs to be enabled. Any file renamed to .heic will do - the decode fails either way.

1. Add an Image block.
2. Upload a .heic file.
3. Wait for the upload to fail.

Expected: the error snackbar appears on its own. No "Upload complete", and the "Uploading — …" snackbar comes down.

For the partial case, add a Gallery block and upload one good image and one broken .heic together. Expected: "Uploaded 1 of 2", no checkmark.

A file type the server refuses is the other way in, and needs no special browser: upload an OpenEXR image (.exr, one is attached to WordPress/wordpress-develop#13068). No browser maps that extension to a mime type, so the file goes to the server, which rejects it. Expected: "Sorry, you are not allowed to upload this file type." on its own. That is #81708, and it fails the same way with or without client-side media processing.

And to confirm the normal path is untouched, upload a good image on its own. Expected: "Upload complete" with the checkmark, as before.

The .exr case is covered by e2e tests in test/e2e/specs/editor/various/upload-progress-snackbar.spec.js, one per upload path. Both fail on trunk, recording the notices as "Uploading - openEXR.exr", "Sorry, you are not allowed to upload this file type.", "Upload complete".

Testing Instructions for Keyboard

No new UI, so nothing to operate by keyboard. The screen reader announcements do change with the outcome: "Media upload complete" for a clean batch, the "Uploaded 1 of 2" text for a partial one, and "Media upload failed" when nothing uploaded.

Screenshots or screencast

Before	After

Use of AI Tools

Written with 🤖 Claude Code. I will review and test.

[github-actions]

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
Co-authored-by: kushagra-goyal-14 <kush123@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: aaronjorbin <jorbin@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Size Change: +267 B (0%)

Total Size: 7.92 MB

📦 View Changed

Filename	Size	Change
build/scripts/editor/index.min.js	581 kB	+208 B (+0.04%)
build/scripts/upload-media/index.min.js	16.4 kB	+59 B (+0.36%)

_{compressed-size-action}

[github-actions]

Flaky tests detected in bffee85.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/33419632842
📝 Reported tests:

can update page link to a custom URL in /test/e2e/specs/editor/blocks/navigation.spec.js, passed after 1 failed attempt.

Error: apiRequestContext.fetch: socket hang up
Call log:
  - → DELETE http://localhost:8889/wp-json/wp/v2/pages/168?force=true
    - user-agent: Playwright/1.62.1 (x64; ubuntu 24.04) node/20.20 CI/1
    - accept: */*
    - accept-encoding: gzip,deflate,br
    - X-WP-Nonce: 443bacc067
    - cookie: wordpress_test_cookie=WP%20Cookie%20check; wordpress_logged_in_23778236db82f19306f247e20a353a99=admin%7C1788370700%7CdqqpAZ34QchKKCDJoD7QtURgD3X3MhosyxBY0uvseIF%7C1e17ea55d49e353aad42c64744bc595fd685be3ba4aa6a3afbc9c7e0b7506d51; wp-settings-time-1=1788197900

    at RequestUtils.rest (/home/runner/work/gutenberg/gutenberg/packages/e2e-test-utils-playwright/src/request-utils/rest.ts:112:39)
    at RequestUtils.call (/home/runner/work/gutenberg/gutenberg/packages/e2e-test-utils-playwright/src/request-utils/pages.ts:27:20)
    at map (/home/runner/work/gutenberg/gutenberg/packages/e2e-test-utils-playwright/src/request-utils/pages.ts:56:37)
    at RequestUtils.deleteAllPages (/home/runner/work/gutenberg/gutenberg/packages/e2e-test-utils-playwright/src/request-utils/pages.ts:56:9)
    at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/blocks/navigation.spec.js:1936:4

[adamsilverstein]

Putting this on the 7.1 board so we can consider fixing it in 7.1.1.

[adamsilverstein]

#82046 is stacked on this branch, so this one goes in first. Agreed on getting the fix into 7.1.1.

@adamsilverstein apologies, I can't tell from your comment if your reply is the Claude generated response or your own take on this topic. I was suggesting re-ordering things and landing that PR first, but this comment just states that the other PR is stacked on this one, and not why.

IMO it would be easier to review and test this change if the other change landed first.

Oh, got it, I misunderstood what you were asking. Will rebase the other PR onto trunk and we can get it merged first.

When you add Claude-generated replies, would you mind adding them in a block quote or something? I want to feel like we're collaborating together, and it will help me a lot if I can know when I'm chatting with you, or chatting with an LLM.

I like that idea. So far I have been adding a single sentence prefix along the lines of Claude came back with the following: and having Claude avoid using "I" where it might sound like me. I'll try adding the block quote or similar formatting to make it clearer.

[adamsilverstein]

Merged:

• Media: Refuse a multi-file drop on a placeholder that takes one file #82046

[andrewserong]

Thanks for the updates and for wrapping comments in block quotes, that helps a lot!

This is testing really nicely for me with client-side media processing enabled and disabled, and since it's a bug fix for 7.1.1, I think we should get it in.

One slight question for me is whether we should make the new selector for getFailureCount private for now, and it's to do with our thread discussing failureCount:

Something I'm wondering about in how we're calculating things here is that we're now trying to keep a running total of all failures in this editor session and performing some arithmetic to compare the failures value to failuresAtStart. This signals to me that we might want to lean into the batch idea that we've already got in some places, and instead have a batch record that is attached to the batch that tracks how many succeeded, how many failed, etc for the batch.

Then, for the consumer, there might be less calculation that needs to happen. As we only really have one consumer for this (the editor snackbar) it isn't really a problem for now. But might it be if/when we want to roll out client-side media processing and uploading elsewhere across WordPress? I think that's why the shape here is on my mind: it feels like we're solving this bug, but haven't quite landed on something that feels neat yet.

In any case, all that's likely well beyond the scope of this fix, and I mostly wanted to capture the idea and see what you had in mind for the future of this.

Thanks again for the fix!

[andrewserong]

(thanks for wrapping in a block quote, that helps readability for me! 🙇)

I think the thing that comes up for me looking over this file is that it sort of signals that our state here might be cleared out sooner than we need it if the existing behaviour means we need to store values outside of state.

For now, and to get this fix over the line, I like the rename to cumulativeFailures that feels clearer to me 👍

An idea for further down the track perhaps, but is there a way to rewrite the tracker state so that it's an object like:

{ total, succeeded, failed }

And it gets cleared out later in the cycle, i.e. once the snackbar is closed? Or do we need the current shape because of the possibility of overlapping batches (i.e. a second batch starts uploading before the first has finished) 🤔

[adamsilverstein]

Thanks for the review and the approval!

Both of the smaller ones are in 6f08a4d:

• getFailureCount is now a private selector on @wordpress/upload-media, read through unlock in the snackbar. Agreed that this is the right place for it while the shape is still an open question - easier to promote it later than to take it back.
• The partial string drops the noun: "Uploaded 3 of 5". No pluralized word, so no _n() needed and nothing for the extra plural forms to trip over.

On the batch record idea:

Claude went back through the tracker for this one:

Overlapping batches are the reason for the current shape. addFiles deliberately folds a new batch into the session already in progress rather than starting a second record, so two blocks uploading at once produce one "3 of 5" line instead of two competing snackbars. A per-batch record would leave the snackbar picking which batch to show, or merging them back together anyway.

Clearing later is the more interesting half. state is cleared inside finish the moment completed >= total, and that transition is exactly what the snackbar watches to know the queue drained - so the record can't simply live longer without something else marking the end. It would need the consumer to say when it is done with the session, which moves lifecycle ownership from the tracker to the snackbar. That is probably the right shape, but it is a behavior change rather than a bug fix.

Worth noting the CSM side already has some of the pieces: queue items carry a batchId, and there is a private isBatchUploaded selector. A { total, succeeded, failed } record that both paths report into, with the snackbar releasing it when it closes, would let both tallies stop being cumulative and would remove the subtraction entirely.

Happy to open a follow up issue with that so it doesn't get lost - it feels like the right thing to sort out before client side media processing shows up anywhere beyond the editor. Want me to?

[andrewserong]

Happy to open a follow up issue with that so it doesn't get lost - it feels like the right thing to sort out before client side media processing shows up anywhere beyond the editor. Want me to?

That'd be great if you don't mind 👍

Not at all a blocker for a bug fix PR like this, but agreed it'd be good to figure it out for usage beyond the editor.

[adamsilverstein]

Opened the follow up: #82264