Can we make these methods private please? There's no reason to be extending a plugin like this so let's not get ourselves into some sort of stupid b/c issue because someone is extending this class.

So instead of adding

$item = $this->app->getMenu()->getItem($query['Itemid']);
if ($this->mode_sef
&& (!$this->params->get('remove_default_prefix', 0) || $lang != JComponentHelper::getParams('com_languages')->get('site', 'en-GB') || $item->home))
{
$uri->setPath($uri->getPath() . '/' . $sef . '/');
}

at line 176, you are rewriting everything and add around 60 LoCs?

you are rewriting everything and add around 60 LoCs?

Answer:

For better understanding it comes with a some more refactoring and removing duplicated code.

Ok, regardless of what I think about the fix: It doesn't work.
@test fail
Setup: Site (latest staging, patch applied via patchtester) with french and english language, english set as default. Surfing on the french part of the site and the cookie correctly says "fr-FR" as the language. calling the root of the site does not redirect and show the french site, but the english one.

I wouldn't make such a fuzz if it really were an issue and if that issue was solvable in some proper way.

Tested successfully before applying patch.
Local install in test environment without mod_rewrite

@designbengel what did you test? The expected behavior is, that you are redirected to your last language, in my case domain.tld/fr. I got my default language displayed, which according to a few people is wrong. Please document the behavior that you saw.

@Hackwar See:

Local install in test environment without mod_rewrite

Without mod_rewrite it works as expected.

The behavior until now was, that the homepage of the default language was the root (/) not /index.php. Besides that, a fix that only works for non-mod_rewrite is still a non-fix, since it means it is not changed for 70% of the rest of the Joomla sites out there.

I mean, you are replacing the default language prefix simply with index.php.

Von Samsung Mobile gesendet

-------- Ursprüngliche Nachricht --------
Von: Thomas Hunziker notifications@github.com
Datum:

