Allow make_clickable() to add target="_blank"

[nicolamustone]

I use quite often the function make_clickable() but i can't use it like i would because it does not allow to add the target attribute.

The patch attached adds a second optional attribute to make_clickable() to allow to add this attribute by passing a bool value to the function (false by default).

If true, it will perform a str_replace on the HTML anchor before to return it to add the target="_blank" attribute.

[joedolson]

Thanks for the contribution, Nicola! However, for accessibility, I strongly recommend against this; links should not open in new windows except in very special cases, and implementing a method to do this globally is something I don't think is a good idea.

[nicolamustone]

Replying to joedolson:

Thanks for the contribution, Nicola! However, for accessibility, I strongly recommend against this; links should not open in new windows except in very special cases, and implementing a method to do this globally is something I don't think is a good idea.

Hey joedolson, thanks for sharing your idea about this.
Why do you think is not good for accessibility?

You can still use it without the second attribute, which by default is setted to false, so you can completely ignore it if you don't need it explicitely.

[joedolson]

Yes, but if you do set it explicitly, then you're applying the attribute globally (unless I misunderstand what you're doing). If you're making all URLs that are automatically linked also open in a new tab, then you're definitely not considering them individually according to whether they're in a situation that would be appropriately opened in a new window.

In almost all cases, links should not be opened in new tabs for accessibility, so I don't think it's a wise to make it easier to do it without careful consideration of context.

[nicolamustone]

Not sure why you say that i'm applying it globally.

Let me do an example of where/when i would use it.

I have a given file URL, escaped and all. In this case, i would open the URL in a new tab.

make_clickable( $file_url, true );

If instead i have a post URL, i'd not open it in a new tab, so i would make

make_clickable( $post_url );

If i have these 2 line of codes on the same page, they won't open both in a new tab, but only the first one, right?

[Daniel Koskinen]

@nicolamustone: I'm jumping in with my own thoughts here. Regarding your example, perhaps in your case it would be fine, but my understanding is that make_clickable is usually meant to be applied to blocks of arbitrary text where you do not necessarily know what the URL or URL's' are beforehand (or even if there are any URL's at all, because they're coming from user-inputted content). They might be even links within the current site.

If you have a variable containing only a URL, I would suggest simply wrapping it in HTML yourself.

[nicolamustone]

Replying to Daniel Koskinen:

@nicolamustone: I'm jumping in with my own thoughts here. Regarding your example, perhaps in your case it would be fine, but my understanding is that make_clickable is usually meant to be applied to blocks of arbitrary text where you do not necessarily know what the URL or URL's' are beforehand (or even if there are any URL's at all, because they're coming from user-inputted content). They might be even links within the current site.

If you have a variable containing only a URL, I would suggest simply wrapping it in HTML yourself.

That makes sense, but in my specific case i use that on a custom field which could be simple text, numbers, or URLs. In the case that is an URL, it needs to open in a new window, always.

It's something similar to what you described, but it will only contain one URL, just i can't know if it's an URL or not, so i can't use HTML manually.

[joedolson]

It's not difficult to test whether or not a string is a URL; assuming you're working with a PHP version above 5.2.0, you can use this:

if (filter_var($field, FILTER_VALIDATE_URL) === FALSE) {
   $return = "<a href='" . esc_url( $field ) . "'>" . esc_url( $field ) . "</a>";
} else {
   $return = esc_html( $field );
}

​http://php.net/manual/en/function.filter-var.php

I think that your use case is an outlier use case, and probably not ideal for core.

[nicolamustone]

I guess we can close this then :)

thanks for reviewing!