User tests: Successful: Unsuccessful:
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.
Labels |
Added:
?
|
Labels |
Added:
?
|
Category | ⇒ | Libraries Plugins |
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();
Looking for a language cookie when the plugin is not enabled is useless imho.
JM, I think that from a "logical" point of view you're absolutely right, but then:
JPluginHelper::isEnabled()
and/or JPluginHelper::getPlugin()
JPluginHelper::load()
where: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...
Maybe removing that optimization from JLanguageMultilang::isEnabled()
could help...
@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:
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...)
My bet on PlgSystemLanguageFilter::parseRule()
...
for naked domain $uri->getPath() returns the default language sef... (should return nothing...)
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.
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...
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?
Have a look at Hackwar#20 if you wish...
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-01-06 11:32:54 |
Closed_By | ⇒ | Hackwar |
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 thePlgSystemLanguageFilter
class...I think that in an ideal world we should only use globally accessible
setLanguageCookie()
andgetLanguageCookie()
methods, so that if we change their implementation (e.g. we don't usegetHash()
any more) we'll face no issues.Do you have any advice about into which class those method should be introduced?
JLanguage
?JLanguageHelper
?