onKernelRequest overwrites the locale

[spantaleev]

Requests (unless I'm missing something) have an en locale by default when created automatically (Request::createFromGlobals()) or when created based on a previous request inside TwigCoreExtension::render(), no matter if that previous request has a different locale.

Since 38d505, onKernelRequest does:

$this['locale'] = $event->getRequest()->getLocale();

This causes $app['locale'] to always get set to en. Apps usually determine and set $app['locale'] inside a before() filter, so this doesn't matter that much for the first (master) request. Filters, however, are not executed for subrequests. The first subrequest that comes up will set the locale to en and it won't get restored when going back to the master request.

It seems like setting the locale based on the request data is not a good idea here. Especially since there is no way for a Request object to have a non-en locale set unless it's specifically created and setLocale() or setDefaultLocale() is called on it.

If this behavior in onKernelRequest really needs to remain in place, we can add some more code to restore the locale after a subrequest.

I'd say it's best to avoid touching $app['locale'] anywhere, but maybe I don't see the whole picture.

[ghost]

If I'm understanding you correctly, you're assuming that a request with the "es" locale, for example, overrides the "en" locale.

I think that is the intended behavior.

If the "es" locale is not available, the TranslationServiceProvider is configured to fallback to what ever you have $app['locale_fallback'] set to.

[spantaleev]

Yes, a request with any locale overrides the currently set $app['locale'].

When a subrequest starts being handled this takes places again, and once it's handled the previous $app['locale'] value is not restored.

So, if you have a before() filter that sets $app['locale'] to es and make a subrequest somewhere, you'll see that when back on the master request $app['locale'] is now not the expected es, but most likely en (default for Request objects).

This whole thing seems to make TwigCoreExtension::render() quite useless, as any Request created by it has the en locale, which mess up your initial $app['locale'] value and when you get back to the master request where you used render() you'll find yourself wondering what happened to the locale value.

I am against Silex doing such kind-of-before() "magic" where it touches the container values. If this needs to happen for some reason, maybe it should be moved to an extension that registers the correct before/after event handlers with the kernel and does it properly.

[stof]

@spantaleev there is 2 ways to have a different locale in the request:

• having a locale stored in the session
• matching a _locale placeholder during the routing

[spantaleev]

@stof, I see. This is handled by the LocaleListener subscriber in Symfony's HttpKernel component.

I'm not using any of those (and I could imagine other people sometimes doing the same), so all of my requests are for the default en locale. I am managing the $app['locale'] myself (for the master request only), so something else touching it later on every subrequest is a bit of a letdown.

I can see this as being my fault, for not going the "righteous" path of using the session or _locale methods.

Still, I think that if Silex changes $app['locale'] on every subrequest, it should at least be polite enough to restore it back.

[stof]

@spantaleev it is indeed an issue for subrequests. But anyway, subrequests will be painful on Silex as the clean way to handle them is to have a scoped container (which is why scopes have been introduced in the Symfony2 DI component whereas the first preview releases of Symfony did not have this feature). But such a container is too much for a micro-framework like Silex.

[spantaleev]

Sure. Pimple lacking scopes can make subrequests usage more painful by sometimes requiring certain workarounds. For all the cases I've used subrequests so far I don't recall it being problematic though.

[fabpot]

I've added some unit tests for the locale behavior. The problem is not how the locale is managed but indeed the fact that's for sub-requests, the before filter is not executed.

Unfortunately, the only way to manage the locale yourself is to choose a name different from locale.

[spantaleev]

I'm confused what the purpose of $app['locale'] is. It seems to do these things:

1) Configure a default locale value for requests (injected to the LocaleListener) 2) Store the "currently" handled request's locale - it actually stores the last-passed-to-handle() request's locale, because when we're back from a subrequest it doesn't get restored to what it was

What's the purpose of 2)?

[fabpot]

I've just updated the description of the locale parameter in the doc (see #): "The locale of the user. When set before any request handling, it defines the default locale (en by default). When a request is being handled, it is automatically set according to the _locale request attribute of the current route."

Right now, the locale is stored in the session if it exists so that it can be restored afterwards even if the request does not contain the _locale attribute. But as this is not a good practice, it will be removed in 2.1.