WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 years ago

#29892 reviewing defect (bug)

get_available_languages() can alternately too and not enough permissive

Reported by: imath Owned by: ocean90
Milestone: Future Release Priority: low
Severity: normal Version: 4.0
Component: I18N Keywords: has-patch
Focuses: administration Cc:

Description

Since 4.0, it's possible to switch site languages from the language dropdown in options-general.php. This is a great feature.

But i think there can be cases when get_available_languages() is too permissive and others not enough.

1/ Too permissive If for some reason, a theme or plugin language file is added at the root of WP_LANG_DIR, then the theme or plugin language file will be listed in the dropdown because it names does not start with 'continents-cities' or 'ms-' or 'admin-'. It can be confusing for the site admin.

2/ Not enough (might be an edge case) get_available_languages() accepts a $dir param. So although it's not used in core, i guess plugins/themes can use this function to list language files of a given directory. In this case, if one of the file starts with 'continents-cities' or 'ms-' or 'admin-' then it won't be listed.

The attached patch is suggesting a way to be sure only site language files will be listed if the $dir param is null or == WP_LANG_DIR else to list all language files.

Attachments (3)

29892.patch (903 bytes) - added by imath 4 years ago.
29892.2.patch (3.1 KB) - added by ocean90 4 years ago.
29892.3.patch (3.2 KB) - added by ocean90 4 years ago.

Download all attachments as: .zip

Change History (12)

@imath
4 years ago

#1 follow-up: @ocean90
4 years ago

  • Component changed from Administration to I18N
  • Focuses administration added

This is related to #29297.

My plan is to deprecate get_available_languages() in favor of wp_get_installed_translations().

wp_get_installed_translations() needs a change for core translations: It should only return the language when all 4 files exists.

#2 in reply to: ↑ 1 ; follow-up: @nacin
4 years ago

Replying to ocean90:

wp_get_installed_translations() needs a change for core translations: It should only return the language when all 4 files exists.

Not sure if I agree with that. Only one is necessary, maybe two. One is multisite-only, while a fourth is not at all required (continents-cities) and is even omitted by one of the English translations (Australian or Canadian).

#3 in reply to: ↑ 2 @ocean90
4 years ago

  • Keywords needs-patch added

Replying to nacin:

Only one is necessary, maybe two. One is multisite-only, while a fourth is not at all required (continents-cities) and is even omitted by one of the English translations (Australian or Canadian).

Totally right. I think we should start to replace get_available_languages() with wp_get_installed_translations( 'core' ) and see what happens.

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

@ocean90
4 years ago

@ocean90
4 years ago

#4 @ocean90
4 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Awaiting Review to 4.1
  • Type changed from enhancement to defect (bug)

29892.2.patch makes get_available_languages( $dir ) a wrapper for wp_get_installed_translations( 'core', $lang_dir ) which only returns the locales for the default textdomain.

29892.3.patch checks also if the admin textdomain exists.

#5 @ocean90
4 years ago

  • Keywords 4.2-early added
  • Milestone changed from 4.1 to Future Release

#6 @iseulde
3 years ago

  • Milestone changed from Future Release to 4.2

has-patch 4.2-early so moving to 4.2.

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


3 years ago

#8 @DrewAPicture
3 years ago

  • Owner set to ocean90
  • Status changed from new to reviewing

Looks like 29892.3.patch still applies.

@ocean90: Can you please review here and make a recommendation for or against 4.2 inclusion? :)

#9 @ocean90
3 years ago

  • Keywords 4.2-early removed
  • Milestone changed from 4.2 to Future Release
  • Priority changed from normal to low

Yeah, it still applies. But I don't like adding $lang_dir to wp_get_installed_translations().

Note: See TracTickets for help on using tickets.