wp_targeted_link_rel filter can result in rel attribute with no value

[spartank]

The Problem

A function for filter wp_targeted_link_rel that returns an empty string will result in a rel attribute without a value. It would be better in that case to not insert the rel attribute.

How To Reproduce

Create function attached to wp_targeted_link_rel that returns empty string.

function my_targeted_link_rel($rel_values) {
        return '';
}
add_filter('wp_targeted_link_rel', 'my_targeted_link_rel');

Resulting link will contain rel=""

<a href="https://www.google.com" target="_blank" rel="">Link</a>

Possible Solutions

Return original link from wp_targeted_link_rel_callback after applying wp_targeted_link_rel if $rel variable is empty.

[mcmwebsol]

patch for 45352

[spartank]

@mcmwebsol I'm new to contributing to core, so I may be wrong, but I feel like it would be a little simpler to put the change in the wp_targeted_link_rel_callback function, right after the apply_filters call.

function wp_targeted_link_rel_callback( $matches ) {
        $link_html = $matches[1];
        $rel_match = array();

        /**
         * Filters the rel values that are added to links with `target` attribute.
         *
         * @since 5.0.0
         *
         * @param string The rel values.
         * @param string $link_html The matched content of the link tag including all HTML attributes.
         */
        $rel = apply_filters( 'wp_targeted_link_rel', 'noopener noreferrer', $link_html );

//New If Statement      
//------------------------------
        if ( ! $rel ) {
                return $matches[0];
        }
//------------------------------
//The rest of the function as is

[pento]

[42770] is being reverted from the 5.0 branch, so I'm moving this ticket to 5.1, so it can be addressed there, instead.

[SergeyBiryukov]

45352.2.diff​ looks good at a glance.

filter_empty_targeted_link_rel() can be replaced with the existing __return_empty_string() function.

[spartank]

The proposed change makes it impossible to use a filter to not set a value for rel on links that open in a new tab. Why shouldn't this be possible as opposed to falling back to "noreferrer noopener" if the filter returns an empty string?

Shouldn't the test case be:

public function test_filter_non_empty_value() {
   add_filter('wp_targeted_link_rel', array( $this, 'filter_empty_targeted_link_rel' ) );
   $content  = '<p>Links: <a href="/" target="_blank">No rel</a></p>';
   $this->assertEquals( $content, wp_targeted_link_rel( $content ) );
   remove_filter( 'wp_targeted_link_rel', array( $this, 'filter_empty_targeted_link_rel' ) );
}

[peterwilsoncc]

I'd rather take the approach that if a developer clears wp_targeted_link_rel they don't wish to use the feature. 45352.3.diff​ returns early if that is the case.

@spartank to answer your question about the test: filters (along with actions and a few other globals) are reset by the test suite automatically between tests so there is no need for the remove_filter() call.

[peterwilsoncc]

45352.4.diff​ removes the unneeded empty() call.

[peterwilsoncc]

In 44691:

Formatting: Prevent wp_targeted_link_rel() adding an empty attribute.

Bypass adding a rel attribute when the wp_targeted_link_rel filter returns an empty string or other falsy result.

Props mcmwebsol, spartank, meatman89fs.
Fixes #45352.