Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 10 years ago

#24786 closed defect (bug) (invalid)

filter set-screen-option not triggering

Reported by: robbcaldwell's profile robbcaldwell Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5.2
Component: Administration Keywords:
Focuses: Cc:

Description

Displaying my own table of records, I want to have a "Per Page" option in the Screen Options pull down. So I setup my option:

function add_options()
{
    $option = 'per_page';

    $args = array(
        'label' => 'Properties',
        'default' => 20,
        'option' => 'my_custom_per_page'
    );

    add_screen_option( $option, $args );
    add_filter('set-screen-option', 'my_custom_set_option', 10, 3);
}

function my_custom_set_option($status, $option, $value)
{
    return $value;
}


This works fine, the option is created and displayed in the Screen Options pull down. However, my_custom_set_option is NEVER called and when you change the value of my_custom_per_page in the Screen Options, it will revert back to its original value and never save.

Furthermore, looking at the core code, wp-admin/includes/misc.php, line 361, version 3.5.2

$value = apply_filters('set-screen-option', false, $option, $value);

$value is always returned as false.

Looking up the apply_filters function, wp-includes/plugin.php, line 137

function apply_filters($tag, $value)

The second parameter of apply_filters is $value, but passed 'false'

If I comment out this line (361) it saves fine.

Shouldn't line 361 be more like:

$value = apply_filters('set-screen-option', $value, $other_arguments);

Change History (10)

#1 @AntoineH
12 years ago

I'm having the same problem in my plugin. Would be nice if there was a response to this bug report.

#2 @DrewAPicture
12 years ago

  • Keywords close added

I think this is expected behavior -- that the filter defaults to false.

Also, is there a particular reason why you're adding the filter right after adding the screen option?

I only ask because moving the add_filter() call outside of add_options() works and saves as expected for me, with add_options() hooked to the in_admin_header action.

<?php
function add_options() {
        $option = 'per_page';

        $args = array(
                'label' => 'Properties',
                'default' => 20,
                'option' => 'my_custom_per_page'
        );

        add_screen_option( $option, $args );
}
add_action( 'in_admin_header', 'add_options' );

function my_custom_set_option( $status, $option, $value ) {
        return $value;
}
add_filter( 'set-screen-option', 'my_custom_set_option', 10, 3 );
Last edited 12 years ago by DrewAPicture (previous) (diff)

#3 follow-up: @DrewAPicture
12 years ago

  • Keywords reporter-feedback added

#4 in reply to: ↑ 3 ; follow-up: @robbcaldwell
12 years ago

Replying to DrewAPicture:

My add_options() and my_custom_set_option() functions are part of a class, therefore calling the add_filter outside, in your example, is not an option, unless I call it after I run add_options() during my class construction. I will have to give it a try.

#5 @robbcaldwell
12 years ago

  • Keywords reporter-feedback removed

#6 in reply to: ↑ 4 @DrewAPicture
12 years ago

Replying to robbcaldwell:

Replying to DrewAPicture:

My add_options() and my_custom_set_option() functions are part of a class, therefore calling the add_filter outside, in your example, is not an option, unless I call it after I run add_options() during my class construction. I will have to give it a try.

My question was more along the lines of why you want to filter it right after setting the default. From a logic standpoint, the filter should be called separately anyway. I still can't figure out why you'd want to add the filter inside of add_options().

#7 @SergeyBiryukov
12 years ago

  • Component changed from General to Administration
  • Keywords close removed
  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Which action add_options() is hooked to in your example? Looks like you're adding the filter too late, it should not be in the same function as add_screen_option().

The example from comment:2 works fine. You can do the same with class methods, it's just a matter of using the correct hooks and the appropriate calling sequence.

#8 follow-up: @tychay
10 years ago

To expand on this (for future people finding this in search).

If add_screen_option() works then it's too late in your code for add_filter('set-screen-option') to ever be triggered.

As you noted, the filter for set-screen-option is defined in misc.php where it is triggered in set_screen_options() which is called in the admin bootstrap (admin.php) almost 100 lines before set_current_screen() which is needed for add_screen_option() to work (since it needs the $current_screen).

Put another way, add the filter very early in your code (notably before admin_init) so it will actually be triggered. Then have it try to get_current_screen() and see that it is null. That's how early set-screen-option gets triggered.


It is common for you to add_screen_option()/screen::add_option() after you know what the screen identifier is so it doesn't trigger on everything…

        public function create_admin_menu()
        {
                $this->_options_suffix = add_options_page( 'page title', 'menu title', 'manage_options', 'SLUG', array( $this, 'show_settings_page' ) );
                if ( $this->_options_suffix ) {
                        add_action( 'load-'.$this->_options_suffix, array($this, 'loading_settings_page') );
                }
        }
        public function loading_settings_page() {
                $screen    = get_current_screen();
                $screen->add_option(
                        'per_page', // built-in type
                        array(
                                'label'   => $optinfo['display'], // Label to use in screen_options
                                'default' => $optinfo['default'], // default # when empty
                                'option'  => $optinfo['name'],    // db option name
                        )
                );
        }

but you’ll need to add the set-screen-option very early: wp-loaded and init is fine but before admin_init. In the example above, I call the function that adds both…

        public function run_admin() {
                add_action( 'admin_menu', array( $this, 'create_admin_menu' ) );
                add_filter( 'set-screen-option', array( $this, 'filter_screen_option'), 10, 3 );
                //add_action( 'admin_init', array( $this, 'admin_init') );
        }

in a hook on plugins_loaded.

Last edited 10 years ago by tychay (previous) (diff)

#9 in reply to: ↑ 8 ; follow-up: @AntoineH
10 years ago

Replying to tychay:

To expand on this (for future people finding this in search).
[ .. ]
in a hook on plugins_loaded.

I followed the steps in your post, but it is still not working in my plugin. Call me stupid, but I just don't get it. And I have no idea on how to debug this to see what I am doing wrong. Do you have any tips?

The function attached to set-screen-option is never called. The correct meta_key is created in my wp_usermeta data, but it remains an empty value. If I change the value in the database then the page shows the correct value, but upon saving the value in the admin page, the DB record is empty again and the page shows the default value.

#10 in reply to: ↑ 9 @kitchin
10 years ago

Replying to AntoineH:

I followed the steps in your post, but it is still not working in my plugin.

I just posted a sample plugin on GitHub, linked at https://wordpress.org/support/topic/howto-adding-screen-options-tab-to-wp_list_table. If you comment there, and maybe post a gist of your work, I can respond.

Note: See TracTickets for help on using tickets.