Make WordPress Core

Opened 13 years ago

Closed 13 years ago

#16262 closed task (blessed) (fixed)

Remove AJAX and extendability from list tables

Reported by: nacin's profile nacin Owned by: nacin's profile nacin
Milestone: 3.1 Priority: highest omg bbq
Severity: normal Version: 3.1
Component: Administration Keywords: has-patch
Focuses: Cc:

Description (last modified by markjaquith)

We keep discovering new bugs for the AJAX in the list tables. Some current ones include issues with pushState (browser history is broken), bulk actions URI redirects and resets (which is a worse issue for usability than we thought). These strike at the core of the AJAX component. These are not easy to address, and will require some new API in some cases (such as transients for updated notices).

The leads have discussed this and the decision is to remove AJAX from the list tables for 3.1. Without AJAX, the user functionality is unchanged from 3.0. This allows the re-architecture to be spread over multiple releases.

Considerations:

  1. Don't enqueue the JavaScript.


  1. Don't add the ajax nonce fields or any spinners.


  1. The top pagination input doesn't work. Solution is to only have the pagination input on the top.


  1. The search box button shouldn't have a name.


  1. Sorting needs to kick back to page one.


  1. Remove the filters in get_list_table() and merge get_list_table() and require_list_table() into a private _get_list_table(). Mark all tables as private.
  1. Postponed until 3.2. The architecture of the list tables needs to be reviewed this week for potential changes. Right now it isn't at the state where we are comfortable with release, especially when considering future requirements for backwards compatibility. By removing the JS for 3.1, we can spend the next week getting the classes exactly how we want them.
  1. edit-comments.dev.js and theme.dev.js both declare list-table a dependency. I don't see anything in theme.dev.js, but there are some things in edit-comments.dev.js that will need to be neutered.
  1. The comment river needs to be fixed. (Gary has a patch up)
  1. Paging needs to work when using keyboard shortcuts (i.e. pressing J at the bottom of page 1, when there is more than one page).

Attachments (12)

16262.diff (22.6 KB) - added by nacin 13 years ago.
16262.2.diff (22.6 KB) - added by nacin 13 years ago.
garyc40.16262.diff (5.1 KB) - added by garyc40 13 years ago.
removed quick edit and reply links on edit_comments.php for now, until ajax actions are implemented again, neutered setCommentsList() and commentReply in edit-comments.js
garyc40.16262.2.diff (2.8 KB) - added by garyc40 13 years ago.
make commentReply work again, since it works in WP 3.0 and should still work properly in 3.1
garyc40.16262.3.diff (3.3 KB) - added by garyc40 13 years ago.
fix another notice in commentReply, restore comment list functionality in post.php
garyc40.16262.4.diff (4.4 KB) - added by garyc40 13 years ago.
neutered theme.dev.js ajax filter as well
garyc40.16262.5.diff (5.3 KB) - added by garyc40 13 years ago.
restore AJAX approve / unapprove / spam / trash functionality
garyc40.16262.river.diff (5.0 KB) - added by garyc40 13 years ago.
restore river functionality
garyc40.16262.river.2.diff (5.0 KB) - added by garyc40 13 years ago.
previous patch accidentally includes list-table.js
garyc40.16262.hotkey.diff (545 bytes) - added by garyc40 13 years ago.
fix prev / next hotkey when navigating to first / last comment
garyc40.16262.hotkey.2.diff (1.1 KB) - added by garyc40 13 years ago.
don't reload when next or prev buttons are disabled, also don't pass hotkeys_hightlight_(first|last) to pagination links
markjaquith.16262.issue09.river.3.diff (4.8 KB) - added by markjaquith 13 years ago.
Fix small conflict introduced by committing issue 10 fix.

Download all attachments as: .zip

Change History (45)

#1 @nacin
13 years ago

  • Owner set to nacin
  • Status changed from new to accepted

#2 follow-up: @nacin
13 years ago

  • Keywords has-patch added

Patch attached handles points 1 through 6. I'm adding one more, which koopersmith said he can handle.

  1. edit-comments.dev.js and theme.dev.js both declare list-table a dependency. I don't see anything in theme.dev.js, but there are some things in edit-comments.dev.js that will need to be neutered.

@nacin
13 years ago

#3 @scribu
13 years ago

  • Cc scribu added

#4 in reply to: ↑ 2 @ocean90
13 years ago

Replying to nacin:

I don't see anything in theme.dev.js

http://core.trac.wordpress.org/browser/trunk/wp-admin/js/theme.dev.js#L33

