@infograf768 ...

I can't understand Travis complains about the query I introduced in JLanguageHelper::detectLanguage()... Can someone please help me?

In any case, that query is meant to get a list of installed languages (and as far as I can tell it works...).
I would be most happy to substitute it with a standard method call, but I couldn't find one: is something actually available for that?

Ouch, I must be blind... the method I was looking for was just under my eyes... (Actually above my eyes!)

@Bakual Thomas, can you have a look at this? Travis is complaining about:

1) JLanguageHelperTest::testDetectLanguage
RuntimeException: No database selected SQL=SELECT element
FROM jos_extensions
WHERE type='language' AND state=0 AND enabled=1 AND client_id=0
/home/travis/build/joomla/joomla-cms/libraries/joomla/database/driver/mysqli.php:610
/home/travis/build/joomla/joomla-cms/libraries/joomla/database/driver.php:1317
/home/travis/build/joomla/joomla-cms/libraries/joomla/language/helper.php:49
/home/travis/build/joomla/joomla-cms/libraries/joomla/language/helper.php:93
/home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/joomla/language/JLanguageHelperTest.php:48
phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/TextUI/Command.php:152
phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/TextUI/Command.php:104

I thought it was about a query I introduced in JLanguageHelper::detectLanguage(), but now I removed it and it is still complaing... Thanks!

The problem seems to occur in JLanguageHelper::createLanguageList(), line 49, $installed_languages = $db->loadObjectList('element'); which is code I haven't touched...

Test units issue?

The test class right now doesn't do any database tests. With your changes, there are now paths where a database query needs to be run. At a minimum, https://github.com/joomla/joomla-cms/blob/staging/tests/unit/suites/libraries/joomla/language/JLanguageHelperTest.php will need to extend TestCaseDatabase to enable this.

Not sure I understand the purpose of this PR.
If languagefilter is not enabled, any parameter set there is no use. Then why is that part of the testing instructions?
As far as I know, the only occurrence, else than in languagefilter (= when multilang is on), to check the browser language is when installing Joomla in order to propose, if possible, the installation UI in the said browser language.
Otherwise, the site default language should be used and, when a user logs in —and if set— the user default language.

There must be something I did not catch.

@mbabker Thanks, Michael!

Now I see: JLanguageHelper::createLanguageList() (an unmodified method) already could imply database operations, but in https://github.com/joomla/joomla-cms/blob/staging/tests/unit/suites/libraries/joomla/language/JLanguageHelperTest.php it was is tested in a way (fourth parameter omitted and thus assumed to false) that precluded that.

IMHO we should change that...

@infograf768 Hi, JM!

If languagefilter is not enabled, any parameter set there is no use. Then why is that part of the testing instructions?

Because (good or bad) it is the only place where we can control that behavior (browser language detection).

As far as I know, the only occurrence, else than in languagefilter (= when multilang is on), to check the browser language is when installing Joomla in order to propose, if possible, the installation UI in the said browser language.

JApplicationSite::initialiseApp() already performed that test and the only reason to perform it there is if you want browser detection for non-multilingual sites. It hasn't worked till now because of the problems I explained in this PR description, but now it should work.

Otherwise, the site default language should be used and, when a user logs in —and if set— the user default language.

In about 3 hours a lady will come to my house that needs my help to have some text entered (copy-and-paste) in an otherwise fully mono-lingual Arab site (it's true, but it is a Wordpress site!): this is a typical scenario where having browser detection in a mono-lingual site will help a lot (at least me, who can't read Arab...). But again, apparently this is a functionality that seems to have already been in Joomla, but broken.

that would be a fundamental change in joomla behaviour as it would mean that site administrators interest could be to install as many languages as possible + making sure that the installed extensions also include all these ini files.
In any case, such a change should not depend on language filter parameter, but on a specific global configuration Setting.

Dont forget that joomla.com sites come with all languages installed

that would be a fundamental change in joomla behaviour as it would mean that site administrators interest could be to install as many languages as possible + making sure that the installed extensions also include all these ini files.

I think that's already there, JM: when you install Joomla you can install as many languages as you want and then turn on (or not) the multilingual feature. And as you said the language choice is already possible in mono-lingual sites for logged-in users.

Browser detection in JApplicationSite::initialiseApp() (i.e. for mono-lingual sites) is already there since at least May 2010 (see: ddc5cf7)

In any case, such a change should not depend on language filter parameter, but on a specific global configuration Setting.

I totally agree on that, but... it is the way it is...

If you want I can move that choice to be a com_languages option (personally I think this is the place where it should be). This would be a little bit tricky as, for B/C, we should migrate the current value from PlgSystemLanguageFilter, but not a big deal (at first sight...)

... and don't forget that this PR fixes other browser detections issues for multi-lingual sites:

say you have an jp-JP (default) and en-GB multi-lingual site with browser detection turned on and you come to visit from Washington, DC: you'll probably prefer to be offered the British site than the Japanese one.

This, at this time, will not work as there are more issues for browser detection in the language filter itself, but to fix those I wait to have #7044 #7061 merged, or I'll have GitHub conflicts (and this is why I introduced the browser detection fix for mono-lingual sites).

