Thanks @brianteeman, great work!

Thanks to coaching from @roland-d this should be mergable again

Now updated to remove QQ

Updated again after changes to captcha to ensure it is mergeable

I repeatedly said to NOT change the QQ in url and classes for now until com_localise can deal with it.
Happily we MAY have a patch today which I am now testing.
If it does not work, you may have to revert this.

Anyhow, this PR would only merged, as agreed upon, AFTER beta release.

You can say whatever you want as much as you want about com_localise. That
is not part of the Joomla distribution and Joomla should NOT and must NOT
be held up and use antiquated and completely unnecessary code because of
some other extension.

As for the merge - yes we all know it will not be merged until after Beta.

I know you dont want to see the core en-GB files be updated - you have made
that perfectly clear. The PLT have decided that they will be so please stop
trying to put down the work of volunteers who you dont agree with. Enough
is enough.

On 27 January 2015 at 07:47, infograf768 notifications@github.com wrote:

I repeatedly said to NOT change the QQ in url and classes for now until
com_localise can deal with it.
Happily we MAY have a patch today which I am now testing.
If it does not work, you may have to revert this.

Anyhow, this PR would only merged, as agreed upon, AFTER beta release.

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

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

Let's keep calm please.

I sent a pull request to com_localise to fix the issue with strings detected as wrong: joomla-projects/com_localise#260

If translators are supposed to use com_localise I understand @infograf768's pain.

But we have a solution and the time to apply it so avoid discussing please.

Exactly. This wouldnt be an issue for com_localise until AFTER the 3.4 release -plenty of time for everyone to help test com_localise

If translators are supposed to use com_localise

Sorry for barking in here but it would be really interesting if com_localise is used mainly by translators or its primary use is "only" the creation of language packages - a task that could be accomplished by a tool oh soo much simpler....

I am aware that com_localise is also a piece of community work and it should be honored as such.

Great work here @brianteeman

Com_localise has nothing to do with this PR.
On 27 Jan 2015 15:13, "Nikolai Plath" notifications@github.com wrote:

If translators are supposed to use com_localise

Sorry for barking in here but it would be really interesting if
com_localise is used mainly by translators or its primary use is "only" the
creation of language packages - a task that could be accomplished by a tool
oh soo much simpler....

I am aware that com_localise is also a piece of community work and it
should be honored as such.

Great work here @brianteeman https://github.com/brianteeman

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

Exactly. This wouldnt be an issue for com_localise until AFTER the 3.4 release -plenty of time for everyone to help test com_localise

Wrong. The issue is that we have changes in JLanguage and we do need com_localise to translate/update/make language packs.

Beta will not contain this PR here, so, indeed, if we can correct com_localise after beta (and before RC) and before this PR -as is- is merged, we will be OK.
Otherwise, as I said above the changes from "_QQ_" to " for the urls and classes will have to be reverted.

We can easily postpone that change which is unnecessary for core to work fine.

The issue is with com_localise not core. No further comment it is obvious
where the "problem" is
On 27 Jan 2015 16:12, "infograf768" notifications@github.com wrote:

Exactly. This wouldnt be an issue for com_localise until AFTER the 3.4
release -plenty of time for everyone to help test com_localise

Wrong. The issue is that we have changes in JLanguage and we do need
com_localise to translate/update/make language packs.

Beta will not contain this PR here, so, indeed, if we can correct
com_localise after beta (and before RC) and before this PR -as is- is
merged, we will be OK.
Otherwise, as I said above the changes from "QQ" to " for the urls and
classes will have to be reverted.

We can easily postpone that change which is unnecessary for core to work
fine.

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

Stupidity is more common than many think. Period.

IMO it's an issue that com_localise had the same problems as JLanguage's debug mode yet JLanguage itself (and inherently JText) has worked OK without this special handling for some time. And IMO it's an issue that com_localise is slowing down changes to the core platform, whether they're considered unnecessary or otherwise.

Just my blunt two cents...

I agree with Brian and Michael. I'm absolutely in favor of merging this PR as is after beta is released. I'm quite sure com_localise can be fixed to cope with this correct change.

And please stop insulting eachother. That doesn't help anyone.

Folks, our tests (Roberto and I) show that using doublequotes in core strings value (not speaking of com_localise) is NO use at all.
If it is used for urls: the browser adds them on the fly, thus giving the impression it works
if it used for classes, they are just ignored. One has to replace them by singlequotes or \".

The only solution which apparently always works is escaping them as \"

I can NOT confirm that at all

I tested every link that I changed and they all worked and all opened
either in the current window or in _blank as defined in the string

