I have tested this for plugins by changing in plugins/models/plugin.php, lines 273 +

		// Load the core and/or local language file(s).
			$lang->load('plg_' . $folder . '_' . $element, JPATH_ADMINISTRATOR, null, false, true)
		||	$lang->load('plg_' . $folder . '_' . $element, JPATH_PLUGINS . '/' . $folder . '/' . $element, null, false, true);

to

		// Load the core and/or local language file(s).
			
			$lang->load('plg_' . $folder . '_' . $element, JPATH_PLUGINS . '/' . $folder . '/' . $element, null, false, true);
			$lang->load('plg_' . $folder . '_' . $element, JPATH_ADMINISTRATOR, null, false, true);

And it worked fine. Debug Language still working fine too.

I guess we do have, as @Bakual states above, to modify ALL $lang->load to solve the issue globally.
Thanks @nikosdion !

Any volunteer to do it everywhere? ?

I guess I can do that and update this PR

That's a lot of work! Thanks.

Yes, I actually have to do this for the PR to be complete. The same code was copied all over the place, from com_categories to com_fields and everything in between ? Give me an hour, I'm on this.

We have about 201 $lang->load in staging but quite a few of them shall not be changed.

There are 167 + 142 occurrences which need some time, love and care. BTW, instead of looking for $lang->load (which assumes that the Language object is always called $lang) use the regular expression #->load\(.*JPATH.*\)# instead. This matches all the uses of language loading we're interested in since each of those pairs (or quartets) of language loading references a JPATH_SITE or JPATH_ADMINISTRATOR or JPATH_BASE path in at least one of the load() calls.

Also fixing in this PR: old code which loaded the default language explicitly instead of passing the $default parameter. This was causing inconsistencies. Code which explicitly loaded the default language would also be triggered when debug language is set to Yes. Code which uses the $default parameter in load() will NOT load the default language if debug language us set to Yes. I guess when the $default parameter was added nobody did a full refactoring of the existing code.

@nikosdion Before you go further

We do have an issue though. It totally changes the behavior for what concerns the same language.
I mean: if we have a fr-FR.ini in core and also a fr-FR.ini in extension, now the extension one would override the core one. Until today it is the opposite.
Same for any language.

@infograf768 This was a bug since Joomla! 1.6. Until around 1.6.0-alpha2 the intention was to completely remove system-wide language files and replace them with extension files. This was abandoned and you could still see some old untouched code mentioning that. The rest of the code written since i.e. in the last seven years was trying to do the opposite: load the extension language files ONLY when the system-wide ones don't exist, i.e. system overrides extension.

This worked very well until we made a change in JLanguage to always load the default language file (instead of only loading it when the specific language file doesn't exist). Now Joomla! would try to load EITHER the current language translation file OR the default language file from the system wide directory. Only if BOTH loads failed would it look in the extension folder.

What I did was normalize these rules into the intended load order (language strings from a file override the same strings from all previous files):

• Extension-specific, default language file (components/com_example/language/en-GB.com_example.ini)
• Extension-specific, current language language file (components/com_example/language/fr-FR.com_example.ini)
• System-wide, default language file (language/en-GB.com_example.ini)
• System-wide, current language language file (language/fr-FR.com_example.ini)

Why so? As a developer I can ship language files with my extension. They are installed inside my extension's folder. You cannot touch them, they'd be overwritten next time you update my extension. How do you, the French TT, provide a better translation for my software without me having to release a new version? By creating language packs which are installed in the system-wide folder. Therefore the system-wide files MUST override the extension files. If you don't, you can never fix language issues unless the developer publishes a new version of the extension, completely missing the point of Joomla! having user-installable language files.

if we have a fr-FR.ini in core and also a fr-FR.ini in extension, now the extension one would override the core one. Until today it is the opposite.

No, it's the same behavior. Global ones will override the component ones. There is never a case where an extension one will override a global one.
The only change we get is an improvement inm that we get per string fallbacks and no longer per file.

Tested this patch.
It solves at the same time the missing en-GB in the same folder as the fr-FR.ini and, indeed, keeps the priority to the language ini file in the core folders.
I guess because you also modified language.php

Tested for now only for plugins, but looks great!!

@infograf768 Hahaha, no. You just had it wrong all along because the code was so darned confusing :D The only change I did to language.php was rewrite it to be more legible. I didn't touch its logic. The legibility improvements make it easier to understand what it does, therefore harder to abuse or break it. Hopefully this will reduce bugs in the long run.

As long as we don't end up in a spot where conceivably the en-GB strings from the system language directory can overload another language's strings loaded from the extension's directory, I think this looks fine.

As long as we don't end up in a spot where conceivably the en-GB strings from the system language directory can overload another language's strings loaded from the extension's directory, I think this looks fine.

That would happen with this PR. But currently, the system en-GB file would override the whole extension language file as well. So it's not really a change in behavior. Only that it changes from "whole file" to "string".
But then, it doesn't make any sense to have an en-GB file present in the system folder while having translations in the extension one. I don't see a use case for that.

The use case doesn't exist. But, people do crazy things then blame us when they do them and it breaks things.

Sure, but it doesn't change behavior. It didn't work before and will not work afterwards. So nothing to blame :)

