? Pending

User tests: Successful: Unsuccessful:

avatar mbabker
mbabker
3 Aug 2017

Summary of Changes

Right now within Joomla\CMS\Language\Language when loading a default language's strings, those strings are mixed in with the strings of the active language. While the behavior is technically correct, this results in two message catalogues being mixed into one with data being overwritten during the various calls. Instead of doing this, a more preferential behavior is to treat each language's strings as a separate message catalogue, and that is what this PR implements.

A new class property, $defaultStrings, is introduced which is similar to the existing $strings array except it will only hold the strings of the default language when those are loaded, resulting in the $strings array now only holding strings of the active language. Methods which do read/write operations to this array (the _ method handling converting language keys to translated strings, the load and loadLanguage methods when reading files into memory, and the hasKey method when checking for existence of a key) are updated to work with this new methodology of splitting strings into different properties.

Testing Instructions

Install a second language to your Joomla site and make that your active language. With this PR applied, the translation keys should continue to be displayed in your active language if available, fall back to English if not available (when debugging is not active, when active we will skip this check), and display the untranslated key if not present at all.

Documentation Changes Required

If accepted, anyone who has subclassed this class and is working with the $strings class property should update their code to be aware of the new $defaultStrings class property. If they don't, the worst thing that happens is they would no longer get the English language fallbacks as those won't be included in the existing property.

avatar joomla-cms-bot joomla-cms-bot - change - 3 Aug 2017
Category Libraries
avatar mbabker mbabker - open - 3 Aug 2017
avatar mbabker mbabker - change - 3 Aug 2017
Status New Pending
avatar mbabker
mbabker - comment - 3 Aug 2017

@Bakual this is what I was referencing in the other issue.

avatar mbabker mbabker - change - 3 Aug 2017
Labels Added: ?
avatar ggppdk
ggppdk - comment - 3 Aug 2017

A little more memory (compared to total memory usage) is worthwhile for this change

and since this will go to a major version change 3.7 -> 3.8 ?
if any 3rd party language-related extension need some updating, they will have time to do it

avatar mbabker
mbabker - comment - 3 Aug 2017

and since this will go to a major version change 3.7 -> 3.8 ?

  1. We aren't WordPress, that's not a major version ?
  2. Technically it is fully B/C, unless you really want to start getting into semantics about how much data is stored in one class property (which I don't believe is covered under any sane B/C definition)
avatar Bakual
Bakual - comment - 3 Aug 2017

Looks nice from what I see and understand ?

avatar infograf768 infograf768 - test_item - 4 Aug 2017 - Tested successfully
avatar infograf768
infograf768 - comment - 4 Aug 2017

I have tested this item successfully on 98539de

Behavior after this patch is the same as before here and as detailed in the test instructions. No specific issue.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17398.

avatar brianteeman
brianteeman - comment - 9 Sep 2017

@mbabker can you fix the conflicts please

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 26 Oct 2017

Reminder @mbabker for solving conflicting Files so this PR can be tested.

avatar mbabker
mbabker - comment - 12 Dec 2017

Rebased onto 3.9 branch with conflicts resolved.

avatar mbabker mbabker - change - 31 Mar 2018
Labels Added: ?
Removed: ?
avatar mbabker
mbabker - comment - 2 Jun 2018

I'm abandoning this. Working on some stuff in the Framework package (having the message catalogue extracted to a separate object with the catalogue having its own awareness of configurable fallback catalogues) that this approach conflicts with.

avatar mbabker mbabker - change - 2 Jun 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-06-02 01:26:00
Closed_By mbabker
Labels Added: ?
Removed: ?
avatar mbabker mbabker - close - 2 Jun 2018

Add a Comment

Login with GitHub to post a comment