Make WordPress Core

Opened 7 years ago

Closed 3 years ago

Last modified 3 years ago

#43700 closed enhancement (fixed)

Language switcher on the login screen

Reported by: johnbillion's profile johnbillion Owned by: joedolson's profile joedolson
Milestone: 5.9 Priority: normal
Severity: normal Version: 4.7
Component: I18N Keywords: has-patch has-screenshots commit has-dev-note
Focuses: accessibility Cc:

Description (last modified by johnbillion)

Since [41692] it's possible to control the language of the login screen using the wp_lang query variable (eg. wp-login.php?wp_lang=it_IT). This is a hidden feature that's only used by the interim login modal.

A language switcher should be introduced on the login screen that makes use of this so users can use the login screen, the password reset screens, and the registration screen in their own language. The behaviour would be the same as the language choosers that are used elsewhere in core, for example on the General Settings screen and on the Multisite registration screens.

One thing that needs to be considered is how the language persists between screens, how to persist POSTed values such as redirect_to, and whether it should persist in user meta once the user successfully logs in. Suggestions welcome.

Attachments (17)

43700.1.diff (3.1 KB) - added by Nikschavan 7 years ago.
43700.diff (6.2 KB) - added by johnbillion 6 years ago.
43700.2.diff (7.1 KB) - added by Nikschavan 5 years ago.
43700.png (16.4 KB) - added by afercia 4 years ago.
Focus style cut-off
43700.3.diff (6.5 KB) - added by sabernhardt 4 years ago.
using "Change" submit button
wp-login-language-switcher-change.jpg (28.6 KB) - added by sabernhardt 4 years ago.
switcher with submit button
wp-login-submit-button-focus.jpg (9.5 KB) - added by sabernhardt 4 years ago.
submit button with focus outline
Screen Shot 2020-10-21 at 5.26.00 PM.png (74.3 KB) - added by garrett-eclipse 4 years ago.
Concept: Language Selector at bottom, register/lostpassword links inside form white, and no need for back as logo does that.
Screen Shot 2020-10-21 at 5.29.03 PM.png (79.0 KB) - added by garrett-eclipse 4 years ago.
Same concept but with a back link
43700.4.diff (6.1 KB) - added by sabernhardt 3 years ago.
removing language switcher from interim login, small CSS changes
43700.top.diff (6.2 KB) - added by sabernhardt 3 years ago.
option: moving switcher to the top, below heading/logo
43700.5.diff (6.3 KB) - added by sabernhardt 3 years ago.
reducing top margin for privacy link
43700-options-Edge-en-US.png (118.7 KB) - added by sabernhardt 3 years ago.
screenshots in English
43700.6.diff (6.3 KB) - added by joedolson 3 years ago.
Code adjustments to 43700.5.diff
43700.7.diff (6.5 KB) - added by joedolson 3 years ago.
CSS improvements for narrow viewports/text zoom
43700.8.diff (7.1 KB) - added by joedolson 3 years ago.
Set locale on user registration
43700.9.diff (7.1 KB) - added by joedolson 3 years ago.
validate locale before saving to user meta

Download all attachments as: .zip

Change History (94)

#1 @johnbillion
7 years ago

  • Description modified (diff)

#2 follow-up: @swissspidy
7 years ago

How has this been implemented on WordPress.org?

#3 in reply to: ↑ 2 @netweb
7 years ago

Replying to swissspidy:

How has this been implemented on WordPress.org?

I've not looked at it personally but the code is here: https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/themes/pub/wporg-login

#4 @Nikschavan
7 years ago

Seems this was the main commit for adding this to WordPress.org - https://meta.trac.wordpress.org/changeset/6677

@Nikschavan
7 years ago

#5 @Nikschavan
7 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

Patch 43700.1.diff is the first pass at adding the language switcher to the login window.

One thing that needs to be considered is how the language persists between screens, how to persist POSTed values such as redirect_to, and whether it should persist in user meta once the user successfully logs in. Suggestions welcome.

