Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#30073 closed defect (bug) (duplicate)

Dynamic filters/actions using WP_Screen->id can lead to some unexpected results for plugins using them.

Reported by: nerrad's profile nerrad Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.0
Component: Plugins Keywords:
Focuses: administration Cc:

Description

Coming up with a good summary title for this is a bit difficult.

What do I mean by "dynamic filter"

Here's an example taken from the WP_List_Table class:

$views = apply_filters( "views_{$this->screen->id}", $views );

So basically that allows one to add extra views only on a certain admin screen.

The Issue

There is a part of the screen_id that is generated in part from an array in the $admin_pages_hooks global. This is used for the page prefixes. For core wp routes this doesn't apply so much, but for any plugins adding custom admin pages using add_menu_page there is this line that potentially causes problems:

$admin_page_hooks[$menu_slug] = sanitize_title( $menu_title );

What that is basically doing is setting what eventually become the screen id hook prefix to be a sanitized slug of whatever was sent in as the menu_label.

The issue is that the menu label is very likely internationalized, which means that unless plugin authors are aware of this, hardcoding the filters/actions they add for dynamic filters/actions that use screen_id won't work if that top level menu item's label is translated.

An example:

Say I do this:

add_menu_page( __('Some Page', 'i18n_slug'), __('Some Menu Label', 'i18n_slug'), 'some_cap', 'some-menu-slug', 'callback' );
add_submenu_page( 'some-menu-slug', __('Some Sub Page', 'i18n_slug'), __('Some Sub Page Menu Label', 'i18n_slug'), 'some_cap', 'some_sub_menu_slug', 'callback' );

The above setup means that the registered submenu page will have a $screen->id of some-menu-label_page_some_sub_menu_slug.

So let's say there's a custom WP_List_Table setup on this subpage. But another dev, develops an addon for this plugin which adds a custom view to this custom WP_List_Table. So they do something like this:

add_filter( "views_some-menu-label_page_some_sub_menu_slug", 'callback' );

And therein is the problem. That will only work when the "Some Menu Label" is not translated.

Of COURSE I understand there is a workaround, in that the addon developer could just dynamically setup their hook. But that's not easily apparent and easily missed because it's not obvious that the screen_id could be different because of translated values.

So what to do?

Proposed potential Solutions?

  1. Don't localize the screen_id prefix.
  2. Allow screen_id to be explicitly set via add_menu_page, add_submenu_page (and if not set, leave existing methods in place for backward compat).

So I've classified this as a bug, but it's kinda not really a bug it's more of a quirk that if developers aren't aware of could cause headaches trying to resolve.

No worries if this is seen as a non-issue cause frankly it is possible to workaround. However might be something worth looking into improving in later iterations.

Change History (2)

#1 @SergeyBiryukov
11 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #18857, #21454, #24274, #27732.

#2 @nerrad
11 years ago

sorry for the dupe didn't find anything in my trac search.

Note: See TracTickets for help on using tickets.