WordPress.org
Get WordPress

Make WordPress Core

Opened 7 years ago

Last modified 6 months ago

#47546 new defect (bug)

The language used for the `lang` attribute isn't validated before being used

Reported by: johnbillion's profile johnbillion Owned by:
Milestone: Future Release Priority: low
Severity: minor Version:
Component: I18N Keywords: has-patch has-test-info changes-requested
Focuses: Cc:

Description

In the admin area, the lang attribute can be populated directly from the WPLANG option or the user's locale meta field. In these cases, no check is performed to ensure that the corresponding language files exist before using the value for the lang attribute.

This can come about if the language files that are used by a user or the site are deleted, or the value is otherwise modified to include an incorrect value.

For example, running wp user meta update admin locale it_IT will result in lang="it_IT" being added to the <html> element, even if the corresponding language files don't exist. This can result in the browser being confused about the language of the page, and can also result in assistive technology handling or pronouncing text incorrectly.

It would be preferable if the existence of the corresponding language files was checked before the lang attribute was output, falling back to en_US as necessary. I'm not sure if it's best to make this change in the get_language_attributes() function, or lower in the determine_locale() or get_bloginfo() functions.

Attachments (1)

47546.diff​ (409 bytes) - added by krynes 6 years ago.
I suggest to check if core language is installed and then display. If not return default Wordpress language

Download all attachments as: .zip

Change History (12)

@krynes
6 years ago

I suggest to check if core language is installed and then display. If not return default Wordpress language

#1 @krynes
6 years ago

  • Keywords has-patch added; needs-patch removed

#2 @johnbillion
6 years ago

  • Keywords needs-testing added

Thanks for the patch @krynes.

Is there a performance overhead with calling get_available_languages()?

#3 @justinahinon
5 years ago

get_available_languages() takes ~1(exp-2) milliseconds more time to run than just get_locale().

Also I tested 47546.diff and it's applying correctly.

#4 @swissspidy
22 months ago

  • Keywords needs-patch added; has-patch removed

I wouldn't necessarily want to call get_available_languages() on the frontend. The function is cached now though in 6.5, which is good.

However, IMO this should be handled when saving the user meta, like edit_user() does.

That could be further improved by registering these options with register_meta / register_setting and a dedicated sanitization callback.

#5 @swissspidy
13 months ago

Circling back here, why not simply use is_textdomain_loaded( 'default' ) or has_translation( 'html_lang_attribute' )?

#6 @swissspidy
13 months ago

  • Milestone changed from Awaiting Review to Future Release
  • Priority changed from normal to low
  • Severity changed from normal to minor

This ticket was mentioned in ​PR #8942 on ​WordPress/wordpress-develop by ​@sukhendu2002.
6 months ago #7

  • Keywords has-patch added; needs-patch removed

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

#8 @SirLouen
6 months ago

  • Keywords needs-test-info added; needs-testing removed

Can you ​provide test info for your proposal

Last edited 6 months ago by SirLouen (previous) (diff)

#9 @SirLouen
6 months ago

cc @sukhendu2002

#10 @sukhendu2002
6 months ago

Hi @SirLouen, I've added test information to the PR.

#11 @SirLouen
6 months ago

  • Keywords has-test-info changes-requested added; needs-test-info removed

Test Report

Description

  • ❌ This report validates whether the indicated patch is working as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8942.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 137.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-One 2.5
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Test Instructions

  1. So here there ​are some clear instructions
  2. But I have combined them with the ones in the OP
  3. Make sure you have the Spanish language installed. Got to wp-content/languages and check that you have the es_ES files.
  4. Issue the wp-cli command wp user meta update admin locale es_ES to set a custom locale for admin interface
  5. Check the HTML tag in the WP administration area
  6. ✅ lang=es in the HTML tag, as expected, since es_ES is installed
  7. Now Make sure you don't have Italian language installed. Go to wp-content/languages and remove all *it_IT* files.
  8. Issue the wp-cli command wp user meta update admin locale it_IT to set a custom locale for admin interface
  9. Check the HTML tag in the WP administration area
  10. 🐞 lang=it_IT despite that there is are no it_IT lang files installed

Expected Results

  • Considering that en_US is the default installed language, it should default to this if the selected custom lang is not installed.

Actual Results

  1. ❌ Issue not resolved with patch: it forces to en_US in all scenarios

Additional Notes

  • @sukhendu2002 review this. When I have for example es_ES lang files, I set es_ES in cli and then I go into the admin, it shows en_US. This doesn't happen without the patch, so it's a regression.

Also, by the way, try to write everything here in Trac, not in GitHub. It's better to keep full context here in Trac, because it doesn't matter if anyone else opens a new PR, we will have all the context always here by all the collaborators that worked in the report.

Note: See TracTickets for help on using tickets.