This attached patch adds the $_GET parameters to hidden variables so that they are retained after the page is reloaded.
Currently three $_GET parameters are handled interim-login, redirect_to, action.

@johnbillion
6 years ago

#6 follow-up: @johnbillion
6 years ago

43700.diff is a work in progress patch that continues the work of @Nikschavan. Changes:

  • Ensure the language switcher only appears when there are language choices available
  • Store the selected language in a cookie so it persists between all of the possible login screens and actions
  • Take the cookie into account when determining the user's locale
  • Allow the user to switch to en_US when it's not the site's default language

#7 @swissspidy
5 years ago

  • Milestone changed from Awaiting Review to Future Release

@Nikschavan
5 years ago

#8 in reply to: ↑ 6 @Nikschavan
5 years ago

43700.2.diff Refreshes @johnbillion's patch with latest trunk and fixes 2 PHP notices.

  • Set default value of explicit_option_en_us as false in wp_dropdown_languages() to avoid PHP notice for undefined index on Line 1511 in src/wp-includes/l10n.php
  • Use esc_url_raw() instead of sanitize_url() to remove depracation warning in wp-login.php

@johnbillion, @swissspidy - Can this ticket be considered for the 5.3 release cycle?

#9 @Nikschavan
5 years ago

Another question would be - should the value of wp_lang be set to user locale after successful login, If user locale is currently empty?

#10 @johnbillion
5 years ago

  • Milestone changed from Future Release to 5.5
  • Owner set to johnbillion
  • Status changed from new to reviewing

This ticket was mentioned in PR #248 on WordPress/wordpress-develop by johnbillion.


5 years ago
#11

Trac ticket: https://core.trac.wordpress.org/ticket/43700

This continues the work on the patches already uploaded to 43700. It introduces a language switcher to the login screen, which also affects its derivatives such as the password reset screens and user registration.

The following closely related functionality has also been implemented:

  • Add the language parameter to the login and password reset URLs in the email sent to a user when they are added to a site.
  • Ensure the password reset email is always sent to the user in their chosen language.
  • Add the language parameter to the URL in the password reset email.

Still to do:

  • Decide whether, upon login, the user's locale should be updated to the one they chose on the login screen. Probably yes.

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


4 years ago

#13 @davidbaumwald
4 years ago

@johnbillion IS this one still on your list to tackle before 5.5 Beta 1 in a few days time? Anything else needed here besides testing?

#14 @afercia
4 years ago

  • Focuses accessibility added
  • Keywords needs-patch added; has-patch removed

Testing 43700.2.diff there are a couple accessibility issues that need to be fixed:

  1. Using the change event on a select is a usability and accessibility issue and needs to be avoided. Users should be in control of triggering the language switch, thus there's the need of a button to submit the language form. See also #40925 and #25103.
  2. Minor CSS issue: the language form uses overflow: hidden thus the select focus style is slightly cut-off on the top and bottom edges. See attached screenshot.

@afercia
4 years ago

Focus style cut-off

#15 follow-up: @johnbillion
4 years ago

  • Keywords needs-testing removed

This needs some more work, including further functional work that I haven't finalised yet, plus design work to improve the layout now that we have several links of different styles at the bottom of this screen, plus the a11y issues mentioned above. Punting to 5.6.

#16 in reply to: ↑ 15 @afercia
4 years ago

  • Milestone changed from 5.5 to 5.6

Replying to johnbillion:

Punting to 5.6.

Forgot to punt? :)

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


4 years ago

#18 @johnbillion
4 years ago

  • Milestone changed from 5.6 to Future Release

This needs some design discussion, design work, and accessibility work. Punting for now.

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


4 years ago

@sabernhardt
4 years ago

using "Change" submit button

#20 @sabernhardt
4 years ago

  • Keywords has-patch needs-testing needs-design-feedback added; needs-patch removed

