Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#16335 closed defect (bug) (fixed)

Media Library search result sorting is broken

Reported by: stephdau Owned by:
Milestone: 3.1 Priority: normal
Severity: major Version: 3.1
Component: Administration Keywords: has-patch
Focuses: Cc:

Description

Under the current trunk (r17344), when going to /wp-admin/upload.php and submitting a search, the latter search query gets lost when trying to change the results' sort order.

  1. go to /wp-admin/upload.php
  2. submit a search, like the always popular "img_"
  3. once you have results, try to sort
  4. watch how the results are now unfiltered (but sorted right).

The issue is with _POST. _GET (/wp-admin/upload.php?s=img_) works as expected and is carried throughout.

Found while testing r17344 merge on wp.com.

Attachments (1)

16335.patch (1.6 KB) - added by SergeyBiryukov 11 years ago.

Download all attachments as: .zip

Change History (16)

#1 @scribu
11 years ago

  • Milestone changed from Awaiting Review to 3.1

#2 @SergeyBiryukov
11 years ago

It's not just Media Library. Comments, Links and Users too.

#3 @scribu
11 years ago

  • Severity changed from normal to major

Ah, of course it doesn't work. The link that you click to sort a column isn't updated to contain the search string.

#4 @nacin
11 years ago

Need to revert all form method=POST back to GET again to fix this.

#5 @SergeyBiryukov
11 years ago

  • Keywords has-patch added; search media removed

#6 @nacin
11 years ago

Might as well just re-open that bulk action URI ticket now.

#7 @scribu
11 years ago

Patch works for me.

#8 @SergeyBiryukov
11 years ago

Plugins search form still has method="post", however there are no sortable columns now, so it's good.

#9 @ryan
11 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [17345]) From post back to get. Props SergeyBiryukov. fixes #16335

#10 @scribu
11 years ago

Related: #16341

#11 @stephdau
11 years ago

Sweet! Thanks scribu, SergeyBiryukov, nacin and ryan.

#12 @_ck_
10 years ago

Did you seriously switch admin functions that can potentially pass up to several thousand bytes of data to GET via URL just avoid fixing the sort routine with POST?

That is just dumbfounding. It defies all "best practice" programming techniques not to mention the security issues it can create.

#13 @scribu
10 years ago

Thanks for the helpful comment, _ck_. You're sure helping the project along.

If it's so awful, just fork it and make it conform to whatever best practices you want. Show us mortals how it's done.

#14 @_ck_
10 years ago

scribu do you feel it's appropriate to attack me personally in trac when I am commenting on the approach and not any individual? If you feel this change was appropriate then defend the change, don't attack me.

I don't claim to be an expert but that's my point - anyone with a base level of experience should have seen this is a bad move.

Don't take my "opinion" on it, go ask on Stack Overflow for some independent feedback on the wisdom of changing a form (in a legacy environment no less) that passes up to several Kbytes of data from POST to GET.

Let's forget about the security implications for a moment.

There are browsers still used today that won't accept urls over 2000 bytes.

Some proxies won't accept urls over 255 characters.

#15 @scribu
10 years ago

You don't attack an individual, you just tacitly imply that we're all idiots. Of course we know that using POST would be better.

In fact, we did switch to POST, but then found bugs that couldn't be fixed in time for 3.1, so the decision was made to revert the change.

Note: See TracTickets for help on using tickets.