So nothing to blame :)

Well, there is, but they won't like the answer I give them ?

Like I said, this looks fine as is.

@mbabker Uh, it actually does exactly that. Darn.

Working around that requires doing the following for every language load:

• Load the default language from the extension folder
• Load the current (with fallback to default) language from the system folder
• Load the current (WITHOUT fallback to default) language from the extension folder

Caveats:

• It's 3 lines instead of 2
• The default language is still added even if Debug Language is set to Yes unless we add another if-block raising the total number of lines to 6 and adding one more tight coupling in the code (it has to know the meaning of the debug property of the Language package)

I guess put this on hold until I get some time today to do that.

@nikosdion I wouldn't do that. It makes the code more complex without gaining anything (ther is no use case for that)
PR is fine as it is and Michael is fine with it as well ?

I agree that the use case is not existent. I think we can keep the code as it is but document that the en-GB language MUST be system wide and NEVER in the extension to prevent any weird bug reports in the future. In other words, let's save institutional knowledge somewhere publicly accessible.

Nicholas K. Dionysopoulos
Director, Akeeba Ltd

From: Thomas Hunziker notifications@github.com
Sent: Tuesday, August 1, 2017 5:14:52 PM
To: joomla/joomla-cms
Cc: Nicholas K. Dionysopoulos; Mention
Subject: Re: [joomla/joomla-cms] RFC: Make Joomla! language handling consistent across the entire core code base (#17372)

@nikosdionhttps://github.com/nikosdion I wouldn't do that. It makes the code more complex without gaining anything (ther is no use case for that)
PR is fine as it is and Michael is fine with it as well ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub#17372 (comment), or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAPoKbFXFjsIUWdampEkkqX-wYmWE6gIks5sTzLcgaJpZM4Ootyw.

Would that mean, that it is recommended that every extension dev changes:
$lang->load('com_foo', JPATH_ADMINISTRATOR);
to
$lang->load('com_foo', JPATH_ADMINISTRATOR, null, false, true);
?

Currently, these are the default values for the optional parameters. IIRC they were not always like that. In fact, $default didn't exist at all a few years ago.

I made them explicit because in case of a future refactor we will be left without an idea of what we were expecting the code to do. Normally this wouldn't be an issue if you do refactoring properly. Based on what I saw in the language-related code yesterday I wouldn't call the refactoring done to it "proper" in any way. It was mostly a hodgepodge of different generations of code. So I normalized everything to be on the safe side.

I would recommend that all extension developers uses all five parameters with explicit values just in case. Leaving language load with just two parameters means that you accept the risk that Joomla! might change something in the future and you won't catch it when beta testing (especially if you don't beta test in a different language). That said, that's my personal recommendation, not a hard rule that extension developers must follow.

I think we can keep the code as it is but document that the en-GB language MUST be system wide and NEVER in the extension to prevent any weird bug reports in the future.

