#14579 closed task (blessed) (fixed)
Ajaxify list-type screens in the admin
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Priority: | normal | Milestone: | 3.1 |
| Component: | Administration | Version: | |
| Severity: | normal | Keywords: | ongoing-project |
| Cc: | ryan, westi, WordPress@…, jdub@…, kawauso, mikeschinkel@…, admin@…, andymacb, batmoo@… |
Description (last modified by scribu)
This is the master ticket for my GSoC project.
What the patch does:
- makes most columns sortable: #9259
- introduces a new pagination style: #12179
- ajaxifies sortable columns, pagination & search
- provides an API for adding custom list screens: #11517
- replaces WP_User_Search with get_users(): #14123
- removes _wp_get_comment_list() in favor of get_comments(): #14163
- makes bulk actions filterable: #12732
- fixes problems with long URIs: #10762, #11114
Attachments (17)
Change History (186)
comment:4
voyagerfan5761 — 3 years ago
- Cc WordPress@… added
- Cc westi added
Great to see the results of all your summer work as a mega patch.
It might be better to land this as incremental patches which achieve the different goals along the way so that it is easier to see what is changing and why.
Would it be much work to split down into per feature patches?
It wouldn't matter if there was an order in which we had to apply them it would just make it easier to review and commit.
Would it be much work to split down into per feature patches?
Unfortunately, yes. Identifying the bugs that were fixed was a retroactive process.
It might be better to land this as incremental patches which achieve the different goals along the way so that it is easier to see what is changing and why.
You can take a look at my commit log to get a sense of what was changed and why.
Awesomesauce.
Small things I noticed on first read. Note I only really looked at modified files:
AJAX fetch-list looks like it needs a cap/referer check. Also, die('-1') on insufficient privs, die('0') on failure.
default-list-tables.php -- I'm not sure I like an includes file having a require at the top. Also, should we split these tables up into individual files? Some of these classes are quite large.
template.php, right line 1817, this should be show_ui, not public.
minor style issues: false instead of FALSE, single if statements should not be on one line, we like our semicolons, CSS declarations should not be on one line, etc.
looks like merging turned wp-admin/network/*.php back into ms-*.php a bit.
<br class="clear"> in plugins.php and a few other places.
_wp_search_sql() probably doesn't need to be in functions.php.
Script localization needs the convert entities bit at the end.
jquery.query.js should be minimized. Also, the crunching of admin-table.js looks like it didn't touch local variables, also. munging is fine.
General concerns about back compat:
We obviously changed quite a bit of markup, as well as some CSS. Should the old CSS selectors remain, in order not to hurt the styling of old-style tables by plugins?
Renaming the "p" query arg on edit-comments, does that break anything? Is that the URL included in emails? Now it's longer… p should be an alias at least.
Probably need to deprecate, not outright remove, WP_User_Search. I did not carefully read through a lot of the red, so there may be other things we can't get rid of without breaking a lot of plugins.
comment:10
nacin — 3 years ago
comment:11
scribu — 3 years ago
Related: #14596
comment:12
automattor — 3 years ago
comment:13
scribu — 3 years ago
comment:14
ramoonus — 3 years ago
- Cc ramoonus@… added
comment:15
follow-up:
↓ 17
jdub — 3 years ago
How will custom columns fit into this? Additional hooks for plugins to provide the data to the ajax call?
comment:16
jdub — 3 years ago
- Cc jdub@… added
comment:17
in reply to:
↑ 15
scribu — 3 years ago
Replying to jdub:
How will custom columns fit into this? Additional hooks for plugins to provide the data to the ajax call?
You don't need to worry about AJAX at all. See this tutorial:
comment:18
follow-up:
↓ 21
demetris — 3 years ago
Two small issues I noticed in the Plugins screen, and that I confirmed on two different setups:
- The rows in the Recently Active view do not show any info. See attached screenshot.
- It does not remember the view you were on. E.g., when I am on the Active view and click to deactivate a plugin, it then sends me to the All view.
comment:19
scribu — 3 years ago
comment:20
scribu — 3 years ago
comment:21
in reply to:
↑ 18
;
follow-up:
↓ 23
scribu — 3 years ago
Replying to demetris:
Two small issues I noticed in the Plugins screen, and that I confirmed on two different setups:
- The rows in the Recently Active view do not show any info. See attached screenshot.
Confirmed.
- It does not remember the view you were on. E.g., when I am on the Active view and click to deactivate a plugin, it then sends me to the All view.
I think this has been fixed in [15496].
Another bug I noticed is that clicking on 'Delete' gives you an 'Are you sure you want to do this?' screen.
comment:22
scribu — 3 years ago
Related: #14612
comment:23
in reply to:
↑ 21
demetris — 3 years ago
Replying to scribu:
Replying to demetris:
It does not remember the view you were on. E.g., when I am on the Active view and click to deactivate a plugin, it then sends me to the All view.
I think this has been fixed in [15496].
I am still seeing it in r15497. Confirmed on two different setups again. The query strings are like this:
http://example.com/wp-admin/plugins.php?deactivate=true&plugin_status=&paged=
(The plugin_status field has no value.)
comment:24
markmcwilliams — 3 years ago
The attached patch fixes the action links for wp-admin/network/sites.php which all mustn't have been sync'ed before you made the commit scribu, anyway, admin_url(); now becomes network_admin_url(); in the above patch! :) All links to the ms-*.php files currently! :(
comment:25
scribu — 3 years ago
comment:26
scribu — 3 years ago
comment:27
scribu — 3 years ago
comment:28
scribu — 3 years ago
comment:29
scribu — 3 years ago
Related: #14637
comment:30
kawauso — 3 years ago
- Cc kawauso added
comment:31
follow-up:
↓ 32
kawauso — 3 years ago
Removal of register_column_headers() breaks popular plugins such as WP E-Commerce and MiniMeta Widget with an E_FATAL
comment:32
in reply to:
↑ 31
markmcwilliams — 3 years ago
comment:33
Lazy79 — 3 years ago
i still have a problem on one blog running with trunk.
on dashboard, post screen (..) it says
Fatal Error: Call to undefined method stdClass::get_column_headers() in template.php (/wp-admin/includes)
on plugin screen it works (of course). its sure because of a plugin (i think) but maybe it helps you in trunk / dev a bit..
comment:34
scribu — 3 years ago
It would help if you could also figure out which plugin is causing it. Add this code to a file in your mu-plugins folder:
function scb_error_handler($errno, $errstr) {
echo $errstr;
ob_start();
debug_print_backtrace();
$out = explode( "\n#", ob_get_clean());
$out = array_slice( $out, 1);
echo '<pre>' . "\n#" . implode( "\n#", $out ) . '</pre>';
}
set_error_handler('scb_error_handler', E_WARNING|E_ERROR|E_RECOVERABLE_ERROR|E_USER_WARNING|E_USER_ERROR);
Then, please post back the displayed backtrace on http://wp.pastebin.ca/
comment:35
Lazy79 — 3 years ago
Hi Scribu,
sorry i was stupid writing the post above. i figured out which plugin really makes this error (and thx for the function, its greadt!)
it is this plugin:
comment:36
scribu — 3 years ago
Related: #14651
comment:37
scribu — 3 years ago
comment:38
follow-up:
↓ 41
scribu — 3 years ago
comment:39
scribu — 3 years ago
comment:40
hakre — 3 years ago
Realted: #11517
comment:41
in reply to:
↑ 38
;
follow-up:
↓ 47
demetris — 3 years ago
comment:42
demetris — 3 years ago
Correction:
s/Recently View/Recently Active/
comment:43
scribu — 3 years ago
comment:44
scribu — 3 years ago
- Description modified (diff)
comment:45
scribu — 3 years ago
comment:46
nacin — 3 years ago
comment:47
in reply to:
↑ 41
;
follow-up:
↓ 53
demetris — 3 years ago
Replying to demetris:
Replying to scribu:
r15525 fixed the issue with the Recently Active, but it seems that the problem remains for other views.
For example, if you are on the Inactive view and activate a plugin, you are then sent to the All view.
It seems this is fixed in current trunk (r15550) and now all views are remembered.
A different problem that I’ve been seeing since the beginning is that you cannot navigate between pages in themes.php. (You need more than 15 installed themes to see this.)
comment:48
scribu — 3 years ago
comment:49
scribu — 3 years ago
comment:50
scribu — 3 years ago
comment:51
scribu — 3 years ago
comment:52
scribu — 3 years ago
comment:53
in reply to:
↑ 47
demetris — 3 years ago
comment:54
scribu — 3 years ago
comment:55
demetris — 3 years ago
Thanks for the quick fix.
Another issue that may be related to the ajaxification:
When I try to delete a plugin or theme, in the Connection Information screen the FTP password (saved and remembered by the browser) flashes once and then disappears. So, I have to re-enter it manually.
(This only happens for deleting. When I update themes/plugins everything works fine in the Connection Information screen.)
comment:56
scribu — 3 years ago
comment:57
scribu — 3 years ago
comment:58
mikeschinkel — 3 years ago
- Cc mikeschinkel@… added
comment:59
scribu — 3 years ago
comment:60
scribu — 3 years ago
Also see #14776 for a usage example.
comment:61
scribu — 3 years ago
comment:62
scribu — 3 years ago
comment:63
scribu — 3 years ago
comment:64
scribu — 3 years ago
comment:65
scribu — 3 years ago
comment:66
follow-up:
↓ 67
demetris — 3 years ago
The bottom group of bulk actions in plugins screens has not been working for some time now. I suppose it is something to do with the ajaxification.
I can look and narrow it down to a revision, if needed.
comment:67
in reply to:
↑ 66
duck_ — 3 years ago
Replying to demetris:
The bottom group of bulk actions in plugins screens has not been working for some time now. I suppose it is something to do with the ajaxification.
I can look and narrow it down to a revision, if needed.
This is because $_REQUESTaction? is never empty as it will either be the bulk action selected (from the top dropdown) or "-1" by default, therefore $_REQUESTaction2? (set by the lower bulk actions dropdown) never gets a chance. See attached patch for fix, change the default value of the bulk actions to be value='0' or alternatively could just be left blank (I think this is the case pre-3.1: value="")
comment:68
hakre — 3 years ago
Related: #14927
comment:69
scribu — 3 years ago
14579-lower-bulk.diff breaks the bottom bulk actions for posts (and probably in other places).
Working on a patch to make this consistent.
comment:70
scribu — 3 years ago
comment:71
duck_ — 3 years ago
Not very detailed as I haven't had time to look into this, but bulk actions (including Empty Trash button) on post type (posts, pages, CPTs) admin pages are not working.
And [15653] introduced a call to the list table get_columns() function in list-table.php line 74 which is before $_screen is instantiated, this raises a notice in the Media library. Just moving that call down a few lines will fix this, don't think it will mess with anything else.
comment:72
scribu — 3 years ago
comment:73
dd32 — 3 years ago
comment:74
ramoonus — 3 years ago
- Cc ramoonus@… removed
comment:75
duck_ — 3 years ago
I'm guessing this is related to this ticket.
The behaviour on the 'post type' screens (I'm counting all of the items in the 2nd block of the admin menu) is inconsistent about showing the "Filter" buttons/dropdowns when there are no posts, pages, media items, etc... to display.
For example, if you have no Posts you still see dates/categories filter options and button, but for Media, Pages and CPTs you see just a "Filter" button, for Links you see an empty dropdown and a button, and Comments shows nothing. 3.0.1 shows none of this for any of the pages except comments which shows the dropdowns and buttons.
comment:76
scribu — 3 years ago
comment:77
Utkarsh — 3 years ago
- Cc admin@… added
Attached patch to remove @ error suppression and use array_key_exists instead.
comment:78
nacin — 3 years ago
Generally better to use isset() or empty() checking for keys (depending), unless there's the possibility that a key's value could be null. In this case, we still want to return something.. Though I guess it would be null regardless if it doesn't exist.
comment:79
Utkarsh — 3 years ago
Learned something new today :) isset seems to be faster than array_key_exists.
Patch refreshed.
comment:80
ramoonus — 3 years ago
- Cc ramoonus@… added
comment:81
ramoonus — 3 years ago
- Cc ramoonus@… removed
comment:82
scribu — 3 years ago
comment:83
andymacb — 3 years ago
- Cc andymacb added
comment:84
scribu — 3 years ago
comment:85
scribu — 3 years ago
comment:86
kawauso — 3 years ago
[15925] creates invalid ID values if a directory separator is present in the plugin path
comment:88
scribu — 3 years ago
Indeed. Besides slashes, there are a lot of other characters which would make the id invalid. Reverted.
comment:89
voyagerfan5761 — 3 years ago
MediaWiki has a sanitizer class with a method to use on strings before using them as IDs or class names. Maybe borrow from there? (MW is GPLv2, just like WordPress)
comment:90
nacin — 3 years ago
We have sanitize_html_class.
comment:91
voyagerfan5761 — 3 years ago
Pardon my incomplete research. I have a fondness for those MediaWiki functions because of their past usefulness to me... Next time I'll more thoroughly search WordPress's code before making a suggestion like that.
comment:92
azaozz — 3 years ago
(In [15939]) Clean list-table.js: when declaring JS object a comma after the last property is invalid, remove unused vars.
comment:93
scribu — 3 years ago
comment:94
nacin — 3 years ago
Reworks get_list_table() to conditionally include classes. Introduces require_list_table() to include core classes. Allows for plugins to prefix classes instead of by default forcing them into WP_*_Table.
comment:95
scribu — 3 years ago
14579-rework-get_list_table.3.diff:
Rename filter to 'get_list_table' and apply it before calling require_list_table().
comment:96
nacin — 3 years ago
comment:98
nacin — 3 years ago
comment:99
nacin — 3 years ago
About 1.86 MB of memory was saved on edit.php by only loading what we needed. As WP_Posts_Table is the largest class (nearly double the size of the next largest), the savings are surely greater on other pages.
comment:100
nacin — 3 years ago
comment:101
nacin — 3 years ago
comment:102
scribu — 3 years ago
- Keywords needs-patch added; has-patch needs-testing removed
While loading items, a horizontal scrollbar appears; on all screens.
comment:103
jane — 3 years ago
- Keywords ongoing-project added; needs-patch removed
comment:104
scribu — 3 years ago
comment:105
scribu — 3 years ago
comment:106
nacin — 3 years ago
comment:107
rovo89 — 3 years ago
I justed tested a plugin with the latest trunk which I started with WordPress 3.0. It uses a custom post type and the manage_edit-myposttpe_columns filter to modify the columns in the edit screen. This works for the first page, but when I navigate to the second page (ajaxified), the default columns are shown.
I tracked this down to a change in [15956]. $current_screen is used in the constructor of WP_Posts_Table, so it needs to be set before creating the instance of this class. The attached patch fixes this.
comment:108
scribu — 3 years ago
comment:109
scribu — 3 years ago
comment:110
solarissmoke — 3 years ago
I was editing some link categories (debug enabled) and came across the following when trying to "quick edit" a category:
(Rev 16123)
PHP Notice: Undefined variable: type in \wp-admin\includes\list-table-terms.php on line 344 PHP Notice: Undefined property: stdClass::$taxonomy in \wp-admin\includes\list-table-terms.php on line 344
These are passed to the quick_edit_custom_box action. I'm not sure what they were supposed to represent - can they be removed altogether?
comment:111
westi — 3 years ago
comment:112
westi — 3 years ago
comment:113
westi — 3 years ago
comment:114
westi — 3 years ago
comment:115
follow-up:
↓ 116
nacin — 3 years ago
solarissmoke - can you open a new ticket?
comment:116
in reply to:
↑ 115
solarissmoke — 3 years ago
comment:117
scribu — 3 years ago
comment:118
scribu — 3 years ago
comment:119
scribu — 3 years ago
comment:121
scribu — 3 years ago
comment:122
scribu — 3 years ago
comment:123
scribu — 3 years ago
comment:124
scribu — 3 years ago
comment:125
scribu — 3 years ago
comment:126
scribu — 3 years ago
comment:127
scribu — 3 years ago
comment:128
scribu — 3 years ago
comment:129
josephscott — 3 years ago
r16182 re-introduced an issue:
Notice: Trying to get property of non-object in /f/www/dev.josephscott.org/public_html/wp/trunk/wp-admin/includes/class-wp-list-table.php on line 88
When replying to a comment in wp-admin.
comment:130
scribu — 3 years ago
comment:131
scribu — 3 years ago
I wrote some code for testing that all the hooks in WP 3.0 can still be found in WP 3.1-alpha, after the ajaxification:
http://core.trac.wordpress.org/attachment/ticket/14579/test-list-tables.php
There don't seem to be any incompatibilities left.
That said, problems still remain:
comment:132
ryan — 3 years ago
I think we need to drop sorting for columns that don't have an index. For example, display_name for the users list and registered for the sites list.
comment:133
scribu — 3 years ago
Wouldn't it be better to add those indexes instead?
comment:134
ryan — 3 years ago
I don't know if they're worth their weight. Those are both global tables that can be huge.
comment:135
ryan — 3 years ago
We could make Registered use the ID for sorting.
comment:136
scribu — 3 years ago
comment:137
scribu — 3 years ago
Related: #15386
Restore "Submitted on" which was missing since 3.0. See the current Comments Help tab.
comment:138
scribu — 3 years ago
comment:139
ocean90 — 3 years ago
See also #15414
comment:140
scribu — 3 years ago
It would be nice to have hashes for all ajaxified actions so that views are bookmarkable.
comment:141
scribu — 3 years ago
@zeo: Thanks for the patches. Please keep in mind that no notifications are sent when you upload a patch is uploaded, so you should announce them in a comment.
comment:142
scribu — 3 years ago
comment:143
scribu — 3 years ago
comment:144
scribu — 3 years ago
comment:145
scribu — 3 years ago
comment:146
scribu — 3 years ago
comment:147
demetris — 3 years ago
It seems r16429 broke page navigation in themes.php.
comment:148
scribu — 3 years ago
Ah, that's because themes.php uses 'pagenum' instead of 'paged'. I'll patch it up soon.
comment:149
scribu — 3 years ago
comment:150
scribu — 3 years ago
Related: #15481
comment:151
scribu — 3 years ago
hashes.14579.diff is an attempt to make ajaxified screens bookmarkable via URL hashes.
So far, the rows are updated based on the hash, but the other UI elements (sort arrow, search field, page field) are not.
comment:152
batmoo — 3 years ago
- Cc batmoo@… added
Patch fixes error notice for Comment Quick Edits.
Notice: Trying to get property of non-object in C:\wamp\www\wp31\wp-admin\includes\class-wp-list-table.php on line 8
comment:153
scribu — 3 years ago
comment:154
batmoo — 3 years ago
Patch sanitizes the entered page number before changing the page:
- Garbled, non-numeric text reverted to page 1 (currently returns NaN)
- Higher than total pages reverted to the last page
- Lower than page 1 reverted to first page
comment:155
scribu — 3 years ago
The sanitization should happen in change_page(), not outside of it.
comment:156
scribu — 2 years ago
comment:157
scribu — 2 years ago
comment:158
dd32 — 2 years ago
Just a note on the "Loading..." text:
- Opera - Ends up as a block div ~200px? high above the header bar
- IE - Doesnt show at all
- FF - Loads and dims the rows as expected
comment:159
scribu — 2 years ago
Related: #15580
comment:160
ocean90 — 2 years ago
14579.network.users.fix.patch will fix problems with hidden columns on network users screen.
comment:161
scribu — 2 years ago
comment:162
scribu — 2 years ago
comment:163
scribu — 2 years ago
Related: #15607
comment:164
mikeschinkel — 2 years ago
I've noticed that when you sort by title in a list of hierarchical posts you loose the hierarchy indicators (which is what I would expect) but there seems to be no way to go back to the hierarchical listing without refreshing the entire page. Not having some way to revert could be confusing for people and while a page refresh will fix, that being the only way feels like it was just missed in the design process.
Had you considered this issue before and if so did I just miss how to get back to the hierarchical listing without refreshing the page?
comment:165
scribu — 2 years ago
- Resolution set to fixed
- Status changed from accepted to closed
I replied here: http://core.trac.wordpress.org/ticket/15580#comment:36
I'm going to close this ticket now, as it's got pretty huge. Feel free to open new tickets for related issues.
comment:166
hakre — 2 years ago
Related: #16022
comment:167
hakre — 2 years ago
comment:168
hakre — 2 years ago
Related: #16185

The images go in wp-admin/images/