Language file management improvements

[phproberto]

Fixes #257

The main idea is that we stop using helpers and hardcoded checks and we use a common base class. I just called it LocaliseLangFile. Ideally we should create a lib_localise library to store all this things.

This only cleans localise helper and the models involved in file checking but I'm sure we can use it in more places to get things simpler.

LocaliseLangFile basics

To check a file for errors:

// $path contains the path to the language file
$langFile = LocaliseLangFile::getInstance($path);

if (!$langFile->check())
{
    // Errors contains detailed errors
    $errors = $langFile->getLinesErrors();
}

To get the list of language strings:

$langFile = LocaliseLangFile::getInstance($path);

$strings = $langFile->getStrings();

// Errors encountered
if (false === $strings)
{
    // All the errors
    $errors = $langFile->getErrors();

    // Last error
    $error = $langFile->getLastError();
}

// Process $strings here

While testing errors I found that core has some inconsistency between file error checking && files not loaded in languages. This system is better than core checks and works the same for codemirror and on PHP checks. I tried to maximize the error detection so we can be sure that if a string isn't loaded it will be marked as wrong. Additionally I added errors that core "passes" but produce crap strings.

In fact I think this should go to core so language file handling & checking is abstracted from the language itself and can be reused by third part devs. I'd be happy to contribute it directly to core or as a more complex thing with a common interface and support for different type of language files in https://github.com/joomla-framework/language. I don't know really where it should go.

[infograf768]

@phproberto
You solved the issue in the reference file but not in the translation field:

[infograf768]

OK, I had a mistake in the file (urls should be "_QQ_" or ", but happily as I found in time that any " in the value breaks that value at the "

[infograf768]

To test, manually add a " or even 2 as "some word" in any value and edit in com_localise

[phproberto]

After 21965327895345 tests I'm still unable to reproduce the issue.

I'll work on the key.php and the translation edit view to fix the JS there because it's not working anyway. So I'll do a cleanup.

Meanwhile if someone wants to help with the ghost issue this are the steps to reproduce it (I cannot do it so I cannot fix the issues there):

• Use a clean install from latest staging
• Install an additional language (I'll use french as example)
• Install com_localise from my branch. Checkout it or download it from https://github.com/phproberto/com_localise/archive/langcheck.zip
• Edit manually the language file for the plugin languagecode in administrator/language/fr-FR/fr-FR.plg_system_languagecode.ini and modify the PLG_SYSTEM_LANGUAGECODE_XML_DESCRIPTION string to:

PLG_SYSTEM_LANGUAGECODE_XML_DESCRIPTION="Ce plug-in permet de changer le code de langue dans le document HTML généré pour cibler son référencement.<br />Note: les champs apparaissent lorsque le plugin est activé puis &quot;enregistré&quot;.<br />Plus d'information sur <a href="_QQ_"http://www.w3.org/TR/xhtml1/#docconf"_QQ_">W3.org</a>"

• Edit the language string from com_localise and see that the string is parsed correctly in the Strings tab

[phproberto]

Ready to test

[infograf768]

@phproberto
I have not tested all your changes, but concerning the display issue in the translation field, it is not solved here.
To make it short: it breaks when we have a ; in the value.

After PR:

Original ini string:
PLG_SYSTEM_LANGUAGECODE_FIELDSET_DESC="Modification du code de langue pour le document HTML généré.<br />Exemple d'utilisation: le site est en français avec le pack de langue 'fr-FR' installé, mais il s'adresse aux canadiens francophones ; pour l'indiquer aux moteurs de recherche, ajoutez le tag 'fr-CA' dans le champ correspondant en remplacement de 'fr-FR'."

As you can see, anything after francophones is not displayed.

PHP 5.3.14 and php 5.4.4

[infograf768]

Looks like it could be related to this
https://bugs.php.net/bug.php?id=51094
But why does it work in core?

[infograf768]

I see now: you use
$scannerMode = isset($options['scanner_mode']) ? $options['scanner_mode'] : INI_SCANNER_RAW;

[infograf768]

Results:
existing doublequotes would not display
Adding a doublequote: it would be deleted on saving
"_QQ_" would display as " and obviously disapear on save

[phproberto]

Travis should be ok now. Ready for the final test before merging :)

[infograf768]

@test
Great. No more apparent issue for me.

[infograf768]

Oh, you changed more, Will test later.

[phproberto]

Based on joomla/joomla-cms#5907 we cannot merge this. We first need to know what is allowed or not in language strings and then adjust com_localise to follow that standard.

So basically I lost 3 days for nothing

[mbabker]

Nobody's trying to waste your time Roberto, and I don't think you lost those days at all. I think once we answer the question on how to manage the quotes (escape them in JLanguage or expect them to be in the INI files), then this should be good to go following whatever standard is decided.

[phproberto]

Yes. That's why I haven't closed this PR. Expecting to adapt it to whatever is decided as standard.

I don't think anybody is trying to waste my time. I just feel that we started to build the house by the roof.

@infograf768 has been blamed for slowing down core for trying to support the new standards in com_localise but the fact is that standards (we don't have that :P) have been "changed" without being prepared to do so. And is thanks to com_localise that we have discovered the issue.

Someone in PLT should add an action to write that standards.

[wilsonge]

Yeah without this PR roberto we wouldn't have known about the escape issue in the first place. So actually it was super important that you did do this!!

[infograf768]

@phproberto
where do we stand now? Shall we merge this or shall we systematically correct a " to \" if no \ present (and when it is not in a "_QQ_")

[phproberto]

Exactly @infograf768. That's the only thing we have to do: escape double quotes. Not sure if it's better to do the work for the translators. I mean they have to know that double quotes need to be escaped and if we do it for them they may think unescaped double quotes are ok.

[infograf768]

To complete (as we discussed elsewhere):
we decided that a string using a non-escaped " would show as error in com_localise translations manager and that we would check for the error in the value when saving an edited file and auto replace an entered " by \" when saving.
This will be easier than validating but does not replace a future validation.

[Valc]

Hi, :)
I think that wait to the new feature or apply this new rules before done in core can make crash few stuff.

As far as i can to see this one is not fully implemented on the last 3.4.0 beta.

Apply this new rules at Language overrides in this beta have the same result than always, each " is replaced by "QQ" constant.

So, if we go there to create an override that have any "escaped doublequote" present it will return an "QQ" for each escaped doublequote and we are creating a problem.

And if we have any " under the new rules we will get the same reply: now is replaced by "QQ".

When this one is fully applied and running from core, seems that the stuff is handle the wrong double quotes usage in two ways:

Wrong usage within text.
Wrong usage within "URLs or class" that are allowed cases.

That sure is easy to apply when really works from core.

Now we are running in the old way and seems that the normal is apply pull requests adjusted to the real working mode.

Regards.

[infograf768]

@phproberto
Because of the changes we have to bring to pkg (adding method upgrade), I have to release a new version right now.
When your PR will be ready, no problem to create a new version again.

[phproberto]

ok :)

