symfony
symfony copied to clipboard
symfony
[HttpFoundation] Follow language preferences more accurately in `getPreferredLanguage()`
| Q | A |
|---|---|
| Branch? | 7.3 |
| Bug fix? | yes? |
| New feature? | no |
| Deprecations? | no |
| License | MIT |
This PR changes the getPreferredLanguage() method in the HttpFoundation\Request class to more accurately reflect the user's preferences. This new implementation also falls more in line with the HTTP and IETF standards, in particular RFC9110 and RFC4647 / BCP-47.
Let's take an example header from the HTTP spec (RFC9110): Accept-Language: da, en-gb;q=0.8, en;q=0.7
With that header the following function call $request->getPreferredLanguage(['da-DK', 'en-GB']) will result in:
Currently: en-GB
This PR: da-DK
Currently if you call this it will match en-GB as this is an exact match with one of the preferred languages. This is strange though, as the user has specifically said they would prefer Danish over English. With the new implementation it will always choose the language with the highest preference if it's available to them.
Three tests (which basically tested the inverse) have been changed to reflect this new behaviour.
I don't know if this change should classify as a bugfix or a change in design, so please advise.
Hey!
I see that this is your first PR. That is great! Welcome!
Symfony has a contribution guide which I suggest you to read.
In short:
- Always add tests
- Keep backward compatibility (see https://symfony.com/bc).
- Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
- Features and deprecations must be submitted against the 7.3 branch.
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.
When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.
I am going to sit back now and wait for the reviews.
Cheers!
Carsonbot
I agree about this qualifying as a feature. And then it's a behavior change for which I think we can anticipate complains. Could we make this opt-in somehow?
@Spomky Is there anything I need to do to tag it as an enhancement?
@chalasr I think given that the current situation does not follow any of the three matching schemes as defined in RFC4647, it would be kind of odd to retain this behaviour in Symfony itself. As it's quite trivial to implement this in your own code base by simply using array_intersect() on $request->getLanguages(), I personally don't see much reason to complicate the current implementation and maintain that old behaviour in some way.