Umm, are you sure you didn't mix up something? With this PR, that rule doesn't make sense. Without, it would make sense to a degree but only to work around a bug.

With this PR this is the behaviour (which imho is expected):

Now, I'm going to ask, why is the doesn't work scenario listed that way? Languages don't have a filesystem priority order like layouts or JHtml loaded media do, that system is entirely driven by what order files are loaded in. The key concern I pointed out is we shouldn't conceivably make it easy for a validly translated file to be overwritten by trying to load the same file from another path and the default language (generally en-GB) existing in that other path causing existing keys to be overwritten.

Internally, part of the problem is we are shoving it all into a single message catalogue. The reality of it is each language should have its own catalogue and we push loaded files into either the active language or the default language store based on the load method parameters (basically you split it up and have the concept of a fallback catalogue). Even that doesn't 100% address the issue of trying to load a (whatever language code here) file from one path then another version of it is loaded from another path with different translations.

I guess if you want to do that, you need to refactor the whole language loading and introduce possible B/C breaks. Thus being something for 4.0.
This PR on the other hand is actually a quite simple improvement without changing any APIs.

Agree totally this PR helps. I'm willing to merge, but it'd be nice to get some tests first ?

FWIW this could be done without a B/C break; in the end it's adding a new $defaultStrings class property and making the loadLanguage() method aware of whether a default language is being loaded; if so then the strings are added to the new property else the existing behavior of loading them to the $strings property is done. Then the _ method adds another lookup to determine if the key exists in the $defaultStrings property. Finally the hasKey method also looks in the $defaultStrings property for a key's presence). Public API doesn't change at all, if someone's subclassing JLanguage the worst thing that happens is they might get some untranslated strings if they aren't aware of the new property.

I'm not going to unit test 90% of the core library code just because I tried fixing a language issue I'm not even affected by :D I'd rather document the following:

DO NOT put language files in your extensions' folders, it will break when someone puts language files system wide. ALWAYS put translation files system wide.

Same effect as this PR, no code, no unit tests needed. If it breaks because someone tried to use a feature broken the last eight years it's 3PD's fault per Joomla's SOP, not a core bug. Done.

From: Michael Babker notifications@github.com
Sent: Wednesday, August 2, 2017 8:14:56 PM
To: joomla/joomla-cms
Cc: Nicholas K. Dionysopoulos; Mention
Subject: Re: [joomla/joomla-cms] RFC: Make Joomla! language handling consistent across the entire core code base (#17372)

Agree totally this PR helps. I'm willing to merge, but it'd be nice to get some tests first ?

FWIW this could be done without a B/C break; in the end it's adding a new $defaultStrings class property and making the loadLanguage() method aware of whether a default language is being loaded; if so then the strings are added to the new property else the existing behavior of loading them to the $strings property is done. Then the _ method adds another lookup to determine if the key exists in the $defaultStrings property. Finally the hasKey method also looks in the $defaultStrings property for a key's presence). Public API doesn't change at all, if someone's subclassing JLanguage the worst thing that happens is they might get some untranslated strings if they aren't aware of the new property.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub#17372 (comment), or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAPoKUUck6JWMBEpW-zS3lBcA2-gbf9Hks5sUK6PgaJpZM4Ootyw.

I mean people testing to ensure nothing's broken, not unit testing. Though we really do need more unit testing, but that's a story that'll never end.

I know that you're happy with this PR but last night I realised it's not. So my comment holds, I'm afraid :( It's not just a quip. Even with this PR, having language files in the extension's folder will break under the circumstances that you explained. That makes me unhappy. It's not a good solution and it's definitely not an elegant one either.

Moreover, it doesn't protect 3PDs from themselves (mostly guilty as charged here). When does a 3PD need to load languages manually? In several cases. In plugins when loading the component's language files so that Models don't generate untranslated strings, for example. Or in plugins when the developer hasn't figured out $this->loadLanguage(). Or when you want to load a different component's (possibly core component's) language file in your Model. These are just the cases I had to deal with the last 15 days. In all these cases the developer does something like JFactory::getLanguage()->load('com_foobar', JPATH_ADMINISTRATOR);. He tests, it works, he moves on... but the language files from the component's directory are never loaded. So we actually end up putting the onus to the developer to load the correct language files in the correct order.

