Opened 15 years ago
Last modified 7 years ago
#18954 new enhancement
Automatically whitelist screen options added via add_screen_option()
| Reported by: | mdawaffe | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | Plugins | Version: | |
| Severity: | normal | Keywords: | has-patch |
| Cc: | Focuses: | administration |
Description
set_screen_options() currently maintains a hardcoded whitelist of screens that can set options.
The list is extendable via the filter:
apply_filters('set-screen-option', false, $option, $value);
but that's sort of a pain. If a plugin calls add_screen_option(), it should be added to the whitelist automatically.
Also note that set_screen_options() is called so early, there's no convenient place for plugins to add to the set-screen-option filter. The best workaround I've been able to come up with is to manually call set_screen_options() again inside the page's load-$hook.
Example naive plugin code that does not work in <=3.3.
# Inside a menu page's load-$hook // Display per-page settings in screen options add_screen_option( 'per_page', array( 'label' => __( 'Things' ) ) );
Attachments (1)
Change History (12)
#6
@
12 years ago
- Keywords has-patch added; needs-patch removed
What's the purpose of the whitelist? Patch submitted with whitelist removed.
#7
@
12 years ago
Would love to see this as well. Spent the last couple hours digging through core to implement custom screen options for a plugin. I had to bypass add_screen_option() entirely, use the screen_settings filter to create the HTML form fields, and use the check_admin_referer action to hook into saving the custom form fields since. The first call in set_screen_options() is check_admin_referer(), so hooking into the action called at the end of that function seemed to be the only way to get access to my fields before the redirect happened.
#8
@
12 years ago
Also, we should be able to call add_screen_option() and have it either rendered by the render_per_page_options() function, or be able to pass a callback to render the option ourselves (probably a better solution).
Maybe a more encompassing solution would be similar to the settings API, with functions for add_screen_group(), register_screen_option(), etc.
#9
@
11 years ago
The filter for updating unknown options in misc.php,
$value = apply_filters( 'set-screen-option', false, $option, $value );
has a major flaw. It implies plugins should return false by default, rather than $value. If more than one plugin is hooking in, they will wipe out each other's options if they return false on any $option they don't recognize, which I've seen in several tutorials online.
The filter is followed by:
if ( false === $value ) return; // ... update_user_meta($user->ID, $option, $value); wp_safe_redirect(...);
Because it's an early hook, plugins have to hook in universally, unless they parse $_GET['page'].
Conflicting goals:
- Do not save unconfirmed data to the db.
- Plugins should pass through data that does not belong to them.
I can write a patch to add a hook if people think it would be accepted:
$value = apply_filters( "set-screen-option_$option", false, $value );
Then maybe deprecate the old filter (can we do that before a redirect?), but in any case let the new filter override it.
#10
@
11 years ago
Also, I have a working sample plugin that explains the somewhat crazy hook stuff needed to work with Screen Options, here: https://wordpress.org/support/topic/howto-adding-screen-options-tab-to-wp_list_table. Sorry for the bugspam.
#11
@
11 years ago
One more comment - I reconsidered and plugins are correct to send back $status (which is false) for unknown options. The correct way to prevent conflicts is for plugins to do
if ( ! empty( $_GET[ 'page' ] ) && $menu_slug === $_GET[ 'page' ] ) {
add_filter( 'set-screen-option', ..., 10, 3);
}
And use the same $menu_slug in add_menu_page().
Still think my new filter would be the best solution, since it would prevent plugins going nuts on the input.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
We may be able to get away with killing this whitelist come 3.4.