Opened 12 years ago
Last modified 21 months ago
#29892 reviewing defect (bug)
get_available_languages() can alternately too and not enough permissive
| Reported by: | imath | Owned by: | ocean90 |
|---|---|---|---|
| Priority: | low | Milestone: | Future Release |
| Component: | I18N | Version: | 4.0 |
| Severity: | normal | Keywords: | has-patch needs-refresh dev-feedback |
| Cc: | Focuses: | administration |
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)
Change History (14)
#2
in reply to: ↑ 1
;
follow-up:
↓ 3
@
12 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
@
12 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.
#4
@
12 years ago
- Keywords has-patch added; needs-patch removed
- Milestone Awaiting Review → 4.1
- Type enhancement → 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.
This ticket was mentioned in Slack in #core by drew. View the logs.
11 years ago
#8
@
11 years ago
- Owner set to
- Status new → 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
@
11 years ago
- Keywords 4.2-early removed
- Milestone 4.2 → Future Release
- Priority normal → low
Yeah, it still applies. But I don't like adding $lang_dir to wp_get_installed_translations().
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
This is related to #29297.
My plan is to deprecate
get_available_languages()in favor ofwp_get_installed_translations().wp_get_installed_translations()needs a change for core translations: It should only return the language when all 4 files exists.