[phproberto]

I've modified the content preprocess to try to show language strings exactly as they would be entered inside an ini file which I think will teach people to do things properly.

Sample string:

PLG_SYSTEM_LANGUAGECODE_XML_DESCRIPTION="Provides the ability to change the language code in the generated HTML document to improve SEO.<br />The fields will \"appeara\" when the plugin is enabled and saved.<br />More &quot;information&quot; <span class=\"label label-important\">permet</span> at <a href="_QQ_"http://www.w3.org/TR/xhtml1/#docconf"_QQ_">W3.org</a> "

Please @infograf768 check it ant tell me if this is ok for you. Note that THIS IS ONLY DONE IN THE STRINGS view. I won't update the codemirror validations until we know exactly what we want.

Sample string:

Will be loaded as:

[infograf768]

Tested:
although we have an issue for that plugin as we have a bug in com_localise which presents the language present IN the extension instead of the core folder one when both present (editing English source though shows the core folder one => to solve), I managed to test by deleting the language folder in the plugin system languagecode folder.

For the sake of the demonstration I modified the reference en-GB ini file:
PLG_SYSTEM_LANGUAGECODE_XML_DESCRIPTION="Provides the ability to change the language code in the generated HTML document to improve SEO.<br />The fields will \"appeara\" when the plugin is enabled and saved.<br />More &quot;information&quot; <span class=\"label label-important\">permet</span> at <a href="_QQ_"http://www.w3.org/TR/xhtml1/#docconf"_QQ_">W3.org</a> "
and then pasted the same value in the translation field.

The Translation part (right column) displays OK before save, not after.
The original does not display the \ and "_QQ_"

[phproberto]

Thanks!

I'll work on the rest now that we agree on how to show strings.

[infograf768]

Hmm, thinking again:
should not we display always " and never "
it looks like saving the value gives what we have in my screenshot above.

[infograf768]

obsolete