Make WordPress Core

Opened 9 years ago

Closed 7 years ago

#31280 closed enhancement (wontfix)

Remove need for numeric keys for admin post screen messages

Reported by: f-j-kaiser's profile F J Kaiser Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.1
Component: Plugins Keywords: has-patch
Focuses: Cc:

Description

By now we can only use absolute integers for post screen messages to notify the user of something. This is forced right after the filter that allows plugins to add their own messages:

$messages = apply_filters( 'post_updated_messages', $messages );

The problem is the following line:

if ( isset($_GET['message']) ) {
	$_GET['message'] = absint( $_GET['message'] );

Now plugin devs can do an arms race with PHP_INT_MAX -1 as filter callback priority and zzzz_ prefixed callback names and then try to ++ their messages until they find a ! isset place to put their message. Not really ideal and as PHP allows mixed arrays, I'd suggest to simply allow strings there as well so everyone can just use for e.g. automattic-foobar as their key to avoid conflicts.

If there's support for this, I can come up with a patch.

Attachments (1)

0001-31280-patch-messages-for-admin-post-screens.patch (1.1 KB) - added by F J Kaiser 9 years ago.
Check for strings as well

Download all attachments as: .zip

Change History (5)

#1 @F J Kaiser
9 years ago

  • Keywords has-patch added

#2 @F J Kaiser
9 years ago

In case someone stumbles upon this ticket while searching for a solution of the problem, here's the temporary work around until this patch is applied to a stable core release. Note that this will only help you to quickly find the lowest unused index. As it's a moving target, you can't pass around a dynamic key/index from post_updated_messages to for e.g. save_post.

add_filter( 'post_updated_messages', function( $messages )
{
	$index = 5;
	$post_type = 'post';
	while ( isset( $messages[ $post_type ][ $index ] ) )
	{
		$index++;
	}
	$messages[ $post_type ][ $index ] = __( 'Your custom message', 'your_textdomain' );
	return $messages;
} );
Last edited 9 years ago by F J Kaiser (previous) (diff)

This ticket was mentioned in Slack in #core by kaiser. View the logs.


9 years ago

#4 @DrewAPicture
7 years ago

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

Unfortunately, the post updated messages "API" such as it is is very much legacy code that's likely beyond the scope of being fixed because back-compat concerns. Granted, you're effectively asking to add string support, but I'm not sure that's a great solution either – it would further complicate an already convoluted system.

Short of straight up replacing it (which would be my vote), perhaps through an admin notices extension of the Notifications API @johnbillion is interested in working on, I'm not sure introducing an additional vector through which extend post messaging support is the way to go.

Closing as wontfix.

Note: See TracTickets for help on using tickets.