Make WordPress Core

Opened 3 years ago

Last modified 21 months ago

#52695 new defect (bug)

Incorrect way to updateLocale in moment.js add_inline_script

Reported by: yoancutillas's profile yoancutillas Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 5.6.2
Component: External Libraries Keywords:
Focuses: Cc:

Description

In [wp-includes/script-loader.php L146](https://build.trac.wordpress.org/browser/branches/5.7/wp-includes/script-loader.php#L146)

moment.updateLocale( '%s', %s );

WP updates the locale
1. with incomplete data
2. regardless of the existing data

The plugins / themes calling for the removed data expect a String, and they will get an undefined, triggering an Uncaught TypeError for any further process.

This is not too visible yet because of the bug #51142.
But when it will be fixed, a lot of scripts will be affected.


1. Incomplete data

1.1
WP currently updates 'longDateFormat' with null parameters. Parameters values should not be null. I suggest to use the 'en_US' values as default.

The parameters should not be null: 'LTS', 'L', 'LLLL'

'longDateFormat' => array(
	'LT'   => get_option( 'time_format', __( 'g:i a', 'default' ) ),
	'LTS'  => null,
	'L'    => null,
	'LL'   => get_option( 'date_format', __( 'F j, Y', 'default' ) ),
	'LLL'  => __( 'F j, Y g:i a', 'default' ),
	'LLLL' => null,
)

1.2
WP currently updates 'week' with 1 parameter instead of 2 (https://momentjs.com/docs/#/customization/dow-doy/).

Missing parameter: 'doy'

'week' => array(
	'dow' => (int) get_option( 'start_of_week', 0 ),
),

2. Existing data

Before calling updateLocale, WP should check if the locale data exists (with moment.localeData( 'en' ) - [docs](https://momentjs.com/docs/#/i18n/locale-data/)).

I suggest to replace only the missing data, and keep the existing one as is.

Change History (7)

#1 @noisysocks
2 years ago

  • Milestone changed from Awaiting Review to 6.0

Thanks for spotting this! The fix I opened for #51142 also addresses this.

This ticket was mentioned in Slack in #core by mike. View the logs.


2 years ago

#3 @mukesh27
2 years ago

  • Milestone changed from 6.0 to Future Release

This was discussed in a bug scrub earlier.

Let's set it to Future Release feel free to change the milestone if it is on your to-do list for the next release.

#4 @yoancutillas
2 years ago

I don't understand, why would you postpone this?

Last week again, another WP user experienced this bug. (https://wordpress.org/support/topic/js-errors-with-plugin-files/#post-15579274)

The solution was: go to Settings > General > Save changes (without changing any option).
It feels very much like a WP bug, right?

#5 @noisysocks
2 years ago

  • Milestone changed from Future Release to 6.1

I think it would be good to include https://github.com/WordPress/wordpress-develop/pull/2456 or some variation of it in 6.1. Just waiting on a code review.

This ticket was mentioned in Slack in #core by chaion07. View the logs.


21 months ago

#7 @chaion07
21 months ago

  • Milestone changed from 6.1 to Future Release

@yoancutillas thank you for reporting this. We reviewed this ticket during a recent bug-scrub session. Considering the lack of progress on the ticket we are updating the milestone to Future Release. We can always revert back to the current release should there be significant progress in the near future.

Cheers!

Props to @thisisyeasin for the suggestion

Note: See TracTickets for help on using tickets.