do_meta_boxes() does not display correctly with screen names that contain charaters outside sanatize_key()'s limits

[GregRoss]

Specifically the "Screen Options" tab is not displayed.

This is because WP_Screen sanitizes the slug where as add_meta_box() does not, resulting in each looking for a different key in the $wp_meta_boxes global.

When WP_Screen->show_screen_options() checks the list of meta boxes for the screen it does not find any and does not display the "Screen Options" tab as it looks for the sanitized slug but the unsanitized slug was to populate the global.

To make matters worse, add_menu_page()'s default behaviour is to return a screen name in the format of "toplevel_page_" + plugin_base(), which will include a / between the directory name and the file name of the plugin's main php.

There are a few options on what to do to resolve this:

1. Change the call to sanitize_key() to something else to support more characters in the WP_Screen->id field.

2. Change add_menu_page() (and probably add_submenu_page as well) to use the same logic and call sanitize_key() on the slug before it is returned.

3. Update the documentation and note the incompatibility but leave it be.

Option 1 and 2 both have significant impact on existing code and plugins.

In option 1 any code that relies on the slug being safe to use in SQL or other places would have to be reviewed.

In option 2 any code the has hard coded a menu slug based on the current behaviour would have to be updated to use the new slug that is returned by the code.

Option 3 is probably the safest :)

[MikeHansenMe]

@GregRoss do you want to see if there is a way to write a patch that takes back-compat into consideration?

[GregRoss]

@MikeHansenMe I doubt the effort to try and resolve this through code is worth it.

Option 1 is easy enough to do, just remove the sanitize_key() call in WP_Screen object and it works, however I'm sure there must be a reason for it to be there so likely other things will break. Replacing sanitize_key() with something more permissive would only be a partial solution so not a very good option.

Option 2 could break a lot of plugins for a corner case with do_meta_boxes(). That seems like a bad tradeoff.

I think the best solution is to update the docs for both add_menu_page()/add_submenu_page() and do_meta_boxes()/add_meta_box() to indicate the proper format for the menu_slug/screen_id.

I've attached a patch which updates the doc blocks to reflect this.

[GregRoss]

Update doc blocks.

[DrewAPicture]

In 40967:

Docs: Provide best practice guidance for achieving parity between $menu_slug values supplied when adding menu and submenu pages, and later trying to compare those initial values against sanitized screen IDs derived from $menu_slug.

At the heart of the matter, the $menu_slug parameter in add_menu_page() and add_submenu_page() is not sanitized with sanitize_key(). When the screen object is later built for the admin page, the screen ID is derived from that $menu_slug value, though passed through sanitize_key(), which can produce unexpected results in comparison check.

Changing the sanitization code to provide actual parity is out of the question at this juncture, so updating the docs to describe how to avoid this edge case is the next best option.

Props GregRoss.
Fixes #35305.