Thanks, @RolandD

This solves the issue we just discussed. Agree with @zero-24 for the @since :)
RTC for me as soon as this is corrected.

TEST: set a user default site language to another language than the default site language.
Set the languagefilter plugin to Automatic Change
Login in frontend with that user, the user should be redirected to the correct language.
before patch, the redirection was always to the site default language.

@test OK for me too

@test
able to reproduce then #6904 works as described...
oops, just realise it was already RTC ;)

Thanks for the extra test @MAT978 Can never have enough :)

IMHO there is no need to have $db as a protected class property...
If there is agreement, can we fix this before 3.4.2 (or we will live with a B/C legacy...)

Please see #7130...

In case #7130 isn't acceptable, for whatever reasons, please make $db private

Also beware the usage of !in_array($lang_code, $lang_codes): in_array() is very-very lenient and it is a really dangerous function to use...

@smz The $db is protected because then the parent class will initialize it.

Care to explain why in_array() is really dangerous? You sound like we should remove it from the Joomla core altogether.

Hi Roland!

@smz The $db is protected because then the parent class will initialize it.

I'm not sure what you mean with this... Maybe your concern was not to mix $db with other other possible uses of variables of the same name in parent classes...

But variables declared at the class level (class properties) follows these visibility rules:

1. private: methods of the same class only (which I think was what you needed)
2. protected: methods of the same class + inherited and parent classes
3. public: everywhere

So, with protected, you are sharing $db with possible new classes extending PlgSystemLanguageFilter and with JPlugin which is PlgSystemLanguageFilter parent class

For a better explanation, please see: http://php.net/manual/en/language.oop5.visibility.php
Choosing the lowest visibility needed is always good programming practice.

Furthermore, due to our (very) strict B/C policy, I think that once you introduce a public/protected method/property, you'll have to live with that until the next major Joomla release.

Care to explain why in_array() is really dangerous? You sound like we should remove it from the Joomla core altogether.

Maybe I've been too harsh and overprotective with that!
in_array() is not the devil, and in this case it could had probably been fit, but the way it performs searches is quite permissive. If you have to check if something (string or integer) is present in an array's values, a safer way is to use array_key_exist() on the flipped (keys and values exchanged) array:
if (array_key_exist(array_flip($myarray))) ...

@smz The protected $db is correct. There is some magic happening in the JPlugin constructor which will populate $db automatically. It's similar to $app.
See https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/plugin/plugin.php#L108

@Bakual ah, OK! So we are not introducing any legacy here and, in case, we could get rid of the protected $db here if we don't need it any more (see: #7130), right?

@smz I am fully aware of how visibility works in PHP. Follow the code and you will find magic in JPlugin as pointed out by Thomas. You are correct, the variable $db can be removed as you did in your PR #7130

As for the in_array, that is not the case here because the value we are checking for can only be string and no longer needed with your change.

@roland-d
Sorry, I misinterpreted and thought that your wish was to share $db between onUserLogin() and the construct where I thought (but I was wrong) it was initialised. My bad!

As for B/C, it's not a problem anyway. Here we are looking at a plugin. This isn't meant to be extended and anyone doing so should be marked as a bad developer on next JaB.
B/C promise is only valid for library code and some other code which would break stuff for the end user. Like HTML output if changed drastically.

That's sadly not true. The b/c rules promised it for all code including extension code which is why we're so screwed in some areas (and why I wrote a long talk at Joomla Day Madrid moaning about it :P)

Umm, either you're interpreting what's published at http://developer.joomla.org/cms/development-strategy.html#backward_compatibility differently or there's a revision that isn't published, it says only /libraries is guaranteed right now. Otherwise you could make the argument you can't ever implement "new MVC" or the router refactorings in components without a major version bump.

Apparently I'm talking shit. But I was sure that's why we proxied the com_config MVC transition

Afaik com_config was rewritten before we adopted the new strategy.
There are also edge cases where components may be extended and/or interacted from other 3rd parties code. Com_config may have been such a case. Com_content may be another such tricky case where we would have to do changes sensible.
However officially, we do not guarantee B/C there