Global Styles: Refactor wp_add_inline_style() to use HTML API.

[dmsnell]

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.

Ticket: https://core.trac.wordpress.org/ticket/61828

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[ramonjd]

Thanks a lot for working on this @dmsnell

I've tested this with theme.json, and also adding custom CSS in the site editor (global styles).

Everything works as expected.

✅ "valid" custom CSS works as expected, including nested rules and other selector manifestations
✅ Other inline styles, such as core-block-supports-inline-css have no regressions

I added some tests to ensure current functionality is preserved (stripping balanced style tags and injected content), and also testing the new behaviour - I hope you don't mind!

Also, when adding inline CSS that wp_add_inline_style() doesn't like, we'll know 😄

Do you think the changes to the HTML API needs testing as well?

[peterwilsoncc]

Some quick notes but will come back to actually test the code.

[ramonjd]

Thanks @peterwilsoncc - updated!

[swissspidy]

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

[dmsnell]

@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 STYLE context that would be fine too.

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

[ramonjd]

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?

[swissspidy]

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 .

[dmsnell]

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.

[dmsnell]

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

you definitely won't hear 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

[ramonjd]

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.

[dmsnell]

The HTML API update has been extracted into #7150

[dmsnell]

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