[wp-trac] [WordPress Trac] #16031: New bulk actions hook missing catcher behavior

WordPress Trac noreply at wordpress.org
Wed Oct 21 05:33:24 UTC 2015


#16031: New bulk actions hook missing catcher behavior
----------------------------+-----------------------------
 Reporter:  Veraxus         |       Owner:  DrewAPicture
     Type:  enhancement     |      Status:  accepted
 Priority:  normal          |   Milestone:  Future Release
Component:  Administration  |     Version:  3.1
 Severity:  normal          |  Resolution:
 Keywords:  has-patch       |     Focuses:
----------------------------+-----------------------------

Comment (by Veraxus):

 Thanks for taking charge of this ticket, @DrewAPicture !

 When I first decided to tackle this problem, I actually started with
 `WP_List_Table`, as per previous tweaks and suggestions by @scribu and
 @edward mindreantre - but quickly discovered that it was impossible. Some
 notes about the admin screens...

 * Every admin screen handles actions ''outside'' of the `WP_List_Table`
 class, on its respective screen.
 * After hard-coded default actions are handled, a custom redirect is built
 by feeding the current URL through `add_query_arg()` as necessary. This
 ensures that duplicate actions aren't accidentally run by flushing the
 request data after it's handled.
 * `$wp_list_table->prepare_items()` is usually run after the
 aforementioned redirect (and after actions are caught and handled),
 meaning our request data will never reach it. It's also not executed
 consistently before or after actions are handled.
 * When and where the redirect is handled by each screen sometimes varies
 wildly. But each time the hook needs to occur ''before'' the forced
 redirect occurs (which flushes our request data), but before
 `$wp_list_table->prepare_items()` is executed.

 And this is why the comprehensive patch adds the hooks to each admin
 screen, contingent on the redirect, and not `WP_List_Table`.

 Unfortunately, we simply won't be able to neatly package this behavior
 into `WP_List_Table` without dramatically reworking every single core
 admin screen as well. We'd have to merge the action-handler logic and
 redirect handling into each screens respective list table child class...
 but that seems like a different project/ticket.

 BUT... maybe that's what we ''should'' do given the wild lack of any
 common style or convention in those files (i.e. use it as an opportunity
 to tidy up the code for each screen). My point is: putting the handler
 inside `WP_List_Table` is anything ''but'' straightfoward due to the way
 these screens are constructed. It would be a pretty substantial
 refactoring project.

 Thoughts?

 P.S. I just realized that the comprehensive patch includes some extra
 indentation on a couple files. I remember addeding it to make it easier
 for me to follow the code, which is something of a mess tbh, and forgot to
 revert it for a cleaner patch. *sigh* :-/

--
Ticket URL: <https://core.trac.wordpress.org/ticket/16031#comment:65>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list