Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#16335 closed defect (bug) (fixed)

Media Library search result sorting is broken

Reported by: stephdau's profile 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 13 years ago.

Download all attachments as: .zip

Change History (16)

#1 @scribu
13 years ago

  • Milestone changed from Awaiting Review to 3.1

#2 @SergeyBiryukov
13 years ago

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

#3 @scribu
13 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
13 years ago

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

#5 @SergeyBiryukov
13 years ago

  • Keywords has-patch added; search media removed

#6 @nacin
13 years ago

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

#7 @scribu
13 years ago

Patch works for me.

#8 @SergeyBiryukov
13 years ago

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

#9 @ryan
13 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
13 years ago

Related: #16341

#11 @stephdau
13 years ago

Sweet! Thanks scribu, SergeyBiryukov, nacin and ryan.

#12 @_ck_
12 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
12 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_
12 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
12 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.