WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#16166 closed defect (bug) (fixed)

Ajax Table Sorting Session Broken with Bulk Actions

Reported by: hakre Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.1
Component: General Keywords: has-patch needs-testing dev-feedback
Focuses: Cc:

Description

The 3.1 new ajax table sorting is loosing data in its session when js + bulk requests are used in the session.

I ran over it while analyzing why pagination is lost in #16162 and it came to my attention that sorting is lost as well. While the number of the page is easier to aquire, sorting information can get lost in a session of http requests (as a more formal description of "using the admin").

The loss is a session break between Ajax and non-Ajax requests:

  • If JavaScript is enabled, the table contents is requested via an Ajax request, the location of the page does not change while navigating the table's data.
  • If JavaScript is disabled, the table contents is requested via a standard HTTP "full page" request, the location of the page does change.

The session break now is when bulk actions come into play: Bulk actions are triggering a "full page" request but without leaving any information (or at least without leaving sorting information) to the HTTP request scope as long as those information has been gathered by JavaScript processes.

With JavaScript disabled, the sorting information is not lost.

Bulk actions should always have all navigation related information regardless how the data to create that request has been acquired by the user: Via ajax calls or by simple HTTP "full page" requests.

Related: #14579; #15580

Attachments (5)

garyc40.16166.diff (5.6 KB) - added by garyc40 3 years ago.
don't reset order and orderby when after bulk edit
garyc40.16166.2.diff (6.3 KB) - added by garyc40 3 years ago.
tidy things up a bit
16166.diff (4.9 KB) - added by nacin 3 years ago.
16166.users-first-pass.diff (3.9 KB) - added by nacin 3 years ago.
16166.2.diff (9.4 KB) - added by nacin 3 years ago.
Untested.

Download all attachments as: .zip

Change History (31)

comment:1 hakre3 years ago

I tested the fix in #15416 and it has the same problem. Sorted pagination is broken with bulk actions.

comment:2 hakre3 years ago

Another symptom of this bug: Navigating through pages via Ajax (let's say you go to page 5 in the media library by pressing four times the "go to right"-arrowed button) and then enabling sorting on a column (let's say File) will bring you back to page 1. This symptom is even without a Bulk request. It's just js requests.

Last edited 3 years ago by hakre (previous) (diff)

comment:3 scribu3 years ago

  • Keywords ux-feedback added
  • Milestone changed from Awaiting Review to 3.1

It sends you back to page 1 on purpose, after performing a search or a sort.

Maybe it doesn't make sense to reset pagination when sorting?

comment:4 hakre3 years ago

Next to that it doesn't make sense, from the serverside, I missed the full pagination data (page-number, sort-column, sort-direction) to create usefull redirect locations server-side. The request only has "paged" from the input.

comment:5 follow-ups: jane3 years ago

I just looked at a couple web sites/apps that have both paging and sorting to see what they do. Typical behavior is to go to page one when a sort is done. The go-to app for behavior checks, gmail, doesn't have paging anymore, since they switched over to river-style (a la twitter).

comment:6 scribu3 years ago

  • Summary changed from Ajax Table Sorting Session Broken with Bulk Actions to Pagination is reset after sorting

comment:7 scribu3 years ago

  • Keywords ux-feedback removed

Thanks Jane.

I guess we then have to find another way to fix the problems caused by this.

comment:8 in reply to: ↑ 5 ; follow-up: hakre3 years ago

Replying to jane:

I just looked at a couple web sites/apps that have both paging and sorting to see what they do. Typical behavior is to go to page one when a sort is done. The go-to app for behavior checks, gmail, doesn't have paging anymore, since they switched over to river-style (a la twitter).

From what I can see in well-made (web)apps is, that if you change the view, the focus stays. Let's say, you're in a list on element 60 (out of 500), numerically sorted ASC, so #60 is POS:60 as well, after sorting the opposite, you're still with focus on #60 while pos is POS:500-60 now. But never is pos POS:1.

