Make WordPress Core

Opened 7 weeks ago

Last modified 6 weeks ago

#64467 new defect (bug)

Grid view displays attachments in the wrong order when an order query var is present but not normalized

Reported by: trivedikavit's profile trivedikavit Owned by:
Milestone: 7.0 Priority: normal
Severity: minor Version:
Component: Media Keywords: has-screenshots changes-requested has-patch has-unit-tests
Focuses: javascript, administration Cc:

Description (last modified by sabernhardt)

Media Library Grid view displays attachments in the wrong order when an order query var is present but not normalized (e.g. order=desc or order=pizza). Not a WP_Query / database issue: the server returns correctly ordered results; the UI reverses them client‑side in JavaScript.

Are you using either the latest version of WordPress, or the latest development version?

Yes. Reproduced on:

  • A clean local WordPress install running WordPress 6.9
  • Multiple production sites, showing the same behavior. The local reproduction confirms this is core behavior and not environment-specific.
Direct URL reproduction (most deterministic)

Visit: /wp-admin/upload.php?mode=grid&orderby=date&order=desc

Observe the ordering displayed in the grid.
Control cases:

  • Works as expected:
    • /wp-admin/upload.php?mode=grid&orderby=date&order=DESC
    • /wp-admin/upload.php?mode=grid&orderby=date
  • Also triggers incorrect ordering:
    • /wp-admin/upload.php?mode=grid&orderby=date&order=pizza

Screenshots

Incorrect Order: order=desc
Incorrect Order in Grid View with order=desc (lowercase)

Correct Order: order=DESC
Correct Order in Grid View with order=DESC (uppercase)

Common User Path (List Mode with Filters => Grid Mode)

  • Visit List mode with sorting: /wp-admin/upload.php?mode=list&orderby=date&order=desc
  • Click the Grid view toggle (the generated link preserves orderby/order).
  • Grid view renders the first batch of attachments in the wrong order.

Does the problem occur even when you deactivate all plugins and use the default theme?

Yes. Reproduced on a clean local install with:

  • Default theme (twentytwentyfive)
  • No plugins

What steps should be taken to consistently reproduce the problem?

Precondition: Media Library contains a few images where this sorting issue would be apparent.

What is the expected output or result? What did you see instead?

Expected

For: mode=grid&orderby=date&order=desc The grid should display attachments newest-first (DESC), consistent with server-side behavior and the AJAX response.

Actual

With order=desc (lowercase) or invalid values such as order=pizza:

The server-side query and the AJAX response contain attachments in the correct order (DESC by date).

The Grid UI renders them reversed (appears ASC / oldest-first).

This demonstrates the defect is not in WP_Query and not in SQL ordering; it is a client-side UI sorting issue in the Media Library grid JavaScript.

Please provide any additional information that you think we'd find useful.

Patch

Here's the patch:

