Making WordPress.org

Opened 9 years ago

Closed 8 years ago

#1850 closed defect (bug) (fixed)

Language suggestor on the beta plugin repository is incorrect

Reported by: clorith's profile Clorith Owned by: ocean90's profile ocean90
Milestone: Plugin Directory v3.0 Priority: normal
Component: Plugin Directory Keywords: has-patch has-unit-tests
Cc:

Description

The language suggestor on the rest of WordPress.org suggest Danish, Norwegian (Bokmål) and Swedish, which are appropriate given me being in Norway and the headers I send.

Accepted-Language header
en-US,en;q=0.8,da;q=0.6,nb;q=0.4,sv;q=0.2
Hitting up the page from 79.160.197.135 in case GeoIP is kicking in and doing strange things.

Tested with the Yoast SEO plugin at https://wordpress.org/plugins-wp/wordpress-seo/ and being suggested Bulgarian (Български)

Attachments (2)

meta-1850.patch (1.2 KB) - added by SergeyBiryukov 8 years ago.
meta-1850.2.patch (3.2 KB) - added by SergeyBiryukov 8 years ago.

Download all attachments as: .zip

Change History (11)

#1 @Otto42
9 years ago

The beta repo doesn't have a custom language suggester, yet.

#3 @Otto42
9 years ago

Ahh, okay then. That's different than what I was looking for.

#4 @SergeyBiryukov
9 years ago

Confirmed:

accept-language: en-US,en;q=0.8,ru;q=0.6,cs;q=0.4

Results in: "This plugin is also available in Български (also: Русский)".

The rest of WordPress.org suggests Russian and Czech, as expected.

Last edited 8 years ago by SergeyBiryukov (previous) (diff)

#5 @SergeyBiryukov
8 years ago

  • Keywords has-patch commit added
  • Milestone set to Plugin Directory v3.0

Traced the issue to two bugs in Locale_Banner class:

  1. In Locale_Banner::locale_banner(), $suggest_locales is an array intersection between locales based on the HTTP Accept-Language header and all available locales. However, due to $locales_from_header being the second argument, the order of preference is defined by the list of all locales (which appears to be randomly returned from DB) instead of the locales provided by Accept-Language header. As a result, the primary language is determined incorrectly (in my case, Russian should be the first, but only comes second).
  2. In Locale_Banner::map_locale(), if an exact match is not found (e.g. when trying to map cs_CS to cs_CZ), an attempt is made to omit the region and only compare the language part. However, what it actually does is return the first non-empty locale from the list of all available locales, which happens to be bg_BG.

meta-1850.patch addresses both issues. Here's some debug info before the patch:

$http_locales: Array ( [0] => en-US [1] => en [2] => ru [3] => cs ) 
$lang: ru, $region: RU
$mapped: ru_RU
$lang: cs, $region: CS
$mapped: bg_BG
$translated_locales: Array ( [0] => bg_BG [1] => uk [2] => fr_FR [3] => tr_TR [4] => de_DE [5] => ru_RU [6] => cs_CZ ) 
$locales_from_header: Array ( [0] => ru_RU [1] => bg_BG ) 
array_intersect: Array ( [0] => bg_BG [5] => ru_RU ) 
$suggest_locales: Array ( [0] => bg_BG [1] => ru_RU ) 
$suggest_named_locales: Array ( [bg_BG] => Bulgarian [ru_RU] => Russian ) 

and after:

$http_locales: Array ( [0] => en-US [1] => en [2] => ru [3] => cs ) 
$lang: ru, $region: RU
$mapped: ru_RU
$lang: cs, $region: CS
$mapped: cs_CZ
$translated_locales: Array ( [0] => bg_BG [1] => uk [2] => fr_FR [3] => tr_TR [4] => de_DE [5] => ru_RU [6] => cs_CZ ) 
$locales_from_header: Array ( [0] => ru_RU [1] => cs_CZ ) 
array_intersect: Array ( [0] => ru_RU [1] => cs_CZ ) 
$suggest_locales: Array ( [0] => ru_RU [1] => cs_CZ ) 
$suggest_named_locales: Array ( [ru_RU] => Russian [cs_CZ] => Czech ) 

#6 follow-up: @ocean90
8 years ago

  • Keywords needs-unit-tests added; commit removed
  • Owner set to ocean90
  • Status changed from new to accepted

Thanks @SergeyBiryukov. We should add some unit tests for that.

#7 in reply to: ↑ 6 @SergeyBiryukov
8 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Replying to ocean90:

We should add some unit tests for that.

Yep, I was working on a test. See meta-1850.2.patch.

This ticket was mentioned in Slack in #meta-i18n by sergey. View the logs.


8 years ago

#9 @ocean90
8 years ago

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

In 5008:

Plugin Directory: Fix two incorrect cases when trying to match a HTTP header to a WordPress locale.

Includes tests.

Props SergeyBiryukov.
Fixes #1850.

Note: See TracTickets for help on using tickets.