Focus in attachment custom fields is lost when updating the value.

[lucymtc]

I ran into a bug related to attachment custom fields in the media modal as well as the attachment edit screen.
It's an accessibility issue where the focus of the text inputs is lost when their value is modified, needing to click twice to update their value. Reproducible when having multiple custom fields.

This is caused by a re-render of the view used for this region, which is wp.media.view.AttachmentCompat

In the initialize function the view re-renders when the model changes.

initialize: function() {
	this.listenTo( this.model, 'change:compat', this.render );
}

Steps to reproduce:

• Create some custom attachment fields using the appropriated hooks attachment_fields_to_edit and attachment_fields_to_save
• Open the Medial Modal through Add Media button and click on an image to view details.
• Click on the one of the custom fields input and type any text.
• Then directly click on the next custom field input.
• Focus gets lost and needs to click again in the input in order to modify the value.

I don't see the reason for the view needing to be re-rendered, removing the listener fixes the issue.
If re-rendering is needed for a particular reason, a possible solution would be to keep track of the last focused input and re-focus on re-render.

The code was added in this commit ​https://github.com/WordPress/wordpress-develop/commit/57b09463e67

[lucymtc]

A temp fix for anyone that needs it is overriding initialize to stop listening for re-rendering.

/**
 * Extends wp.media.view.AttachmentCompat
 * Override initialize to stop listening the compat change to
 * prevent re-rendering the attachment compat view.
 * Fixes issue losing focus on atatchment custom fields in the modal.
 */

var OriginalAttachmentCompat = wp.media.view.AttachmentCompat;
wp.media.view.AttachmentCompat = OriginalAttachmentCompat.extend({

    initialize: function() {
        OriginalAttachmentCompat.prototype.initialize.apply( this, arguments );

        this.stopListening( this.model, 'change:compat', this.render );
    }
});

[syhc]

We (at work) have been running into the same problem.

The problem seems to happen when there is more than 1 custom field, and that you have tabbed to a new field before wp.media.model has save the previous field.

Our solution was to add an option to media-views.js AttachmentCompat.save function

this.model.saveCompat( data, {silent:'true'} )

I solved the problem as suggested by ​syhc in the Trac ticket #40909.

Trac ticket: https://core.trac.wordpress.org/ticket/40909

[afrin29]

Test Report

This report validates that the indicated patch doesn't address the issue.

Patch tested: ​https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/1585.diff

Environment

• OS: macOS 12.5.1
• Web Server: Nginx
• PHP: 7.4.33
• WordPress: 6.2-alpha-54642-src
• Browser: Safari 15.4
• Theme: Twenty Nineteen
• Active Plugins: No plugins activated

Actual Results

• ❌ Patch doesn't resolve the issue.

Additional Notes

• I have added two custom fields using twenty nineteen functions.php and I found this issue that I have to click two times into the second field after adding text into the field one. However, I have added the patch but still facing this same issue.

Supplemental Artifacts

Before: ​https://d.pr/v/thZKnH
After: ​https://d.pr/v/JMPpd8

Prevents loss of focus on 40909

Trac ticket: https://core.trac.wordpress.org/ticket/40909

[joedolson]

The PR fixes the loss of focus state by removing the 'initialize' method in AttachmentCompat.

Like @lucymtc, I'm unable to see any reason that this method is required, but would appreciate a second opinion from somebody who might know. @adamsilverstein @joemcgill, perhaps you know something?

Removing the method resolves the accessibility issue by allowing focus to flow without disruption.

The alternate suggestion by @lucymtc, monitoring the last focus, is not quite what we'd need to do, so the alternative gets more tricky - we'd actually need to monitor the expected next focus. The save runs when focus leaves the field but before focus moves to the next field, so setting focus back to the previous location would actually be confusing - you'd leave the field and then be immediately sent back to it.

[joedolson]

PHP snippet to add custom fields for testing:

add_filter( 'attachment_fields_to_edit', 'test_fields_40909', 10, 2 );
function test_fields_40909( $form_fields, $post ) {
	$field_1                = get_post_meta( $post->ID, 'field_1', true );
	$form_fields['field_1'] = array(
		'label' => __( 'Field 1' ),
		'input' => 'text',
		'value' => $field_1,
	);
	$field_2                = get_post_meta( $post->ID, 'field_2', true );
	$form_fields['field_2'] = array(
		'label' => __( 'Field 2' ),
		'input' => 'text',
		'value' => $field_2,
	);

	return $form_fields;
}

add_filter( 'attachment_fields_to_save', 'test_save_fields_40909', 10, 2 );
function test_save_fields_40909( $post, $attachment ) {
	if ( isset( $attachment['field_1'] ) ) {
		update_post_meta( $post['ID'], 'field_1', sanitize_text_field( $attachment['field_1'] ) );
	} else {
		delete_post_meta( $post['ID'], 'field_1' );
	}
	if ( isset( $attachment['field_2'] ) ) {
		update_post_meta( $post['ID'], 'field_2', sanitize_text_field( $attachment['field_2'] ) );
	} else {
		delete_post_meta( $post['ID'], 'field_2' );
	}

	return $post;
}

[joedolson]

That last PR was run against the wrong ticket ID. Whoops!

[joedolson]

After discussing this ticket in the core media component meeting, we've decided to commit it despite being unsure whether removing the initialize method could have side effects for extenders, to get as broad a reach as possible during the beta cycle to identify possible issues.

[antpb]

Left a comment in the PR

"This looks good. We should get this in asap to see if this has any impact on plugins using custom fields in media.

It would not surprise me if this is not doing anything and maybe once did."

[joedolson]

In 55299:

Media: Fix focus loss updating custom fields in media modal.

When the save action runs to dynamically save changes to custom fields in the media modal, the custom fields container is re-rendered, losing focus. Remove the re-rendering of custom fields to prevent loss of focus.

Props lucymtc, teebee, syhc, afrin29, antpb, joedolson.
Fixes #40909.

Fixed in https://core.trac.wordpress.org/changeset/55299