WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#12596 closed defect (bug) (invalid)

"screen options" tab disappearing for plugins

Reported by: arena Owned by: westi
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Plugins Keywords: reporter-feedback
Focuses: Cc:

Description

(works in 2.9 not in 3.0)

Unable to create a 'screen options' tab for plugin admin pages.

In 2.7, filter ’screen_meta_screen’ was created to give the ability to name the $screen variable for plugins. (http://core.trac.wordpress.org/changeset/9788)

In 3.0 the code sequence has changed in wp-admin/includes/template.php, for function screen_meta

new code is :

if ( is_string($screen) ) $screen = convert_to_screen($screen);

Result is that with the "is_string" test, for plugins admin pages, "convert_to_screen" function is never called and filter ’screen_meta_screen’ is never called ...

Consequence is : no screen options tab and corresponding div’s for plugin admin pages.

Attachments (4)

sample1 W2.9.jpg (73.6 KB) - added by arena 5 years ago.
sample1 W2.9
sample1 W3.0.jpg (49.6 KB) - added by arena 5 years ago.
sample1 W3.0
sample2 W2.9.jpg (77.8 KB) - added by arena 5 years ago.
sample2 W2.9 this one with meta_boxes
sample2 W3.0.jpg (60.4 KB) - added by arena 5 years ago.
sample2 W3.0 this one with meta_boxes

Download all attachments as: .zip

Change History (27)

comment:1 @dd325 years ago

From what i can see, all the referenced locations to the code you mention were added in their current location in:

[12728] ryan 16/01/2010 3:58:36 AM Merge edit-pages.php into edit.php. see #9674

Maybe you could attach a example plugin which uses the functionality which previously worked, and no longer does?

comment:2 follow-up: @TobiasBg5 years ago

  • Keywords reporter-feedback added
  • Priority changed from high to normal
  • Severity changed from critical to normal
  • Summary changed from 'screen options' tab desappearing for plugins to "screen options" tab disappearing for plugins

The screen options tab is actually not hidden because of that mentioned additional check.

It is hidden, because you are not using any meta boxes on your page.

If you use add_meta_box(...) (before the show callback for your submenu/menu page is executed, thus e.g. in load-$page_hook), you will see that the screen options tab is there.

Also, please describe what you are actually trying to achieve with that filter. If you just want to change the name of the current screen, you can also use set_current_screen() for that.

Pending reporter-feedback, tempted to close as worksforme.

comment:3 in reply to: ↑ 2 @arena5 years ago

Wrong !

what about admin pages with a list and columns you want to hide or display ...

comment:4 @TobiasBg5 years ago

Ok, forgot about that use case of the "screen options" tab.

Nevertheless, it would be very helpful, if you could answer the questions from above and would provide a more detailed explanations on what was working in WP 2.9 but is not in WP 3.0.

@arena5 years ago

sample1 W2.9

@arena5 years ago

sample1 W3.0

@arena5 years ago

sample2 W2.9 this one with meta_boxes

@arena5 years ago

sample2 W3.0 this one with meta_boxes

comment:5 @arena5 years ago

The plugin is MailPress

comment:6 @arena5 years ago

@dd32 sequence code has changed from 2.9 to 3.0

if ( is_string($screen) ) always return false for admin plugin pages ...

so no chance to go to convert_to_screen($screen); where filter ’screen_meta_screen’ applies to set the value of variable $screen.

@TobiasBg

set_current_screen ends 'current_screen' filter is a wrong options as 'current_screen' requires an object to be returned ...

@all,

Good patch is in in wp-admin/includes/template.php,

for function screen_meta

$screen = apply_filters('screen_meta_screen', $screen);
if ( is_string($screen) )

$screen = convert_to_screen($screen);

for function convert_to_screen

$screen = apply_filters('screen_meta_screen', $screen);

comment:7 @arena5 years ago

there is apparently an issue for the initialisation of javascript variables pagenow and adminpage ...

comment:8 follow-up: @TobiasBg5 years ago

Thanks for the clarification.

Could you describe a little more, what you are trying to achieve with the "screen_meta_screen" filter?

Also, for the set_current_screen() function: You don't have to use any filter in it. Just call the function with your wanted screen ID string.

Also, are you making all relevant actions (registering meta boxes, registering those table columns) before the "screen options" tab is produced, i.e. in a hook that is called before the one you specify in the add_submenu_page calls?

comment:9 follow-up: @TobiasBg5 years ago

To clarify my first question: From your patch suggestion, it seems to me that you are trying to set the screen ID with the "screen_meta_screen" filter, in which case "set_current_screen()" is the better solution, as far as I have experienced.

comment:10 in reply to: ↑ 8 @arena5 years ago

Replying to TobiasBg:

Thanks for the clarification.

Could you describe a little more, what you are trying to achieve with the "screen_meta_screen" filter?

Also, for the set_current_screen() function: You don't have to use any filter in it. Just call the function with your wanted screen ID string.

Also, are you making all relevant actions (registering meta boxes, registering those table columns) before the "screen options" tab is produced, i.e. in a hook that is called before the one you specify in the add_submenu_page calls?

I do works since 2.7 for my plugin

comment:11 in reply to: ↑ 9 @arena5 years ago

Replying to TobiasBg:

To clarify my first question: From your patch suggestion, it seems to me that you are trying to set the screen ID with the "screen_meta_screen" filter, in which case "set_current_screen()" is the better solution, as far as I have experienced.

"screen_meta_screen" was working in previous version so this is a bug !!

calling "set_current_screen()" whenever in my code is not the solution at all.

"screen_meta_screen" by giving the screen a specific name used to have that screen properly set in javascript and php (javascript 'screen' name used in setting the columns in a list or ordering meta boxes in a page, i am using both of them) ..

the plugin is mailpress !

comment:12 follow-up: @TobiasBg5 years ago

I haven't yet have time to look at your plugin and I'm only saying all of this from recent experience when working with the metaboxes. I, too, had the problem to be in need of setting the screen ID which resembles the ID the JavaScript uses to store/ajax the information about the state of the meta boxes. My first idea was to use the "screen_meta_screen" filter, but then I basically found out what you mention in the initial post: that it is not called regularly so that I could rely on it. Then I found out about the "set_current_screen()" function. At first I wanted to hook into it, but couldn't really find a good hook in there. That's when I noticed that I could just call the function myself, with my desired screenID/pagenow as the parameter - and all my problems were solved.

What I'm trying to say: Also the filter "screen_meta_screen" might have worked before, it is not necessarily a bug if it doesn't anymore, because it might not have been used in the intended fashion (which I don't know, as I'm not too familiar with the background of that part of the code).

"screen_meta_screen" by giving the screen a specific name used to have that screen properly set in javascript and php (javascript 'screen' name used in setting the columns in a list or ordering meta boxes in a page, i am using both of them) ..

That's exactly what I'm doing with set_current_screen() as outlined above. Instead of setting it with the filter, just set the screen with that function, called in the load-{$hook} filter of the admin page.

comment:13 in reply to: ↑ 12 ; follow-up: @arena5 years ago

Replying to TobiasBg:

I haven't yet have time to look at your plugin and I'm only saying all of this from recent experience when working with the metaboxes. I, too, had the problem to be in need of setting the screen ID which resembles the ID the JavaScript uses to store/ajax the information about the state of the meta boxes. My first idea was to use the "screen_meta_screen" filter, but then I basically found out what you mention in the initial post: that it is not called regularly so that I could rely on it. Then I found out about the "set_current_screen()" function. At first I wanted to hook into it, but couldn't really find a good hook in there. That's when I noticed that I could just call the function myself, with my desired screenID/pagenow as the parameter - and all my problems were solved.

What I'm trying to say: Also the filter "screen_meta_screen" might have worked before, it is not necessarily a bug if it doesn't anymore, because it might not have been used in the intended fashion (which I don't know, as I'm not too familiar with the background of that part of the code).

"screen_meta_screen" by giving the screen a specific name used to have that screen properly set in javascript and php (javascript 'screen' name used in setting the columns in a list or ordering meta boxes in a page, i am using both of them) ..

That's exactly what I'm doing with set_current_screen() as outlined above. Instead of setting it with the filter, just set the screen with that function, called in the load-{$hook} filter of the admin page.

'screen_meta-screen' worked in previous versions.
set_current_screen() only exists since 3.0 ...

comment:14 in reply to: ↑ 13 @arena5 years ago

Replying to arena:

Replying to TobiasBg:

I haven't yet have time to look at your plugin and I'm only saying all of this from recent experience when working with the metaboxes. I, too, had the problem to be in need of setting the screen ID which resembles the ID the JavaScript uses to store/ajax the information about the state of the meta boxes. My first idea was to use the "screen_meta_screen" filter, but then I basically found out what you mention in the initial post: that it is not called regularly so that I could rely on it. Then I found out about the "set_current_screen()" function. At first I wanted to hook into it, but couldn't really find a good hook in there. That's when I noticed that I could just call the function myself, with my desired screenID/pagenow as the parameter - and all my problems were solved.

What I'm trying to say: Also the filter "screen_meta_screen" might have worked before, it is not necessarily a bug if it doesn't anymore, because it might not have been used in the intended fashion (which I don't know, as I'm not too familiar with the background of that part of the code).

"screen_meta_screen" by giving the screen a specific name used to have that screen properly set in javascript and php (javascript 'screen' name used in setting the columns in a list or ordering meta boxes in a page, i am using both of them) ..

That's exactly what I'm doing with set_current_screen() as outlined above. Instead of setting it with the filter, just set the screen with that function, called in the load-{$hook} filter of the admin page.

'screen_meta-screen' worked in previous versions.
set_current_screen() only exists since 3.0 ...

and set_current_screen() is WEIRD !!!

see following lines where a string separated with a dash contains the content of 2 variables ... herk !

list( $id, $typenow ) = explode('-', $id, 2);

comment:15 follow-up: @nacin5 years ago

There's nothing wrong with that logic.

You're not giving us a lot to work on here. "It worked" and "It doesn't work" is not enough information. Nor are we going to comb our way through the codebase of a plugin such as MailPress.

Please upload a sample plugin that works on 2.9 and does not on 3.0, and shows *only* the bug you describe.

comment:16 follow-up: @arena5 years ago

I tried to use set_current_screen()
and called it on following hooks :

'init' => Call to undefined function set_current_screen()

'admin-init' => not working

'admin-head' => not working

more info required on how it should be coded !

comment:17 in reply to: ↑ 16 @arena5 years ago

Thanks !

comment:18 in reply to: ↑ 15 @arena5 years ago

Replying to nacin:

There's nothing wrong with that logic.

You're not giving us a lot to work on here. "It worked" and "It doesn't work" is not enough information. Nor are we going to comb our way through the codebase of a plugin such as MailPress.

Please upload a sample plugin that works on 2.9 and does not on 3.0, and shows *only* the bug you describe.

I did that on Ticket #11231 and this ticket has not changed since more than one month. Investing time with such a result do not gives me the motivation to invest time on this new bug ! sorry and thank you for your time !

comment:19 follow-up: @TobiasBg5 years ago

'screen_meta-screen' worked in previous versions. set_current_screen() only exists since 3.0 ...

Code changes... While the WP developers usually will do everything they can to maintain backwards compatibility, that is not always possible. Also, it should not be too hard to include a version check or a function_exists() check.

I tried to use set_current_screen() and called it on following hooks:

As I said: Use the load-$hook action, where $hook is the result of

$hook = add_(sub)menu_page(...)

Calling it earlier (in init/admin_init) won't work, because that is before the actual call of set_current_screen() by WordPress, which then overrides your call again. Calling it later (admin_head) also won't work, because all the magic regarding the tab has been done by then, as the output of the page has already started.

and set_current_screen() is WEIRD !!!

That may be true, but I'm pretty sure the original coder did the best he could to make it work effectively. While it can be argued that an additional in-between-step might make the code more readable, keep in mind that that part of the code is run pretty often (on every page load of the admin area) and therefore should be as quick as possible. And sometimes it is not always required to fully understand a function, just to be able to use it :-)

comment:20 in reply to: ↑ 19 @arena5 years ago

$hook = add_(sub)menu_page(...)

An admin page is not always the result of a menu item ...

comment:21 @TobiasBg5 years ago

Then consider the actions load-$plugin_page or load-$pagenow. One of the three now mentioned load-... will fire, otherwise it's not a plugin's admin page - if I understand wp-admin/admin.php correctly.

Also, please keep in mind that I'm just trying to help you, so turning down my suggestions without really trying (or, if you are, describing your progress), is not really motivating. I don't want to have to apologise for my suggestions on what you could try.

comment:22 @arena5 years ago

  • Resolution set to invalid
  • Status changed from new to closed

SOLVED at last !!! using 'current_screen' filter

Thank you for your time TobiasBg & nacin

comment:23 @nacin5 years ago

  • Milestone 3.0 deleted
Note: See TracTickets for help on using tickets.