An: joomla/joomla-cms joomla-cms@noreply.github.com
Cc: Hannes Papenberg info@joomlager.de
Betreff: Re: [joomla-cms] Fix for languagefilter problem (#6428)

@Hackwar See:

Local install in test environment without mod_rewrite

Without mod_rewrite it works as expected.

—
Reply to this email directly or view it on GitHub.

designbengel only stated that it still works as expected without SEF rewrite turned on. Which is a perfectly valid test. It's important to not only test what is broken, but also to make sure nothing else got broken due to the fix.

My own test with SEF on and mod_rewrite on were actually successful.
Before when I visit www.example.com I always got redirected to the site default language (in my case en-GB).After switching to www.example.com/de/ and visiting www.example.com again, I got redirected to www.example.com/en, which is wrong.
After applying the patch and cleaning all existing cookies, it worked as expected and I got redirected to www.example.com/de as expected with the second visit.

With "Removing URL language code" turned on, it still always redirects to the site default language when I access the site using www.example.com. But here I could see a point in that it's now working as it should, because that URL is actually the correct URL for the site default language.
It's however a change to the behavior in 3.3.6.

@Bakual seriously? The behavior that you are describing is exactly what we have in staging right now, minus #6371. Your first description is what #6371 is fixing and I'd rather have 5 lines moved around, than to have more than 200 lines changed. Compared to this PR, the original PR for the language filter plugin was intensively tested.
Your second description is what we have right now and what you have been calling wrong for the last 3 weeks. At the same time, this PR does not solve that perceived error, but instead replaces the default languages prefix with the static prefix "index.php".

This PR fixes a wrong specified cookie as far as I saw. Your other PR does something else.

what you have been calling wrong for the last 3 weeks

I never called it wrong. My stance was and still is that according to users it's different to how it was and that I'm not going to judge what is correct behavior when the language code is removed for the default language. Personally I find that a not-so-ideal feature to begin with which asks for troubles.

but instead replaces the default languages prefix with the static prefix "index.php".

Hu? Where does it do that? It didn't for me. I think you misunderstood something there.

No, my cookie is correct. It is the order in which they are evaluated that is wrong and that is fixed in #6371.

Regarding what is wrong or right: I've been saying this for the last few weeks that it is actually a bug in the old behavior that it behaved the way it did. I asked you guys to decide if this is a bug or not. You say that you don't want to decide. That leaves us at the point where I say it is wrong and JM says it is right and now we can argue about that until the end of time. Considering that it is the last perceived issue and blocker that did not have and still does not have a fix for 3.4.1, this is pretty serious in my book.

Regarding the replaced prefix: The old and definitely expected behavior was, that / is the homepage of the default language of the site, while /index.php/[lang-code]/ is the homepage of [lang-code]. As well as the homepage without the languagefilter plugin is /. Now, with mod_rewrite disabled, the homepage of the default language is /index.php. That is precisely the move from for example /index.php/en/ to /index.php. With mod_rewrite it moves from /en to /index.php. So we simply replace the language prefix for the default language with "index.php".

That you have the index.php as homepage for the default language is only when you have it already in you url and then link to the page again, that is the same as in 3.3.6.

This does not solve here the cookie issue when URL Language Code is removed (for the site default language)

@infograf768 working on that part, too many options too test

@Hackwar I startet testing first time due to pbf yesterday so i´m sorry for this unclear statement. I wanted to say, that i tested the behaviour before applying the patch with the settings: SEF on, Mod Rewrite OFF the behaviour is how it should be, like descriped above.
First site visit: default languagage, changing to en -> english language, access the site without lang code again: -> english language, It´s always the language i selected actively before. So when i select german and access the site without lang code it´s german.

When testing, make sure that the site default language, the cookie set and the default browser language are all different.

That what I have tested:

Testing enviorement

browser language == de
default lang == fr
other lang == de, en, he
lang switcher on
submenu items for „kategorie“ and single articles

SEF: on

Use URL Rewriting: on

Remove URL Language Code: on

1. cookie cleared, http://site -> http://site (lang==fr)
2. chose de -> http://site/de
3. chose fr -> http://site
4. chose he -> http://site/he (change to rtl)
5. chose de -> http://site/de
6. select kategorie submenu item -> http://site/de/kategorie
7. chose fr -> http://site
8. select article submenu item -> http://site/article-fr

Remove URL Language Code: off
1. cookie cleared http://site -> http://site/fr
2. chose de -> http://site/de
3. chose fr -> http://site/fr
4. chose he -> http://site/he (change to rtl)
5. chose fr -> http://site/fr
6. select kategorie submenu item -> http://site/de/kategorie
7. chose fr -> http://site/fr
8. select article submenu item -> http://site/fr/article-fr

Use URL Rewriting: off

Remove URL Language Code: on

1. cookie cleared, http://site -> http://site (lang==fr)
2. chose de -> http://site/index.php/de/
3. chose fr -> http://site/index.php
4. chose he -> http://site/index.php/he (change to rtl)
5. chose de -> http://site/index.php
6. select kategorie submenu item -> http://site/index.php/de/kategorie
7. chose fr -> http://site/index.php
8. select article submenu item -> http://site/index.php/article-fr

Remove URL Language Code: off
1. cookie cleared, http://site -> http://site/index.php/fr/
2. chose de -> http://site/index.php/de/
3. chose fr -> http://site/index.php/fr/
4. chose he -> http://site/index.php/he (change to rtl)
5. chose de -> http://site/index.php/de/
6. select kategorie submenu item -> http://site/index.php/de/kategorie
7. chose fr -> http://site/index.php/fr
8. select article submenu item -> http://site/index.php/fr/article-fr

SEF off

1. cookie cleared, http://site -> http://site/index.php?lang=fr
2. chose de -> http://site/index.php?option=com_content&view=featured&Itemid=104&lang=de
3. chose fr -> http://site/index.php?option=com_content&view=featured&Itemid=103&lang=fr
4. chose he -> http://site/index.php?option=com_content&view=featured&Itemid=105&lang=he (change to rtl)

So far that is what was shipped with 3.4.0.

The issue is not switching between languages, etc. The issue is, that people want to open domain.tld/ and receive the default language, then switch the language, and if they open domain.tld/ again, they get redirected to the last language that was active.

BTW: site/ != site/index.php Such a change (and regression) is not acceptable here.

Use URL Rewriting: on

Remove URL Language Code: on

1. cookie cleared, http://site -> http://site (lang==fr)
2. chose de -> http://site/de
3. People apparently expect this: open http://site -> http://site/de
4. chose fr -> http://site
5. chose he -> http://site/he (change to rtl)
6. chose de -> http://site/de
7. select kategorie submenu item -> http://site/de/kategorie
8. chose fr -> http://site
9. select article submenu item -> http://site/article-fr

Forget to test this one, worked yesterday but not now. Going to fix that now

1. chose de -> http://site/de
2. People apparently expect this: open http://site -> http://site/de

Yesterday it worked, because /index.php was your marker that you should fall back to the default language. But as I said before, the URL of the homepage is / and not /index.php and since http://site/ should behave differently, there is still only one way to achieve the behavior that JM thinks is right, and that is by discovering the language for http://site/ always by cookie and in the router to generate URLs to the homepage always with a lang prefix and then redirect to the none prefix version for the default language.

The perceived issue is not solvable in the parse method alone.

I am very familiar with multilingual sites and have managed seo successfully for them.

From what I know and from what google suggests the behaviour as its is, is 100% correct!
You will NEVER want a tld root to have two languages it could produce dublicated content and confusion.

You want your english speaking visitors to link: domain.de/en NOT domain.de.
Also they will bookmark that url path when they come back. Another point you: still have the option to read the browsers meta and provide the adequat url and language.

You will NEVER want a tld root to have two languages it could produce dublicated content and confusion.

As far as I know, there is a big difference between a crawler reading an url and a cookie set on a user machine.
For the crawler, when URL Language Code is removed, the tld will always be the site default language. The language is set anyway in the hml of the page, which is enough for the search engines.

For a user, it depends if the cookie is set or not.

The perceived issue is not solvable in the parse method alone.

Indeed.
In 3.3.6 it was taken care of in other places.
$cookie was protected and used in buildRule() to setPath (and in onAfterDispatch() to detect language for associations, but this one is also present in staging).

Here is what I tested with the last changes:

Testing enviorement

browser language == de
default lang == fr
other lang == fr, en, he
lang switcher on
submenu items for „kategorie“ and single articles

SEF: on

Use URL Rewriting: on

Remove URL Language Code: on

1. cookie cleared, http://site -> http://site (lang==fr)
2. chose de -> http://site/de
3. http://site/ -> http://site/de
4. chose fr -> http://site
5. chose he -> http://site/he (change to rtl)
6. chose de -> http://site/de
7. select kategorie submenu item -> http://site/de/kategorie
8. chose fr -> http://site
9. select article submenu item -> http://site/article-fr

Remove URL Language Code: off
1. cookie cleared http://site -> http://site/fr
2. chose de -> http://site/de
3. http://site/ -> http://site/de
4. chose fr -> http://site/fr
5. chose he -> http://site/he (change to rtl)
6. chose de -> http://site/de
7. select kategorie submenu item -> http://site/de/kategorie
8. chose fr -> http://site/fr
9. select article submenu item -> http://site/fr/article-fr

Use URL Rewriting: off

Remove URL Language Code: on

1. cookie cleared, http://site -> http://site (lang==fr)
2. chose de -> http://site/index.php/de/
3. http://site/ -> http://site/index.php/de
4. chose fr -> http://site/index.php
5. chose he -> http://site/index.php/he (change to rtl)
6. chose de -> http://site/index.php
7. select kategorie submenu item -> http://site/index.php/de/kategorie
8. chose fr -> http://site/index.php
9. select article submenu item -> http://site/index.php/article-fr

Remove URL Language Code: off
1. cookie cleared, http://site -> http://site/index.php/fr/
2. chose de -> http://site/index.php/de/
3. http://site/ -> http://site/de
4. chose fr -> http://site/index.php/fr/
5. chose he -> http://site/index.php/he (change to rtl)
6. chose de -> http://site/index.php/de/
7. select kategorie submenu item -> http://site/index.php/de/kategorie
8. chose fr -> http://site/index.php/fr
9. select article submenu item -> http://site/index.php/fr/article-fr

@rdeutz
Url Language Code Removed set to ON
in your test, you should display /he/ and then quit browser.
This would mean the he-il cookie is set.
Launch browser again and enter the tld root url: http://site.com
You then should be redirected to http://site.com/he/

Then do the same test for the default language (in your case fr-fr)

@infograf768 when I close the browser the cookie isn't valid anymore because it is set to session

"As far as I know, there is a big difference between a crawler reading an url and a cookie set on a user machine.
For the crawler, when URL Language Code is removed, the tld will always be the site default language. The language is set anyway in the hml of the page, which is enough for the search engines.

For a user, it depends if the cookie is set or not."

You should not base you decision of what you think the search engines are capable of doing but what’s more attractive in terms of usability. Having two or more languages coming up in one URL is inconsistent and duplicated. It will result in bad off site linking behavior and puts sites at risk to lose integrity and hence trust in search engines.

@peacetree this here is not so much about what is right or better and what is wrong, this here should make sure that we have the same behaviour as we had before.

Right, on my website I did have my configuration set as such I had one and only one language coming up at my root path. All other languages were successful redirected to site.tld/en (for example). If that is what the coming build will support I am happy.

@infograf768 had my cookie lifetime set to session, the procedure you described is working after closing the browser and reopen I get the same language as I had before closing the browser.

One thing I noticed while testing is that the language switcher module doesn't have the lang-code for the default language when the plugin is set to "Remove URL Language Code==Yes". That is a change in the behauviour and it results in showing a language people don't expect. E.G. if you have fr as default and you go to en and then click on fr in the lang switcher module you are still on the english part of the site. That is not wrong because the link is http://site/ what will be redirected based on the cookie setting to http://site/en. To keep a long story short we need to fix the languageswitcher module.

@rdeutz
Please test @Bakual PR. It is solving the issue here. I posted there a remaining small issue and possible solution.
With that one, when hovering the default site language flag from another language, we do get the /xx/ sef and it is correctly redirected (as should) to the root tld without sef url language code
#6452

BTW: cookie should be set to Year to test this feature.

Suggest to close this one for now as #6452 has been merged