say you have an jp-JP (default) and en-GB multi-lingual site with browser detection turned on and you come to visit from Washington, DC: you'll probably prefer to be offered the British site than the Japanese one.

That would mean it would change the behaviour of every multilingual site that exists

That would mean it would change the behaviour of every multilingual site that exists

Sounds to me like it would be a fix? Or are there cases where you would prefer a japanese site when you visit with an en-US browser? Assuming it's a multilingual site with en-GB and jp-JP (default) installed.

That would mean it would change the behaviour of every multilingual site that exists

Yes, at least for languages for which different locales exist, and this will be in accord with http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.4

Attempt to clarify:

When multilanguage is enabled (language filter enabled + content languages), when using browser settings instead of the default site language for new visitors will only happen when the url clicked on is the pure domain, i.e. www.mysite.com, and the browser language corresponds exactly to one of the published Content Languages. Thereafter, the language cookie is taking over.
It is a parameter. The site administrator chooses the behavior in the language filter.

What this is proposing, as far as I understand, is, by default for a monolanguage site, to load the browser preferred language —if that exact language tag is installed in the site, and whatever the default site language defined by the administrator of the site. I suppose that if the browser preferred language is not found it would default anyway to the site default language.

We are NOT speaking here of any Content in that language, just the UI. It would NOT be a Japanese site if jp-JP is installed, the default site language is not Japanese and content is in any other language.

In monolanguage, to be able to change the default site (and admin) language of the UI was introduced for logged users for whom it is authorized to set their user language in their profile (if other languages are proposed by the site administrator).

The behavior proposed would be totally different. It would be a NEW FEATURE.
I do not oppose to new features but the parameter to do so or not (not by default for B/C) has to be in the Global Configuration and only for monolanguage.

Multilanguage sites should be totally B/C and except for first visit, depending on cookie if set such. I can have a en-US preferred language for my browser but I may like to display as default the fr-FR part of a multilingual site.

Such a new feature is certainly not for this 3.4. series.

We are NOT speaking here of any Content in that language, just the UI. It would NOT be a Japanese site if jp-JP is installed, the default site language is not Japanese and content is in any other language.

That's clear (at least to me). The example given by @smz was about someone adding a new article in a foreign language site. Which would be prompted with an UI in the preferred local language.

The behavior proposed would be totally different. It would be a NEW FEATURE.

So it was never supposed to do a browser detect in a monolingual site? Then this needs to be removed instead.
Apparently, there is currently code in there to check for the browser language, but it isn't working.
So we need to decide if we want to remove that code fully or fix it. But leaving it there without function sure is wrong.

I do not oppose to new features but the parameter to do so or not (not by default for B/C) has to be in the Global Configuration and only for monolanguage.

If we need to add a parameter, I do agree on the global configuration part, as it's an application setting.
I'm not convinced it has to be one. As I don't see a downside of offering the UI in the browser language (if said language is installed), it's probably why the admin installed the language. But I may lack imagination and I don't like more parameters than needed

As far as I can remember indeed, browser language settings check was never intended for monolanguage sites (or never worked since the origin of Joomla. You can choose.) and was never required...
It has always been used only for Installation and Multilingual.
I am very curious to find out when this code made its way there...

Thanks to all for the interest in this!

I want to underline that my primary intent with this PR was not to fix/introduce (depending on how you see things) automatic browser language detection for monolingual sites, but to fix (in my opinion, others could say "extend") the language detection logic so that it returns valid a result for languages similar to the ones available on a given site.

The reasons why I did it together with the fixes to JApplicationSite::initialiseApp(), I already explained that: browser detection for monolingual sites is broken/absent (whatever...) and as far as I can tell it is broken for multilingual sites too (can someone confirm this, please?) and I can't fix that while #7061 is un-merged. So there would be no way to test it.

From a very practical stance, if you try this PR it will just work... Agreed that it will be totally un-aesthetic to have the parameter controlling it in an un-published plugin, but my personal opinion is: "What the heck... we can fix that thereafter... the damn thing works anyway..."

I totally agree with @bakual: if monolingual sites browser detection is something we do not want, then the code controlling it shouldn't be there at all, while indeed we even have public static variables controlling that and passing the required behavior between the language filter and the site application class initialisation method. Removing that would break B/C, I guess, so better fix it, don't you think?

I can't fix that while #7061 is un-merged

Fixed that issue for you. It's now merged

Agreed that it will be totally un-aesthetic to have the parameter controlling it in an un-published plugin

It may break if you unistall that plugin.

After thinking a bit about it and talking with JM, I think removing as much as possible from that code makes most sense. Since apparently browser language detection never worked in a monolingual site and isn't supposed to work.
I don't like to add an additional parameter which is only of quite some situational use. I guess it should actually be possible to offer such a browser language detection as a plugin on JED. Kind of a toned down multilang plugin.

Also to consider is what happens when a guest visits the site and wants the site to be shown with the default japanese UI, despite him using an english browser. He wouldn't be able to change the language without a language switcher of some kind. Which would be ugly on a multilingual site.

