WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 2 months ago

#47003 reviewing defect (bug)

i18n: Merge similar translation strings in new user registration screen

Reported by: ramiy Owned by: SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Users Keywords: has-screenshots has-patch needs-refresh
Focuses: administration Cc:
PR Number:

Description

See the attached patch.

Attachments (6)

47003.png (181.4 KB) - added by ramiy 8 months ago.
47003.patch (664 bytes) - added by ramiy 8 months ago.
47003.2.patch (2.3 KB) - added by ramiy 8 months ago.
another 3 similar strings merged into 1 string
47003.2.png (33.1 KB) - added by ramiy 8 months ago.
3 more…
47003.3.png (42.5 KB) - added by ramiy 5 months ago.
47252.4.png (53.2 KB) - added by ramiy 5 months ago.

Download all attachments as: .zip

Change History (18)

@ramiy
8 months ago

@ramiy
8 months ago

#1 @ramiy
8 months ago

  • Keywords has-screenshots has-patch added

@SergeyBiryukov

#2 @ramiy
8 months ago

Actually, this is a bigger issue. We have many similar error messages in several locations. We should use the same error messages in the following functions:

  • register_new_user() (wp-includes/user.php)
  • wpmu_validate_user_signup() (wp-includes/ms-functions.php)
  • check_username() (wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php)

@ramiy
8 months ago

another 3 similar strings merged into 1 string

@ramiy
8 months ago

3 more...

#3 @ramiy
8 months ago

Sorry, the last screenshot is not for this ticket.

#4 @SergeyBiryukov
8 months ago

  • Milestone changed from Awaiting Review to 5.3
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#5 @ramiy
5 months ago

@SergeyBiryukov

This ticket merges the following 3 strings:

  • Usernames can only contain lowercase letters (a-z) and numbers.
  • Username contains invalid characters.
  • <strong>ERROR</strong>: This username is invalid because it uses illegal characters. Please enter a valid username.

to only one string:

  • Usernames can only contain lowercase letters (a-z) and numbers (0-9).

#6 follow-up: @garrett-eclipse
5 months ago

The only string I'd be concerned about here is the one in wp-includes/user.php as all the other error messages there have the Error: prefix, examples;

'<strong>ERROR</strong>: Please enter a username.'
'<strong>ERROR</strong>: This username is already registered. Please choose another one.'

#7 in reply to: ↑ 6 @ramiy
5 months ago

Replying to garrett-eclipse:

The only string I'd be concerned about here is the one in wp-includes/user.php as all the other error messages there have the Error: prefix, examples;

<strong>ERROR</strong>: Please enter a username.
<strong>ERROR</strong>: This username is already registered. Please choose another one.

I know, this is why I mentioned above that this is a bigger issue. I think we should remove the Error: prefix from wp-includes/user.php.

@ramiy
5 months ago

#8 follow-up: @ramiy
5 months ago

As you can see from the screenshot, update nag has no Warning prefix, it only has a yellow color. Same should be in with errors. The prefix should be removed, it has a red color.

Note that many error in WordPress dashboard don't have the error prefix. I'll a few attach screenshots.

@ramiy
5 months ago

#9 in reply to: ↑ 8 @garrett-eclipse
5 months ago

Replying to ramiy:

As you can see from the screenshot, update nag has no Warning prefix, it only has a yellow color. Same should be in with errors. The prefix should be removed, it has a red color.

Note that many error in WordPress dashboard don't have the error prefix. I'll a few attach screenshots.

Thanks @ramiy seeing it there in context I agree there's no need for the Error: prefix as it's redundant since it's already in an error notice. Not sure if that should be handled in a separate ticket or could be done here. @SergeyBiryukov thoughts?

#10 follow-up: @ramiy
5 months ago

I think that this ticket should handled only the merge of similar translation strings. A separate ticket should remove the Error: prefix from the error messages in the dashboard.

#11 in reply to: ↑ 10 @garrett-eclipse
5 months ago

Replying to ramiy:

I think that this ticket should handled only the merge of similar translation strings. A separate ticket should remove the Error: prefix from the error messages in the dashboard.

I agree with you there @ramiy and created #47656 to accommodate it. The two will conflict so one will need a refresh in the end.

#12 @garrett-eclipse
2 months ago

  • Keywords needs-refresh added
  • Milestone changed from 5.3 to 5.4

@SergeyBiryukov made some good points and a proposal in #47656;

Previous discussion: #15887

Specifically, I think comment:5:ticket:15887 sums up the purpose of the ERROR: prefix nicely:

All CAPITAL letters in a word typically denotes Emphasis. It's not meant to be

"Good day gentle, but incorrect, user - there seems to be an error. Be a sport and type in your e-mail address"

But rather

"HEY, USER! (yelling) THERE'S A PROBLEM! ATTENTION! ERROR. Type in your email address please"

We could, however, move the ERROR: prefix out of the strings and add it separately, see the error message in wp-admin/themes.php for example. Related: #38860

I concur and believe isolating the prefix is our patch forward here. @ramiy would you like to handle the refresh?

As it will need a refresh and would be a string change I would feel more comfortable tackling this in 5.4 as we're already in beta2 for 5.3, as well that allows us to coincide #47656 as I'm marking it for 5.4 as well.

Note: See TracTickets for help on using tickets.