`wp_get_global_styles`: return the standard format for CSS Custom Properties

[oandregal]

The wp_get_global_styles functions returns an internal format for CSS Custom Properties instead of the proper CSS format.

Step-by-step reproduction instructions

• Use a theme that has a theme.json.
• Paste the following under styles.blocks:

"core/post-terms": {
    "typography": { "fontSize": "var(--wp--preset--font-size--small)" },
    "color":{ "background": "var:preset|color|secondary" }
}

• Use the wp_get_global_styles functions to retrieve those styles. For example, paste the following in functions.php of the theme:

<?php
add_action( 'init', function(){
        error_log( print_r( wp_get_global_styles( array(), array('block_name'=>'core/post-terms') ), true ) );
} );

The result will be:

<?php
(
    [typography] => Array( [fontSize] => var(--wp--preset--font-size--small) )
    [color] => Array( [background] => var:preset|color|secondary )
)

when it should have been

<?php
(
    [typography] => Array( [fontSize] => var(--wp--preset--font-size--small) )
    [color] => Array( [background] => var(--wp--preset--color--secondary) )
)

Note the color.background value. It should return the value in the standard CSS Custom Property format, not the shortened internal format.

Trac ticket https://core.trac.wordpress.org/ticket/58467
Backports ​https://github.com/WordPress/gutenberg/pull/50366 and ​https://github.com/WordPress/gutenberg/pull/50527
Part of ​https://github.com/WordPress/gutenberg/issues/45171

## What?

This PR fixes wp_get_global_styles so that it always return the standard format for CSS Custom Properties. For a theme.json dataset with the following data:

"core/post-terms": {
    "typography": { "fontSize": "var(--wp--preset--font-size--small)" },
    "color":{ "background": "var:preset|color|secondary" }
}

the wp_get_global_styles function should return:

(
    [typography] => Array( [fontSize] => var(--wp--preset--font-size--small) )
    [color] => Array( [background] => var(--wp--preset--color--secondary) )
)

## Why?

See ​https://github.com/WordPress/gutenberg/issues/49693 The internal syntax shouldn't leak out, so consumers of this function only have to deal with the standard CSS Custom format.

## How?

This PR extract the already existing logic that converts var:preset|color|secondary to var(--wp--preset--font-size--small) to a separate method, then uses the same method to sanitize the output of wp_get_global_styles to only include custom CSS variables and not internal variable syntax.

## Testing Instructions

Make sure the tests pass.

[oandregal]

In 55959:

wp_get_global_styles: return the standard format for CSS Custom Properties.

Props samnajian, hellofromtonya, isabel_brison.
Fixes #58467.

Commited at https://core.trac.wordpress.org/changeset/55959

[spacedmonkey]

@oandregal Do you mind changing 0 === strpos( $value, $prefix ) to str_starts_with? We are trying to get rid of usage of strpos. See #58012.

CC @SergeyBiryukov

[oandregal]

Thanks for bringing that to my attention, I wasn't aware we were polyfilling those functions that are only available in PHP8+.

I've looked at that ticket and https://core.trac.wordpress.org/ticket/58012 It seems there is debate about it being the right direction. I've also seen that there has been issues with these changes with load-styles.php due to the polyfills for those functions not being available. It requires wider testing.

The timing is a bit off for me: we're right just before beta 6.3, and this sounds like a risky/involved change to do with no benefits. The opportunity cost is high. I wouldn't mind investigating and preparing an update to more modern PHP coding standards at some point, though it's not the right time for me to help with this refactoring.

[spacedmonkey]

@oandregal I have created a PR - ​https://github.com/WordPress/wordpress-develop/pull/4663. If you can take a look, I will commit it for you.

[oandregal]

If you can take a look, I will commit it for you.

Unfortunately, I don't know how I can help with this: my concerns are about testing that it doesn't break anything because the polyfills not being available everywhere (as others pointed out in the ticket and the core-performance channel ​https://wordpress.slack.com/archives/C02KGN5K076/p1687362710229799).

I cannot justify investing on this right now, so I'd rather hold this sort of changes off for when other work is not such time-sensitive. It's fine by me if others want to pursue it and make sure it works properly.

[spacedmonkey]

@oandregal As a core committer of this, you have a responsibility to review and action feedback on your commit. As this is your commit, you should look at the patch and merge it before looking at other work. You have to own the life cycle of a core commit and any feedback that comes out of it.

[oandregal]

@oandregal As a core committer of this, you have a responsibility to review and action feedback on your commit. As this is your commit, you should look at the patch and merge it before looking at other work. You have to own the life cycle of a core commit and any feedback that comes out of it.

Absolutely. I just disagree this is the right moment to do that experiment that has proven problematic and performance-wise it has dubious effects (probably negative ones). The direction doesn't have full support from core committers. When it does, I'm happy to follow up with this change and any other that's suggested.

[oglekler]

This ticket was discussed during the bug scrub, and it was agreed that this ticket should be closed in 6.3 milestone and further work need to be moved into another ticket.

@spacedmonkey can you make this, please? 🙏

Props @mrinal, @audrasjb

[spacedmonkey]

This ticket was fixed in other commits.

[SergeyBiryukov]

Fixed by [55959] and [55987].