getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.32k stars 168 forks source link

Problem accessing languages in route closure #1473

Closed lukaskleinschmidt closed 5 years ago

lukaskleinschmidt commented 5 years ago

Describe the bug When adding a closure as a route accessing languages leads to an error.

To Reproduce

  1. Create a multilang site with two languages
  2. Add following to your config.php.
    
    <?php

// config.php

return [ 'languages' => true, 'routes' => function ($kirby) {

    foreach ($kirby->languages() as $language) {
        // do something with it
    }

    return [
      // does not matter
    ];
},

];

3. See error

I testet a bit around and it seems like [this part](https://github.com/getkirby/kirby/blob/master/src/Cms/App.php#L124) of the constructor prevents at least languages from working. When I move mentioned part at at least over [`$this->extensionsFromOptions();`](https://github.com/getkirby/kirby/blob/master/src/Cms/App.php#L117) the error will not be thrown any more. Not sure about any other complications such change would introduce though.

```diff
// setup the I18n class with the translation loader
$this->i18n();

+ // set the singleton
+ Model::$kirby = static::$instance = $this;

// load all extensions
$this->extensionsFromSystem();
$this->extensionsFromProps($props);
$this->extensionsFromPlugins();
$this->extensionsFromOptions();
$this->extensionsFromFolders();

// handle those damn errors
$this->handleErrors();

- // set the singleton
- Model::$kirby = static::$instance = $this;

// bake config
Config::$data = $this->options;

Expected behavior It should be possible to access languages within a route closure.

Kirby Version 3.0.1

WeTurkstra commented 5 years ago

Iam having a similair issue. If you have a plugin which uses a translation rendered in a route closure it picks the wrong language.

I have 'nl' as a default language. But in the I18n class it uses 'en' since the i18n::$locale is not set yet...

The problem is fixed with @lukaskleinschmidt. But like he said i dont know what else breaks moving this code up.

Can this please be reviewed?

distantnative commented 5 years ago

I was having problems with my Retour plugin in this direction as well and came up with the same idea for a fix - if only I would have seen/remembered that issue earlier 🙈 We will check carefully if this indeed is a valid fix or if it breaks other important parts.

bastianallgeier commented 5 years ago

That really fixes the issues so far as it seems. We need to test it a bit further but all unit tests pass right now.