Fixed that issue for you. It's now merged

Thanks! Now I can fix language filter. (Well... let's be honest... I can try to fix it! )

...I don't like to add an additional parameter which is only of quite some situational use.

My personal POW is that who wants to offer personalized GUI language for logged in users should also offer browser detection for front-end GUI as otherwise one would probably not even be able to find out where the bloody login is reading in Arab, Japanese or Korean (and the other way round, of course!)

Also to consider is what happens when a guest visits the site and wants the site to be shown with the default japanese UI, despite him using an english browser. He wouldn't be able to change the language without a language switcher of some kind. Which would be ugly on a multilingual site.

Actually this is (and was) already accounted for: http://example.com/?language=jp-JP, which has precedence over browser detection. And then of course a webmaster could just turn off browser detection if this is deemed preferable (and by the way, off is the default value).

Anyway, I will now concentrate on fixing multilingual browser detection, but I propose to:

• Leave this PR open for a while, just in case...
• Fix test unit in https://github.com/joomla/joomla-cms/blob/staging/tests/unit/suites/libraries/joomla/language/JLanguageHelperTest.php by adding provision for database access or I will not be able to fix multilingual neither...

Thanks again!

OMG! OMG! I have broken browser detection for multilang in #7055!!!
Please merge #7111 ASAP...

Ash on my head...

OK, funny...

Now that I've fixed my moronic mistake in language filter I could test both the old version and my new version of JLanguageHelper::detectLanguage() in multilingual as well.

Guess what? In multilingual I get en-GB with my en-US browser with both of them!!

So in multilingual there must be some fail-safe mechanism that gracefully degrades every en-XX language code to en-GB.

I've not identified it yet, but I think it must lie in some lang_code -> sef -> lang_code double-translation.

In any case I think my JLanguageHelper::detectLanguage() is more RFC compliant and worth being accepted, but I'll now propose it separately as a "Code review".

It will need the test unit fix to pass...

This PR will probably die, and in case I'll propose a separate "Code review" for JApplicationSite::initialiseApp() (which isn't in a great shape anyway...).

For that I'll need guidance on what to do: 1) Introduce/fix browser detection for mono-lingual or 2) clean it up of all the useless language-related stuff.

If you want I can also leave this open (as we have mainly discussed about this issue), purge everything that is not strictly related (i.e. the new browser detection method) and in case change title/description.

Let me know what you prefer...

The serendipity corner:

• You have a multilingual site with en-GB as default and "remove sef" on
• Second language is it-IT with sef=it
• Nothing prevents you to create in en-GB a top-level menu item (e.g. for the "Information Technology" department blog) with "it" as its alias
• This will of course have http://example.com/it as its URL.
• That menu item (luckily enough...) will never be accessible.

Not a very likely scenario, but... should I open an "issue" for that?

The serendipity corner:
You have a multilingual site with en-GB as default and "remove sef" on
Second language is it-IT with sef=it
Nothing prevents you to create in en-GB a top-level menu item (e.g. for the "Information Technology" department blog) with "it" as its alias
This will of course have http://example.com/it as its URL.
That menu item (luckily enough...) will never be accessible.
Not a very likely scenario, but... should I open an "issue" for that?

I let you imagine the necessary checks which should only happen when saving the menu item:
is it in the menu root
is Remove URL Language Code set to Yes
is the alias equal to the sef of a published content language
And what if that menu item had been created before adding a new Content Language with its sef = the existing alias?

We can't cope with all idiotic stuff done by site admins... :)

We can't cope with all idiotic stuff done by site admins... :)

Agreed!

btw, concerning this

Guess what? In multilingual I get en-GB with my en-US browser with both of them!!

I guess this is due to the same behavior that forces "en" (if set such in the browser) to be used as priority.
As far as I could test, it is also alpha-ordered.
Example:
set your browser to "en" instead of "en-gb" or "en-us" and then install Joomla!.
You will be proposed en-AU as installation language...

@infograf768 Correct, and I stand corrected:

• Current implementation of browser detection has provision for partial language tag match too (I didn't got it right at first reading...)

• In case of partial match the first (alpha order) available match is returned

Differences:

• From a theoretical point of view my implementation is more RFC compliant (it takes weights into consideration), but from a practical point of view there is no difference.

• To make it really better we should have a square matrix of assigned desirable weights (so that, e.g., if you have only en-GB and en-AU installed, en-AU is preferable for en-NZ, while en-GB is preferable for en-US)

• My implementation can be a wee faster if there are many installed languages (mine loops on language codes returned by browser, current loops on language codes returned by browser and installed languages)

So there is no real practical advantage with my implementation and it is probably not worth changing anything unless we decide we want browser detection for mono-lingual too.

... well, there is one more difference (maybe a more important one): mine caches results, current doesn't, and I've seen that it can be called (and is actually called) in different places, not only language filter (e.g. com_tags)

I guess we can close this one to not confuse testers.

I guess we can close this one to not confuse testers.

Agreed!