Incorrect page id/name search blocks future searches

[bencroskery]

After an incorrect page id or name search in the media library, ex upload.php?p=123 or upload.php?name=noexist, future searches get blocked in js.

Steps to reproduce:

1. go to upload.php?p=123 (or use another id which doesn't exist)
2. should see 'No media files found.'
3. submit a search in the search box (nothing will happen)

Submitting the form is blocked by wp-admin/js/media.js:90, preventing the submit for (an unrelated?) search #find-posts.

[desrosj]

Welcome to trac, @bencroskery! I was able to reproduce this, but it did take one change. This only happens in the list view in the media library, not the grid view. To reproduce, you must add mode=list to the URL.

upload.php?p=123&mode=list

Are you interested in tackling this @bencroskery?

[tomdude]

Created patch to fix this. Adds check if any posts exist for preventDefault conditional.

[DrewAPicture]

Assigning to mark the good-first-bug as "claimed".

[audrasjb]

Media: Prevent incorrect previous queries to block media search input

[audrasjb]

Hi there,

Adding accessibility focus as this is a blocker for the search input’s accessibility.

The proposed patch perfectly fixes the issue. I tested it and it works fine.

I refreshed the patch since JS dependencies were completely moved.

I think 38221.1.diff is clearly good to go, so marking this for commit.

Cheers,
Jb

[afercia]

Thanks for the patches. Looking a bit into this, I'm not sure I understand what is the scenario where, using the admin interface, parameters like ?p=123 or ?name=noexist get added to the query string. Maybe I'm missing something but I couldn't find such a case.

Appending manually random parameters may break many other admin screens as well. Clarifying whether there are actual cases where these parameters are added while using the UI may help. Otherwise I'm not sure this can be considered an real bug.

Regardless, I'm not sure it should be fixed on the JavaScript side. The root cause of this odd behavior is that there are multiple "submit" buttons in the same form element, e.g.:

• the "Filter" button
• the "Select" button inside the "Attach to existing content" modal dialog: though it's visually hidden and appears only when clicking the "attach" link, it's in the same form and brings some JavaScript into play

When manually appending ?p=123 or ?name=noexist, the "Filter" button is not printed out. Thus, the "Select" button is the one that browsers consider the form submit button and triggers a click event when submitting a search by pressing Enter in the search field. The "Select" button has some JavaScript that prevents the default action when none of the radio buttons representing the posts within the modal is checked so the search submission is prevented.

A more proper fix would be making sure the "Filter" button is always printed out. It isn't because the ! is_singular() check originally introduced in [7251] fails: this is very old code and I'm not sure I understand whether it still needs to be there.

Worth noting this may happen also on other list tables where the same ! is_singular() check is used, it would just need a URL parameter that makes WP_Query consider the page as singular.

I'd like to ask for a second opinion from some experienced contributor with some historical knowledge of this ! is_singular() conditional implementation.

[afercia]

38221.2.diff​ is a proof of concept that makes the "Filter" submit button always visible and thus fixes the search.

Still, I'd lean towards a wontfix if there are no actual cases where those URL parameters are added by using the admin interface.

[afercia]

Another approach could be using the form attribute on the button so that the button is instructed to submit a specific form. No support in IE though. ​https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Browser_compatibility

[SergeyBiryukov]

Replying to afercia:

Thanks for the patches. Looking a bit into this, I'm not sure I understand what is the scenario where, using the admin interface, parameters like ?p=123 or ?name=noexist get added to the query string. Maybe I'm missing something but I couldn't find such a case.

I also could not find such a case. Thinking about it, I went back and forth for a bit between trying to fix it and closing as wontfix, since adding arbitrary parameter to the URL doesn't seem like a common use case.

That said, I think the is_singular() check can be removed here:

• The instance in class-wp-posts-list-table.php was added in [7247].
• The instance in class-wp-media-list-table.php was added in [7251].

As far as I can tell, both instances are now redundant and don't serve any purpose:

• wp-admin/edit.php?p=123 redirects to wp-admin/edit-comments.php?p=123
• wp-admin/upload.php?p=123 displays a single media item (assuming the ID exists), but without the filter.

I think it would make sense to always show the filter here, regardless of the is_singular() check.

See 38221.3.diff​.

[SergeyBiryukov]

In 48240:

Administration: Always show the filters on media and post list tables.

Previously, the filters were hidden for single posts or attachments, which could only be achieved by editing the URL manually.

The is_singular() check was added long before the list tables were introduced, and appears to no longer serve any purpose in the current code.

As a side effect, this resolves an issue where a non-existing attachment ID in the URL would block further search in Media Library.

Props afercia, tomdude, audrasjb, bencroskery, desrosj, SergeyBiryukov.
Fixes #38221.