Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

[HttpRequest] fixes Request::getLanguages() bug #7378

Closed
wants to merge 2 commits into from

5 participants

@jfsimon

This PR adds to suported languages the first segment of all compouds languages codes.
When receiving Accept-Language: en-us header, accepted languages will now be en, en_US.

This should not be a BC break as most browsers already send the long and short versions of language codes... but some dont.

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #6928
src/Symfony/Component/HttpFoundation/Request.php
@@ -1309,6 +1309,11 @@ public function getLanguages()
for ($i = 0, $max = count($codes); $i < $max; $i++) {
if ($i == 0) {
$lang = strtolower($codes[0]);
+ // First segment of compound language codes
+ // is added to supported languages list
+ if(!in_array($lang, $this->languages)) {
@lsmith77 Collaborator

missing space after if

@jfsimon
jfsimon added a note

fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/Symfony/Component/HttpFoundation/Request.php
@@ -1316,7 +1321,9 @@ public function getLanguages()
}
}
- $this->languages[] = $lang;
+ if(!in_array($lang, $this->languages)) {
@lsmith77 Collaborator

missing space after if

@jfsimon
jfsimon added a note

fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fabpot fabpot referenced this pull request from a commit
@fabpot fabpot merged branch jfsimon/issue-6928 (PR #7378)
This PR was squashed before being merged into the 2.1 branch (closes #7378).

Commits
-------

17dc2ff [HttpRequest] fixes Request::getLanguages() bug

Discussion
----------

[HttpRequest] fixes Request::getLanguages() bug

This PR adds to suported languages the first segment of all compouds languages codes.
When receiving `Accept-Language: en-us` header, accepted languages will now be `en, en_US`.

This should not be a BC break as most browsers already send the long **and** short versions of language codes... but some dont.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #6928
70ec4f6
@fabpot fabpot closed this
@stof stof commented on the diff
...ymfony/Component/HttpFoundation/Tests/RequestTest.php
@@ -951,8 +951,8 @@ public function testGetLanguages()
$request = new Request();
$request->headers->set('Accept-language', 'zh, en-us; q=0.8, en; q=0.6');
- $this->assertEquals(array('zh', 'en_US', 'en'), $request->getLanguages());
- $this->assertEquals(array('zh', 'en_US', 'en'), $request->getLanguages());
+ $this->assertEquals(array('zh', 'en', 'en_US'), $request->getLanguages());
+ $this->assertEquals(array('zh', 'en', 'en_US'), $request->getLanguages());
@stof Collaborator
stof added a note

@jfsimon This is a BC break as en would now be preferred over en_US here.

@jfsimon
jfsimon added a note

@stof oh, good one! I have not thought about the order...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jfsimon jfsimon referenced this pull request from a commit in jfsimon/symfony
@jfsimon jfsimon Revert "merged branch jfsimon/issue-6928 (PR #7378)"
This reverts commit 70ec4f6, reversing
changes made to 3a03f3e.
839c78a
@fabpot fabpot referenced this pull request from a commit
@fabpot fabpot merged branch jfsimon/issue-7378 (PR #7438)
This PR was merged into the 2.1 branch.

Commits
-------

c928ddc [HttpFoudantion] fixed Request::getPreferredLanguage()
839c78a Revert "merged branch jfsimon/issue-6928 (PR #7378)"

Discussion
----------

[HttpFoundation] fixed Request::getPreferredLanguage()

Previous PR #7378 was wrong and adding BC break. Resolution for short languages codes is now done in `Request::getPreferredLanguage()` method.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7378
19955c5
@fabpot fabpot referenced this pull request from a commit
@fabpot fabpot Merge branch '2.1' into 2.2
* 2.1:
  #7106 - fix for ZTS builds
  Added '@@' escaping strategy for YamlFileLoader and YamlDumper
  [Yaml] fixed bugs with folded scalar parsing
  [Form] made DefaultCsrfProvider using session_status() when available
  Added unit tests to Dumper
  Update .travis.yml (closes #7355)
  [HttpFoudantion] fixed Request::getPreferredLanguage()
  Revert "merged branch jfsimon/issue-6928 (PR #7378)"
  Routing issue with installation in a sub-directory ref: #7129

Conflicts:
	.travis.yml
	src/Symfony/Bundle/FrameworkBundle/Routing/Router.php
	src/Symfony/Component/Routing/RouteCollection.php
03fc97d
@fabpot fabpot referenced this pull request from a commit
@fabpot fabpot Merge branch '2.2'
* 2.2:
  #7106 - fix for ZTS builds
  Added '@@' escaping strategy for YamlFileLoader and YamlDumper
  [Yaml] fixed bugs with folded scalar parsing
  [Form] made DefaultCsrfProvider using session_status() when available
  Added unit tests to Dumper
  Update .travis.yml (closes #7355)
  [HttpFoudantion] fixed Request::getPreferredLanguage()
  Revert "merged branch jfsimon/issue-6928 (PR #7378)"
  Routing issue with installation in a sub-directory ref: #7129
77ec799
@mmucklo mmucklo referenced this pull request from a commit
@fabpot fabpot merged branch jfsimon/issue-6928 (PR #7378)
This PR was squashed before being merged into the 2.1 branch (closes #7378).

Commits
-------

17dc2ff [HttpRequest] fixes Request::getLanguages() bug

Discussion
----------

[HttpRequest] fixes Request::getLanguages() bug

This PR adds to suported languages the first segment of all compouds languages codes.
When receiving `Accept-Language: en-us` header, accepted languages will now be `en, en_US`.

This should not be a BC break as most browsers already send the long **and** short versions of language codes... but some dont.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #6928
586be23
@mmucklo mmucklo referenced this pull request from a commit
@jfsimon jfsimon Revert "merged branch jfsimon/issue-6928 (PR #7378)"
This reverts commit 70ec4f6, reversing
changes made to 3a03f3e.
ea66d00
@mmucklo mmucklo referenced this pull request from a commit
@fabpot fabpot merged branch jfsimon/issue-7378 (PR #7438)
This PR was merged into the 2.1 branch.

Commits
-------

c928ddc [HttpFoudantion] fixed Request::getPreferredLanguage()
839c78a Revert "merged branch jfsimon/issue-6928 (PR #7378)"

Discussion
----------

[HttpFoundation] fixed Request::getPreferredLanguage()

Previous PR #7378 was wrong and adding BC break. Resolution for short languages codes is now done in `Request::getPreferredLanguage()` method.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7378
496c83b
@mmucklo mmucklo referenced this pull request from a commit
@fabpot fabpot Merge branch '2.1' into 2.2
* 2.1:
  #7106 - fix for ZTS builds
  Added '@@' escaping strategy for YamlFileLoader and YamlDumper
  [Yaml] fixed bugs with folded scalar parsing
  [Form] made DefaultCsrfProvider using session_status() when available
  Added unit tests to Dumper
  Update .travis.yml (closes #7355)
  [HttpFoudantion] fixed Request::getPreferredLanguage()
  Revert "merged branch jfsimon/issue-6928 (PR #7378)"
  Routing issue with installation in a sub-directory ref: #7129

Conflicts:
	.travis.yml
	src/Symfony/Bundle/FrameworkBundle/Routing/Router.php
	src/Symfony/Component/Routing/RouteCollection.php
4105be1
@hackzilla hackzilla referenced this pull request from a commit
@jfsimon jfsimon Revert "merged branch jfsimon/issue-6928 (PR #7378)"
This reverts commit 70ec4f6, reversing
changes made to 3a03f3e.
fcecba0
@hackzilla hackzilla referenced this pull request from a commit
@fabpot fabpot Merge branch '2.1' into 2.2
* 2.1:
  #7106 - fix for ZTS builds
  Added '@@' escaping strategy for YamlFileLoader and YamlDumper
  [Yaml] fixed bugs with folded scalar parsing
  [Form] made DefaultCsrfProvider using session_status() when available
  Added unit tests to Dumper
  Update .travis.yml (closes #7355)
  [HttpFoudantion] fixed Request::getPreferredLanguage()
  Revert "merged branch jfsimon/issue-6928 (PR #7378)"
  Routing issue with installation in a sub-directory ref: #7129

Conflicts:
	.travis.yml
	src/Symfony/Bundle/FrameworkBundle/Routing/Router.php
	src/Symfony/Component/Routing/RouteCollection.php
6af424d
@hackzilla hackzilla referenced this pull request from a commit
@fabpot fabpot Merge branch '2.2'
* 2.2:
  #7106 - fix for ZTS builds
  Added '@@' escaping strategy for YamlFileLoader and YamlDumper
  [Yaml] fixed bugs with folded scalar parsing
  [Form] made DefaultCsrfProvider using session_status() when available
  Added unit tests to Dumper
  Update .travis.yml (closes #7355)
  [HttpFoudantion] fixed Request::getPreferredLanguage()
  Revert "merged branch jfsimon/issue-6928 (PR #7378)"
  Routing issue with installation in a sub-directory ref: #7129
db5ae8b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 14, 2013
  1. @mweimerskirch @jfsimon

    Improve preferred language detection

    mweimerskirch authored jfsimon committed
Commits on Mar 18, 2013
  1. @jfsimon
This page is out of date. Refresh to see the latest.
View
9 src/Symfony/Component/HttpFoundation/Request.php
@@ -1309,6 +1309,11 @@ public function getLanguages()
for ($i = 0, $max = count($codes); $i < $max; $i++) {
if ($i == 0) {
$lang = strtolower($codes[0]);
+ // First segment of compound language codes
+ // is added to supported languages list
+ if (!in_array($lang, $this->languages)) {
+ $this->languages[] = $lang;
+ }
} else {
$lang .= '_'.strtoupper($codes[$i]);
}
@@ -1316,7 +1321,9 @@ public function getLanguages()
}
}
- $this->languages[] = $lang;
+ if (!in_array($lang, $this->languages)) {
+ $this->languages[] = $lang;
+ }
}
return $this->languages;
View
8 src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
@@ -951,8 +951,8 @@ public function testGetLanguages()
$request = new Request();
$request->headers->set('Accept-language', 'zh, en-us; q=0.8, en; q=0.6');
- $this->assertEquals(array('zh', 'en_US', 'en'), $request->getLanguages());
- $this->assertEquals(array('zh', 'en_US', 'en'), $request->getLanguages());
+ $this->assertEquals(array('zh', 'en', 'en_US'), $request->getLanguages());
+ $this->assertEquals(array('zh', 'en', 'en_US'), $request->getLanguages());
@stof Collaborator
stof added a note

@jfsimon This is a BC break as en would now be preferred over en_US here.

@jfsimon
jfsimon added a note

@stof oh, good one! I have not thought about the order...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
$request = new Request();
$request->headers->set('Accept-language', 'zh, en-us; q=0.6, en; q=0.8');
@@ -969,6 +969,10 @@ public function testGetLanguages()
$request = new Request();
$request->headers->set('Accept-language', 'zh, i-cherokee; q=0.6');
$this->assertEquals(array('zh', 'cherokee'), $request->getLanguages());
+
+ $request = new Request();
+ $request->headers->set('Accept-language', 'en-us');
+ $this->assertEquals(array('en', 'en_US'), $request->getLanguages());
}
public function testGetRequestFormat()
Something went wrong with that request. Please try again.