Typography: replace non-dot decimal separator for float to string conversions

[ramonjd]

Resolves:

• Typography: Casting float value to string makes it comma separated in some locales (PHP version < 8) #57607

What?

When coercing a float to a string, replace commas with dots.

This is so we can output valid CSS values where locales use commas as decimal separators.

Why?

In PHP version < 8, PHP uses the current locale for float to string conversions.

That means 1.333 + "px", for example, will be converted to 1,333px.

PHP 8 does not do this.

See: https://php.watch/versions/8.0/float-to-string-locale-independent

How?

String replace before turning floats to strings or trying to use stringified floats for any other business.

E.g.,

str_replace( ',', '.', '$some_number )

Testing Instructions

To test, we need to be running a PHP version of < 8, e.g., 7.4.

If you're not and you're using wp-env, you can update the .wp-env.json file and set the phpVersion value.

Another dependency is locale, which needs to be installed on the server.

Here's a diff that does all that:

wp-env override and locale

diff --git a/.wp-env.json b/.wp-env.json
index 79810b194b..a1d3716f4c 100644
--- a/.wp-env.json
+++ b/.wp-env.json
@@ -1,5 +1,6 @@
 {
 	"core": "WordPress/WordPress",
+	"phpVersion": "7.4",
 	"plugins": [ "." ],
 	"themes": [ "./test/emptytheme" ],
 	"env": {
diff --git a/packages/env/lib/init-config.js b/packages/env/lib/init-config.js
index 318bcae151..eb9962b0de 100644
--- a/packages/env/lib/init-config.js
+++ b/packages/env/lib/init-config.js
@@ -207,6 +207,17 @@ RUN apt-get -qy install git
 # Set up sudo so they can have root access.
 RUN apt-get -qy install sudo
 RUN echo "#$HOST_UID ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers`;
+
+			const locale = 'de_DE.UTF-8';
+			dockerFileContent += `
+# Install locales
+RUN apt-get update && apt-get install -y locales locales-all
+RUN locale-gen ${ locale }
+ENV LANG ${ locale }
+ENV LANGUAGE ${ locale }
+ENV LC_ALL ${ locale }
+RUN update-locale LANG=${ locale }
+`;
 			break;
 		}
 		case 'cli': {

If you don't mind losing your docker containers, once that patch is applied, run the following commands:

npm run wp-env destroy
# Hit `y` to destroy your local docker containers
npm run wp-env start 

Then, somewhere at the top of the typography.php file (not inside a function), add the following line:

setlocale( LC_ALL, 'de_DE.UTF-8' );

This sets the locale to German, which uses commas as decimal separators, e.g., 1,33.

Finally, ensure that tests pass and that fluid typography works on the frontend as expected.

[github-actions]

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files

❔ lib/block-supports/typography.php
❔ phpunit/block-supports/typography-test.php

[ramonjd]

Now accepts negative numbers. This was an unrelated limitation.

-10rem

Also now accepts any pre-stringified floats from other locales:

12,232px

And unit less values:

10.333 to which a px unit will be added by default. This matches existing behaviour only that we move it down to after the regex matching so we can do the string replace on any found values. Note that incoming $raw_value can be anything, e.g., clamp(...) or whatever, so I'm letting the regex do the check before we get clever.

[ramonjd]

This property is an addition of convenience. Any subsequent string coercion runs the risk of triggering the locale behaviour, so do it here once and return the combined value for use in our output.

[andrewserong]

Just double-check my reading here: it's safe to access ['combined'] here without checking it's set, because it's guaranteed to be as $minimum_viewport_width is sure to be an actual / valid value because if it wasn't this function would have returned early?

Or to put it differently: at first I saw the call to gutenberg_get_typography_value_and_unit and thought it might be dangerous because the function can technically return null, but in practice here it looks like it never will / it'll always return a value with combined available 👍

[ramonjd]

because it's guaranteed to be as $minimum_viewport_width is sure to be an actual / valid value because if it wasn't this function would have returned early?

Exactly! But I've replaced this with a regular string replace for another reason 😆

Basically a micro optimization: don't run the viewport offset value through gutenberg_get_typography_value_and_unit() as the values have already been previously checked via gutenberg_get_typography_value_and_unit().

It avoids another regex parsing, and as you say, the function will return early if those values are bunk.

[andrewserong]

Nice work, and thanks for the helpful testing instructions, without them I think it'd have taken me a while to get everything set up!

Looking good:

Trunk (fluid is skipped and we get the real value)	This PR (fluid values get output correctly

LGTM! ✨

[ramonjd]

Nice work, and thanks for the helpful testing instructions, without them I think it'd have taken me a while to get everything set up!

Thanks for testing this one @andrewserong I know it was a bit fiddly 😄

[ramonjd]

I have a more universal approach with a new function here:

• Formatting: add new rounding css number formatting function #57796

This PR remains relevant however, since any float->string coercion will be run through the new function in pretty much the same places.

[ramonjd]

Converting to draft while #57607 plays out

[ramonjd]

Closing for now. I'll come back later if this feature is required and time permits.