--- a/wp-includes/js/media-models.js
+++ b/wp-includes/js/media-models.js
@@ -43,7 +43,7 @@ Query = Attachments.extend(/** @lends wp.media.model.Query.prototype */{

               this.filters.order = function( attachment ) {
                       var orderby = this.props.get('orderby'),
-                              order = this.props.get('order');
+                              order = ( this.props.get('order') || 'DESC' ).toUpperCase();

                       if ( ! this.comparator ) {
                               return true;
@@ -1091,7 +1091,7 @@ var Attachments = Backbone.Collection.extend(/** @lends wp.media.model.Attachmen
        */
       comparator: function( a, b, options ) {
               var key   = this.props.get('orderby'),
-                      order = this.props.get('order') || 'DESC',
+                      order = ( this.props.get('order') || 'DESC' ).toUpperCase(),
                       ac    = a.cid,
                       bc    = b.cid;

@@ -1108,7 +1108,7 @@ var Attachments = Backbone.Collection.extend(/** @lends wp.media.model.Attachmen
                       ac = bc = null;
               }

-              return ( 'DESC' === order ) ? wp.media.compare( a, b, ac, bc ) : wp.media.compare( b, a, bc, ac );
+              return ( 'ASC' === order ) ? wp.media.compare( b, a, bc, ac ) : wp.media.compare( a, b, ac, bc );
       },
       /** @namespace wp.media.model.Attachments.filters */
       filters: {

What the patch changes

The patch makes two small but important changes in wp-includes/js/media-models.js:

1) Normalize order to uppercase in the media models

It changes:

order = this.props.get('order');

and

order = this.props.get('order') || 'DESC';

to:

order = ( this.props.get('order') || 'DESC' ).toUpperCase();

So:

  • 'desc' becomes 'DESC'
  • 'asc' becomes 'ASC'
  • undefined becomes 'DESC'
  • 'pizza' becomes 'PIZZA' (still invalid, but now handled consistently)
  • This ensures the subsequent comparisons behave deterministically.
2) Change the comparator to treat only explicit ASC as ascending (default everything else to DESC)

It changes the comparator decision from:

return ( 'DESC' === order ) ? DESC_logic : ASC_logic;

to:

return ( 'ASC' === order ) ? ASC_logic : DESC_logic;

Effect:

  • If order is exactly 'ASC', the Grid sorts ascending.
  • For 'DESC' (and for any unexpected/invalid value), the Grid sorts descending.
  • That default-to-DESC behavior matches how WP_Query behaves when order is missing or invalid, which prevents the UI from "flipping" the already-correct server ordering.

Testing Instructions

Since this patch modifies the unminified JavaScript file wp-includes/js/media-models.js, to test it properly:

  1. Set define( 'SCRIPT_DEBUG', true ); in your wp-config.php to ensure WordPress loads unminified JavaScript files instead of the minified versions (otherwise you need to rebuild or patch the minified file too.)
  2. Apply the patch to wp-includes/js/media-models.js.
  3. Clear your browser cache or use an incognito/private browsing window to avoid cached scripts.
  4. Follow the reproduction steps above to verify the fix:
    • Visit /wp-admin/upload.php?mode=grid&orderby=date&order=desc and confirm attachments display newest-first.
    • Test control cases: order=DESC, order=asc, and invalid values like order=pizza should all behave correctly (defaulting to DESC where appropriate).

Net Result

  • Grid view displays attachments in the same order as the AJAX response.
  • order=desc and order=DESC behave identically.
  • Invalid order values no longer cause the Grid UI to reverse the display order; they fall back to descending behavior instead.

Attachments (2)

incorrect-order.png (359.5 KB) - added by trivedikavit 7 weeks ago.
Incorrect Order in Grid View with order=desc
correct-order.png (355.5 KB) - added by trivedikavit 7 weeks ago.
Correct Order in Grid View with order=DESC

Download all attachments as: .zip

Change History (13)

@trivedikavit
7 weeks ago

Incorrect Order in Grid View with order=desc

@trivedikavit
7 weeks ago

Correct Order in Grid View with order=DESC

#1 @sabernhardt
6 weeks ago

  • Description modified (diff)
  • Focuses administration added
  • Keywords has-patch needs-testing has-test-info javascript removed
  • Summary changed from Media Library Grid view displays attachments in the wrong order when an order query var is present but not normalized (e.g. `order=desc` or `order=pizza`). Not a `WP_Query` / database issue: the server returns correctly ordered results; the UI reverses them client‑side in JavaScript. to Grid view displays attachments in the wrong order when an order query var is present but not normalized

Hi and welcome to WordPress Core Trac!

In trunk, the script involves both query.js and attachments.js.

There is also a 'menuOrder' option that relies on ASC uppercase.

Last edited 6 weeks ago by sabernhardt (previous) (diff)

#2 follow-up: @adamsilverstein
6 weeks ago

@trivedikavit Thanks for the detailed bug report! I was able to reproduce this in trunk following the common path you described:

Visit List mode with sorting: /wp-admin/upload.php?mode=list&orderby=date&order=desc
Click the Grid view toggle (the generated link preserves orderby/order).
Grid view renders the first batch of attachments in the wrong order.

Since is the primary or maybe only way users can reach this mis-sorted state (there is no sort option on the grid view itself). I wonder if a simpler fix might be to just change the sort parameter used on the list view from 'desc' to 'DESC'. This seems like the fundamental issue - the sort variable doesn't match what the code is expecting.

#3 in reply to: ↑ 2 @adamsilverstein
6 weeks ago

Since is the primary or maybe only way users can reach this mis-sorted state (there is no sort option on the grid view itself). I wonder if a simpler fix might be to just change the sort parameter used on the list view from 'desc' to 'DESC'. This seems like the fundamental issue - the sort variable doesn't match what the code is expecting.

Reviewing the list table code, I see this won't work: it expects orderby to be lower case and even explicitly converts it to lower case if it wasn't already. I think the JavaScript solution you proposed is likely the best fix.

@trivedikavit - Can you review the feedback from @sabernhardt above to review the additional locations that might need adjusting as well.

#4 @adamsilverstein
6 weeks ago

  • Keywords changes-requested added
  • Milestone changed from Awaiting Review to 7.0

#5 @trivedikavit
6 weeks ago

After digging further into the Query and Attachments models (thanks @sabernhardt for pointing those out as sources), I think there is a more fundamental root cause here and a cleaner fix than normalizing order at individual call sites.

What changed in my understanding

The initial patch (uppercasing order inside filters.order / comparator, and defaulting to DESC unless ASC) is effective, but it treats the symptom: it fixes only the specific places where those methods read this.props.get( 'order' ).

On closer inspection, the underlying issue is that this.props is not initialized consistently across different collection types / instantiation paths. As a result, this.props.get( 'order' ) can contain different values depending on whether the collection is a Query instance or a plain Attachments instance.

The underlying inconsistency

wp.media.model.Query.get( props ) normalizes order

In Query.get() the code:

  • uppercases props.order
  • validates it against ASC/DESC
  • defaults invalid values back to the default (DESC)

So any Query created via Query.get() ends up with normalized props.order.

// Normalize the order.
props.order = props.order.toUpperCase();
if ( 'DESC' !== props.order && 'ASC' !== props.order ) {
    props.order = defaults.order.toUpperCase();
}

Reference: https://core.trac.wordpress.org/browser/trunk/src/js/media/models/query.js?rev=51632&marks=40#L254

This is also done via _requery here, where if the collection is a query, it would create and mirror a Query collection: https://core.trac.wordpress.org/browser/trunk/src/js/media/models/attachments.js?rev=51191&marks=497,514#L443

wp.media.model.Attachments.initialize() does not normalize order

In Attachments.initialize( models, options ), options.props is applied directly via:

this.props.set( _.defaults( options.props || {} ) );

Reference: https://core.trac.wordpress.org/browser/trunk/src/js/media/models/attachments.js?rev=51191&marks=497,514#L47

If options.props.order is passed in as lowercase (e.g. 'desc') or invalid, it can remain unnormalized in this.props.

Because Query inherits from Attachments, we end up with different this.props states depending on how the collection is constructed:

  • A Query instance created via Query.get() will have normalized this.props.get( 'order' )
  • An Attachments instance constructed with raw options.props.order may not have normalized this.props.get( 'order' )

This inconsistency becomes visible as a bug whenever downstream UI logic performs case-sensitive comparisons against DESC / ASC (e.g. the comparator branching on 'DESC' === order), which can lead to the Grid UI effectively sorting in the opposite direction from what the server returned.

How I confirmed it

I added a temporary log inside the comparator to inspect the instance type and the order value:

console.log( 'comparator in', this instanceof wp.media.model.Query ? 'Query' : 'Attachments', 'with order:', order );

For example, visiting /wp-admin/upload.php?mode=grid&orderby=date&order=desc yields:

comparator in Query with order: DESC
comparator in Attachments with order: desc

Proposed fix: normalize once at initialization

Instead of normalizing in every method that consumes props.order, normalize at the point where props are first applied in Attachments.initialize() (i.e. normalize options.props.order before setting it on this.props):

options.props = options.props || {};

if ( 'string' === typeof options.props.order ) {
    options.props.order = options.props.order.toUpperCase();
    if ( 'ASC' !== options.props.order && 'DESC' !== options.props.order ) {
        options.props.order = 'DESC';
    }
}

this.props.set( options.props );

Reference (replace this line): https://core.trac.wordpress.org/browser/trunk/src/js/media/models/attachments.js?rev=51191&marks=497,514#L47

Testing

Testing with the fix in place does lead to consistent normalized order:

comparator in Query with order: DESC
comparator in Attachments with order: DESC

Why this is better

  • Fixes the root cause: this.props starts out normalized regardless of whether the collection is Query or Attachments.
  • More maintainable: avoids having to normalize order everywhere this.props.get( 'order' ) is referenced.
  • Inheritance-proof: subclasses inheriting Attachments.initialize() get consistent normalization automatically.

Regarding the menuOrder:

This proposed change preserves existing behavior for valid ASC/DESC values and improves reliability for menuOrder (which already relies on ASC being uppercase). It only changes behavior for previously unnormalized inputs (e.g. asc, desc, invalid strings), aligning Attachments initialization with Query.get()’s existing normalization/defaulting.


@adamsilverstein Let me know if this looks good.

#6 @adamsilverstein
6 weeks ago

@adamsilverstein Let me know if this looks good.

Great investigation @trivedikavit - thanks for all the details and context. Your new approach seems like a much better solution! Please proceed to a patch or PR if you can!

This ticket was mentioned in PR #10680 on WordPress/wordpress-develop by @trivedikavit.


6 weeks ago
#7

  • Keywords has-patch added

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

### Summary
This PR addresses the Media Library Grid view ordering issue when the order query parameter is not normalized (e.g., order=desc instead of order=DESC). It consolidates order normalization logic in the base Attachments model to ensure consistent behavior across all attachment collections.

### Changes Made
In attachments.js:

  • Added order normalization in the initialize method: converts options.props.order to uppercase and defaults to 'DESC' if invalid.
  • Changed this.props.set( _.defaults( options.props || {} ) ); to options.props = _.defaults( options.props || {} ); followed by normalization and then this.props.set( options.props ); for better control.

In query.js:

  • Removed duplicate order normalization code, as Query inherits from Attachments and will now use the parent's normalization logic.

### Why This Change?

  • Fixes Root Cause: Ensures this.props starts normalized in Attachments.initialize(), preventing inconsistencies between Query and Attachments instances.
  • Consistency: All attachment collections now handle the order property uniformly, fixing the grid view ordering bug.
  • DRY Principle: Eliminates code duplication between Attachments and Query models.
  • Robustness: Prevents invalid order values from causing UI sorting issues.
  • Backwards Compatibility: No breaking changes; existing valid 'ASC'/'DESC' values work as before, and invalid values now default to 'DESC'.

### Safety and Scope

  • Scope: The changes only affect how order props are normalized internally in wp.media.model.Attachments and its subclass wp.media.model.Query. This doesn't alter public APIs or external behavior.
  • Normalization Is Defensive: We're ensuring order is always 'ASC' or 'DESC' (defaulting to 'DESC'), which aligns with existing expectations in the comparator and filters. Invalid inputs (like 'desc' or 'pizza') now behave predictably instead of causing silent failures.
  • Inheritance: Since Query extends Attachments, all media collections benefit from consistent normalization without duplication.
  • No Breaking Changes: Valid 'ASC'/'DESC' values remain unchanged; only unnormalized inputs are fixed. The Trac ticket confirms this resolves the grid view ordering bug without side effects.
  • Potential Edge Cases: If a plugin or theme manually instantiates these models with custom order values, they might see normalized output, but that's actually an improvement (consistent behavior). The media JS isn't loaded on the frontend by default, so no impact on public-facing sites unless explicitly enqueued.

### Testing

  • Verified that attachment sorting works correctly in the media library grid view.
  • Ensured Query instances still normalize order via inheritance.
  • Tested with URLs like /wp-admin/upload.php?mode=grid&orderby=date&order=desc to confirm correct ordering.
  • No adverse effects on other WordPress Core functionality.

#8 @soyebsalar01
6 weeks ago

I tested and confirm that media library grid sorting works correctly for both asc and desc.

This ticket was mentioned in PR #10682 on WordPress/wordpress-develop by @adamsilverstein.


6 weeks ago
#9

  • Keywords has-unit-tests added

@adamsilverstein commented on PR #10680:


6 weeks ago
#10

I tried adding some QUnit tests to validate the order normalization (with help from Claude) in https://github.com/WordPress/wordpress-develop/pull/10682

#11 @trivedikavit
6 weeks ago

@adamsilverstein

You mentioned this in the Github PR:

can this use options.props.order = defaults.order.toUpperCase(); like the original code in query.js that you remove below?


We cannot normalize order directly on defaults (e.g., defaults.order.toUpperCase()) in the Attachments model, as defaults refers to Query.defaultProps, which is scoped to the Query class and unavailable in the base Attachments class.

References:

Attempting defaults.order.toUpperCase() in Attachments.initialize() would fail, as defaults is undefined in that scope.

Behavior in Attachments Instances

Attachments is the base Backbone collection for media attachments. On instantiation (e.g., new wp.media.model.Attachments(models, options)), initialize:

  • Sets up this.props with options.props.
  • If options.props.order is a string, normalizes it: uppercases and defaults to 'DESC' if invalid.
  • Ensures consistent order for sorting/filtering, handling case and invalid inputs.

Previously, without normalization in Attachments.initialize(), plain Attachments instances (non-Query subclasses) could use unnormalized order (e.g., 'desc' or 'pizza'), causing inconsistent media library grid sorting.

Impact on Query Instances

Query inherits from Attachments, so normalization is inherited.

  • Before: Normalization in Query.get() ensured valid order for Query instances.
  • After: Normalization in Attachments.initialize() during instantiation (via inheritance). Query.get() applies defaults first, then new Query() triggers normalization.
  • No change in Query behavior; the fix prevents bugs in other Attachments subclasses or direct usage.
Note: See TracTickets for help on using tickets.