IMHO this is a strong indicator of a badly thought out API. The API should be DRY and abstract those decisions from the developer. It's NOT a low level API (loadLanguage() is the low level API and we are supposed to abstract it for the developer through load()). When I tell the high level load() API to load the language files for com_foobar, mod_whatever, plg_system_whatchacallit or tpl_yadayadayada I am explicitly telling it which extension I am interested in and which application folder (site or admin). I am giving it all the information it needs to take a decision. Yet, for some silly reason lost in the sands of time, I have to call it two to four times to have it load the correct files, i.e. I have to take a decision that belongs in the API.

I propose that we trash this PR and start a new one which only changes Language::load(). Have it analyze the first argument. If it looks like an extension name AND the second argument is either JPATH_SITE or JPATH_ADMINISTRATOR then and only then try to load the translation files in the correct order:

• Component-specific en-GB (if $default is true)
• System-wide en-GB (if $default is true)
• Component specific current language
• System-wide current language

One stone kills about 200-odd birds. Making all those 200-odd changes in various places in the code is no longer necessary since Boolean short-circuit evaluation means that only the first leg, ostensibly trying to load the system-wide files, will get called. Even better, we don't have to tell 3PDs to do anything. The core takes care of everything. Even better, we can Unit Test the bloody thing making the need for human testers moot.

I appreciate your feedback on this.

Would be a nice move as IMO it is the responsibility of the core to work properly out of the box.

I propose that we trash this PR and start a new one which only changes Language::load(). Have it analyze the first argument. If it looks like an extension name AND the second argument is either JPATH_SITE or JPATH_ADMINISTRATOR then and only then try to load the translation files in the correct order: ...

The above is much better choice that making several calls to Language::load() (2 or 4 calls !)
just after such a PR is made and accepted, which also in 1 out of 4 cases it will have the issue mentioned above of overriding current language with english (i know it is rare and that it exists already, but why not fix properly this case too)

The or-chain code of calling Language::load() will no longer be needed (will be redundant), and at some point it should be removed in order to have cleaner code, this again will involve changing code all these places ...

Imho, this PR would be a quick win without breaking anything.

Of course it would be nice to improve the language loading in general. But instead of trying to do that in a B/C way with some guessing and magic, why not refactor it for J4 and do it properly then?

Because the refactoring you need to do is what you call "guessing and magic" which is already b/c. SO why not put it in Joomla! 3?

Also, to clarify, it's neither guessing nor magic. It's taking core API business decisions based on a. user input and b. system architecture. Both are reasonably expected when you use a high level API. Not taking them is a bug. Granted, it has persisted for 8 years but that doesn't make it less of a bug.

Argueabley it is not B/C break it is a needed bug-fixing, also what exactly will break ?

Component-specific en-GB (if $default is true)
System-wide en-GB (if $default is true)
Component specific current language
System-wide current language

This looks right to me.

If we can shove more logic into the language class so devs don't have to be aware of the out-of-the-box possibilities and account for those on their own, then let's do it.

Concerning lang load see also #17398

I rebased this PR to the new staging and implemented my idea for Language::load(). Can you please take a look at it? When that part is ready I will modify all of the other touched files to use the new features of the Language::load() method instead of using repeated code.

I'd appreciate a prompt response. This week the public-facing support is closed and I had to cancel my vacation due to the wildfires in Greece. This means I'm in the office (it has A/C!), bored out of my skull and am going to spend quite some time writing code ;)

Looks good to me.

@nikosdion
After these last changes, can you specify the tests we should do?

@infograf768 The changes I submitted last night were just low level changes to the API. I wanted to have another developer look at them (two sets of eyes are better than one). I will change all the code that calls the Language::load() API today. After I submit my next bunch of changes you should just do the same tests as before.

Remember that the language loading order is as follows (later items overwrite newer items):

