Site Health: Allow 'target' in 'a' tags in the debug data description

[kraftbj]

When extending the debug data to include custom information for a plugin, it would be handy to include a link offsite to the plugin's support page. In this setup, I would like to open in a new tab instead of taking someone off of their site without a clean way to return.

Suggest adding target as an acceptable attribute for the a tag when running the description through wp_kses.

[jeherve]

Should we add rel in there too, so one can set rel="noopener noreferrer" for their links?

[jeherve]

Add rel to list of authorized attributes

[Clorith]

If we do this, we also need to allow span and class for screen reader text informing of new tabs opening and such.

[mukesh27]

If span is allow then can we add div p ?

[mukesh27]

Added span, div and p with class attribute

[Clorith]

Div and p are excessive and block elements, so I say no, it's overcomplicating things and making uneven outputs I would say, if your debug information is this excessive then you may not be categorising it properly.

[mukesh27]

@Clorith thanks for your information so needs add span with class right? and remove div and p from attached patch

[audrasjb]

Patch refresh, and add hreflang attribute and span element

[audrasjb]

Hi,

In 46878.3.diff:

• I refreshed the previous patch
• I added hreflang attribute since I think it could be helpful to provide information about the language of any linked document when different from the website language
• I added span element with class attribute

[audrasjb]

Adds span element to existing documentation for debug_information filter

[Clorith]

46878.4.diff​ looks good to me.

[pento]

@Clorith: Can you provide some context for why it's being run through KSES? Anything that's able to hook into the debug_information filter will be able to print its own arbitrary HTML, so it doesn't seem like there'd be any security benefit.

If a plugin wants to add ridiculous HTML, I don't think there's much we can realistically do about that. Instead of adding it directly to the description, they'd just have to write a little bit of JS to alter the HTML in the browser, instead.

[Clorith]

That was the intention, yes, to limit the potential for crazy markup and such.

This was mostly to maintain the sanity of the copy-paste output, since excessive markup would risk making that much less usable.

[pento]

WP_Debug_Data::format() should probably just run wp_strip_all_tags() on everything to avoid that problem.

[Clorith]

True, we can do that, my concern is someone adds a set of li elements, and when stripped by the formater it ends up being one long string with no separation for example.

Sure, this would then be on whomever added the list items for not making the consideration, but I also like to make it harder to mess up if possible, for everyone's benefit.

[Clorith]

I've been checking further on this, and after looking over the source I see some new elements have gone in that means this isn't as big of a concern any more after the introduction of the debug entry, so one can differentiate between the visual, and copied data.

Let's roll with what @pento suggested then as that solves the concerns I have been stuck with.

[azaozz]

Replying to Clorith:

Yep, I'm with pento on this as well. Generally WordPress lets plugins and themes do... anything they want in order to provide better user experience. Trying to restrict that would result in either the plugin using more "hacky" way to achieve the same result, or the user experience being not as good as it can be.

As most of the data for the copied info is generated separately, we can strip_tags() only in cases where we are reusing the "view" strings intended for the HTML. There is no point in restricting these strings when added by plugins.

Patch coming up.

[azaozz]

In 46878.5.diff:

• Do not use kses to sanitize section descriptions.
• Add a bit more docs explaining the expected format of the strings.

The section descriptions come directly from plugins, and are not used in the data for copying. There's no need to sanitize them as mentioned above.

The fields names and values are run through esc_html() and all of the data for copying is sanitized with esc_attr(). They do not support HTML tags. Don't think they need additional sanitization.

[azaozz]

In 45259:

Site Health: Allow some HTML (inline tags only) in the section descriptions. Add some more docs about expected formatting of the gathered data.

Props kraftbj, jeherve, mukesh27, audrasjb, azaozz.
Fixes #46878.