Skip to content

Fix: Media upload from the post edit screen shows an inaccurate number when uploading multiple media files#11585

Open
hbhalodia wants to merge 1 commit intoWordPress:trunkfrom
hbhalodia:fix/issue-65053
Open

Fix: Media upload from the post edit screen shows an inaccurate number when uploading multiple media files#11585
hbhalodia wants to merge 1 commit intoWordPress:trunkfrom
hbhalodia:fix/issue-65053

Conversation

@hbhalodia
Copy link
Copy Markdown

Trac ticket: https://core.trac.wordpress.org/ticket/65053

Use of AI Tools

AI assistance: Yes
Tool(s): GitHub Copilot, Claude
Model(s): Claude Sonnet 4.6
Used for: Identifying root cause and for a fix. Have reviwed the final implementation and raised the PR.

Detailed Claude Analysis and fix

Root Cause Analysis: "Showing X of Y media items" shows incorrect total count

Trac ticket: https://core.trac.wordpress.org/ticket/65053
Related PR: WordPress/gutenberg#77213
Affected files: src/js/media/models/attachments.js


The Symptom

When opening the media library modal (e.g. via "Set featured image" or an Image block), the footer media count displayed an incorrect total count:

Showing 8 of 7 media items

The displayed count (8) was correct, but the total (7) was one short of the real total.


Background: How the count works

The media library uses a Backbone.js collection architecture with three key classes:

Class Role
wp.media.model.Query Fetches paginated attachments from the server via AJAX. Stores totalAttachments parsed from the X-WP-Total response header.
wp.media.model.Attachments A client-side collection that can mirror a Query and/or observe additional collections (e.g. the selection).
wp.media.model.Selection Extends Attachments. Holds the currently selected items in the modal.

The "Showing X of Y" text in the Load More bar is rendered by AttachmentsBrowser.updateLoadMoreView():

sprintf(
    __( 'Showing %1$s of %2$s media items' ),
    this.collection.length,             // items currently displayed
    this.collection.getTotalAttachments() // total on the server
)

getTotalAttachments() reads directly from the mirrored Query collection's totalAttachments property:

getTotalAttachments: function() {
    return this.mirroring ? this.mirroring.totalAttachments : 0;
},

The totalAttachments on the Query is updated in real-time via two private methods bound as event listeners:

_addToTotalAttachments: function() {
    if ( this.mirroring ) {
        this.mirroring.totalAttachments = this.mirroring.totalAttachments + 1;
    }
},
_removeFromTotalAttachments: function() {
    if ( this.mirroring ) {
        this.mirroring.totalAttachments = this.mirroring.totalAttachments - 1;
    }
},

The Root Cause

In wp.media.model.Attachments, the observe() method is responsible for watching another collection and replicating its add/remove/reset events. Before this fix, it also registered the total-count trackers on every observed collection:

// BEFORE (buggy):
observe: function( attachments ) {
    this.observers = this.observers || [];
    this.observers.push( attachments );

    attachments.on( 'add change remove', this._validateHandler, this );
    attachments.on( 'add',    this._addToTotalAttachments,      this ); // ← too broad
    attachments.on( 'remove', this._removeFromTotalAttachments, this ); // ← too broad
    attachments.on( 'reset',  this._validateAllHandler,         this );
    this.validateAll( attachments );
    return this;
},

The critical detail is that observe() can be called on more than just the Query collection. Both wp.media.controller.FeaturedImage and wp.media.controller.ReplaceImage call:

// featured-image.js (line 83) and replace-image.js (line 86):
library.observe( this.get('selection') );

This is intentional and correct behaviour for display purposes — it ensures that a pre-existing featured image (which might not be in the first page of query results) is always visible in the grid even if the server hasn't returned it yet.

However, it also meant that the total-count handlers were now bound to the Selection collection as well as the Query collection.

The event cascade that causes the wrong count

  1. User opens the "Set featured image" modal. A featured image is already set (ID: 42).
  2. library.observe(selection) is called. The selection contains 1 item (attachment 42).
  3. The server responds with 8 total attachments → query.totalAttachments = 8. ✅
  4. User clicks a different image (ID: 99) to set as the new featured image.
  5. Selection.add() is called for attachment 99. Because this.multiple === false, it first calls this.remove(this.models) — removing attachment 42 from the selection.
  6. The remove event fires on the Selection collection.
  7. _removeFromTotalAttachments is triggered (it was bound to the selection in step 2) → query.totalAttachments is decremented: 8 → 7. ❌
  8. Selection.add() completes, adding attachment 99, firing add_addToTotalAttachments → 7 → 8. ✅ (briefly correct again)
  9. But updateLoadMoreView is debounced (10ms). If it fires between steps 7 and 8 (or if the add event is never fired due to validation), the UI shows "Showing 8 of 7".

Additionally, in the general case, any add/remove on the selection (without a corresponding add/remove on the query) permanently corrupts totalAttachments for the lifetime of the modal session.


The Fix

The total-count event bindings (_addToTotalAttachments / _removeFromTotalAttachments) belong exclusively on the primary query collection being mirrored — not on every additionally-observed collection.

The fix moves these bindings from observe() into mirror(), which is the method that establishes the relationship with the Query collection:

// AFTER (fixed):
observe: function( attachments ) {
    this.observers = this.observers || [];
    this.observers.push( attachments );

    attachments.on( 'add change remove', this._validateHandler, this );
    // _addToTotalAttachments / _removeFromTotalAttachments removed from here
    attachments.on( 'reset', this._validateAllHandler, this );
    this.validateAll( attachments );
    return this;
},

mirror: function( attachments ) {
    if ( this.mirroring && this.mirroring === attachments ) {
        return this;
    }

    this.unmirror();
    this.mirroring = attachments;

    this.reset( [], { silent: true } );
    this.observe( attachments );

    // Bind total attachment count tracking only to the mirrored query
    // collection, not to additional observed collections (e.g. selection).
    attachments.on( 'add',    this._addToTotalAttachments,      this );
    attachments.on( 'remove', this._removeFromTotalAttachments, this );

    this.trigger( 'attachments:received', this );
    return this;
},

Why this is safe

  • mirror() is only ever called with a wp.media.model.Query instance (via _requery()). The query accurately reflects server-side counts, so attaching the total-count trackers there is correct.
  • observe() is called for both the query (via mirror()) and supplemental collections like the selection. After this fix, observe() only handles item visibility/validation, which is its original purpose.
  • The unmirror() path is unaffected: it calls unobserve(this.mirroring) which calls .off(null, null, this) on the query, removing all handlers bound with this as the context — including the total-count ones added in mirror().
  • There are no other callers of observe() in core that expect the total-count side effect.

Files Changed

File Type Change
attachments.js Source Moved _addToTotalAttachments/_removeFromTotalAttachments bindings from observe() to mirror()

Fix Location: Core, not Gutenberg

Although this bug is reproducible through the "Set featured image" panel in Gutenberg, the root cause is in the legacy media library JavaScript stack in WordPress Core. The Attachments, Selection, and Query Backbone models are part of wp.media which lives in Core (wp-includes/js/). Gutenberg's MediaUpload component is ultimately a wrapper that opens this same modal. Adding multiple={false} to MediaUpload (as proposed in the Gutenberg PR #77213) would address the UX problem of allowing multi-select but would not fix this count bug, which is triggered by any single-selection change in the modal, including image blocks and other media contexts.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@github-actions
Copy link
Copy Markdown

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.

Core Committers: Use this line as a base for the props when committing in SVN:

Props hbhalodia.

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

@github-actions
Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant