Make Joomla! language handling consistent across the entire core code base #17372

nikosdion · 2017-07-31T17:58:32Z

Pull Request for Issue #17272

Pinging @b2z @laoneo @infograf768 and @Bakual

Summary of Changes

Objectives:

Joomla! must ALWAYS load the default language file (typically: en-GB, depends on language config) no matter if it's stored in the system-wide or extension-specific folder.
Joomla! must ALWAYS override the extension-specific language files with the system-wide language files, allowing TTs to improve upon built-in translations.
Joomla! must ALWAYS override the default language with the user's language.
These language rules must apply to all extensions, no matter which part of Joomla's core code is loading them.
Language loading in Joomla! should be in a consistent code format, making it easy to refactor without developers being prone to forgetting any instances.

Actions taken:

Normalised all instances of JFactor::getLanguage() to point to a $lang variable.
Refactored all calls to lang() separated by Boolean Or (||) into separate lines.
Added comments on all refactored calls with a link back to this PR so that future contributors will not try to refactor the code back to the broken state.
Refactored Language::load(). You may want to push this upstream @mbabker
Some namespaced libraries under the Joomla\CMS namespace have been touched with the rules above. Do we need to merge upstream anywhere? I can't find any info Ping @mbabker @wilsonge

Technical notes

Removal of Boolean Or (||) from language file loading

Joomla! currently loads language files from the system-wide folder (language or administrator/language). If no files are found there then, and only then, will Joomla! look in the component folder (components/com_example/language or administrator/components/com_example/language).

The problem is that a component can only ship its language files inside the component directory, including the default language (en-GB). If the user now installs a language pack for this extension they will get system-wide translation files installed, let's say for fr-FR. If that translation (in our case fr-FR) is incomplete the user will receive untranslated strings (e.g. COM_EXAMPLE_SOMEVIEW_LBL_WHATEVER) instead of the default (en-GB) translation. This happens because of Boolean short-circuit evaluation which causes Joomla! to stop looking for language files once the system-wide files are loaded.

This PR removes the Boolean OR and replaces it with two separate lines. The order of the lines, as discussed in gh-17272, is such that system-wide language files override component-specific language files.

Load order of language files

As noted in the comments of this PR, I normalized the language loading rules from a hodgepodge of decisions made on different generations of code and ad-hoc accidental rules into a sensible load order. The load order for all extension types, loaded anywhere in core code, now is:

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, let's say the French Translation Team, provide a better French 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 we did it the other way around (extension overrides system-wide languages) you can never have an up to date translation until I, the developer of Joomla! extensions, publish a new version of the extension. This would completely miss the point of Joomla! having user-installable language files.

When is the default language loaded and what exactly is the default language?

This is not a change in this PR. I am documenting the current behavior of Joomla! since there seems to be an abundance of confusion surrounding it.

When Joomla's Debug Language is set to Yes it will only ever load the translation file for the user's currently set language. If the file is missing language strings, is invalid or doesn't exist at all you get untranslated strings.

In any other case, Joomla! will first try to load the default language file. Only then will it try to load the user's currently set language. Any language strings not defined in the current language file will be taken from the default language file. This may result in the display of mixed language content e.g. French mixed with (some) English but this is generally better for the user than throwing unsightly untranslated keys to their faces.

In the current Joomla! code base the default language is always English (en-GB). This is something that can be globally changed in theory, albeit Joomla! doesn't take advantage of this feature (yet).

Documentation Changes Required

I would strongly recommend adding the bits I wrote above about language load order and default language loading in the Wiki.

@b2z

…specific folders PR for gh-17272 Pinging @b2z @laoneo @infograf768 and @Bakual # Summary of changes Joomla! currently loads language files from the system-wide folder (`language` or `administrator/language`). If no files are found there then, and only then, will Joomla! look in the component folder (`components/com_example/language` or `administrator/components/com_example/language`). The problem is that a component can only ship its language files inside the component directory, including the default language (en-GB). If the user now installs a language pack for this extension they will get system-wide translation files installed, let's say for fr-FR. If that translation (in our case fr-FR) is incomplete the user will receive untranslated strings (e.g. COM_EXAMPLE_SOMEVIEW_LBL_WHATEVER) instead of the default (en-GB) translation. This happens because of Boolean short-circuit evaluation which causes Joomla! to stop looking for language files once the system-wide files are loaded. This PR removes the Boolean `OR` and replaces it with two separate lines. The order of the lines, as discussed in gh-17272, is such that system-wide language files override component-specific language files. # Documentation changes required You _may_ want to document that default language files can be placed in either system-wide or component-specific folders starting Joomla! 3.8.0 (or whatever version this is merged) whereas in previous versions they HAD TO be placed in the system-wide folders. Just so that this bit of institutional knowledge is not lost to the sands of time and the interwebz ;)

Bakual

Code change looks fine.
If this is accepted we likely have to search for other instances of language file loading and apply the same type of change there.

infograf768 · 2017-08-01T07:03:32Z

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 !

infograf768 · 2017-08-01T07:05:42Z

Any volunteer to do it everywhere? 😃

nikosdion · 2017-08-01T07:19:28Z

I guess I can do that and update this PR

infograf768 · 2017-08-01T07:28:51Z

That's a lot of work! Thanks.

nikosdion · 2017-08-01T07:29:29Z

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.

infograf768 · 2017-08-01T07:31:20Z

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

nikosdion · 2017-08-01T07:37:09Z

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.

infograf768 · 2017-08-01T08:33:40Z

@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.

@mbabker

We may have to push this upstream. Ping @mbabker

… base Objectives: * Language loading in Joomla! should be in a consistent code format, making it easy to refactor without forgetting any instances. * Joomla! must ALWAYS load the default language file (typically: en-GB, depends on language config) no matter if it's stored in the system-wide or extension-specific folder. * Joomla! must ALWAYS override the extension-specific language files with the system-wide language files, allowing TTs to improve upon built-in translations. * Joomla! must ALWAYS override the default language with the user's language. * These language rules must apply to all extensions, no matter which part of Joomla's core code is loading them. Actions taken: * Normalised all instances of JFactor::getLanguage() to point to a $lang variable. * Typehinted all $lang variables to the framework Language class (helps IDEs recognize the method calls, reducing the likelihood of writing bad code). * Refactored all calls to lang() separated by Boolean Or (||) into separate lines. * Added comments on all refactored calls with a link back to this PR so that future contributors will not try to refactor the code back to the broken state.

… base Remove typehints. They are out of scope for this PR, I guess.

… base I had missed a few spots

nikosdion · 2017-08-01T09:08:56Z

@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.

zero-24 · 2017-08-01T09:14:15Z

administrator/components/com_categories/helpers/categories.php

+					 */
+
+					// Loading language file from the administrator/components/*extension*/language directory
+					$lang->load($component, JPath::clean(JPATH_ADMINISTRATOR . '/components/' . $component), null, false, true);
 					// Loading language file from the administrator/language directory then


please take a look into the drone error on this line:

FILE: ...omla-cms/administrator/components/com_categories/helpers/categories.php -------------------------------------------------------------------------------- FOUND 1 ERROR(S) AFFECTING 1 LINE(S) -------------------------------------------------------------------------------- 69 | ERROR | Please consider a blank line preceding your comment --------------------------------------------------------------------------------

http://213.160.72.75/joomla/joomla-cms/7502

I protest :) We can easily remove the comments but this will most definitely result in developers in the not so distant future screwing up the language loading since there'd be nothing warning them against putting a Boolean Or again (or doing something sillier).

I am a practical guy. Between satisfying a set of generic rules regarding comments and making sure that the application won't break next time someone touches it I will choose the latter any time of the day.

I'd strongly recommend reading Ponderings on Odoriferous Syntactical Constructifications. If you're not familiar with the author Google him ;)

Sorry you got me wrong. I was just suggesting to add a clean line before the comment not to remove them ;)

But it looks like george has already done that: ff95399 Thanks.

Bakual · 2017-08-01T09:33:40Z

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.

infograf768 · 2017-08-01T09:48:46Z

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!!

nikosdion · 2017-08-01T10:19:52Z

@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.

mbabker · 2017-08-01T11:59:06Z

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.

Bakual · 2017-08-01T13:08:11Z

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.

mbabker · 2017-08-01T13:17:59Z

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

Bakual · 2017-08-01T13:36:07Z

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

mbabker · 2017-08-01T13:42:11Z

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.

nikosdion · 2017-08-01T14:02:52Z

@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.

infograf768 · 2018-05-23T05:26:25Z

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

# Conflicts: # administrator/components/com_menus/models/items.php # build/helpTOC.php # components/com_contact/contact.php # libraries/src/Installer/InstallerAdapter.php # plugins/system/updatenotification/updatenotification.php

izharaazmi · 2018-05-23T07:42:28Z

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

I have just merged latest staging into your branch at https://github.com/nikosdion/joomla-cms/pull/3. Please merge and reopen this.

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

Ping @infograf768

Update PR #17372 with current staging

nikosdion · 2018-05-23T07:50:05Z

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 :)

infograf768 · 2018-05-23T08:12:35Z

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

infograf768 · 2018-05-23T08:13:54Z

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

izharaazmi · 2018-05-23T08:32:41Z

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.

infograf768 · 2018-05-23T08:37:37Z

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

izharaazmi · 2018-05-23T08:41:23Z

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.

nikosdion · 2018-05-23T08:43:03Z

@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.

infograf768 · 2018-05-23T08:44:33Z

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

izharaazmi · 2018-05-23T08:51:40Z

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.

infograf768 · 2018-05-23T08:51:51Z

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

infograf768 · 2018-05-23T08:52:35Z

@izharaazmi
Just trying to point obvious problems for now.

izharaazmi · 2018-05-23T08:54:06Z

Sure, that is very helpful. Thanks.

nikosdion · 2018-05-23T20:04:02Z

@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).

joomla-cms-bot added the PR-staging label Jul 31, 2017

Bakual added the Documentation Required label Jul 31, 2017

Bakual approved these changes Jul 31, 2017

View reviewed changes

Nicholas K. Dionysopoulos added 4 commits August 1, 2017 11:39

Refactored Language::load() to make it more readable

dadf64d

We may have to push this upstream. Ping @mbabker

Make Joomla! language handling consistent across the entire core code…

0b9ddae

… base Remove typehints. They are out of scope for this PR, I guess.

Make Joomla! language handling consistent across the entire core code…

e4b1138

… base I had missed a few spots

nikosdion changed the title ~~Load component language files from both system and component folders~~ Make Joomla! language handling consistent across the entire core code base Aug 1, 2017

zero-24 reviewed Aug 1, 2017

View reviewed changes

Add new line to fix codestyle

ff95399

infograf768 changed the title ~~Make Joomla! language handling consistent across the entire core code base~~ RFC: Make Joomla! language handling consistent across the entire core code base Aug 1, 2017

Merge pull request #3 from izharaazmi/lang-load-sequence

647a4ff

Update PR #17372 with current staging

nikosdion reopened this May 23, 2018

nikosdion requested review from laoneo, mbabker, rdeutz and wilsonge as code owners May 23, 2018 07:49

ghost added the J3 Issue label Apr 5, 2019

ghost changed the title ~~RFC: Make Joomla! language handling consistent across the entire core code base~~ [RFC] Make Joomla! language handling consistent across the entire core code base Apr 6, 2019

joomla-cms-bot added the RFC Request for Comment label Apr 6, 2019

ghost removed the J3 Issue label Apr 19, 2019

ghost changed the title ~~[RFC] Make Joomla! language handling consistent across the entire core code base~~ Make Joomla! language handling consistent across the entire core code base Apr 20, 2019

nikosdion closed this Jul 2, 2019

nikosdion deleted the patch-3 branch July 2, 2019 12:00

joomla-cms-bot removed the RFC Request for Comment label Jul 2, 2019

Make Joomla! language handling consistent across the entire core code base #17372

Make Joomla! language handling consistent across the entire core code base #17372

Conversation

nikosdion commented Jul 31, 2017 • edited

Summary of Changes

Technical notes

Removal of Boolean Or (||) from language file loading

Load order of language files

When is the default language loaded and what exactly is the default language?

Documentation Changes Required

Bakual left a comment

Choose a reason for hiding this comment

infograf768 commented Aug 1, 2017

infograf768 commented Aug 1, 2017

nikosdion commented Aug 1, 2017

infograf768 commented Aug 1, 2017

nikosdion commented Aug 1, 2017

infograf768 commented Aug 1, 2017

nikosdion commented Aug 1, 2017

infograf768 commented Aug 1, 2017

nikosdion commented Aug 1, 2017

zero-24 Aug 1, 2017

Choose a reason for hiding this comment

nikosdion Aug 1, 2017

Choose a reason for hiding this comment

zero-24 Aug 1, 2017

Choose a reason for hiding this comment

Bakual commented Aug 1, 2017

infograf768 commented Aug 1, 2017

nikosdion commented Aug 1, 2017

mbabker commented Aug 1, 2017

Bakual commented Aug 1, 2017

mbabker commented Aug 1, 2017

Bakual commented Aug 1, 2017

mbabker commented Aug 1, 2017

nikosdion commented Aug 1, 2017

infograf768 commented May 23, 2018

izharaazmi commented May 23, 2018

nikosdion commented May 23, 2018

infograf768 commented May 23, 2018

infograf768 commented May 23, 2018 • edited

izharaazmi commented May 23, 2018

infograf768 commented May 23, 2018

izharaazmi commented May 23, 2018

nikosdion commented May 23, 2018

infograf768 commented May 23, 2018

izharaazmi commented May 23, 2018

infograf768 commented May 23, 2018 • edited

infograf768 commented May 23, 2018

izharaazmi commented May 23, 2018

nikosdion commented May 23, 2018

nikosdion commented Jul 31, 2017 •

edited

infograf768 commented May 23, 2018 •

edited

infograf768 commented May 23, 2018 •

edited