Media Library: deleting all items on the last page loses the pagination/navigation buttons and shows message

[donsony]

While deleting all items on the last page of Media Library, the page loses the pagination/navigation buttons and shows the message "No Media Files found" with no way to access the earlier pages other than by going back to the "Media" link

[antpb]

illustration of the behavior

[antpb]

I've added an illustration of the behavior above, we have two options here that @desrosj outlined in the recent #core-media meeting:

"I would either return the user to page 1, or, send them to the _new_ last page."

[desrosj]

I looked at how other admin areas handle this. This is what I found:

• Posts: reloads the page with paged - 1 (new last page).
• Categories: reloads the page with paged - 1 (new last page).
• Users: This flow is a bit different because there is a confirmation page in between. But, the user is always returned to the first page of users after deleting users.

My vote is to keep things consistent with the other post and term areas in core and redirect the user to the new last page (paged-1).

[Mista-Flo]

Hi there, it took me ages to understand what was wrong here.

For example, if you have just two pages of attachments and you are on page 2, just change the paged argument in the URL to 37 and it still loads the UI with no attachments at all.

After some digging, I have found out that $wp_query->found_posts and $wp_query->max_num_pages are equal to 0.

So even if it's on page 450 that does not exist, the posts list table have a count of the number of total posts which helps to identify the maximum number of pages in the pagination and redirect to the last existing page if the requested page is too high, check set_pagination_args function in wp-admin/includes/class-wp-list-table.php for more information.

So finally I just used the wp_count_attachments() function to mimic the behaviour of the post list table and it worked.

[justinahinon]

39968.1.patch is fine and correctly solve the issue on my Trunk version.
When I delete all the items in the last page, it returns to the new last page.

[antpb]

This works well in my testing and is related to another patch in 5.6. Because of this, we've moved this ticket into the milestone ahead of Beta 4.

[antpb]

In 49567:

Media: Improve count in Media Library pagination.

Deleting all visible items on the last page of the media library previously left a blank page with no media items available. Using wp_count_attachements instead of found_posts solves the problem.

Props donsony, karmatosed, desrosj, mista-flo, justinahinon.

Fixes #39968.

[iCaleb]

Hi all,

This changeset means that the total count and pagination in the media library will no longer take additional query filtering into account.

Example issue we just ran into with this patch applied:

add_action( 'pre_get_posts', function( $query ) {
        $display_all = filter_input( INPUT_GET, 'example-filter', FILTER_SANITIZE_STRING );

        if ( 'all' !== $display_all ) {
                $query->set( 'meta_key', 'some_meta_key' );
        }
} );

Before this patch, the pagination/total count would be correct with and without the filtering. But now with the patch applied, the pagination displays far more pages than needed with the latter half all being blank.

I can open a new ticket if needed for this bug, but seemed more fitting to go here. Need to take another look at how to resolve this bug without removing the ability to filter the $wp_query.

[trepmal]

inaccurate item total with r49567

[trepmal]

Adding onto @iCaleb's comment, a simpler demonstration of the issue can be found with the media type and date filters in list view. Note that the pagination area shows 38 items and 2 pages, but there is only result that matches the date filter.

[peterwilsoncc]

I've been able to reproduce this with @trepmal's steps above

• upload media over the course of two or more months (you might need to mess with server time to do this, or run a production server on RC1)
• view media library list view
• in pagination, note the number of items and max pages displayed
• limit images to a single month
• observe neither of the number have changed in the pagination section

I suggest reverting [49567].

Filtering either via the admin or using WP_Query seems less of an edge case than deleting all the images on the final page.

[SergeyBiryukov]

Replying to peterwilsoncc:

I suggest reverting [49567].

I agree, that appears to be the safest option for RC2.

[SergeyBiryukov]

In 49720:

Media: Revert [49567].

This addresses a regression with the pagination section in Media Library no longer taking additional query filtering into account.

Props iCaleb, trepmal, peterwilsoncc.
See #39968.

[SergeyBiryukov]

In 49721:

Media: Revert [49567].

This addresses a regression with the pagination section in Media Library no longer taking additional query filtering into account.

Props iCaleb, trepmal, peterwilsoncc.
Reviewed by peterwilsoncc, SergeyBiryukov.
Merges [49720] to the 5.6 branch.
See #39968.

[SergeyBiryukov]

Moving to a future release for further work.

[Mista-Flo]

Second patch

[Mista-Flo]

Hey guys, I have uploaded another patch that fixes the issue with filters.

Unfortunately there are some backend logic somewhere else that prevent non existing pages to redirect to the last page when a filter search is made like with the date.

Steps to reproduce:

1. Go to wp-admin/upload.php
2. Enable list mode
3. Filter by a given month
4. Go to page 2
5. Then directly change the paged parameter to a random page like 56
6. It won't show any attachment, but it does not redirect you to the last existing page

So it seems to partially fix the original issue.

[antpb]

@Mista-Flo that behavior is the current experience even without the fix. I personally don't think we have to solve for that edge case in this ticket. Might be good to document in a new ticket, but I think it's somewhat unrelated to the core issue here.

The code looks good from my high level view. Will test it locally here soon. :) thank you!