Multiple add media buttons all have same ID attribute

[helgatheviking]

If you add an editor to a metabox (using wp_editor()) and that editor has media buttons then the content's media buttons and the new media buttons have the same ID, because the media buttons link has the same ID no matter what, which is invalid markup.

The source of the problem is the media_buttons() function:

/**
 * Adds the media button to the editor
 *
 * @since 2.5.0
 *
 * @param string $editor_id
 */
function media_buttons($editor_id = 'content') {
	$post = get_post();
	if ( ! $post && ! empty( $GLOBALS['post_ID'] ) )
		$post = $GLOBALS['post_ID'];

	wp_enqueue_media( array(
		'post' => $post
	) );

	$img = '<span class="wp-media-buttons-icon"></span> ';

	echo '<a href="#" id="insert-media-button" class="button insert-media add_media" data-editor="' . esc_attr( $editor_id ) . '" title="' . esc_attr__( 'Add Media' ) . '">' . $img . __( 'Add Media' ) . '</a>';

	/**
	 * Filter the legacy (pre-3.5.0) media buttons.
	 *
	 * @since 2.5.0
	 * @deprecated 3.5.0 Use 'media_buttons' action instead.
	 *
	 * @param string $string Media buttons context. Default empty.
	 */
	$legacy_filter = apply_filters( 'media_buttons_context', '' );

	if ( $legacy_filter ) {
		// #WP22559. Close <a> if a plugin started by closing <a> to open their own <a> tag.
		if ( 0 === stripos( trim( $legacy_filter ), '</a>' ) )
			$legacy_filter .= '</a>';
		echo $legacy_filter;
	}
}
add_action( 'media_buttons', 'media_buttons' );

[ericlewis]

We can't get rid of an ID here, since we need a unique identifier to pass to the wp_editor() options hash for tabfocus_elements.

In attachment:28090.diff​, introduce a $static variable to keep track of the ID attribute for the anchor, and set tabfocus_elements to insert-media-button-1 in edit-form-advanced.php.

[wonderboymusic]

In 28837:

In media_buttons(), add a static var $instance to increment the id attribute of insert-media-button-%d on each call.

Props ericlewis.
Fixes #28090.

[nacin]

I suspect someone may be relying on the first button being insert-media-button. I have two alternative solutions:

• Is there a reason why IDs are needed at all here for all subsequent instances?

• Or, can we make it so the first one is insert-media-button, and all later ones receive an ID appended?

[helen]

• Is there a reason why IDs are needed at all here for all subsequent instances?

Sounds like an ID is required for tabfocus.

• Or, can we make it so the first one is insert-media-button, and all later ones receive an ID appended?

Agree.

[pento]

attachment:28090.2.diff​ sets the first button's id to insert-media-button, subsequent button ids to insert-media-button-1, -2, etc.

I briefly considered starting at -2, but it implied that there should be a -1 somewhere.

[nacin]

Let's go with 28090.3.diff​. Another core or lead dev can come by and commit this if it suits them.

[nacin]

Fine to be committed.

[SergeyBiryukov]

In 29671:

Restore 'insert-media-button' as an ID attribute for the first instance of media_buttons(). Remove IDs for subsequent instances.

props nacin.
fixes #28090.