Set items per page to 1 and try with wordpress ;)

comment:9 in reply to: ↑ 8 ; follow-up: jane3 years ago

Replying to hakre:

From what I can see in well-made (web)apps is, that if you change the view, the focus stays.

Which ones are you looking at?

comment:10 scribu3 years ago

hakre, I think you're suggesting "local" sorting, i.e. to only sort the items currently in view, right?

I think that's less useful than the global sorting we currently have.

comment:11 in reply to: ↑ 9 hakre3 years ago

Replying to jane:

Replying to hakre:

From what I can see in well-made (web)apps is, that if you change the view, the focus stays.

Which ones are you looking at?

Any application of which has some list - the viewable part of the list is comparable to the viewable page in wordpress. paging is scrolling.

Try Yahoo Mail if you're looking for some table/list that is actually sortable.

I think tine 2.0 has this as well.


Replying to scribu:

hakre, I think you're suggesting "local" sorting, i.e. to only sort the items currently in view, right?

No, infact, it's without the word "only" so probably what you called "global".

But I have no problem to focus on the same item, if that's what you call "local".

Last edited 3 years ago by hakre (previous) (diff)

comment:12 hakre3 years ago

This is one test I did: Compare use with javascript on and off. Compare the different behavior. If you do bulk actions both things are broken. But w/o bulk actions, I have the feeling that w/o javascript works a bit more useful then with javascript.

garyc403 years ago

don't reset order and orderby when after bulk edit

comment:13 garyc403 years ago

  • Keywords has-patch added

My patch doesn't prevent pagination from resetting when you sort. However, it does prevent order and orderby from resetting when you bulk edit.

comment:14 scribu3 years ago

  • Summary changed from Pagination is reset after sorting to Ajax Table Sorting Session Broken with Bulk Actions

Yeah, we should focus on fixing the bulk action behaviour.

garyc403 years ago

tidy things up a bit

comment:15 garyc403 years ago

  • Keywords needs-testing dev-feedback added

nacin3 years ago

comment:16 nacin3 years ago

(In [17270]) Keep sorting and paging for bulk actions. props garyc40, see #16166.

comment:17 nacin3 years ago

We need to go through the other list tables to see where else that add_query_args code needs to end up.

comment:18 in reply to: ↑ 5 voyagerfan57613 years ago

  • Cc WordPress@… added

Replying to jane:

The go-to app for behavior checks, gmail, doesn't have paging anymore, since they switched over to river-style (a la twitter).

I can't find any river-style view in Gmail, even with &labs=0. But Gmail doesn't have sorting anyway, nor does it have the ability to switch between ajax and full-page requests within a session. (Users either have full ajax or a basic HTML page.)

comment:19 nacin3 years ago

(In [17273]) #blamenacin, see #16166.

comment:20 nacin3 years ago

(In [17275]) Move wp_redirect calls to the end of the switch in users.php. Fix unrelated bug where the user's cap should be check, rather than their role's cap. see #16166.

comment:21 nacin3 years ago

(In [17276]) Move edit.php bulk actions code to a handler in the list table class. see #16166.

comment:22 nacin3 years ago

(In [17277]) Revert [17275] and [17276]. The rabbit hole is too deep. see #16166.

comment:23 nacin3 years ago

After some additional discussion, the 11 list tables that implement bulk actions have too many differences and idiosyncrasies to implement what we were thinking. (You can see the general idea in [17275].) We'll be adding bandaids at this point.

comment:24 scribu3 years ago

Yeah, I've been down that rabbit hole myself:

http://gsoc.trac.wordpress.org/changeset/757

nacin3 years ago

Untested.

comment:25 nacin3 years ago

(In [17321]) Revert [17270], [17273], see #16166, see #16262.

comment:26 nacin3 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Last edited 3 years ago by nacin (previous) (diff)
Note: See TracTickets for help on using tickets.