I tested every class that is used and they all worked ((although most of
the classes have zero effect anywhere except in the Hathor template as
span=red is only defined in hathor and not in isis for example)

Of course I might have missed one or two but I really tried to test every
single instance. That is why you will see in the PR that they were
submitted one extension at a time and were not done as a global
search/replace.

Instead of a blanket statement that they do not work PLEASE provide a
specific example of a link and a class that do not work when changed to "
instead of QQ

On 28 January 2015 at 11:12, infograf768 notifications@github.com wrote:

Folks, our tests (Roberto and I) show that using doublequotes in core
strings value (not speaking of com_localise) is NO use at all.
If it is used for urls: the browser adds them on the fly, thus giving the
impression it works
if it used for classes, they are just ignored. One has to replace them by
singlequotes.

The only solution which apparently always works is escaping them as \"

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

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

Simple test:
/administrator/language/en-GB/en-GB.plg_system_languagecode.ini
add a <span> by changing the string to:

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

you will get:

then change the singlequotes to doublequotes in the span:

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

you will get:

btn-primary is ignored.
Now escape the " in the span, and you will get the good classes and display again.
PLG_SYSTEM_LANGUAGECODE_XML_DESCRIPTION="Provides the ability to <span class=\"btn btn-primary\">change</span> the language code in the generated HTML document to improve SEO.<br />The fields will appear when the plugin is enabled and saved.<br />More information at <a href="_QQ_"http://www.w3.org/TR/xhtml1/#docconf"_QQ_">W3.org</a> "

Concerning urls: just look at the source of the page when you use only non escaped double quotes: they do not show at all... The browser is the one that makes them work.

Oh, I have to add:
php 5.3.14 or php 5.4.4

@infograf768 is right. You cannot replicate it because browsers are adding the missing double quotes but you get unexpected behaviors like only applying first class (the example @infograf768 posted). In fact I found it while trying to replicate core behavior in com_localise.

But I have a patch ready as soon as we confirm it works on com_localise so let's focus on solve the issue

Here is the video of my tests

https://www.dropbox.com/s/36vw1hw3pqokmh3/qq.mp4?dl=0

So based on what @phproberto has said the issue is NOTHING to do with this patch. The issue is in the core of joomla for which you have a patch so is unrelated to this patch and does not prevent this PR from being applied. Sorry J-M nice try but we will continue with improving the core en-GB language of Joomla

Did I say it was related to this patch? No.
I said we found out this issue and if this is NOT corrected in core, we would have to deal with it by adding escaping.
Cool down.

After fixing core this can be merged. Before it can't because we would be introducing language strings that cause unexpected behaviors. But I have the patch ready so let's focus on improving joomla!

About your example you are only using a single class. Try to use this in a language string to reproduce the issue:

<a href="http://joomla.org" class="btn btn-primary">Blue button</a>

Without patching the button will be grey (only btn class applied). After the patch it should work properly.

I can confirm your finding of the double class.

Again it is not related to this PR so I dont see why it was even mentioned
here

On 28 January 2015 at 12:16, Roberto Segura notifications@github.com
wrote:

After fixing core this can be merged. Before it can't because we would be
introducing language strings that cause unexpected behaviors. But I have
the patch ready so let's focus on improving joomla!

About your example you are only using a single class. Try to use this in a
language string to reproduce the issue:

Blue button

Without patching the button will be grey (only btn class applied). After
the patch it should work properly.

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

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

It's not even a core Joomla issue. It's PHP's parse_ini_* functions in general. I tested the same behavior against a Symfony based project using INI language files and had the same issues; unescaped quotes don't render in the output but browsers are handling it fine. The solution that will work every time is to escape quotes (\").

Well it's a Joomla issue if Joomla is using the strings that are not parsed properly.

JM successfully tested the fix and now I'm preparing a PR to fix core issue.

Then Symfony's translator (which parses files the same way we do) has the same bug

The PHP parse_ini_* functions consider those valid strings which is why everything still looks like it works (aside from a few cases like multiple classes in an HTML element). Our debug code in JLanguage can be updated to check for this condition, but without seeing a patch, I wouldn't suggest to do something much more complex.

PR sent. Let's keep this PR on topic and discuss there about the bug:

#5907

@brianteeman whilst we are still unsure about how the escaping is going to practically work combined with the fact I want this merged before beta 2 ideally, could you please revert the "_QQ_" changes?

It's not related to the com_localise but to the fact it appears that we were misguided about the parsing working perfectly without. Hopefully we can then fit in another PR before 3.4 stable that can deal with removing "_QQ_" but this will then not be dependent on all these changes.

Everyone of the uses of QQ in this PR works. The only uses of QQ that have
been shown not to work are not in this PR.

Actually, they work because of browser rendering. This string (https://github.com/joomla/joomla-cms/pull/5615/files#diff-f1920e8ff815e60e6de2d140d2b18e42R24) renders ending date <span class=when>%s</span> <span class=date>%s</span> as the HTML output in its current state.

Are you talking about reverting ALL instances of QQ or just the ones where
we can't use & quot

@mbabker does it matter how they work. The fact is they do.

It results in a technically invalid INI file and produces invalid HTML markup. Like Roberto showed, it becomes an issue if there's a space in an attribute like class (which I'm pretty sure almost no core language string has this). But yes, it does render as expected when viewed in a browser.

You are correct that NO language string in core has this issue.

From reading the other issue it sounds like you are recommending to use \" which makes more sense to me as that is a much more common standard for escaping a character than using the unreadable "_QQ_"

There are two different times that we used QQ

1. Whenever we displayed a quotation
2. When we needed a quotation mark in a href or class

Can you please clarify which needs changing.

I dont see reverting to QQ as a sensible option myself. If it has to be escaped only when using a href or class then that is easy to do to update in this PR

For expected results 100% of the time, any double quote within the string should always be escaped regardless of what type of use case.

The style guide currently stipulates that to DISPLAY a double quote you
should use the html characters
https://github.com/joomla/user-interface-text/blob/master/punctuation.md#quotes-and-speech-marks

That was added to the PR very early on

/me confused on what I am being asked to do considering all discussions
taking place on all issues

On 28 January 2015 at 17:41, Michael Babker notifications@github.com
wrote:

For expected results 100% of the time, any double quote within the string
should always be escaped regardless of what type of use case.

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

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

All the class and href QQ should be back - for some reason revert wouldnt work on all so i have to do one or two manually

That's fine. Thanks Brian! I'm all in favour of removing QQ_ too but I just want to do it at a time when we are all confident and have a clearly written procedure for how to write the alternative. I'll do a final review and hopefully merge tonight

Remember I did remove the QQ when used to display a quotation mark https://github.com/joomla/user-interface-text/blob/master/punctuation.md#quotes-and-speech-marks

I have not reverted that

@phproberto the " does work currently right? It's just raw " that need to be removed correct?

@mbabker

For expected results 100% of the time, any double quote within the string should always be escaped regardless of what type of use case.

Could this sentence be included to the (new) style guide?

@wilsonge Yes. HTML entities are handled correctly in HTML output.

@elkuku I'd say yes on that.

I am happy to add that they should be escaped in the style guide once you
decide how they are to be escaped. QQ make is completely unreadable and
\" is far more normal escape style and is readable

On 28 January 2015 at 19:04, Michael Babker notifications@github.com
wrote:

@wilsonge https://github.com/wilsonge Yes. HTML entities are handled
correctly in HTML output.

@elkuku https://github.com/elkuku I'd say yes on that.

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

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

+1 for \" instead of _QQ_

Yes. Anything not a direct " is correct

Just reviewed it and it looks ok (I haven't checked all the files). Replacing "_QQ_" with \" is ok too if you want to definitely get rid of QQ.

So basically:

• To add a double quote in text you can use: "_QQ_" or \" or "
• To add a double quote inside HTML markup like classes, URLs, etc. you can use: "_QQ_" or \". " does not work here.

So I think if we want to get rid of "_QQ_" I'd recommend as a general rule to use always \" because it works everywhere.

As a general rule it is much easier than that

To display a Quotation Mark use the html character (this is already in
the docs)
To use a quotation mark use the escaped character (hopefully this is
\'' with QQ kept for BC)

On 28 January 2015 at 20:25, Roberto Segura notifications@github.com
wrote:

Just reviewed it and it looks ok (I haven't checked all the files).
Replacing "QQ" with \" is ok to if you want to definitely get rid of
QQ.

So basically:

• To add a double quote in text you can use: "QQ" or \" or(&)quot;` (github does not allow me to put it correctly)
• To add a double quote inside HTML markup like classes, URLs, etc. you can use: "QQ" or \". quot; does not work here.

So I think if we want to get rid of "QQ" I'd recommend as a general
rule to use always \" because it works everywhere.

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

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

I think Brian's one makes sense to me

@wilsonge

I'll do a final review and hopefully merge tonight

This is not so urgent, please wait we decide how to handle it globally ( #5907 ), so that we also know how to do in com_localise and what to say to TTs.

This PR does not effect TT
The string has been reverted to QQ
What else can you come up with to stop this being merged

Stop the paranoia, Brian. I have nothing against making en-GB better. I am concerned by formats and code.
I am just saying that IF we decide to use escaped doublequotes \" instead of "_QQ_", we could already do it here.

Just stop it JM you are playing games with your threats/promises to leave
Joomla (again) if this was committed without QQ.

We have done what you asked for
Now lets commit the dam thing so people are able to test that there aren't
any silly mistakes.
When/if a decision is made on escaping characters a new PR can be done for
that in seconds.

(Turning off notifications)

When/if a decision is made on escaping characters a new PR can be done for that in seconds.

Correct.
@wilsonge
Ok to merge for me. RTC.