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

REGRESSION: Language selection is not "remembered" when returning to site #6213

Closed
pieter-groeneweg opened this issue Feb 27, 2015 · 22 comments

Comments

@pieter-groeneweg
Copy link

read: http://forum.joomla.org/viewtopic.php?f=711&t=874852&p=3272103#p3272103

I did a quick test.

When selecting a language (en), cookie is set in path /en/ with value "en-GB" expiration this date next year.

When returning to the site, the language shown is dutch (nl).. Not even by choice of language default as "en" is set as default.

@engelweb
Copy link

I believe we're experiencing the same issue with our site; after updating, Joomla 3.4 seems to default to system language/browser language (depending on the setting of the language filter), completly ignoring the stored value in the cookie.

We noticed becuase it affects all SEF URL's not containing a language segment on our site.
Any URL like "example.com/pages" missing the language (ex. /en/) will automatically redirect to the default of the filter now, rather than checking with the cookie to retrive the current language.

In the previous version of Joomla, this was not an issue.

@pekollik
Copy link

I am running Joomla! 3.4.0 at localhost and I was not able to reproduce the issue. I installed dutch language packet, changed default language both for Site and Administrator back and forth between En and NL couple of time and everything was ok.

@pieter-groeneweg
Copy link
Author

Aha, yes indeed... it listens to the browser preference...

...but NOT to the cookie....

@infograf768
Copy link
Member

I confirm the regression.
The cookie is saved indeed, but when we load the url without languagecode it does not redirect to the last language used but to the default site language, whatver the settings for a new visitor (which the user is not, as a cookie exists).
this happens when "Remove URL Language Code" is set to Yes

@infograf768 infograf768 changed the title Language selection is not "remembered" when returning to site REGRESSION: Language selection is not "remembered" when returning to site Feb 28, 2015
@Hackwar
Copy link
Member

Hackwar commented Feb 28, 2015

@engelweb What you are describing is expected behavior. The cookie should never change the URL, but only hint which language you want. Since we don't want duplicate content, you can only remove the language code from the URL for the default language of the site, not of the specific user.

@infograf768
Copy link
Member

@Hackwar
what I describe is unwanted behaviour.
The cookie should be used, even when "Remove URL Language Code" is set to Yes

@davdebcom
Copy link
Contributor

There is no discussion possible about whether it should work this way or not, because it used to work this way. It is a backwards compatibility issue that needs to be fixed, because the project decided that in the 3.x upgrades there would be no BC incompatible changes.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6213.

@brianteeman
Copy link
Contributor

Agreed - this is a B/C change that needs to be fixed


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6213.

@Hackwar
Copy link
Member

Hackwar commented Feb 28, 2015

I don't see anybody denying that. I simply said that the behavior that @engelweb!! experiences is the expected behavior. Or does anybody claim that it would be ok to get dutch or english content under the exact same URL simply based on a cookie?

@HermanPeeren
Copy link
Contributor

When english is the default language, but the cookie says dutch was last used, then the url should be redirected to the dutch url.

Now https://github.com/joomla/joomla-cms/blob/staging/plugins/system/languagefilter/languagefilter.php#L241-L247 just take the default site language without looking at the cookie.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6213.

@davdebcom
Copy link
Contributor

Exactly, what @HermanPeeren says.
When english is the default language, but the cookie says dutch was last used, then the url should be redirected to the dutch url.

Of course with a correct redirect, not 301 but 303?

@Bakual
Copy link
Contributor

Bakual commented Feb 28, 2015

Closing this since Hannes did a PR #6230.
Please test so we can fix this in staging. Thanks all!

@Bakual Bakual closed this as completed Feb 28, 2015
@HermanPeeren
Copy link
Contributor

This should be reopened as #6230 is closed now and this issue not yet solved. Working on a PR.

@Bakual
Copy link
Contributor

Bakual commented Mar 1, 2015

Agreed

@Bakual Bakual reopened this Mar 1, 2015
@infograf768
Copy link
Member

Found another issue:
http://forum.joomla.org/viewtopic.php?f=711&t=875100

Folks, I wonder now if we should not simply revert the original PR.

@engelweb
Copy link

engelweb commented Mar 2, 2015

Sorry for this late reply, but I forgot a detail in my previous description of my case, which seem to have possibly caused a bit of confusion.

In the case I described earlier in this issue, the URLs are supposed to contain the language string, ex. "example.com/en/".
The problem occurs if I have a link in ex. a "Custom HTML" module which links to an address without the language code included.
Upon clicking said link, it will then redirect me to the default language and add it to the URL (ex. /en/), rather than the language stored in the cookie (ex. /nl/).
In the previous version, the Joomla would detect the lack of language segment, then search the cookie for the current language in order to put it back in and direct you to the correct page, rather than simply throw you back to the default.

If this is the new expected behavior, then I find that quite bothersome, since it's a "feature" that we have been taking advantage of in pretty much all our projects.

Either way, I hope that clearifies things a bit.
Other than that, it's good to see so many people actively working figuring out the problem.
Keep up the good work everyone!

@HermanPeeren
Copy link
Contributor

Main lesson: better, more systematically testing before commiting.

I think we only have some minor cases left now. Unfortunately today I have to work first, but hope to finish my PR tonight and it solves this issue. It took some more time because I want to get the code cleaner; easier to reason about.

Hannes' #5140 PR was a very good and necessary step in cleaning up the code-mess in the original languagefilter-plugin, that might have worked but was unmaintainable. The other, menu-item-alias issue, is probably not related to the languagefilter-plugin, see #6254. 'Simply' reverting a possibly related PR could cause other issues and could be counter-productive.

@Hackwar
Copy link
Member

Hackwar commented Mar 2, 2015

The menu alias is fixed with #6254

Regarding issues with language: There is quite a bit more cruft in JApplicationSite::initialiseApp(). Most of those checks there are unnecessary and quite a bit of those checks never evaluate to true. If the project is interested in cleaning up old code, that would be the next area to work on to improve the language system.

@infograf768
Copy link
Member

#6254 does not solve the menu item alias here, or, rather, introduces other error. See PR.

@infograf768
Copy link
Member

New #6254 and #6278 solve Falang, Virtuemart and menu item alias issues.

Just remains to solve this cookie one to be fine for 3.4.1

@Bakual
Copy link
Contributor

Bakual commented Mar 9, 2015

Hannes did another attemp of solving this. Can you guys please test #6371?

@rdeutz
Copy link
Contributor

rdeutz commented Mar 20, 2015

Problem fixed with #6452 so closing this one

@rdeutz rdeutz closed this as completed Mar 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.