(A sadly decision :()

@nacin
13 years ago

#5 @nacin
13 years ago

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

#6 @nacin
13 years ago

(In [17322]) Remove AJAX from list tables. first pass. see #16262.

#7 @nacin
13 years ago

[17322] addresses points 1 through 6.

We'll execute point 7 early this week.

Point 8 needs someone to tear out some JS.

#8 @markjaquith
13 years ago

  • Description modified (diff)
  • Keywords has-patch removed
  • Priority changed from normal to high

#9 @Denis-de-Bernardy
13 years ago

So, we're back in alpha? :-D

@garyc40
13 years ago

removed quick edit and reply links on edit_comments.php for now, until ajax actions are implemented again, neutered setCommentsList() and commentReply in edit-comments.js

#10 @garyc40
13 years ago

Attached a patch that addresses point 8.

Comment Reply requires ajax, but there's no alternate no-js interface, as far as I know. We might need to work on that as well for 3.1 .

Another thing, the comment list in post.php doesn't work because it requires ajax as well.

I'm really sorry you guys have to make this drastic decision :(

@garyc40
13 years ago

make commentReply work again, since it works in WP 3.0 and should still work properly in 3.1

#11 @garyc40
13 years ago

Attached another patch that restores Comment Reply and Quick Edit comment so that it should work as in 3.0. Please disregard my comment above about no-js comment reply for now.

@garyc40
13 years ago

fix another notice in commentReply, restore comment list functionality in post.php

@garyc40
13 years ago

neutered theme.dev.js ajax filter as well

@garyc40
13 years ago

restore AJAX approve / unapprove / spam / trash functionality

#12 @garyc40
13 years ago

Sorry for the constant annoying stream of patches. I was thoroughly combing through each line of code in "edit-comments.js" and disabling AJAX features or restoring them one by one if they're in 3.0 .

All ajax functionalities that are not depending on list_table have been restored.

However, it's currently not refilling the extra list. Trashing a comment will still add another comment from the extra buffer to the list for a while, however there are only 8 extra comments in the buffer when the page loads.

In 3.0, refilling the extra list is done by submitting the form #get-extra-comments. However as we abstracted things into WP_Comments_List_Table, it got deleted and we have been using listTable.fetch_list instead.

What's the favorable way to proceed with restoring the extra list's functionality? Use the 3.0 approach, or just create a custom ajax request without going through listTable.fetch_list ?

#13 @demetris
13 years ago

  • Cc dkikizas@… added

#14 @garyc40
13 years ago

  • Keywords has-patch dev-feedback added

#15 @nacin
13 years ago

(In [17325]) s/get_list_table/_get_list_table/ in admin-ajax. see #16262.

#16 @markjaquith
13 years ago

(In [17326]) Several list table JS fixes. restores XHR comment status changes. props garyc40. see #16262

#17 follow-up: @nacin
13 years ago

Use the 3.0 approach, or just create a custom ajax request without going through listTable.fetch_list ?

What's the difference? I would think restoring 3.0 behavior would be the easiest way. edit-comments.dev.js can probably be largely reverted to the 3.0 state.

#18 in reply to: ↑ 17 @garyc40
13 years ago

Replying to nacin:

What's the difference? I would think restoring 3.0 behavior would be the easiest way. edit-comments.dev.js can probably be largely reverted to the 3.0 state.

No difference in terms of outcome. It's just that the 3.0 state using a form is a bit messy. I'd go with moving the ajax request out of listTable.

#19 @nacin
13 years ago

Whatever is least impact and easiest to implement, at this point.

#20 @nacin
13 years ago

  • Description modified (diff)
  • Keywords needs-patch added; has-patch dev-feedback removed
  • Priority changed from high to highest omg bbq

Modifying the 8 original points and adding point 9.

The implicit point 10 is that absolutely everything needs testing to ensure parity with 3.0.

@garyc40
13 years ago

restore river functionality

@garyc40
13 years ago

previous patch accidentally includes list-table.js

#21 @nacin
13 years ago

  • Keywords has-patch added; needs-patch removed

#22 @markjaquith
13 years ago

  • Description modified (diff)
  1. Paging needs to work when using keyboard shortcuts (i.e. pressing J at the bottom of page 1, when there is more than one page).

#23 @markjaquith
13 years ago

Probably best not to work on num 10 if it's going to break the patch for num 9, which is slightly more critical. Patch for num 9 looks sane to me. Can someone else review and weigh in on num 9?

@garyc40
13 years ago

fix prev / next hotkey when navigating to first / last comment

#24 @garyc40
13 years ago

The patch I attached fixes point num 10 without affecting num 9. It's not a combo patch though, only fixes num 10.

@garyc40
13 years ago

don't reload when next or prev buttons are disabled, also don't pass hotkeys_hightlight_(first|last) to pagination links

#25 follow-up: @garyc40
13 years ago

Please delete garyc40.16262.river.2.2.diff, I attached it by mistake.

garyc40.16262.hotkey.2.diff is the one you want to look at.

#26 in reply to: ↑ 25 @scribu
13 years ago

Replying to garyc40:

Please delete garyc40.16262.river.2.2.diff, I attached it by mistake.

Done.

#27 follow-up: @nacin
13 years ago

Bit confused on which patches do what. Correct me if I'm wrong:

#28 in reply to: ↑ 27 @garyc40
13 years ago

Replying to nacin:

Exactly. Sorry for the confusion.

#29 @markjaquith
13 years ago

(In [17343]) Fix inter-page navigating on Edit Comments screen when using keyboard shortcuts. props garyc40. see #16262

#30 @markjaquith
13 years ago

  • Description modified (diff)

10 is in. Reviewing updated patch for issue 9.

@markjaquith
13 years ago

Fix small conflict introduced by committing issue 10 fix.

#31 @markjaquith
13 years ago

(In [17344]) Fix the comments "river." issue 9. props garyc40. see #16262

#32 @markjaquith
13 years ago

  • Description modified (diff)

That's all 10. Anything else?

#33 @nacin
13 years ago

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

Marking as fixed. New tickets for all new bugs please. Thanks!

Note: See TracTickets for help on using tickets.