Global Styles: Refactor wp_add_inline_style() to use HTML API

[ramonopoly]

The current implementation of wp_add_inline_style strips matching <style /> tags in the incoming data, and removes them, preserving the nested content.

The function's preg_replace regex assumes balanced tags, e.g., <style>...content</style>.

It cannot however detect when incoming data contains a closing style tag </style>.

The consequence is that the style tag generated by WordPress in WP_Styles() can be short circuited.

Furthermore:

• the stripos won't detect closing style tags with attributes (which is allowed)

Example:

<?php

function like_wp_add_inline_style( $data ) {
    if ( false !== stripos( $data, '</style>' ) ) {
        return trim( preg_replace( '#<style[^>]*>(.*)</style>#is', '$1', $data ) );
    }
    return 'yay';
}

//  ## Do what wp_add_inline_style expects:
$result = like_wp_add_inline_style( ".captain {<style><p>Do it to me one more time...</p></style>}");

/*
Output in frontend:
<style id='my-inline-css' type='text/css'>
.captain {<p>Do it to me one more time...</p>}
</style>
*/

// -------------------------------------------------

// ## Do what wp_add_inline_style doesn't expect:
$result = like_wp_add_inline_style( ".captain {</style><p>Do it to me one more time...</p><style>}");


/*
Output in frontend:
<style id='my-inline-css' type='text/css'>
.captain {</style><p>Do it to me one more time...</p><style>}
</style>
*/

wp_add_inline_style should be updated to escape closing style tags, except in the case of balanced tags, where it should match existing functionality.

Props to @dmsnell, @peterwilsoncc and @costdev for helping diagnose and address this issue

The existing wp_add_inline_style() code attempts to ensure that the provided code doesn't include STYLE tags, but this is difficult to do. In this change, the HTML API is employed to ensure that the HTML tags are properly detected.

With the ability to detect these more carefully, new options are available to the function when things _go wrong_. This makes the attempt to auto-detect when someone has provided a <style>-wrapped input by mistake, and will unwrap that properly. Any other defects are passed transparently into the styles.

Thanks @peterwilsoncc - updated!

FWIW, the same goes for wp_add_inline_script, where this was first introduced.

IMHO this is overly defensive programming and overkill for this use case.

In these functions we do a simple regex check _out of courtesy_ to tell developers they are doing it wrong. If they added some weird markup that we don't detect, they deserve to get yelled at by the browser or whatever

---

@swissspidy I added the fallback conditions just because they were easy and because it wasn't clear what the right end goal should be. if you think we ought instead to truncate the provided content once it exists the SCRIPT context that would be fine too.

For example, if provided runAThing();</script><p>Done!</p> then we could simply send along runAThing(); and not do any more work. The only real condition I considered was that many people might send <script>runAThing();</script> and it seemed reasonable to auto-correct this.

The only real condition I considered was that many people might send <script>runAThing();</script> and it seemed reasonable to auto-correct this.

I agree, and this was the genesis of this PR. Why not tighten the checks up a little, even if out of courtesy?

If they added some weird markup that we don't detect, they deserve to get yelled at by the browser or whatever

Even when that weird markup comes from external sources, such as a theme's custom CSS?

The current code in core was not meant to magically fix a dev‘s broken inline css/js. We could actually even consider removing that logic with the doing_it_wrong .

I've updated this by adjusting the comments and test names. Happy to have someone revert if they don't like the slightly-more descriptive test names. I've also moved the interpolated $script var in the test failure output to the end after a : to more clearly separate it from the error message.

Why not tighten the checks up a little, even if out of courtesy?

you definitely won't here me shouting this virtue 😉

IMO each case warrants its own review. here, we can reliably detect this condition and resolve it, so it seemed fine. whether we want to is more about what we allow and which habits we want to discourage.

leaving it in and taking it out are equally easy to do

IMO each case warrants its own review. here, we can reliably detect this condition and resolve it, so it seemed fine. whether we want to is more about what we allow and which habits we want to discourage.

Fair point.

My thinking comes from the following assumptions:

• the current code attempts a code replace, so I'm assuming this should be retained in some fashion for backwards compatibility. (This PR does that).
• It's possible to add scripts that do things in a theme.json's "css" field. A theme can be installed by any user, whether they've had a hand in producing the code or not. We should therefore tighten up the check to prevent it.

Everything else I'd defer to you folks for advice.

The HTML API update has been extracted into ​https://github.com/WordPress/wordpress-develop/pull/7150

The HTML API update has been merged into trunk and trunk has been merged into this branch.