WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 2 weeks ago

#48244 new defect (bug)

script-loader.php Need to use _n() when more than one results are found

Reported by: tobifjellner Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.5
Component: Script Loader Keywords: good-first-bug needs-patch
Focuses: Cc:
PR Number:

Description

Location:
https://build.trac.wordpress.org/browser/trunk/wp-includes/script-loader.php?marks=1178#L1178

Content:

'manyResults'  => __( '%d results found. Use up and down arrow keys to navigate.' ),

should (if possible) be converted to use _n() in order for some languages to be able to correctly construct the phrase.

Attachments (6)

48244.patch (905 bytes) - added by shaharia.azam 2 weeks ago.
_n() has been applied in script-loader.php when more than one results are found
48244.2.patch (742 bytes) - added by shaharia.azam 2 weeks ago.
String is returning correctly. Removed .replace() from plugin.js
48244.3.patch (651 bytes) - added by shaharia.azam 2 weeks ago.
String is returning correctly. Removed .replace() from tags-suggest.js
48244.4.patch (1.8 KB) - added by shaharia.azam 2 weeks ago.
It's for 4.5 . Both plugin.js and script-loader.php has been updated
48244.5.diff (2.1 KB) - added by shaampk1 2 weeks ago.
Merged plugin.js, tags-suggest.js, and script-loader.php patches into one .diff file
48244.5.patch (1.6 KB) - added by shaharia.azam 2 weeks ago.
As per @swissspidy on slack, _n implemented on plugin.js & tags-suggest.js

Download all attachments as: .zip

Change History (10)

#1 @swissspidy
2 months ago

The correct number is only known client-side, so for correct results one would need to use _n() from the @wordpress/i18n JS package.

This would need to happen in at least two places:

https://github.com/WordPress/wordpress-develop/blob/3b14a06c8cb187d8abd3d4af8ea9c0ea3a475228/src/js/_enqueues/vendor/tinymce/plugins/wplink/plugin.js#L459-L469
https://github.com/WordPress/wordpress-develop/blob/715a65c56142262b938d6860f840e201b1fe074a/src/js/_enqueues/admin/tags-suggest.js#L135-L144

However, this is by far not the only place where plural forms in JS could or should be improved. In #20491 you can find a huge patch with possible changes.

#2 @SergeyBiryukov
2 months ago

  • Version changed from trunk to 4.5

Introduced in [36806].

#3 @shaampk1
6 weeks ago

Hey folks,

This ticket really got my attention. I found it an interesting issue to work with. I know there has been already said a lot about L10n which I have been reading and trying to catch up with for the past few days now but still, I am not sure how far I have come regarding this issue. I'd appreciate it if you could review the following code snippet and let me know what I am missing.

For script-loader.php

	// Strings for 'jquery-ui-autocomplete' live region messages
	did_action( 'init' ) && $scripts->localize(
		'jquery-ui-autocomplete',
		'uiAutocompleteL10n',
		array(
			'noResults'    => __( 'No results found.' ),
			/* translators: %s: Number of results found when using jQuery UI Autocomplete. */
			'manyResults' => _n( '%s result found. Use up and down arrow keys to navigate.', '%s results found. Use up and down arrow keys to navigate.', did_action( 'init' ) ),
			'itemSelected' => __( 'Item selected.' ),
		)
	);

For plugin.js & tags-suggest.js

messages: {
	noResults: ( typeof window.uiAutocompleteL10n !== 'undefined' ) ? window.uiAutocompleteL10n.noResults : '',
	results: function( number ) {
		if ( typeof window.uiAutocompleteL10n !== 'undefined' ) {
			if ( number > 1 ) {
				return window.uiAutocompleteL10n.manyResults.replace( '%s', number );
			}

			return window.uiAutocompleteL10n.oneResult;
		}
	}
}

Thank you!

Last edited 2 weeks ago by shaampk1 (previous) (diff)

@shaharia.azam
2 weeks ago

_n() has been applied in script-loader.php when more than one results are found

@shaharia.azam
2 weeks ago

String is returning correctly. Removed .replace() from plugin.js

@shaharia.azam
2 weeks ago

String is returning correctly. Removed .replace() from tags-suggest.js

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


2 weeks ago

@shaharia.azam
2 weeks ago

It's for 4.5 . Both plugin.js and script-loader.php has been updated

@shaampk1
2 weeks ago

Merged plugin.js, tags-suggest.js, and script-loader.php patches into one .diff file

@shaharia.azam
2 weeks ago

As per @swissspidy on slack, _n implemented on plugin.js & tags-suggest.js

Note: See TracTickets for help on using tickets.