Patch is updated for accessibility concerns and a few other changes. (The since note is still the placeholder because this may not be ready for 5.6.)

  • Reduces padding above logo
  • Removes border around language switcher
  • Shows overflow (focus outline)
  • Adds "Change" submit button (reusing text string from wp-admin/includes/class-wp-users-list-table.php)
  • Centers language switcher instead of aligning under the links
  • Centers submit button below the select and label at screen widths up to 400px
  • Code consistency:
  1. replacing spaces with tabs
  2. method="get" lowercase
  3. adding zero for "0.5em"
  4. moving aria-hidden attribute after classes to match the show password button's order

#21 @johnbillion
4 years ago

Thanks @sabernhardt could you add a screenshot please?

@sabernhardt
4 years ago

switcher with submit button

@sabernhardt
4 years ago

submit button with focus outline

#22 @sabernhardt
4 years ago

I also considered moving the language switcher above the username/password form, but that probably would mean changing the focused element on page load.

This ticket was mentioned in Slack in #design by paaljoachim. View the logs.


4 years ago

#24 @garrett-eclipse
4 years ago

  • Keywords needs-refresh added; needs-design-feedback removed

Thanks for the patch and screens @sabernhardt discussing in a design triage it's looking great.

We would suggest leaving it after the form as the primary focus of the page is the login. And this is a secondary action which once used isn't going to be re-used as often as login would.

Also the icon seems like it's baseline it would be good to have that vertically centred.

#25 @johnbillion
4 years ago

I think this needs some more in-depth design discussion. The bottom of the login screen can contain quite a few elements now and it's getting messy as the four existing items have three different appearances between them.

  • Register
  • Lost your password?
  • ← Back to [site]
  • Privacy Policy
  • Language switcher

#26 @karmatosed
4 years ago

  • Keywords needs-design-feedback added

I sort of think putting it before the lost password could be better, sorry to pop in with a counter point. How about we pop back the label as I do agree with you @johnbillion here that section can get quite large if those things are added. Maybe some mocks even if quickly produced?

#27 @johnbillion
4 years ago

Some visual mockups would be great

@garrett-eclipse
4 years ago

Concept: Language Selector at bottom, register/lostpassword links inside form white, and no need for back as logo does that.

@garrett-eclipse
4 years ago

Same concept but with a back link

#28 @garrett-eclipse
4 years ago

I put together two quick concepts using login.wordpress.org as the basis (so yes there's no button for submit in my concept but should be one for accessibility). I moved the Register and Lost your password links into the form area as they're tied to the login. And for the Back I didn't really feel it needed as there's the logo for that, but did provide a mock including it above the form as I assume it's beneficial for accessibility.

We can also try a mock with the selector above the form, just note the logo is up there and currently is fairly clean.

This ticket was mentioned in Slack in #design by paaljoachim. View the logs.


4 years ago

#30 @paaljoachim
4 years ago

  • Keywords needs-design-feedback removed

I am removing the Needs design feedback keyword from this ticket. If it is a mistake then please add it back in again.

Last edited 4 years ago by paaljoachim (previous) (diff)

This ticket was mentioned in Slack in #accessibility by sabernhardt. View the logs.


4 years ago

#32 @keyur5
4 years ago

Tested @sabernhardt patch (43700.3.diff). Worked as expected.

#33 @paaljoachim
4 years ago

At what stage is this ticket at?
I see that the patch has been approved.

#34 @paaljoachim
4 years ago

I have tested the latest patch and it works well.

Last edited 4 years ago by paaljoachim (previous) (diff)

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


4 years ago

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


4 years ago

#37 @Clorith
4 years ago

  • Keywords needs-patch added; has-patch removed

I like the direction of those patches @garrett-eclipse, big fan of keeping it simple and grouped by relevance like that, some quick feedback from looking at them, and my first thoughts on what might need refining;

The "back" link shouldn't come so close to the WordPress logo, since the logo is a link to WordPress.org, while the back link takes you to the main URL of that site. These two types of links should ideally be separated by a non-link element of some kind to avoid accidental navigation to the wrong place. (I saw your mention of this, so making note that the logo always links to WordPress.org, unless someone has changed this with code)

Could we also get some mockups for the password reset, and password recovery screens, these should also have the language switcher available ideally, in case you somehow got the wrong language, or (after WordPress 5.7) a reset link was sent directly to you, and you did not set your language from the login page.

There are also more "back"-type links on those two pages, on the lost password screen for example there's the same link back to the front page, but also a link back to the login page, which perhaps need to be presented in a different way from the login view it self? (I don't know here, but bringing it up for the sake of completeness)

