WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 3 months ago

#25280 new enhancement

wp_localize_script unexpectedly converts numbers to strings

Reported by: adamsilverstein Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.2
Component: I18N Keywords: has-patch dev-feedback needs-testing
Focuses: Cc:

Description

Running wp_localize_script on an an array has the unexpected result of converting numbers into strings. Core WordPress files work around this issue by explicitly casting variables to int, for example in revisions.js.

Although it is easy to work around the issue (if you are expecting it), JavaScript is loosely typed and will merrily add numbers as strings without any warning - introducing potential bugs when code that expects a number is instead passed a string. Properly passing numbers as numbers would resolve this issue.

The root of the problem is that in PHP versions before 5.3.3, json_encode encoded numbers as strings. PHP 5.3.3 includes an option JSON_NUMERIC_CHECK that eliminates this issue.

For example & testing this bug, add the following code to your template's setup function:

	wp_enqueue_script( 'localize_test', get_template_directory_uri() . '/localize_script.js', array(), '1.0.0', true );
	$array_to_localize = array( 
		'a_string' => __( 'Some String' ),
		'a_number' => 10,
		);

	wp_localize_script( 'localize_test', 'localized_variable', $array_to_localize );

Then add a file named localize_script.js in your theme folder containing the following code:

console.log(localized_variable);

Reloading shows that the number has been converted into a String (console screenshot).

The attached patch corrects the issue by checking for the presence of the JSON_NUMERIC_CHECK constant and if present, passes it to json_encode - telling it to encode numbers as numbers and resulting in the expected behavior (console screenshot). I checked this under PHP 5.2.17 verifying that no errors are created (although the old behavior of converting numbers still occurs). Under PHP 5.3.3 the variable is now properly passed as a number. The Codex documentation needs to be updated if this patch is accepted.

Attachments (3)

25280.diff (682 bytes) - added by adamsilverstein 7 months ago.
correct wp_localize_script encoding numbers as strings
25280.2.patch (737 bytes) - added by c3mdigital 7 months ago.
Don't run booleans through html_entity_decode() cast as a string
25280.patch (737 bytes) - added by adamsilverstein 3 months ago.
update against trunk

Download all attachments as: .zip

Change History (14)

adamsilverstein7 months ago

correct wp_localize_script encoding numbers as strings

comment:1 c3mdigital7 months ago

  • Keywords close added; dev-feedback removed

It also converts booleans to strings as well and if you don't have control of the javascript reading the vars you can't use numbers or booleans.

json_encode is not why numbers are converted to strings. They are cast as a string on purpose:

$l10n[$key] = html_entity_decode( (string) $value, ENT_QUOTES, 'UTF-8');

I know the general accepted practice is to use wp_localize_script to pass variables to javascript but that is not what the function was intended for (it's a bonus feature and I blame Otto's awesome blog post). It's for string translations and I don't think it should be changed because numbers don't need to be translated.

I'm thinking wontfix.

Last edited 7 months ago by c3mdigital (previous) (diff)

comment:2 follow-up: c3mdigital7 months ago

Also to note the way JSON_NUMERIC_CHECK handles booleans.

wp_localize_script( 'testing', 'TESTING', array( 'false' => false, 'true' => true, 'integer' => 234 ) );

Before patch:
var TESTING = {"false":"","true":"1","integer":"234"};

after patch:
var TESTING = {"false":"","true":1,"integer":234};

Last edited 7 months ago by c3mdigital (previous) (diff)

comment:3 in reply to: ↑ 2 ; follow-up: adamsilverstein7 months ago

Replying to c3mdigital:

Also to note the way JSON_NUMERIC_CHECK handles booleans.

wp_localize_script( 'testing', 'TESTING', array( 'false' => false, 'true' => true, 'integer' => 234 ) );

Before patch:
var TESTING = {"false":"","true":"1","integer":"234"};

after patch:
var TESTING = {"false":"","true":1,"integer":234};