• Extension-specific default language e.g. components/com_example/language/en-GB/en-GB.com_example.ini
• System-wide default language e.g. language/en-GB/en-GB.com_example.ini
• Extension-specific current language e.g. language/fr-FR/fr-FR.com_example.ini
• System-wide current language e.g. language/fr-FR/fr-FR.com_example.ini

At the very least we want to test this for components, modules, plugins and templates. The code also supports libraries. However, all library packages I've seen either do their own thing (um, I plead guilty) or just shove everything into the system-wide languages folder. In either case you won't be able to meaningfully test them. The other four types of extensions, though, are the bulk of 3PD Joomla! code and we definitely need to have them tested :)

I'll ping you when I finish writing the code.

OK, everyone. I think that this is feature-complete.

Please test as I mentioned above, in my reply to @infograf768. Thank you :)

@nikosdion @infograf768 Why was this closed? without any comments!

I have no idea. I guess because nobody fully tested it (I missed Nicholas last message).

I'd suggest reopening this and consider for 4.0 at least, if not 3.9 for whatever reason. However, I am not sure until I understand the reason it was closed.

I have deleted the code since it's been so long that nobody seemed interested in my contribution. Sorry. My software is not affected anymore as I'm using FOF 3 for everything. Everyone else not using FOF 3: good luck :)

Well, GitHub let me restore the branch. If anyone is crazy enough to rebase it to the current staging (just the 2018 copyright commit would take you a day or three to process) go for it and redo the PR. I am not that crazy and I now have a 4 month old baby demanding my attention. Also, no incentive to push myself since I'm no longer affected by this.

4 months old baby means someone is not sleeping much at your home ?

@nikosdion Lots of love to the 4 month old baby ?

I have just merged latest staging into your branch at nikosdion#3. Please merge and reopen this.

If you want me to open a new PR instead please let me know.

Ping @infograf768

OK, here we go. I have re-opened this PR. I don't see a reason why it couldn't be included in 3.9 or 3.10 since it's b/c. Hopefully this time things will get moving :)

Now starting to test on staging as it would not go on 3.9 (issue with language.php)

Notice: Undefined offset: 1 in /Applications/MAMP/htdocs/installmulti/trunkgitnew/libraries/src/Language/Language.php on line 1526

Failed to start the session because headers have already been sent by "/Applications/MAMP/htdocs/installmulti/trunkgitnew/libraries/src/Language/Language.php" at line 1526.

And in logs
[23-May-2018 10:13:20 Europe/Berlin] PHP Warning: session_regenerate_id(): Cannot regenerate session id - headers already sent in /Applications/MAMP/htdocs/installmulti/trunkgitnew/libraries/joomla/session/handler/native.php on line 151

Index: libraries/src/Language/Language.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- libraries/src/Language/Language.php	(date 1527064051000)
+++ libraries/src/Language/Language.php	(date 1527064051000)
@@ -1511,7 +1511,7 @@
 		}
 
 		// All valid extension names are at least 5 characters long (3 character prefix, underscore and extension name)
-		if (strlen($extension) < 5)
+		if (strlen($extension) < 5 || $extension === 'joomla')
 		{
 			return false;
 		}

I am not sure how to commit to this branch without disturbing Nicholas. Here is the patch.

languages can't be switched in backend. It stays in English

Well, looks like a simple merge was not enough. I'll try to look deeper and test it myself first. Will come back when I have it working.

@izharaazmi Send me a PR on my repo and I'll merge it ;) Generally, I am open to merging such PRs. I may not be very fast in doing so but I will get there within a business day at most.

I still have the undefined offset when I display the Manage manager
sys.ini definitely don't load

Thanks @nikosdion :)

@infograf768 I'll look all bits and pieces together and try to fix as much as I can. This will save time for all 3 of us.

Extensions language files only present in the extension folder do NOT load.

@izharaazmi
Just trying to point obvious problems for now.

Sure, that is very helpful. Thanks.

@infograf768 Are you sure this is not a case of what I had explained? Is your non-loading file en-GB or a different language? Look at the load order in the comment I linked and check if any of these files is present. Each item on the list overrides the items preceding it on the list (last item found wins).