Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

system.languages.session_store_active: Language never stored in, or retrieved from session #3269

Closed
pamtbaau opened this issue Mar 14, 2021 · 3 comments
Assignees

Comments

@pamtbaau
Copy link
Contributor

pamtbaau commented Mar 14, 2021

The following setting in system.yaml is supposed to store the chosen language extension in the url into the session:

languages:
  supported: [it, en]
  session_store_active: true

However, the chosen language will never be stored in, nor retrieved from, the session, because function Language::setActiveFromUri($uri) is called before the session has been initialized.

InitializeProcessor.php
In InitializeProcessor::process(), the line $this->initializeUri($config);, which calls Language::setActiveFromUri($uri) is executed before $this->initializeSession($config);

// Initialize URI.
$this->initializeUri($config);
// Grav may return redirect response right away.
$redirectCode = (int)$config->get('system.pages.redirect_trailing_slash', 1);
if ($redirectCode) {
$response = $this->handleRedirectRequest($request, $redirectCode > 300 ? $redirectCode : null);
if ($response) {
$this->stopTimer('_init');
return $response;
}
}
// Load accounts (decides class to be used).
// TODO: remove in 2.0.
$this->container['accounts'];
// Initialize session.
$this->initializeSession($config);

Language.php
Because of the above, the following if-statements, meant to store and retrieve the language value from the session, will always evaluate to false:

// Store in session if language is different.
if (isset($this->grav['session']) && $this->grav['session']->isStarted()
&& $this->config->get('system.languages.session_store_active', true)
&& $this->grav['session']->active_language != $this->active
) {
$this->grav['session']->active_language = $this->active;
}
} else {
// Try getting language from the session, else no active.
if (isset($this->grav['session']) && $this->grav['session']->isStarted() &&
$this->config->get('system.languages.session_store_active', true)) {
$this->setActive($this->grav['session']->active_language ?: null);
}

@mahagr mahagr self-assigned this Mar 15, 2021
@mahagr mahagr added the bug label Mar 15, 2021
@mahagr
Copy link
Member

mahagr commented Mar 15, 2021

The language code should really be inside an event SessionStartEvent (class).

@mahagr
Copy link
Member

mahagr commented Apr 9, 2021

@pamtbaau Can you test my fix? :)

@pamtbaau
Copy link
Contributor Author

pamtbaau commented Apr 9, 2021

Just did and it seems to be working fine.

  • Fresh install Grav 1.7.10
  • system.yaml
    languages:
      supported: [de, en]
      session_store_active: true
    
  • browse to 'localhost/en/typography'
  • browse to 'localhost/typography' ---redirect---> localhost/en/typography
  • browse to 'localhost/de/typography'
  • browse to 'localhost/typography' ---redirect---> localhost/de/typography

@mahagr mahagr closed this as completed Apr 13, 2021
nbusseneau added a commit to nbusseneau/grav that referenced this issue Oct 12, 2021
At the moment and since 1.7.19, the `system.languages.session_store_active`
setting has no effect.

Session must be initialized before URI for `$language->setActiveFromUri($uri)`
(called from `$this->initializeUri($config) -> $uri->init()`) to
properly retrieve / store `active_language` in Session.

This was previously detected in getgrav#3269 as per the code comment, but got
reversed in 2e9fe80.
mahagr pushed a commit that referenced this issue Oct 20, 2021
At the moment and since 1.7.19, the `system.languages.session_store_active`
setting has no effect.

Session must be initialized before URI for `$language->setActiveFromUri($uri)`
(called from `$this->initializeUri($config) -> $uri->init()`) to
properly retrieve / store `active_language` in Session.

This was previously detected in #3269 as per the code comment, but got
reversed in 2e9fe80.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants