? Success
Pull Request for # 6213

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
28 Feb 2015

Fixes #6213
This fixes the case when the default language SEF-part should not be added to the URL and the user is typing in the domain of the site, already visited the site and thus has a cookie with his prefered language.

avatar Hackwar Hackwar - open - 28 Feb 2015
avatar joomla-cms-bot joomla-cms-bot - change - 28 Feb 2015
Labels Added: ?
avatar HermanPeeren
HermanPeeren - comment - 28 Feb 2015

Started testing, but looking at code see something I don't understand:

https://github.com/joomla/joomla-cms/blob/e7341c5a8f55725966a50db595233c279df5d13e/plugins/system/languagefilter/languagefilter.php#L245-L249 :

if (strlen($sef))
{
$found = true;
$lang_code = JComponentHelper::getParams('com_languages')->get('site', 'en-GB');
}

If there is a slug on first position, but it is not a languageslug, then you take the default language string????? What is your rationale for that??? Or am I completely misinterpreting?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6230.
avatar Hackwar
Hackwar - comment - 28 Feb 2015

The situation is as follows: The default language is not supposed to have a SEF code in the URL. So if we have a URL like /something/really/cool, the code at the beginning separates that into $parts = array('something', 'really', 'cool'). It takes the first of those as $sef, that makes it $sef = 'something'. That is not a language code in our system and that means that we don't have a lang-code yet. Now, if the path were just empty, we would be calling the homepage and thus would have to take the choice from the cookie. But since we have a valid URL (/something/really/cool), that URL should be treated as being in the default language. Otherwise the whole "drop default language SEF code" does not work.

avatar zero-24 zero-24 - change - 28 Feb 2015
Rel_Number 6213
Relation Type Pull Request for
avatar zero-24 zero-24 - change - 28 Feb 2015
Category Plugins Router / SEF
avatar HermanPeeren
HermanPeeren - comment - 28 Feb 2015

if cookie at last url was set to non-default-language and new visit is to url /something/really/cool,

  • you say the default language is found
  • I say a 303 redirect should be made to /sef/something/really/cool (with sef being the language slug of that specific not-default-language}.
    This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6230.
avatar HermanPeeren
HermanPeeren - comment - 28 Feb 2015

I meant: "if cookie at last visit..." not "at last url..."


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6230.
avatar HermanPeeren
HermanPeeren - comment - 28 Feb 2015

We must make it as it was, BC. So first we must agree what that was. My thoughts about that, written out in some pseudo-code, for SEF-urls (where "OK" means: we have found the language and don't have to redirect;don't process further, but continue at the end):

1. LangSlug found
    a. DontRemoveDefault => OK
    b. RemoveDefault :
         1. LangSlug != DefaultLang => OK
         2. LangSlug == DefaultLang:
              - set LangCookie
              - remove LangSlug
              - 301 redirect

else

2. LangCookie set
    a. DontRemoveDefault:
         1. LangCookie != DefaultLang => OK
         2. LangCookie == DefaultLang:
              - set LangSlug
              - 301 redirect
    b. RemoveDefault:
         1. LangCookie != DefaultLang:
              - set LangSlug
              - 303 redirect
         2. LangCookie == DefaultLang => OK

else

3. LangBrowser found (and parm set that we want to check that)
    a. DontRemoveDefault:
         - set LangSlug
         1. LangBrowser != DefaultLang: 303 redirect 
         2. LangBrowser == DefaultLang: 301 redirect
     b. RemoveDefault:
         1. LangBrowser != DefaultLang:
              - set LangSlug
              - 303 redirect
         2. LangBrowser == DefaultLang => OK

else

4. take DefaultLang
    a. DontRemoveDefault:
         - set LangSlug
         - 301 redirect
    b. RemoveDefault => OK

If OK: set LangCookie and continue.

Hope I didn't make any mistakes while writing this out. Our discussion above was especially about option 2.b.1 (no LangSlug found + LangCookie set + RemoveDefault + LangCookie != DefaultLang).

     <hr /><sub>This comment was created with the <a href="https://github.com/joomla/jissues">J!Tracker Application</a> at <a href="http://issues.joomla.org/tracker/joomla-cms/6230">issues.joomla.org/joomla-cms/6230</a>.</sub>
avatar Hackwar
Hackwar - comment - 28 Feb 2015

Why should I ever redirect the URL /how-to-use-joomla to /de/how-to-use-joomla? If I request an english page, I expect to get an english page. I don't expect to get a mixed language page or to be only possible to access german pages. There is exactly one situation where the requested page is ambiguos: When a user is a returning visitor and requests simply domain.tld and the site is set to hide the default languages sef code. In that case it should take the value from the cookie. In any other cases, the URL overrides anything else.

HermanPeeren notifications@github.com schrieb:

if cookie at last url was set to non-default-language and new visit is to url /something/really/cool,

  • you say the default language is found
  • I say a 303 redirect should be made to /sef/something/really/cool (with sef being the language slug of that specific not-default-language}.
    This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6230.

Reply to this email directly or view it on GitHub:
#6230 (comment)

avatar HermanPeeren
HermanPeeren - comment - 28 Feb 2015

(indentation was lost in copying, grrrrr)


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6230.
avatar Hackwar
Hackwar - comment - 28 Feb 2015

There are a lot more cases than the few that you listed and they are all correctly covered in the current plugin except for the one that I described above and that is solved with this PR.

HermanPeeren notifications@github.com schrieb:

We must make it as it was, BC. So first we must agree what that was. My thoughts about that, written out in some pseudo-code, for SEF-urls (where "OK" means: we have found the language and don't have to redirect;don't process further, but continue at the end):

1. LangSlug found
a. DontRemoveDefault => OK
b. RemoveDefault :
1. LangSlug != DefaultLang => OK
2. LangSlug == DefaultLang:
- set LangCookie
- remove LangSlug
- 301 redirect

else

2. LangCookie set
a. DontRemoveDefault:
1. LangCookie != DefaultLang => OK
2. LangCookie == DefaultLang:
- set LangSlug
- 301 redirect
b. RemoveDefault:
1. LangCookie != DefaultLang:
- set LangSlug
- 303 redirect
2. LangCookie == DefaultLang => OK

else

3. LangBrowser found (and parm set that we want to check that)
a. DontRemoveDefault:
- set LangSlug
1. LangBrowser != DefaultLang: 303 redirect
2. LangBrowser == DefaultLang: 301 redirect
b. RemoveDefault:
1. LangBrowser != DefaultLang:
- set LangSlug
- 303 redirect
2. LangBrowser == DefaultLang => OK

else

4. take DefaultLang
a. DontRemoveDefault:
- set LangSlug
- 301 redirect
b. RemoveDefault => OK

If OK: set LangCookie and continue.

Hope I didn't make any mistakes while writing this out. Our discussion above was especially about option 2.b.1 (no LangSlug found + LangCookie set + RemoveDefault + LangCookie != DefaultLang).

    <hr /><sub>This comment was created with the <a href="https://github.com/joomla/jissues">J!Tracker Application</a> at <a href="http://issues.joomla.org/tracker/joomla-cms/6230">issues.joomla.org/joomla-cms/6230</a>.</sub>

Reply to this email directly or view it on GitHub:
#6230 (comment)

avatar HermanPeeren
HermanPeeren - comment - 1 Mar 2015

Lately I had a client with this use case: urls like somedomain.com/de/hotel-A, somedomain.com/en/hotel-A, etc., where he didn't want to advertise with the language slug, forinstance on radio-advertisements, but wanted the visitors being redirected to the respective language specific urls. Besides that: this is the behaviour we had in the language filter and we agreed upon backwards compatibility. Of course if the old way would obviously be a mistake we could do it differently now, but that is not the case AFAIK.

I tried to systematically list all possible combinations of cases (within SEF-urls) of LangSlug, LangCookie, LangBrowser, DefaultLang, RemoveDefault and DontRemoveDefault. Within those variables do you have any other case I forgot to list???


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6230.
avatar mbabker
mbabker - comment - 1 Mar 2015

Not knowing how everything was working before and going off the comments here, I think Hannes has it right with this PR. Regardless of any cookies that are set, I think the default behavior is that you are served the content (and language) you requested. If no code is specified and no cookie found, the system should rightfully try to set the language based on browser configuration then default settings for the site. Conditions beyond that would imply that a cookie is in place or that the site administrator has additional options configured, which I take it isn't a possibility with this plugin as is.

avatar HermanPeeren
HermanPeeren - comment - 1 Mar 2015

An example. If we would have a multiligual site joomlapersons.org, with default language en-GB. For some weird reason, which would not be my choice, it would have been decided that the language slug for the default language should be removed (by setting the parameter in this language filter plugin for it). You just visited joomlapersons.org/michael-babker. You indicate, by clicking the French flag on top of the site, you want to visit this site in French. You are redirected to joomlapersons.org/fr/michael-babker. So far so good and we all agree and are happy.

Now, in the above scenario, next time, you type the url: joomlapersons.org/hannes-papenberg. I say you should now be 303-redirected to joomlapersons.org/fr/hannes-papenberg, the French page, because the language cookie indicated your last visit was in fr-FR. But the code in Hannes' PR would not do that and keep you on joomlapersons.org/hannes-papenberg, showing information in English.

Moreover, when your browser language in the above example would be French and your visit to joomlapersons.org/michael-babker would be your first visit to this site, you should not even have needed to chose the French language, but should have been 303-redirected to joomlapersons.org/fr/michael-babker on that very first visit, based on your browser language. This does not happen in Hannes' code.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6230.
avatar infograf768
infograf768 - comment - 1 Mar 2015

@test fails
(Remove URL Language Code set to Yes)
Although the cookie is now respected and closing browser and reopening it does work when the cookie is set to another language than the default , it is now impossible to load/switch to the default language via the language switcher.

avatar HermanPeeren
HermanPeeren - comment - 1 Mar 2015

@infograf768 Yes, that also: because in your test case when building the new url the cookie is not set to the new language, cannot be made up from the url and so you will be redirected to the last non-default language based on the cookie info. But I don't agree that the cookie is fully respected now: I still see the problem when we are not requesting the home-page and leave out the language slug from our request url, as outlined in my last example.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6230.
avatar Hackwar
Hackwar - comment - 1 Mar 2015

See, and that happens when you confuse me. This is actually not a bug but expected behavior. You can either have language SEF codes in your URL and discover the language via cookie if no SEF code is given or you can hide it for the default language, but than you can't use cookie discovery, because as JM rightfully noticed, you don't get back to the default languages homepage.

All that said, this is not a bug, but expected behavior. We actually discussed exactly this in the original PR #5140 and agreed on this behavior there. Thus I'm closing my PR. If you still think you know better, feel free to write your own PR, but leave me out of this.

avatar Hackwar Hackwar - change - 1 Mar 2015
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-03-01 09:43:42
avatar Hackwar Hackwar - close - 1 Mar 2015
avatar Hackwar Hackwar - close - 1 Mar 2015
avatar HermanPeeren
HermanPeeren - comment - 1 Mar 2015

Will do. I started working on a PR yesterday, but you were faster with this PR. Thanks anyway, for it forces us all to better analyse what we want and how to accomplish that.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6230.
avatar Hackwar Hackwar - head_ref_deleted - 1 Mar 2015
avatar Bakual
Bakual - comment - 15 Mar 2015

My attemp at fixing this PR to allow switching back to the default language: #6452

avatar infograf768
infograf768 - comment - 16 Mar 2015

I guess this one can be closed in favour of #6542

avatar Hackwar
Hackwar - comment - 16 Mar 2015

This one was closed for 2 weeks already.

Add a Comment

Login with GitHub to post a comment