interesting points, thanks for the feedback! the boolean evaluation in JS won't change, right? "1" and 1 are both truthy.

the patch does not rely on PHP 5.3; instead it improves the behavior of if 5.3 is available. in 5.2 nothing changes.

agree wp_localize_script is not intended for this purpose, but in practice thats how its used- even internally by core. if we aren't going to try to fix the function, how about adding a new function for passing non-string values from php to js - providing a standard method is better than leaving it up to developers in my opinion.

comment:4 in reply to: ↑ 3 c3mdigital7 months ago

Replying to adamsilverstein:

interesting points, thanks for the feedback! the boolean evaluation in JS won't change, right? "1" and 1 are both truthy.

the patch does not rely on PHP 5.3; instead it improves the behavior of if 5.3 is available. in 5.2 nothing changes.

agree wp_localize_script is not intended for this purpose, but in practice thats how its used- even internally by core. if we aren't going to try to fix the function, how about adding a new function for passing non-string values from php to js - providing a standard method is better than leaving it up to developers in my opinion.

Also those results were after the booleans and numbers were run through html_entity_decode cast as a string. I don't fully understand why scaler variables are run through html_entity_decode()?

I would love a better way to pass data to javascript as a script dependency. wp_localize_script would be fine for most use if there was a way to pass a real boolean. Maybe we could start with omitting booleans from the is_scalar check. I'm not against JSON_NUMERIC_CHECK but know core doesn't like to use stuff dependent on php 5.3 even if there is a sanity check.

comment:5 c3mdigital7 months ago

  • Keywords dev-feedback added; close removed
  • Type changed from defect (bug) to enhancement
  • Version changed from trunk to 3.2

Related: #19825, #11520

c3mdigital7 months ago

Don't run booleans through html_entity_decode() cast as a string

comment:6 adamsilverstein7 months ago

that looks reasonable, haven't checked locally: will that pass actual true and false to JS now?

adamsilverstein3 months ago

update against trunk

comment:7 adamsilverstein3 months ago

  • Keywords needs-testing added

comment:8 follow-up: tomauger3 months ago

I'm liking this fix, but actually agree with adam that it might be good to have an entirely new way of passing object dependencies into our JS. Right now I'm using a lot of JSDoc to document the "localize" global JS var, but it certainly would be nice to have something a little more robust.

Forking wp_localize_script() to wp_pass_vars_to_script() (for lack of a better name) seems like a pretty good (and backward-compatible) way to go.

comment:9 in reply to: ↑ 8 adamsilverstein3 months ago

Replying to tomauger:

I'm liking this fix, but actually agree with adam that it might be good to have an entirely new way of passing object dependencies into our JS. Right now I'm using a lot of JSDoc to document the "localize" global JS var, but it certainly would be nice to have something a little more robust.

Forking wp_localize_script() to wp_pass_vars_to_script() (for lack of a better name) seems like a pretty good (and backward-compatible) way to go.

I agree a new function probably makes the most sense, that would leave backward compatibility with what we have while allowing a more robust design. i believe this function was originally built with translation in mind and has obviously been used for other purposes to the point where it has become the recommended way to pass variables from PHP->JS in WordPress.

The attached patch corrects some unexpected behavior and might still be worth getting in - what would you envision a new function doing other than having a new name and passing numbers/booleans correctly? If you have some ideas, please respond or go ahead an open a new ticket proposing the new function!

Thanks :)

comment:10 adamsilverstein3 months ago

  • Summary changed from wp_localize_script unexpectedly convert numbers to strings to wp_localize_script unexpectedly converts numbers to strings

comment:11 tomauger3 months ago

I kind of blew my mind when I started modeling how to create a tighter coupling between PHP and JS, particularly in such a way that modern IDEs (I use Netbeans, for example) would be able to pick up on the dependencies and perhaps even offer code completion. At the moment, I can't think of anything more innovative than a spin-off of wp_localize_script(), which is frankly a pretty user-friendly way of communicating between the two "worlds".

Note: See TracTickets for help on using tickets.