? Success

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
4 Mar 2015

This PR removes the early loading of the languagefilter plugin and reduces the number of necessary checks in this condition-cascade. It also always checks for the language cookie and not only when the language filter plugin has been activated. I would argue that there is hardly a situation out there where you enabled the languagefilter plugin, select a language that you don't want and then again disable that plugin.

avatar Hackwar Hackwar - open - 4 Mar 2015
avatar joomla-cms-bot joomla-cms-bot - change - 4 Mar 2015
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 4 Mar 2015
Labels Added: ?
avatar zero-24 zero-24 - change - 6 Mar 2015
Category Libraries Plugins
avatar smz
smz - comment - 30 May 2015

Please have a look at the (merged) PR #7055: In it I introduced a new getLanguageCookie() method that also test if the cookie is for a published content language. But... it is a private method in the PlgSystemLanguageFilter class...

I think that in an ideal world we should only use globally accessible setLanguageCookie() and getLanguageCookie() methods, so that if we change their implementation (e.g. we don't use getHash() any more) we'll face no issues.

Do you have any advice about into which class those method should be introduced? JLanguage? JLanguageHelper?

avatar smz
smz - comment - 30 May 2015

Please also note that there is #7061 (not merged yet) where the language cookie could be delivered as a secure cookie (but I don't think this can affect your PR in any way...), and #7057 (not merged yet) where I deprecate the getCurrentLanguage() methods that were accessing the language cookie too and now get the current language from JFactory::getLanguage()->getTag();

avatar infograf768
infograf768 - comment - 31 May 2015

Looking for a language cookie when the plugin is not enabled is useless imho.

avatar smz
smz - comment - 31 May 2015

JM, I think that from a "logical" point of view you're absolutely right, but then:

  • you'll have to get the languagefilter plugin state by calling JPluginHelper::isEnabled() and/or JPluginHelper::getPlugin()
  • this in turn will call JPluginHelper::load() where:
  • we'll hit the database and call JFactory::getUser() which in turn... (etc., etc.)

I think the rationale behind this PR is essentially to streamline all of the above in the application initialization phase, which IMHO is a good idea.

Another possibility would be to have a specialised and streamlined method to check if the site is multilingual or not. I know we already have JLanguageMultilang::isEnabled() for that, but in it we try to optimize things using JFactory::getApplication()->isSite(): catch 22... :unamused:

Maybe removing that optimization from JLanguageMultilang::isEnabled() could help...

avatar smz
smz - comment - 31 May 2015

Please see #7091, if it can help...

avatar smz
smz - comment - 1 Jun 2015

@infograf768 and @Hackwar, I discovered something interesting (and puzzling...)

First, @hackwar, in theory there is a problem in your patch: $this->_detect_browser is not defined if you don't load the language filter (it is the language filter that initialize it), so the browser detection logic could not work with your patch.

But there is more: while trying to fix this (using a new method to directly get the language filter parameters without first loading it), I discovered that even if you get the language string from the browser, this is royally ignored and (typically) the default language will be used.

The very same happens with plain staging code: browser language is ignored.

Make a test:

  • be sure to have: "Language Selection for new Visitors" set to "Browser Settings" in the language filter
  • assuming you have an en-GB browser
  • set your default language to fr-FR (or de-DE, or whatever else...)
  • close your browser
  • delete the language cookie
  • re-open your browser and visit your naked domain
  • you are not redirected to en-GB, but to the default language

Even more (and this might be very clear to you, but it isn't to myself): It is not here, in JApplicationSite::initialiseApp(), that it is "decided" to which page you're routed.

Try to put a print_r($options['language']) at the end of the decision process and then try switching languages both through the language switcher or adding the language sef to your URLs.

Everytime you switch language you will see that here we have the previous language in $options['language']

And eeeeeeeven more, if you delete all the logic to get a correct $options['language'] here and just put a fixed $options['language'] = 'en-GB';, well... everything will work as before! You'll even be redirected to the correct cookie defined language when visiting the naked domain...everything as before as far as I can see. Browser detection still not working (but the problem must be somewhere else I would say at this point...)

avatar smz
smz - comment - 1 Jun 2015

My bet on PlgSystemLanguageFilter::parseRule()...

avatar smz
smz - comment - 1 Jun 2015

for naked domain $uri->getPath() returns the default language sef... (should return nothing...)

avatar Hackwar
Hackwar - comment - 1 Jun 2015

Sorry, but I don't understand what you are trying to say. JApplication:initialiseApp() is not meant to do the work of the languagefilter plugin. It is only meant to decide the language of a site that is not multi-language and it does that very well. For everything else, there is the languagefilter plugin.

This patch does not claim to make everything perfect in this code. It only tries to clean up some of the mess that is in it right now.

avatar infograf768
infograf768 - comment - 1 Jun 2015

I would argue that there is hardly a situation out there where you enabled the languagefilter plugin, select a language that you don't want and then again disable that plugin.

Even if that was done, looking for a language cookie would be wrong as the nature of the site would have changed.

Sorry, but I don't understand what you are trying to say. JApplication:initialiseApp() is not meant to do the work of the languagefilter plugin. It is only meant to decide the language of a site that is not multi-language and it does that very well. For everything else, there is the languagefilter plugin.

Yes, and that's why I do not see the use to check for the language cookie...

avatar smz
smz - comment - 1 Jun 2015

Sorry, but there is probably something very basic that I'm missing:

If (and at this point I'm sure it is...):

JApplication:initialiseApp() (...) is only meant to decide the language of a site that is not multi-language (...)

then not only the cookie, but also browser detection (a parameter set in language filter) should have no land here, correct?

The only thing we should do is to get the default content language from com_languages, right?

Do we want to respect logged-in user's language preference? This, of course, will only affect Joomla strings and not content (we are talking of a non-multilingual site)...

Am I missing something?

avatar smz
smz - comment - 1 Jun 2015

Have a look at Hackwar#20 if you wish...

avatar Hackwar Hackwar - close - 6 Jan 2016
avatar Hackwar Hackwar - close - 6 Jan 2016
avatar Hackwar Hackwar - head_ref_deleted - 6 Jan 2016
avatar Hackwar Hackwar - change - 6 Jan 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-01-06 11:32:54
Closed_By Hackwar

Add a Comment

Login with GitHub to post a comment