Discover a missing language correctly via the cookie

[Hackwar]

This tries to discover the right language via a potentially set cookie. The cookie overrides the other settings. If no cookie is set, the language is discovered based on the language of the browser. If that language is not supported, we fall back to the application default setting.

This fixes the remaining issue with cookies described in #6213.

Please test.

[N6REJ]

How does that fit in with the system->language filter plugin settings?
Bear
On 3/9/2015 10:07, Hannes Papenberg wrote:

If no cookie is set, the language is discovered based on the language
of the browser. If that language is not supported,

[Hackwar]

What do you mean exactly?

[N6REJ]

if the fallback is to go to browser language it could(?) override the
setting in the plugin which MIGHT be default to SITE language.
If the cookie is not set then next precedent should be plugin, THEN
application THEN browser..
Bear
On 3/9/2015 11:19, Hannes Papenberg wrote:

What do you mean exactly?

—
Reply to this email directly or view it on GitHub
#6371 (comment).

No virus found in this message.
Checked by AVG - www.avg.com http://www.avg.com
Version: 2015.0.5751 / Virus Database: 4299/9261 - Release Date: 03/09/15

[Hackwar]

There is no language set in the plugin. The browser should always override the default setting of the site if possible.

[Hackwar]

Besides that, as you can see in the code, it honours the setting in the plugin regarding discovery via browser.

[N6REJ]

The language filter plugin has a setting for wether to use "SITE" or
"Browser" as default lang. This should not be ignored
Bear
On 3/9/2015 11:30, Hannes Papenberg wrote:

There is no language set in the plugin. The browser should always
override the default setting of the site if possible.

—
Reply to this email directly or view it on GitHub
#6371 (comment).

No virus found in this message.
Checked by AVG - www.avg.com http://www.avg.com
Version: 2015.0.5751 / Virus Database: 4299/9261 - Release Date: 03/09/15

[Hackwar]

Please either read the code or my message preceding yours.

[infograf768]

This does not work here: I always get the site default language fr-FR.

Conditions on my test site:
SEF on
Browser settings (en-GB)
Remove Language Code on
Default site language fr-FR
Cookie set to it-IT

Evidently the url used is the base one.

[infograf768]

Tested further:
If URL Language code is not removed, it looks like working

[Hackwar]

This is indeed for the cases where no SEF code is set and the default language should not be removed from the URL.

[infograf768]

It is damned wrong, Hannes, and a regression.
The cookie has priority over any other settings when the base url is used.
If no cookie, then it depends on the Language Selection for new Visitors

[Hackwar]

@infograf768 I see that differently, but that also is not the point here. This tries to fix the issue that @engelweb described in #6213 with SEF codes enabled for all languages. I am aware that the behavior that you expect requires another change. At the same time, these 2 issues are not connected.

[infograf768]

They should be solved together.

[Hackwar]

They are 2 seperate issues. I agree with the one from @engelweb and I am solving it here. I don't agree with what you want regarding the second issue and thus leave it to someone else to fix. Regardless of that, these are still 2 seperate issues and unless we are forced to always provide fixes for 2 things at a time, this PR stands. It will look funny though when I have to fix JForm and JMail in the future, simply because you always expect 2 issues to be fixed together.

[infograf768]

The issue is the cookie issue and we have there a regression versus 3.3.6.
What "I want" is the desired way of using cookies in multilingual joomla!. They should work, whether we take off or not the url language code.
Not "agreeing" with the normal behavior of multilanguage is somehow weird.
I do not see why this would be separated. The code you provide solves the cookie issue ONLY for some settings, not for all settings as it was before.

[Hackwar]

What you want is to override the URL, but only in some cases, and I say that the URL is king, as does TBL. You should always get the same content for a URL.
Anyway, as I said, I don't agree with you here and thus someone else has to decide this and/or provide code for this.

These are 2 seperate issues, because the situation that I've solved here is, that no language is set so far and it then falls back on the cookie and after that it falls back on the default language. The situation that you have is, that a language is set (the default language) and you then want that changed. Solving one or the other means code that does not have anything to do with each other.