#38 @tobifjellner
4 years ago

What languages should be offered in the drop-down?
One way is to offer any language that is available for WordPress core.
Upside: Great for a real user who want to log in but doesn't know the main language of the site.
Downside: An attacker could force the site it to download a lot of unnecessary language files, using more traffic and storage space. (Remember that once a certain language is installed, WordPress will automatically download available translations of these languages for all installed plugins and themes...

Possible mitigations:
Either only offer languages that are already installed. (If the site uses English US and no other languages are installed, then the language chooser should be removed or at least hidden.)
Or prepare a special micro language package that only covers login/password reset and won't trigger download of any more language files until the user has authenticated or registered correctly.
Perhaps the choice of approach should be available to the site admin, if not as a setting, then via filter or a constant.

#39 @joedolson
3 years ago

I think that defaulting to only languages that are already installed is a good option. It covers most practical cases. A special micro language pack could be created - but that seems like it would be a larger project to cover a handful of edge cases.

#40 @johnbillion
3 years ago

With the current patch the currently installed languages are listed. This means the language picker on the log in screen works the same way as that on the user profile editing screen.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


3 years ago

#42 @joedolson
3 years ago

  • Milestone changed from Future Release to 5.9

Let's try and get this done in 5.9. @johnbillion If you don't think you'll have time to shepherd this, feel free to assign me as the owner.

#43 @johnbillion
3 years ago

  • Owner changed from johnbillion to joedolson

👍

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


3 years ago

#45 @Hareesh Pillai
3 years ago

This ticket was discussed in Slack during the scrub yesterday.

The option to show only the installed languages in the site seems like a good approach. In addition to the reasons stated by @tobifjellner above, these are the languages the user will anyway have to use after logging in.

One case to be handled: How would this behave if no other language (apart from the default) is installed?

#46 @paaljoachim
3 years ago

Hey @hareesh-pillai

If there is only one language installed there there is no need for a language switcher, and the switcher should not be seen on the login screen.

@sabernhardt
3 years ago

removing language switcher from interim login, small CSS changes

@sabernhardt
3 years ago

option: moving switcher to the top, below heading/logo

#47 @sabernhardt
3 years ago

  • Keywords has-patch added; needs-testing needs-refresh needs-patch removed

The switcher does not belong in the interim login modal.

CSS changes:

  • I removed the line-height from the label so it centers vertically.
  • The select dropdown has a right margin of 0.5em, equal to that on the label.
  • The side padding is removed in both patches, but the bottom spacing now uses padding instead of margin when the switcher is at the end of the page (43700.4.diff).

Note: I did not rearrange the links, if that is necessary to complete this ticket.

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


3 years ago

@sabernhardt
3 years ago

reducing top margin for privacy link

@sabernhardt
3 years ago

screenshots in English

#49 @sabernhardt
3 years ago

Keeping the switcher form at the bottom may be preferred, so I only updated the privacy link margin for that option in 43700.5.diff.

I should note that the width of the select depends on the length of the options. A few languages are wider than "English (United States)" (including "Español de República Dominicana"). Also, the screenshots do not include the Register link, but that is on the same line as the lost password link.

This would need testing with RTL languages, too.

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


3 years ago

#51 @audrasjb
3 years ago

  • Keywords has-screenshots added

Thanks for the screenshots @sabernhardt!

Keeping the switcher at the bottom is my fav option, so it doesn't introduce much change in the current implementation.

I think we're pretty good now.

#52 @sabernhardt
3 years ago

  • Keywords needs-testing added

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


3 years ago

@joedolson
3 years ago

Code adjustments to 43700.5.diff

#54 @joedolson
3 years ago

Patch includes changes in sanitizing/escaping & removes the 30px css height on the select element, which causes accessibility issues when text size was increased.

#55 @joedolson
3 years ago

With the css height removed, this looks good. There's a slight horizontal scrollbar on narrow screens with greater than 200% text zoom caused by the margin-right on the select input; it's a pretty minor problem, but I'm going to update the patch again to remove it in the narrow viewport.

@joedolson
3 years ago

CSS improvements for narrow viewports/text zoom

#56 @joedolson
3 years ago

  • Keywords commit added; needs-testing removed

Testing-wise, this is all good. Marking for commit.

#57 @joedolson
3 years ago

  • Keywords commit removed

Nope, never mind. One remaining question to resolve. Per https://github.com/WordPress/wordpress-develop/pull/248, are we going to set the user's locale on log-in/registration to their selected locale?

I'm not sure it makes sense to change a user's locale on log-in; we should possibly just assume that they have already chosen their preferred locale. But for new users, if you choose a language during registration, I think you'd expect not to have to set that again.

@joedolson
3 years ago

Set locale on user registration

#58 @joedolson
3 years ago

Patch 8 sets the user locale only on user registration. I think this is sufficient; doing it on user login just seems unnecessary and could yield unexpected results.

@joedolson
3 years ago

validate locale before saving to user meta

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


3 years ago

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


3 years ago

#61 @joedolson
3 years ago

  • Keywords commit added

#62 @joedolson
3 years ago

Per conversation on Slack, we're agreed that setting the user locale on registration but not on log-in is reasonable. Registration makes sense because it would add user burden to have to select their language twice, but log-in makes less sense because the log-in form is not a reasonable place to configure settings and existing users are likely to have already set their language, anyway.

#63 @joedolson
3 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 52058:

I18N: Add language switcher on login/registration screens.

Load a language switcher on the login and registration screens that allows users to choose any already-installed language. Set user locale on registration.

Props johnbillion, Nikschavan, afercia, sabernhardt, garrett-eclipse, keyur5, paaljoachim, Clorith, tobifjellner.
Fixes #43700.

#64 @paaljoachim
3 years ago

Great!
Thank you Joe and everyone else involved!

#65 @TimothyBlynJacobs
3 years ago

Can we add an action inside the <form> body? AFAICT if you have any other parameters that you need to keep, like redirect_to, there isn't a clean way to do that.

#66 @joedolson
3 years ago

@TimothyBlynJacobs redirect_to is handled as a hidden input added as needed.

#67 @TimothyBlynJacobs
3 years ago

Sorry, to clarify, I mean if you have any other parameters you want to keep. Only the ones built into Core are handled, but plugins have no opportunity to inject their data.

#68 @joedolson
3 years ago

In 52060:

Coding Standards: PHP Code style errors.

Fixes coding standards issues.

Follow up to [52058].

See #43700.

#69 @joedolson
3 years ago

In 52063:

Coding Standards: PHP Code style errors.

Fixes coding standards issues.

Follow up to [52060].

See #43700.

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


3 years ago

#71 @joedolson
3 years ago

@TimothyBlynJacobs Seems reasonable to add that as an enhancement or bug fix follow-up.

#72 @SergeyBiryukov
3 years ago

In 52076:

Coding Standards: Revert unrelated change to wp_send_user_request().

This fixes a mix of tabs and spaces in the email template. Spaces inside a string are not considered WPCS errors, and are not required to be converted to tabs.

Follow-up to [52060], [52063].

See #43700.

#73 @sabernhardt
3 years ago

In [52078]:

Coding Standards: Minor alignment fix after [52058].

#74 @johnbillion
3 years ago

  • Keywords needs-dev-note added

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


3 years ago

Note: See TracTickets for help on using tickets.