Testing Instructions

1. In the admin en-GB.ini language file, change the language string JERROR_COULD_NOT_FIND_TEMPLATE="Could not find template "_QQ_"%s"_QQ_"." to JERROR_COULD_NOT_FIND_TEMPLATE="Could not find template "%s"." (note the removed _QQ_" from each quote)
2. Enable language debug and site debug modes
3. On any backend request, check the parsing errors in language files section of the debug output, you should get a JROOT/administrator/language/en-GB/en-GB.ini : error(s) in line(s) 158 message if you used this same string
4. Apply the patch and reload the page; that error should be gone

Test Status

Partial Success - With this patch applied, there is no longer a parsing error on this line. However, there are now parsing errors on all blank lines in the language files.

I confirm @mbabker findings.

Looks like the regex misses a |
I tested with:
$regex = '#^(|(\[[^\]]*\])|([A-Z][A-Z0-9_\-\.]*\s*=\s*".*"))\s*(;.*)?$#';
and I do not get any more errors.

As #5858 has now been merged, I guess both instances of
$errors[] = $lineNumber;
should be
$errors[] = $lineNumber + 1;

Also to please Travis, add a space between if and (

Should be better now.

`Parse error: syntax error, unexpected '{' in /Applications/MAMP/htdocs/joomla-cms-staging/libraries/joomla/language/language.php on line 884a

Oopsidaisi

This new PR show parsing errors for lines of type
[Configuration]
which we for example use in com_localise to separate parts of the ini file into sliders

Also, a missing " at the beginning or end of the value is no more triggering a parse error indicating the line.
It just prevents the file from loading

test by using:
JSTATUS=Status" or JSTATUS="Status in en-GB.ini instead of JSTATUS="Status"
and display Articles Manager.

Should be fixed now.

Fingers crossed.
Note to self: don't do PR's on 2.5 hrs of sleep!

@test
better.
Still, we do not display a parse error when doing a mistake using "_QQ_"

For example:
JSTATUS="Status "_QQ_"yes"_QQ_" => file will not load at all
or
JSTATUS="Status "_QQ"yes"_QQ_""
will display: Status _QQyes"

Note: although the test described by @mbabker above, i.e.
JERROR_COULD_NOT_FIND_TEMPLATE="Could not find template "%s"."
will let check that using a "will not trigger a parsing error, it is not a good usage example.

When someone wants to display double quotes in text, one should use " (or"_QQ_").
It is only when needing doublequotes for a url or class that " (or "_QQ_") should be used.

@test Thanks for this @nonumber
I tested this with the following extra strings

`TEST1=test
TEST2="test"test"
TEST3="this is a red</span"
TEST4=

TEST6="This is a joomla link"`

The debug plugin correctly identified that the 1st and 4th strings were errors and that the rest were correct.

Awesome stuff.

When this is accepted I will update #5615 to complete the removal of all QQ in the language files in joomla - woohoo

Oops got my markdown wrong. These are the strings I tested

TEST2="test"test"
TEST3="this is a <span="red">red</span"
TEST4=

TEST6="This is a <a href="http://joomla.org">joomla link</a>" <hr /><sub>This comment was created with the <a href="https://github.com/joomla/jissues">J!Tracker Application</a> at <a href="http://issues.joomla.org/tracker/joomla-cms/5862">issues.joomla.org/joomla-cms/5862</a>.</sub>

Third attempt

TEST2="test"test"
TEST3="this is a <span="red">red</span"
TEST4=

TEST6="This is a <a href="http://joomla.org">joomla link</a>"<hr /><sub>This comment was created with the <a href="https://github.com/joomla/jissues">J!Tracker Application</a> at <a href="http://issues.joomla.org/tracker/joomla-cms/5862">issues.joomla.org/joomla-cms/5862</a>.</sub>

Do we need double quotes in a url - I just tested with this string
PLG_NONE_XML_DESCRIPTION="This loads a basic text entry field with a link
joomla"

And the link loads correctly and wth this PR the string is not marked as an
error

On 22 January 2015 at 10:25, infograf768 notifications@github.com wrote:

@test https://github.com/test
better.
Still, we do not display a parse error when doing a mistake using "QQ"

For example:
JSTATUS="Status "QQ"yes"QQ" => file will not load at all
or
JSTATUS="Status "QQ"yes"_QQ""
will display: Status _QQyes"

Note: although the test described by @mbabker https://github.com/mbabker
above, i.e.
JERROR_COULD_NOT_FIND_TEMPLATE="Could not find template "%s"."
will let check that using a "will not trigger a parsing error, it is not
a good usage example.

When someone wants to display double quotes in text, one should use
" (or "QQ").
It is only when needing doublequotes in a url or class that " (or "QQ")
should be used.

—
Reply to this email directly or view it on GitHub
#5862 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

Do we need double quotes in a url - I just tested with this string PLG_NONE_XML_DESCRIPTION="This loads a basic text entry field with a link joomla" And the link loads correctly and wth this PR the string is not marked as an error

Read again my comment above:

When someone wants to display double quotes in text, one should use " (or"_QQ_").
It is only when needing doublequotes in a url or class that " (or "QQ") should be used.

When this is accepted I will update #5615 to complete the removal of all QQ in the language files in joomla

We have 1744 instances of "_QQ_" in core...
This will need a lot of work and is useless. One will have to differentiate between the 2 usages as stated above, i.e. it can't be done by a general find/replace. Doing it in #5615 will just make it harder to test.

CONSTANT= "My dear "Joomla" is now at version 3.4.0"

Test that.

I am happy to do that work yo remove the useless string. Most have already
been removed in the engb patch
On 22 Jan 2015 10:37, "infograf768" notifications@github.com wrote:

Do we need double quotes in a url - I just tested with this string
PLG_NONE_XML_DESCRIPTION="This loads a basic text entry field with a link
joomla http://joomla.org" And the link loads correctly and wth this PR
the string is not marked as an error

Read again my comment above:

When someone wants to display double quotes in text, one should use "
(or"QQ").
It is only when needing doublequotes in a url or class that " (or "QQ")
should be used.

When this is accepted I will update #5615
#5615 to complete the removal
of all QQ in the language files in joomla

We have 1744 instances of "QQ" in core...
This will need a lot of work and is useless. One will have to
differentiate between the 2 usages as stated above, i.e. it can't be done
by a general find/replace. Doing it in #5615
#5615 will just make it harder
to test.

—
Reply to this email directly or view it on GitHub
#5862 (comment).

String checking with invalid use of "QQ" should now be better.
3rd party extensions will still have these, even if they all get removed from core. So the check still needs to deal with these.

@infograf768 The whole idea is that we don't use the QQ for double quotes.
You can simply use a normal ", or if necessary the html entity " (which needs to be decoded in output).

@nonumber
I understand that it should be the aim. Now, as "_QQ_"works in all cases, it is still usable and I think we should stiil be able to use it.
For example, in com_localise, any doublequote entered in the value as ", whether it is in text or used in a class or url, will automatically be changed to "_QQ_" when the ini is saved, making it real easy for Translators.
Therefore we do need to trigger errors when the "_QQ_" has a typo.

Thanks:
JSTATUS="Status "_QQ_"yes"_QQ_"
now triggers a parse error with the correct line
but
JSTATUS="Status "_QQ_yes"_QQ_""
prevents ini from loading but does not display a parse error.

Not a problem. The style guide instructs you to use the html entity and not
the character if you want to display a quote and this has already been
done in the big enGB patch
On 22 Jan 2015 10:39, "infograf768" notifications@github.com wrote:

CONSTANT= "My dear "Joomla" is now at version 3.4.0"

Test that.

—
Reply to this email directly or view it on GitHub
#5862 (comment).

So is it all good now?

@infograf768 Regarding this bit:

Note: although the test described by @mbabker above, i.e.
JERROR_COULD_NOT_FIND_TEMPLATE="Could not find template "%s"."
will let check that using a "will not trigger a parsing error, it is not a good usage example.

When someone wants to display double quotes in text, one should use " (or"QQ").
It is only when needing doublequotes for a url or class that " (or "QQ") should be used.

In a Framework project I'm working on, I have a language string DASHBOARD.NEW.NOTIFICATION.COUNT_MORE="There are <span class="badge">%s</span> new notifications." which displays just fine without any sort of encoding quotes.

Not needing to encode quotes either through " or _QQ_ is consistent with Symfony's Translation component which also supports INI based language files and a system I've used a bit as of late. Given the fact that the strings render correct, whether in debug mode or not, the language parser itself is handling the strings just fine but the regex (which I'm assuming is intended to check a key/string combination for bad characters) was in dire need of updating, accomplished by this PR.

As for testing the PR itself, I tested again and all looks well to me.

DASHBOARD.NEW.NOTIFICATION.COUNT_MORE="There are <span class="badge">%s</span> new notifications."

is exactly behaving as a class or url, as I described. In these cases " is perfectly OK and even more, it is necessary to never use htmlentities ("_QQ_" would also work).
It is when we use direct text as
MY_CONSTANT= "Some text with "doublequotes" around a word"
that it does not display the double quotes.
In that case, we do need to use the htmlentity:
MY_CONSTANT= "Some text with "doublequotes" around a word"

@nonumber
I would appreciate if you could also solve:

JSTATUS="Status "_QQ_yes"_QQ_""
prevents ini from loading but does not display a parse error.

JSTATUS="Status "_QQ_yes"_QQ_""
That technically is not an invalid string.
Consideing "_QQ_" is replaced by ", then the result would be:
Status "_QQ_yes"

Please test by yourself. It is simple. It gives:

btw
JSTATUS="Status "yes"_QQ_""
same problem

I think you have something else messed up @infograf768

I tried JERROR_COULD_NOT_FIND_TEMPLATE="Could not find template "_QQ_%s"_QQ_"." then changed the check in admin so that the exception which shows that message would always render. My output was this:

I also tested JERROR_COULD_NOT_FIND_TEMPLATE="Could not find template "%s"." real quick and the text was **Could not find template isis.** (no quotes)

Last test with JERROR_COULD_NOT_FIND_TEMPLATE="Could not find template \"%s\"." results in **Could not find template "isis".**

All occasions had no parsing errors and correct output site wide.

chnage in en-GB.ini the string to

JERROR_ALERTNOTEMPLATE="The template for this display "_QQis not available."

Then take off the ! line 217 of ROOT/libraries/cms/application/administrator.php
You will get:

Just try
JSEARCH_TOOLS="Search tools "_QQ_yes"_QQ_""

it is almost the last string in en-GB.ini
It breaks loading the whole file
Not loaded : JROOT/administrator/language/en-GB/en-GB.ini

So I can replicate the issue JM's seeing using JSTATUS="Status "_QQ_yes"_QQ_"" but the same problem isn't experienced with JERROR_COULD_NOT_FIND_TEMPLATE="Could not find template "_QQ_%s"_QQ_"." (note both miss a closing quote in the first _QQ_ use). The difference between how the two strings are rendered is that JSTATUS always goes through JText::_() while JERROR_COULD_NOT_FIND_TEMPLATE always goes through JText::sprintf(), which both proxy into JLanguage::_().

What's confusing the heck out of me is that both strings have the same style parse error, one works OK and the other causes the file to incorrectly load.

Maybe this will help someone:

Note:
JSTATUS="Status "_QQ_"yes"_QQ_""
works perfectly.

So is this something that should be fix here. In other words, the debug should show this as an invalid string? => I don't think so, as it is ok in other areas.

Or should this be fixed in the JText that is parsing the strings, which get's my vote.
If so, then that should be in a different PR. This issue was already there before this PR.

The issue did not exist before. It was shown as a parse error in debug lang.
It is unrelated to JText:: as it is at parsing time with this PR that we get the loading error and no error displayed in the debug.

The error with the JSTATUS string is when the file contents get pushed through parse_ini_string(). Before, the contents were getting parsed and a string array returned; now the method is returning a boolean false.

Without this PR, when using JSTATUS="Status "_QQ_yes"_QQ"", I get in debug
JROOT/administrator/language/en-GB/en-GB.ini : error(s) in line(s) 107

Yes, so again. The issue here is whether or not that string should be flagged as an error string.
See comments above. So question is: should it be flagged?

From my point of view:

• This ONLY concerns an extra bit of info. It doesn't concern functioning of Joomla (it's not breaking things).
• This ONLY concerns weird edge cases which will probably never ever happen
• This ONLY concerns the invalid use of the _QQ_ thing which we are trying to deprecate

So is it worth all the attention?

Since we're still performing a str_replace('_QQ_', '"\""', $contents) on the file contents before parsing it, IMO, the uses of _QQ_ need to work until they're fully deprecated and removed.

Ok, but does that mean the string should be flagged as having an error?

The strings with wrong values should be flagged indeed.

I think for a case like WHATEVER="Whatever "_QQ_my quoted text"_QQ_" says" then yes because it's essentially a broken quote using our current rules.

I can't apply this PR anymore here btw. But I kind of remember I also had issues with single " in the values.

For example:
JFEATURED_ASC="Featured "ascending"

will prevent loading en-GB.ini and no parse error....

JFEATURED_ASC="Featured "_QQ_"ascending"

is OK

Done.

JFEATURED_ASC="Featured "ascending"
will prevent loading en-GB.ini and no parse error....

That is then an issue in the handling of the ini files itself. This PR only deals with the debug side of things.

I think it's better. The file doesn't load but that is because of a legitimate parsing error with the PHP function and the error is flagged in the debug output.

There's an extra closing parenthesis on line 892 right now @nonumber

Fixed

JFEATURED_ASC="Featured "ascending"
will prevent loading en-GB.ini and no parse error....

That is then an issue in the handling of the ini files itself. This PR only deals with the debug side of things.

This should show in the debug as an error and with its line number. This is what the debug is for. parse_ini_string apparently does not deal with single doublequotes.

I propose you add, line 898

                // Check for single double quotes
                if (substr_count($line, '"') % 2 != 0)
                {
                    $errors[] = $lineNumber + 1;
                    continue;
                }

Which solves the issue here.

Also please update your branch to staging.

Done

@nonumber
The PR has conflicts.
also, I suggest you change a bit the comment ( I was not precise enough above):
from
// Check for single usage double quotes.
to
// Check for odd number of double quotes.
or better English if judged necessary, as it is not only dealing with a single doublequote

NOTE to all: I also would appreciate some help to correct com_localise error flagging before we start using the doublequotes in core files.

@nonumber
Travis fails
ILE: ...e/travis/build/joomla/joomla-cms/libraries/joomla/language/language.php

FOUND 1 ERROR(S) AFFECTING 1 LINE(S)

897 | ERROR | Whitespace found at end of line

As this will anyway have to be squashed, I can correct it when merging, if you have no time

Great work, thank you @nonumber!