[infograf768]

sigh...

[joomdonation]

Hi

I think I agree with each of you 50% (please forgive me if I am wrong, I am not a developer/user who uses multilingual often).

1. I agree with @infograf768 that language in cookie should be respected
2. I also agree with @Hackwar that url should be king...

So what would be a solution in this case ? I think in this block of code https://github.com/Hackwar/joomla-cms/blob/patch-40/plugins/system/languagefilter/languagefilter.php#L245, we can check and compare cookie language code with the site default language. If they are different, maybe we should perform a redirect.

Below are the code I propose:

$found = true;          
// Check language code              
$lang_code = JComponentHelper::getParams('com_languages')->get('site', 'en-GB');                
$cookieLanguageCode = $this->app->input->cookie->getString(JApplicationHelper::getHash('language'));
if ($cookieLanguageCode != $lang_code)
{                   
    $path = $this->lang_codes[$cookieLanguageCode]->sef . '/' . $path;
    $uri->setPath($path);
    if (!$this->app->get('sef_rewrite'))
    {
        $uri->setPath('index.php/' . $uri->getPath());
    }                   
    $this->app->redirect($uri->base() . $uri->toString(array('path', 'query', 'fragment')));            
}

Could you please look at it and give it a try ?

[Hackwar]

That does not work, since you would never be able to switch back to the default language if you once chose a different one.

There is exactly one solution to this: Create a special exception that creates a URL WITH the SEF code for the default language in the language switcher (and nowhere else) and thus the language is discovered, then a new cookie needs to be written and then you have to redirect to the root again without the SEF code. That solution has so many problems and sucks in so many different ways, that I will not support it. It basically means that you have to write a second router just for the language switcher plugin, it goes against the main rule of the net regarding URLs, it is not helpfull in terms of SEO and it creates ugly code that will again mean issues in the future.

[joomdonation]

Ah, I see. It is complicated that I thought. What if we just redirect the first time in their session. The code can be change a bit like that:

$found = true;          
// Check language code              
$lang_code = JComponentHelper::getParams('com_languages')->get('site', 'en-GB');                
$cookieLanguageCode = $this->app->input->cookie->getString(JApplicationHelper::getHash('language'));
$session = JFactory::getSession();
$redirected = $session->get('redirected', 0);   
if ($cookieLanguageCode != $lang_code && !$redirected)
{                   
    $path = $this->lang_codes[$cookieLanguageCode]->sef . '/' . $path;
    $redirected = $session->set('redirected', 1);
    $uri->setPath($path);
    if (!$this->app->get('sef_rewrite'))
    {
        $uri->setPath('index.php/' . $uri->getPath());
    }                   
    $this->app->redirect($uri->base() . $uri->toString(array('path', 'query', 'fragment')));            
}

Thanks for being patient with me :).

[joomdonation]

Hmm. Please ignore my last message. Seems something still wrong. I will try to think more about it before getting back to this.

[Hackwar]

That doesn't work either, because I might have still a running session and then open domain.tld, hoping to get back to the homepage of my language and would land on the default language homepage. BTW: A nice solution than your "redirected" parameter is $session->isNew().

[Hackwar]

I mean, some site have session lifetimes of several hours. So if you have that set to 3 hours, close the window, go to lunch, come back, open a new window and go to domain.tld, your code wont work. As I said, the only way to do this, is the ugly solution that I posted above.

[joomdonation]

First time know about $session->isNew(); Thanks Hannes. Then we will have to accept one of the two defects and could not having a perfect solution for this case.

Then we will leave it here for PLT to make decision.

[xpallicer]

I can't understand what was wrong with Joomla 3.3

1 - search language cookie
2 - if no cookie is found apply the filter plugin preference

Done

I think it's not that hard

[joomdonation]

@Hackwar I just had a code review of the language filter plugin and have some questions:

1. Seems in the __constructor of the plugin, you forgot to set default_lang property, something like this:

$this->default_lang = JComponentHelper::getParams('com_languages')->get('site', 'en-GB');

