[wp-trac] [WordPress Trac] #30073: Dynamic filters/actions using WP_Screen->id can lead to some unexpected results for plugins using them.

[WordPress Trac]

#30073: Dynamic filters/actions using WP_Screen->id can lead to some unexpected
results for plugins using them.
----------------------------+-----------------------------
 Reporter:  nerrad          |      Owner:
     Type:  defect (bug)    |     Status:  new
 Priority:  normal          |  Milestone:  Awaiting Review
Component:  Plugins         |    Version:  4.0
 Severity:  normal          |   Keywords:
  Focuses:  administration  |
----------------------------+-----------------------------
 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.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/30073>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform