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

Fixing regression in languagefilter related to cookies #6452

Merged
merged 4 commits into from
Mar 16, 2015

Conversation

Bakual
Copy link
Contributor

@Bakual Bakual commented Mar 15, 2015

Issue

There is currently an issue that the cookie setting isn't respected when you access www.example.com. If you visited the site before and switched the language, it should actually redirect you to that language but it currently will always redirect to the site default language.

Solution

There are already several PRs which are trying to solve this issue. Most of the did detect a part of the problem, but missed another part.
So this is my attemp to merge the best from various PRs to try to get this solved. Hopefully I didn't miss anything myself 😄
What I did:

  • From Fix for languagefilter problem #6428 I took the fix for the cookie_path, which now defaults just to / like we do in all other instances where we set a cookie within our CMS.
  • From Discovering the right language if not given by URL #6230 I took the fix to detect the cookie language properly
  • Additional I fixed the buildRule so it allows to switch back to the default language. Basically the language code is added to the URL if the current language isn't the default language, allowing to switch back and override the cookie setting. Also I'm setting the cookie before the redirect in that case.

Testing

  • Set up a multilanguage site and enable SEF with mod_rewrite.
  • Try with and without the setting "Remove language code for default language" in the languagefilter plugin
  • When accessing www.example.com the first time, you should be redirected to the default language content.
  • When you switch the language, you should see the selected language and the URL changed to www.example.com/de (if you switched to german).
  • Now the tricky part comes: Try to reload www.example.com. You should be redirected to the language you selected before (www.example.com/de/), but this fails currently and you get redirected to site default language again.
  • Apply patch and clear the language cookies. Simplest is to just close all instances of your chosen browser. The language cookies is limited to session by default.
  • Now the redirects should work as expected.

@Bakual
Copy link
Contributor Author

Bakual commented Mar 15, 2015

(Hopefully) fixed codestyle and removed some unneeded code based on suggestions from Hannes. Thanks!

@infograf768
Copy link
Member

@test

Much better! Now cookie works OK with URL Language Code removed.

I still get an issue here
Settings:

  1. Browser set to en-gb as preferred language
  2. Default site language set to fr-FR
  3. Settings for Global Configuration:

screen shot 2015-03-16 at 08 35 38

  1. Settings for Languagefilter plugin.
    screen shot 2015-03-16 at 08 33 24

Delete all cookies. Quit browser. Relaunch browser. Enter tld root.
Result: the site default language loads (fr-FR) instead of the Browser setttings language (en-GB)

This happens only when url language code is removed.

@infograf768
Copy link
Member

Tested with this code:

// If the default prefix should be removed and the SEF prefix is not among those
            // that we have in our system, its the default language and we "found" the right language
            if ($this->params->get('remove_default_prefix', 0) && !isset($this->sefs[$sef]))
            {
                $lang_code = $this->app->input->cookie->getString(
                    JApplicationHelper::getHash('language')
                );

                if (!$lang_code && $this->params->get('detect_browser', 0) == 1)
                {
                    $lang_code = JLanguageHelper::detectLanguage();
                }

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

                if ($lang_code == JComponentHelper::getParams('com_languages')->get('site', 'en-GB'))
                {
                    $found = true;
                }
            }
            else
[...]

and looks like it corrects the issue here.

@Bakual
Copy link
Contributor Author

Bakual commented Mar 16, 2015

Added suggestion by @infograf768. Worked when I tested it.

@infograf768
Copy link
Member

@test
Works fine for me. Including correct urls obtained by the langswitcher.

@gwsdesk
Copy link

gwsdesk commented Mar 16, 2015

@test works for me also. Get the correct url's

@Kubik-Rubik
Copy link
Member

Great contribution @Bakual!

Let's get the new version done and released!

Kubik-Rubik added a commit that referenced this pull request Mar 16, 2015
Fixing regression in languagefilter related to cookies
@Kubik-Rubik Kubik-Rubik merged commit 1337366 into joomla:staging Mar 16, 2015
@Kubik-Rubik Kubik-Rubik added this to the Joomla! 3.4.1 milestone Mar 16, 2015
@Bakual Bakual deleted the YetAnotherLanguageFilterCookieFix branch March 16, 2015 15:38
@xpallicer
Copy link
Contributor

This issue has not been fixed, cookie language is not remembered

@infograf768
Copy link
Member

@xpallicer
You should test on 3.4.1 and make sure to tick Year for the cookie time in the languagefilter, so that the cookie still exists after quitting the browser and relaunching.

@xpallicer
Copy link
Contributor

Hello infograf,
Did this: update to Joomla 3.4.1, delete all site's cookies. Access mysite.com/es, access mysite.com I get redirected to my browser default language (I should be redirected to mysite.com/es again)

@infograf768
Copy link
Member

After accessing mysite.com/es, quit the browser and then relaunch and use mysite.com

@xpallicer
Copy link
Contributor

Now it works. I think the bug persists if you do not update (save) Joomla global configuration. Thank you. Best regards!

@Bakual
Copy link
Contributor Author

Bakual commented Mar 23, 2015

The config don't matter. What is important is that you cleared the cookies. Especially the one with the path set to something else than / (root) 😄

@chrisxchrisxchrisx
Copy link

I cannot get this to work under 3.4.3 as it used to under 3.1.5 (yes I had put off upgrading for some time since initial upgrade failed). I followed this process:

  1. Using Firefox 39.0 clear all cookies
  2. Check Language Filter settings via http://saatalk.info/administrator
    Language Selection for new Visitors: Browser Settings
    Automatic Language Change: Yes
    Item Associations: Yes
    Remove URL Language Code: No
    Cookie Lifetime: Year
    Add Alternate Meta Tags: Yes
  3. Check http://saatalk.info
    Correctly redirected to http://saatalk.info/en in line with browser settings
    Cookie containing en-GB stored on path /
  4. Select Deutsch via language switcher on http://saatalk.info
    Correctly redirected to http://saatalk.info/de
    Cookie containing de-DE stored on path /
  5. Check http://saatalk.info
    INCORRECTLY redirected to http://saatalk.info/en
    Cookie containing en-GB stored on path /

What I want to happen is for the language to be specified by the following in order of priority:

  1. via the URL
  2. via the Language Switcher
  3. via the Browser Settings

This is indeed what happened under 3.1.5 and reading about the clever work done in this thread I thought it was what you guys were aiming at too. Now it seems to be ignoring point 2 entirely, and the Language Switcher just sets the the URL in menu items, ignoring the cookie entirely for any other URL without a language specification. Am I still doing something wrong?

@Bakual
Copy link
Contributor Author

Bakual commented Jul 13, 2015

  1. via the URL
  2. via the Language Switcher
  3. via the Browser Settings

The language switcher is the same as "URL". So you either have "URL" or "browser settings". URL obviously has priority over browser settings.
What you mean is the cookie value which gets ignored. But only if Remove URL Language Code is disabled. If it's enabled all seem to work fine.

@chrisxchrisxchrisx
Can you please open a new issue? Because it will not get fixed when you comment on a closed PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants