643641 - Twitter Party: locale detection doesn't recognize some locales in the ab-cd form

Status: VERIFIED FIXED

(www.mozilla.org :: General, defect)

Description

[Staś Małolepszy :stas]

If the country part is given in the Accept Language header, the locale detection is broken -- it moves to the next locale in the header.

I have a working patch.

Comment 1

[Staś Małolepszy :stas]

Here's an example of this behavior:

> $ curl -sI -H'Accept-Language: en-us,en;q=0.7,ar;q=0.3' http://dev2.twitterparty.quodis.com/ | grep Location
> Location: http://dev2.twitterparty.quodis.com/ar

Comment 2

[Staś Małolepszy :stas]

Attachment: Patch

The issue was that once we got into the `if ($this->mapLonglocales == true)` part, the break statement would only break from the smaller loop (foreach ($this->supportedLocales as $var)), but the bigger loop would continue checking the next preferred locale in the Accept Language header.

Comment 3

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 520811 [details] [diff] [review]
Patch

looks good to me and is a simplification of my code

Comment 4

[William Reynolds [:williamr]]

Reassigning to Andre to commit to the repo.

Comment 5

[Andre Torgal]

commited

updated in http://dev2.twitterparty.quodis.com/
and http://dev.twitterparty.quodis.com/

does it really fix? I think an issue with the white list still remains.

Comment 6

[Staś Małolepszy :stas]

> curl -sI -H'Accept-Language: en-us,en;q=0.7,ar;q=0.3' http://dev2.twitterparty.quodis.com/ | grep Location
> Location: http://dev2.twitterparty.quodis.com/en-US

I think this is fixed now.

I would appreciate a thorough QA, though.  FWIW, I tested with more combinations of the Accept Language header and it looked good.

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED:

ohwhatagooseiam:~ stephendonner$ curl -sI -H'Accept-Language: en-us,en;q=0.7, ar;q=0.3' http://twitterparty.mozilla.org | grep Location
Location: http://twitterparty.mozilla.org/en-US

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Post-fix screenshot

Comment 9

[Staś Małolepszy :stas]

The fix for this was lost in a merge from the master branch:

https://github.com/quodis/Twitter-Collage/commit/882bbf6a5ca6cf4bd93df012f2177774deb88d3a#diff-1

Comment 10

[Andre Torgal]

done

updated on our server:

curl -sI -H'Accept-Language: en-us,en;q=0.7,ar;q=0.3'
http://dev-m.twitterparty.quodis.com/ | grep Location

also marked Bug 645851 as Fixed

Comment 11

[Rebecca Billings [:rbillings]]

Verified on quodis:
curl -sI -H'Accept-Language: es,en;q=0.7,ar;q=0.3' http://dev-m.twitterparty.quodis.com/ | grep Location

Location: http://dev-m.twitterparty.quodis.com/es

Comment 12

[Rebecca Billings [:rbillings]]

Verified on twittercollage.allizom.org
curl -sI -H'Accept-Language: es,en;q=0.7,ar;q=0.3' http://twittercollage.allizom.org/| grep Location

Location: http://twittercollage.allizom.org/es