Customize media control button labels should automatically reflect the specified mime type

[celloexpressions]

[39234] updated the button labels for the header video control to reflect that it's used for videos, rather than media in general. However, this approach only fixes this one control instance, and is something that should be brought to all media controls.

For 4.7, we should add an inline note where [39234] is referencing this ticket and explaining that in the future all media controls will default to labeling the buttons based on the mime type (and as a reminder to remove those overrides). Then, this ticket can transition to a future release/4.8 enhancement for updating the default button labels to use the mime_type where possible, and fall back to the existing defaults with media. We'll need an array of known mime types mapped to the corresponding media type labels and should be able to substitute those into each of the default button label strings in the control constructor.

[westonruter]

In 39237:

Customize: Only show video header controls if previewing front page; show explanatory notice when controls are hidden.

Also include todo for the header_video control's button_labels. See #38796.

Props westonruter, joemcgill, celloexpressions.
Fixes #38778.

[Christian1012]

Added 38796.0.diff​.

I don't have tests setup locally for trunk, plus I noticed that tests for WP_Customize_Control are still todos. I could write tests but would need a little help on where to put them. Plus I didn't want to write too many until I got feedback on the first pass.

/**
 * Test WP_Customize_Media_Control::get_default_button_labels() when mime type is 'video'.
 * 
 * @see WP_Customize_Media_Control::get_default_button_labels()
 */
public function test_get_default_button_labels_with_video_mime_type() {
        // Decalare expectations.
        $expected = array(
                'select'       => 'Select Video',
                'change'       => 'Change Video',
                'default'      => 'Default',
                'remove'       => 'Remove',
                'placeholder'  => 'No video selected',
                'frame_title'  => 'Select Video',
                'frame_button' => 'Choose Video',
        );

        // Act.
        // Assumes TestCase container has wp_customize property, instance of WP_Customize_Manager.
        // See /trunk/tests/phpunit/tests/customize/control.php for example that this is based on.
        do_action( 'customize_register', $this->wp_customize );
        $control = new WP_Customize_Media_Control( $this->wp_customize, 'nosetting', array(
                'settings' => array( 'nosetting' ),
                'mime_type' => 'video',
        ) );
        
        // Unit.
        $this->assertEquals( $expected, $control->get_default_button_labels() );

        // Functional.
        $this->assertEquals( $expected, $control->button_labels );
}

[westonruter]

@celloexpressions thoughts on 38796.0.diff​?

[celloexpressions]

Add translator comments for placeholders and reuse automatic labels for image controls

[celloexpressions]

38796.1.diff​ builds on 38796.0.diff​ to remove the type-specific label overwrites for WP_Customize_Image_Control and add translator comments for placeholders. This should be ready for commit.

[westonruter]

Fix reference to undefined variable $label

[westonruter]

Humm. I wonder actually if this refactoring is going to cause problems for translators. For example, there may be languages with verb/object agreement whereby the verb needs to change based on the object being acted upon. This is the same problem we had with trying to translate strings with post type names embedded in them. See #37895. Need input from i18n.

[westonruter]

Unfortunately this isn't going to work for translation purposes. Is there an alternative refactor that would settle on a better way to keep all of the 4 sets of strings distinct?

[celloexpressions]

I'm not going to be available to investigate further in the near future in terms of patching. My thought would be to just write out all of the strings for the four primary media/file types here, rather than trying to split out the one word. That fixes this issue and also leaves room for continued future improvement.

[Christian1012]

My thought would be to just write out all of the strings for the four primary media/file types here, rather than trying to split out the one word. That fixes this issue and also leaves room for continued future improvement.

Added 38796.3.diff​.

Supports full translatable strings for the three supported mime types (audio, video, image) and the default (file) without tokens. I believe this is what @celloexpressions and @westonruter are advocating.

[celloexpressions]

Looks good, @Christian1012, thanks!

[netweb]

This ticket has a patch, setting the owner to that of the person who added the good-first-bug keyword, this ticket update causes the ticket to be removed from the "Unclaimed" section of the Good First Bug Trac Report 44.

[westonruter]

In 41550:

Customize: Let media control button labels better automatically reflect the specified MIME type.

Props Christian1012, celloexpressions, westonruter.
Fixes #38796.