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

VERIFIED FIXED

Status

www.mozilla.org
General
--
critical
VERIFIED FIXED
7 years ago
6 years ago

People

(Reporter: stas, Assigned: Andre Torgal)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
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.
(Reporter)

Comment 1

7 years ago
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
(Reporter)

Comment 2

7 years ago
Created attachment 520811 [details] [diff] [review]
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.
Assignee: nobody → stas
Attachment #520811 - Flags: review?(pascalc)
Comment on attachment 520811 [details] [diff] [review]
Patch

looks good to me and is a simplification of my code
Attachment #520811 - Flags: review?(pascalc) → review+
Reassigning to Andre to commit to the repo.
Assignee: stas → andre
(Assignee)

Comment 5

7 years ago
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.
(Reporter)

Comment 6

7 years ago
> 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.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
(Reporter)

Comment 9

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

https://github.com/quodis/Twitter-Collage/commit/882bbf6a5ca6cf4bd93df012f2177774deb88d3a#diff-1
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 10

7 years ago
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
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
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
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
Status: RESOLVED → VERIFIED
Component: www.mozilla.org/firefox → www.mozilla.org
Product: Websites → Websites
Component: www.mozilla.org → General
Product: Websites → www.mozilla.org
You need to log in before you can comment on or make changes to this bug.