Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpKernel] Set the default locale early#29483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
9fdb130 toc499c47Comparethewilkybarkid commentedDec 6, 2018
Failures are unrelated. |
src/Symfony/Component/HttpKernel/EventListener/LocaleListener.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
makes sense to me
src/Symfony/Component/HttpKernel/Tests/EventListener/LocaleListenerTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| // must be registered after the Router to have access to the _locale | ||
| KernelEvents::REQUEST =>array(array('onKernelRequest',16)), | ||
| KernelEvents::REQUEST =>array( | ||
| array('onKernelRequestEarly',100), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
should we discuss the "100"? (I didn't check - just asking :) )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Was matching the priority from#29186; only thing higher in core isSessionListener (andDebugHandlersListener). IMO these should be set as early as possible (so maybe even higher?).
src/Symfony/Component/HttpKernel/EventListener/LocaleListener.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
ro0NL left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
might deserve a changelog entry
nicolas-grekas commentedDec 13, 2018
which is another hint this should go on master - same as#29186 :) |
096a1e6 to02c9f35Comparefabpot commentedJan 2, 2019
Thank you@thewilkybarkid. |
…kid)This PR was merged into the 4.3-dev branch.Discussion----------[HttpKernel] Set the default locale early| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |Similar to#29186, the default locale isn't set till after the router so isn't available when trying to handle errors there (well, only the _default_ default locale is).Commits-------02c9f35 Set the default locale early
Uh oh!
There was an error while loading.Please reload this page.
Similar to#29186, the default locale isn't set till after the router so isn't available when trying to handle errors there (well, only thedefault default locale is).