Woocommerce-admin: Support HTML in report tables/charts (particularly when added to `woocommerce_price_format`)

[KZeni]

Is your feature request related to a problem? Please describe.
I have the following implemented in my theme's functions.php file:

// Add "USD" to prices on the site to prevent confusion/assumptions which might result in surprise currency conversion rates (based on: https://www.codemyownroad.com/add-currency-code-suffix-prices-woocommerce/)
function amperageAddPriceSuffix($format, $currency_pos) {
    switch ( $currency_pos ) {
        case 'left' :
            $currency = get_woocommerce_currency();
            $format = '%1$s%2$s <span class="woocommerce-Price-currencyFormat">'.$currency.'</span>';
        break;
    }
    return $format;
}
add_action('woocommerce_price_format', 'amperageAddPriceSuffix', 1, 2);

which shows the currency code after prices to help prevent assumptions regarding the currency being used for international customers.

The currency code is wrapped in a <span class="woocommerce-Price-currencyFormat"> element to allow for that part to be styled on the resulting pages & potentially hidden on parts of the site where it isn't needed & would otherwise clutter things.

Unfortunately, the tables/charts in WooCommerce Admin Analytics reports outputs the HTML code as text. You can see an example of this here: https://cloudup.com/cr6346F0pZa

Describe the solution you'd like
I think it'd be safe to allow these parts of the report to allow HTML code to be rendered as HTML since it's pulling from a safe source. This then addresses the issue caused by someone with a use-case that's the same as / similar to mine (have HTML inserted into this content for legitimate reasons.)

Describe alternatives you've considered
I could adjust my code to not wrap the currency code in HTML, but that would prevent the site from hiding it & styling it as appropriate.

I'm thinking there are other cases where HTML might be added into this woocommerce_price_format hook so it seems like this should be something for this to address rather than having use-cases limited due to this setup's limitations (which everything else I've seen handles HTML being part of woocommerce_price_format just fine). So I'm not really aware of a solid alternative to what I've proposed above.

Should this be prioritized? Why?
Normal priority would be fine since the number of sites affected by this is limited and it isn't technically breaking anything in the meantime. However, I'm thinking it really should be addressed, and it might even be a reasonably fast/straightforward change to make.

Additional context
This is focusing on woocommerce_price_format having HTML content introduced since that's the issue I'm experiencing, but I'm thinking it might be something for the other fields being output to follow suit & not just the output of the price (though it might be a universal change anyway...? [not sure at this point.])

[KZeni]

Maybe this is a case where all output for this is getting sanitized. I see https://github.com/woocommerce/woocommerce-admin/blob/master/client/lib/sanitize-html/index.js has export const ALLOWED_TAGS = [ 'a', 'b', 'em', 'i', 'strong', 'p' ]; that might just need to be changed to export const ALLOWED_TAGS = [ 'a', 'b', 'em', 'i', 'strong', 'span', 'p' ]; to accomodate this.

I went with for my currency code wrapper since I didn't want to have it inherit any particular formatting. Meanwhile, I'm thinking having span allowed during HTML sanitization would be just as safe as the others already being allowed.

Then again, I might be looking in the wrong place for this.

[timmyc]

Thanks for the detailed report @KZeni - I believe the root cause of the HTML being sanitized here is React's core rendering functionality. To allow wholesale HTML to be rendered in this context, we would either need to use a whitelist approach ( akin to what you noted here ) or use dangerouslySetInnerHTML which as the name suggests could lead to 🐉

@psealock it seems like this is kind of like the extension example you are building right now in https://github.com/woocommerce/woocommerce-admin/pull/3328, so not sure if you have more info to add in on other ways to approach this problem. One route would be to use the table filter in js

[psealock]

We've seen this a few times, and avoiding dangerouslySetInnerHTML is probably best. Most uses are exactly the one above where a span is inserted for styling.

At the very least, wc-admin can strip tags. So this:

Will become this:

For more extensive manipulation in wc-admin only, we can add a filter so that extensions can simply pass in a custom react component.

const MyCurrency = ( { number, format, symbol } ) => {
    return (
        <div>{ symbol } 🇯🇲 { symbol } 🍗 { format( number ) }</div>
    );
};

addFilter( 'woocommerce_admin_currency_component', 'plugin-domain', MyCurrency );

[timmyc]

@KZeni I'm going to close this out but please let us know if we can help out further. I'm on WooCommerce slack if you ever want to give me a shout there, feel free to do so!