You intended to do that or you just forgot ? I ask this because I see that you use that property later (before it initialized anywhere)
https://github.com/Hackwar/joomla-cms/blob/patch-40/plugins/system/languagefilter/languagefilter.php#L142
2. Should we have an extra property called cookike_lang_code (still could not find a good name). We just use to calculate and store default language code in the following order:

• Cookie
• If not found in cookie and the plugin set detect browser language, then set it to browse language
• If not, set it to site default language

$cookieLangCode = $this->app->input->cookie->getString(JApplicationHelper::getHash('language'));
if (!$cookieLangCode)
{
    if ($this->params->get('detect_browser', '1'))
    {
        $cookieLangCode = JLanguageHelper::detectLanguage();
    }
}
if (!$cookieLangCode || !isset($this->lang_codes[$cookieLangCode]))
{
    $cookieLangCode = $this->default_lang;
}           
$this->cookie_lang_code = $cookieLangCode;

We will store it and use it later (so that we won't have to calculate it each time we need)
3. At this line of code https://github.com/Hackwar/joomla-cms/blob/patch-40/plugins/system/languagefilter/languagefilter.php#L245, you are returning site default language. As people expect to have cookie language of avaialble, should we return $this->cookie_lang_code as calculated in the __constructor ?
4. Seems in this block of code https://github.com/Hackwar/joomla-cms/blob/patch-40/plugins/system/languagefilter/languagefilter.php#L293, users still expect to receive correct language if it is passed in URL, so maybe we should only try to find from cookie ... if the language not found before. Code:

if (!$found)
{
    $found = true;
    $lang_code = $this->cookie_lang_code;
}

Here is the link to the code which I proposed if you want to have a look https://gist.github.com/joomdonation/78f786e0028e74fe7062

[Hackwar]

Hello @joomdonation,
to 1.: That property is set in the parse() method. The parse() method is always called before any URL-build method can be called.
to 2.: We are doing exactly that right now. The value is in $this->default_lang. Yes, it is a bad name.
to 3.: No, since you then can never get back to the default languages homepage. The idea is, that a URL without a language SEF code is in the default language. So unless we remove the "remove default lang sef code" feature, we can't simply fall back here on the cookie value.
to 4.: In the next line, it already receives the value from the cookie. You could only check if the language is already set in $lang_code.

There is no need for a cookie_lang_code property.

[joomdonation]

@Hackwar

#1 => OK

#2 => OK

#3 => Maybe consider this redirect on first time visit which we talked yesterday when the language found on cookie is different from default language. It is not correct 100% but It might work in most cases ?

#4 => Maybe I didn't explain clear enough. In that block of code, seems you are ignoring language which might be set in URL already. Please see comment from a developer in mailing list that language passed on url is ignored in post request https://groups.google.com/forum/#!topic/joomla-dev-general/TajVgOI3NIA

That's why I suggest we only detect language from cookie if it is not found before ?

[Bakual]

There is exactly one solution to this: Create a special exception that creates a URL WITH the SEF code for the default language in the language switcher (and nowhere else) and thus the language is discovered, then a new cookie needs to be written and then you have to redirect to the root again without the SEF code.

After investing some hours into figuring out how it worked in 3.3.6 and what the differences now are, the cited comment is actually the solution. In 3.3.6 this was the behavior of the language switch module which allowed to switch back to the default language. Feel free to check on http://joomla16.hopto.org/ which runs a default 3.3.6 multilang installation.
In 3.4.0 it always generates the URL without the language code for the default language, which is technically the correct URL, but the regression JM and others are talking about.

There is also a separate issue which Robert tried to fix in his code. The cookie path is determined wrong when setting the cookie. $cookie_path = $this->app->get('cookie_path', $uri->base(true)); will result in the cookie path being /en/ which is wrong. Interesting enough, when reading the cookie, the code uses another default value $cookie_path = $this->app->get('cookie_path', '/'); which is actually what we use everywhere else in Joomla. Like for the remember-me, session and banners cookies.
The idea here is that the default value / will work for almost any site. The only instance where it will not work correctly is when you have multiple Joomla installations in subdirectories in the same domain. Then you should set the cookie path in your global configuration.

[Bakual]

So this line https://github.com/Hackwar/joomla-cms/blob/patch-40/plugins/system/languagefilter/languagefilter.php#L176
Changed from this check:

if ($this->params->get('remove_default_prefix', 0) == 0
    || $sef != $this->default_sef
    || $sef != $this->lang_codes[$this->tag]->sef
    || $this->params->get('detect_browser', 1) && JLanguageHelper::detectLanguage() != $this->tag && !$this->cookie)
{

to:
(!$this->params->get('remove_default_prefix', 0) || $lang != JComponentHelper::getParams('com_languages')->get('site', 'en-GB'))

Which is the reason you can't switch back to the default language.

[Hackwar]

After investing some hours into figuring out how it worked in 3.3.6 and what the differences now are, the cited comment is actually the solution. In 3.3.6 this was the behavior of the language switch module which allowed to switch back to the default language. Feel free to check on http://joomla16.hopto.org/ which runs a default 3.3.6 multilang installation.

I never said that it worked differently in 3.3.6. I also have been describing the solution to the problem in several posts in this bugtracker and I think both in the mailinglists and the Skype chat. I'm not making this a secret. But I'm saying that this is a bug and unless we now disagree on this, bugs should be fixed. I asked you guys (the PLT) to decide if the previous behavior is a bug and thus was rightly fixed, or if the change that was done was a bug/regression. So far the PLT did not make an official statement regarding this. I only got a bunch of vocal people that claim that the new behavior is a bug.

There is also a separate issue which Robert tried to fix in his code. The cookie path is determined wrong when setting the cookie. $cookie_path = $this->app->get('cookie_path', $uri->base(true)); will result in the cookie path being /en/ which is wrong.

That is simply not true. Unless JUri is broken, JUri will not return such a path. I checked this myself just a second ago. I see that there is another cookie with that name, but that is related to https://github.com/joomla/joomla-cms/blob/staging/plugins/system/languagefilter/languagefilter.php#L508 this line that gives no default at all. I would prefer if we properly used JUri for the cookie path, but if you don't want to do that, that is fine by me. But then please simply fix this by adding the one character in that one line there and not move around 200+ lines of code.

[Hackwar]

Regardless of all of that, this PR is still valid, still fixes the issue and is still waiting for testers/being merged.

[Hackwar]

I have to correct myself regarding line 508. That is in fact correct the way it is. I don't know where that cookie comes from, but it does not stem from the languagefilter plugin.

[rdeutz]

cookie: I found that when you set the cookie path to "" the path will be added. Seems the browser is doing it, so I used

$base = $uri->base(true) == '' ? '/' : $uri->base(true);

so it works when you have Joomla installed in the docRoot or in subDir of the docRoot

[Hackwar]

Yes, you are right. I apologise. I will correct my code.

[Bakual]

Do we really need to do this calculations for the cookie_path? In all other instances where we are setting a cookie within Joomla, we just default to /, which will work for the most sites anyway.
The only place it may not work is if you have www.example.com/one_joomla and www.example.com/another_joomla. But then the user can set the cookie_path in his global configuration to reflect that.
I would prefer to be consistent when setting the cookie.

By the way, you do it already this way here: https://github.com/Hackwar/joomla-cms/blob/patch-40/plugins/system/languagefilter/languagefilter.php#L459 😄

[rdeutz]

I think we need it, it is maybe a 0.0001 % problem for the sites outside, but for our testing it is more a problem when someone installed more then one multi-language sites for testing a functionality.

[Hackwar]

I've removed the path for the cookie again. We should improve this in general, but maybe not exactly here. I would prefer if we set the value for the cookie path somewhere during application initialisation in the application configuration, so that we don't need a default value at all and simply set this once at some central place. Of course only if it isn't set by the user.

[infograf768]

@Bakual PR works great here, with a small necessary chnage for Browser Settings issue.
Please test:
#6452

[Hackwar]

Looked through the code again and this is actually a non-issue. Closing this one.