Opened 10 years ago
Last modified 6 years ago
#29030 new defect (bug)
Screen Options Poor Update/Rendering Causes Many things to Break
Reported by: | codecandid | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 3.9.1 |
Component: | Administration | Keywords: | dev-feedback has-patch |
Focuses: | ui, javascript, administration | Cc: |
Description (last modified by )
Screen options dont work properly in many different situations. I noticed the first issue when trying to create a sticky header plugin for the wp_list_table. When scrolling down the page the headers stick to the top by cloning the header with javascript and hiding the other original at the same time. However, If screen options are updated the tables break completly even after the plugin is disabled. Wordpress checks the current table headers to determine which ones are hidden and should be added to the manageedit-{$post_type}columnshidden
field in the user_meta
table. So since the cloned table header the plugin created is hidden visually while scrolling up, wordpress thinks that all columns aredisabled and adds all the columns to manageedit-{$post_type}columnshidden
.
This is poor practice because it doesn't seperate presentation well enough from the logic used to render screen options. Any user who has access to wp-admin/edit.php
can completly break their tables if any html/css visually hides the <thead>
or a column-header perhaps by a plugin, or maybe the browser doesn't load a certain script, or perhaps they are just messing with the dev-tools. Beginers that don't know how to properly [remove columns][5], could run into this issue if they ever try to use css instead. manageedit-{$post_type}column
should not rely on the visibility of <thead> and only the actual checked input fields. Also cb
and title
should not be allowed to be added to the manageedit-{$post_type}column
. They should only be able to be removed with unset
.
To recreate this issue:
- open up firebug/chrome dev tools/etc. on http://www.example.com/wp-admin/edit.php
- add
thead {display: none;}
to the style editor - On the page screen options uncheck at least one column ( this is to ensure
manageedit-{$post_type}columnshidden
is a database field for the current user and if not it creates it ) - Hit apply to refresh the page
*The tables will now be broken....*
To chck the columns I used the get_user_meta();
function to print the array of hiddencolumns
on each post types edit.php
admin screen notices:
<?php function get_current_post_type() { global $post, $typenow, $current_screen; if ($post && $post->post_type) return $post->post_type; elseif ($typenow) return $typenow; elseif ($current_screen && $current_screen->post_type) return $current_screen->post_type; elseif (isset($_REQUEST['post_type'])) return sanitize_key($_REQUEST['post_type']); return null; } function get_current_user_manageedit_pagecolumnshidden() { $current_ptype = get_current_post_type(); $user_id = get_current_user_id(); $key = 'manageedit-'.$current_ptype.'columnshidden'; $single = true; if(get_user_meta($user_id, $key, $single)) return get_user_meta($user_id, $key, $single); } function echo_current_user_manageedit_pagecolumnshidden() { global $pagenow; if ( $pagenow !== 'edit.php' ) return; $columnshidden= get_current_user_manageedit_pagecolumnshidden(); echo '<pre>'; print_r( $columnshidden ); echo '</pre>'; } add_action('all_admin_notices', 'echo_current_user_manageedit_pagecolumnshidden');
Output for the broken tables :
Array ( [0] => cb [1] => title [2] => [3] => )
After determining that cb
& title
were in fact added to the $meta_value
you need to fix the table. This will do the trick:
function delete_current_user_manageedit_pagecolumnshidden() { $user_id = get_current_user_id(); $meta_key = 'manageedit-pagecolumnshidden'; if( get_user_meta($user_id, $meta_key) ) delete_user_meta( $user_id, $meta_key ); } add_action ('admin_init', 'delete_current_user_manageedit_pagecolumnshidden');
Side-Notes:
*columnshidden
appears [wp_ajax_hidden_columns()
][1] & [get_hidden_columns()
][2]
*client-side functionality appears to be here in [common.js
][3] which checks for the [hidden table headers][4]
Similar issues with the screen options can be recreated for different situations that have nothing to do with the tables.
Recreate similar issue on nav-menus.php
- Go to http://example.com/wp-admin/nav-menus.php
- Uncheck all the fields in the *"Show advanced menu properties"* Screen-Options tab
- Add the screen options filter to hide them from display:
add_filter('screen_options_show_screen', 'remove_screen_options_tab');
- Reload http://example.com/wp-admin/nav-menus.php
All of the hidden advanced menu properties will now be broken and are all visible even though they were unchecked. I'm not sure if this is the same issue, but it appears that overall screen options have a high change of not working properly
Other-Notes
These issues of broken tables might also have to do with the same functionality problem of how screen options update/render:
http://wordpress.stackexchange.com/questions/144361/wordpress-admin-wp-table-list-show-incorrectly
[1]: https://github.com/WordPress/WordPress/blob/448275cce483138f53ccfa586b2d28b7fe8b0785/wp-admin/includes/screen.php#L55
[2]: https://github.com/WordPress/WordPress/blob/270a57075c290736387b6551670fde34fb3f1851/wp-admin/includes/ajax-actions.php#L1307
[3]: https://github.com/WordPress/WordPress/blob/448275cce483138f53ccfa586b2d28b7fe8b0785/wp-admin/js/common.js#L29
[4]: https://github.com/WordPress/WordPress/blob/448275cce483138f53ccfa586b2d28b7fe8b0785/wp-admin/includes/screen.php#L17
[5]: http://codex.wordpress.org/Plugin_API/Filter_Reference/manage_$post_type_posts_columns
Attachments (1)
Change History (8)
#3
@
10 years ago
- Keywords dev-feedback added
Checking my hunch, I do believe this is in fact a javascript issue:
Ajax handler looks for "hidden columns here:https://github.com/WordPress/WordPress/blob/448275cce483138f53ccfa586b2d28b7fe8b0785/wp-admin/js/common.js#L29
.manage-column
css selector of each <th>
uses :hidden
to check hidden table header's:
hidden : function() { return $('.manage-column').filter(':hidden').map(function() { return this.id; }).get().join(','); },
manageedit-{$post_type}columnshidden
ajax should not check :hidden
table columns but only checked input fields in the screen options form to better seperate presentation and data handling. This way cb
and title
should also never get added to manageedit-{$post_type}columnshidden
.
I will run some tests to make sure this is in fact it and try to write a patch later this weekend. My jquery is average at best, so if anyone else can figure this out be my guest!
#4
@
10 years ago
Reproduced and the column state stayed in the messed up state http://i.imgur.com/alSoT3l.png
I had to search the core for finding solution to restore the checkbox and title column :D
Removing manageedit-postcolumnshidden
row from wp_usermeta
database table helped.
Screencast: https://youtu.be/lZKjUpFx_yc
#6
@
6 years ago
- Keywords has-patch added; needs-patch removed
Based on @codecandid 's findings, I added a patch.
In addition to the code fix in common.js, I made a few more changes. I thought it is better to handle the logic to hide the columns at server-side in addition to client-side.
From what I have tested, this patch seems to fix the issue.
By the way here is the original plugin that I was trying to make that I first noticed the problem.
https://github.com/bryanwillis/sticky-admin-table-headers