Skip to content
This repository has been archived by the owner on Jul 4, 2018. It is now read-only.

Translator not working correctly #983

Closed
MHaendel opened this issue Jul 23, 2014 · 15 comments
Closed

Translator not working correctly #983

MHaendel opened this issue Jul 23, 2014 · 15 comments

Comments

@MHaendel
Copy link

$app['translator']->getLocale() returns empty,
$app['locale'] returns default fallback language (en)

@fabpot
Copy link
Member

fabpot commented Jul 23, 2014

Which version of Silex are you using? Can you submit a small script that reproduces the error?

@kwisatz
Copy link

kwisatz commented Jul 23, 2014

I can reproduce this behavior using Silex 1.2.1 (it still works as expected in 1.2.0)
I believe this is due to 2dcb2b1 where Silex\Translator is initializing its parent (Symfony\Component\Translation\Translator) passing null instead of app['locale'].

Changing the Symfony\Translator to the following returns Silex to its previous behavior, where registering the TranslationServiceProvider would be enough to set the current request locale as the active translator locale:

    public function __construct(Application $app, MessageSelector $selector)
    {
        $this->app = $app;

        parent::__construct($app['locale'], $selector);
    }

I must admit thought, I haven't fully understood how #830 and not passing the current locale to Symfony\Component\Translation\Translator are related.

I'll try to create a small example.

@MHaendel
Copy link
Author

Yes, Version is 1.2.1.

@kwisatz
Copy link

kwisatz commented Jul 23, 2014

OK, here's a simple example:

<?php

// Embed the composer-generated autoload file
require_once __DIR__ . '/../vendor/autoload.php';

$app = new Silex\Application();

$app->register(new Silex\Provider\TranslationServiceProvider(), array('locale_fallbacks' => array('en')));

$app['translator'] = $app->share($app->extend('translator', function ($translator, $app) {
    $translator->addLoader('yaml', new Symfony\Component\Translation\Loader\YamlFileLoader());

    $translator->addResource('yaml', __DIR__ . '/fr.yml', 'fr');
    $translator->addResource('yaml', __DIR__ . '/en.yml', 'en');
    $translator->addResource('yaml', __DIR__ . '/de.yml', 'de');

    return $translator;
}));

$app->get('/', function () use ($app) {
    return "Hello";
});

$app->get('/test/{_locale}', function () use ($app) {
    var_dump($app['locale'], $app['request']->getLocale(), $app['translator']->getLocale());
    return $app['translator']->trans('create');
});

$app->run();

Hit it with http://localhost:8080/test/fr and it should have:

string 'fr' (length=2)
string 'fr' (length=2)
string 'fr' (length=2)
Créer

but it does have:

string 'fr' (length=2)
string 'fr' (length=2)
null
Create

@henrikbjorn
Copy link
Contributor

There is nothing that calls setLocale between the request and the translator. Seems like the LocaleListener needs to be updated or you need a custom one.

@kwisatz
Copy link

kwisatz commented Jul 23, 2014

Yes, indeed, calling $app['translator']->setLocale() explicitely would work.

I'm not sure whether the current behaviour, where this is not done automatically is a bug or intended behavior.
In the second case, http://silex.sensiolabs.org/doc/providers/translation.html should probably get updated to say as much.

@freepius
Copy link
Contributor

I am (very) agree with @kwisatz !
And me too, I don't understand why #830.
For me, the new behaviour of Translator (introduced in Silex 1.2.1) is not good.

$app['locale'] and Translator should be independent.
Indeed, Translator uses $app['locale'] but $app['locale'] can be used by other providers/parts of code.

But, actually, the constructor of Translator (in fact, the one of its parent) modifies $app['locale'] through setLocale() and sets it to null... See :

So, if I define $app['locale'] before to declare the TranslationServiceProvider, my $app['locale'] will be override !

For me, two solutions.

Use the code of @kwisatz :

public function __construct(Application $app, MessageSelector $selector)
{
    $this->app = $app;
    parent::__construct($app['locale'], $selector);
}

Or better, return to the old behaviour ($app['locale'] and Translator independent).

@henrikbjorn
Copy link
Contributor

@freepius44 good catch! Could you do a UnitTest as a PR, maybe even with a fix?

There is two ways to fix this i think.

  1. Do as in your example.
  2. Use the Translator directly from Symfony and create a EventListener that sets the Locale on the translator based on the request (like LocaleListener does).

@freepius
Copy link
Contributor

It's done ! But my changes don't convince myself :-)

@henrikbjorn
Copy link
Contributor

I would go for the EventListener and remove the custom Translator :)

@Virakal
Copy link

Virakal commented Aug 5, 2014

Sorry if this has already been established, I believe this is caused by a Symfony 2.5.2 change (symfony/translation@v2.5.1...v2.5.2#diff-c93f36c15d5c29291cda962db092b47bR68) so that setLocale is now called during construction, forcing $app['locale'] to be nulled out, as opposed to Silex 1.2.1.

Using symfony/Translation v2.5.1 this behaves as expected for me, so that may be a reasonable workaround in the short term?

@futurevision
Copy link

This is still broken and I have to use nasty workarounds for it, when will a patch be available?

Easiest reproduction with Silex 1.2.1 and Symfony Translation 2.5.4:

require_once 'vendor/autoload.php';

$app = new Silex\Application;
$app->register(new Silex\Provider\TranslationServiceProvider);

var_dump($app['locale']);
$app['translator'];
var_dump($app['locale']);

output: string(2) "en" NULL

fabpot added a commit that referenced this issue Sep 26, 2014
This PR was merged into the 1.2 branch.

Discussion
----------

fixed translator

Fixes #1028, #988, #983, #1023

That makes the tests pass again.

Commits
-------

712375b fixed translator
@fabpot
Copy link
Member

fabpot commented Sep 26, 2014

see #1028

@fabpot fabpot closed this as completed Sep 26, 2014
@henrikbjorn
Copy link
Contributor

Wouldnt it have been better to use one of the other PR? Also there is one where the default translator is used instaed, which would mean less custom code in Silex and relying on events instead?

@fabpot
Copy link
Member

fabpot commented Sep 26, 2014

The event listener does not have the same behavior. But the idea is great and I've submitted a similar PR on Symfony for it. As the affected Silex version is 1.2, it was not possible to bump the requirement to Symfony 2.6 (not yet released.) But that's something we will do for Silex 2.0.

symfony/symfony#12046

fabpot added a commit that referenced this issue Apr 11, 2015
…r) (freepius44)

This PR was merged into the 2.0.x-dev branch.

Discussion
----------

A solution to resolve #983 (bad comportment of Translator)

Implementation of @henrikbjorn solution :

Use the Translator directly from Symfony and create a EventListener that sets the Locale on the translator based on the request (like LocaleListener does).

Commits
-------

36cfde2 A solution to resolve #983 (bad comportment of Translator)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

7 participants