Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RfC] Deprecate _QQ_ usage in language files #13421

Closed
3 of 5 tasks
andrepereiradasilva opened this issue Dec 30, 2016 · 16 comments
Closed
3 of 5 tasks

[RfC] Deprecate _QQ_ usage in language files #13421

andrepereiradasilva opened this issue Dec 30, 2016 · 16 comments

Comments

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Dec 30, 2016

Description

The usage of _QQ_in translation files is a joomla custom method that IMHO should be deprecated.

Also:

  • PHP is becoming more strict everyday and having those custom "_QQ_" is not the best thing because we are closing the ini var delimiter at the middle of a string.
  • Also i don't see why we need to have extra processing in replacing this _QQ_ stuff when we need is, for what i tested, to escape double quotes (\").

I could be missing something, but for all my tests, i don't see anything more.

So this RFC is to propose to have a plan to remove those _QQ_ without any B/C break and giving time to adapt.

Basicly what is proposed is at the end of the process

; Replace this:
LANG_VAR="Some var with double quotes in <a href="_QQ_"https://www.joomla.org"_QQ_">link<a>"

; With this:
LANG_VAR="Some var with double quotes in <a href=\"https://www.joomla.org\">link<a>"

Proposed

I can do the PR needed (as time allows) in this sequence:

Translator impact

As @infograf768 expalined i am now aware of the impact of this translation.
In core, currently, for what i count there are 59 language variables with _QQ_ usage in 26 en-GB language files so when _QQ_ is replace by escaped double quotes \" it means this language string will appear as not translated or as changed (in crowin).
Third party extensions use those too.

So this cannot be replaced in one go without B/C break.

Notes

This is a RFC because, before i start making those PR, i want to have feedback/approval from mantainers on this. Otherwise i will not do them.

@mbabker
Copy link
Contributor

mbabker commented Dec 30, 2016

👍

PHP has advanced enough that this workaround is no longer needed. Use native features as able, which means just escape your quotes. Even if the usage of the constant in core isn't phased out during 3.x since that would presumably create an unnecessary workload on translators, it should be gone in 4.0.

@zero-24
Copy link
Contributor

zero-24 commented Dec 30, 2016

So this cannot be replaced in one go without B/C break.

Why do you think the change f the coreis not B/C? JText (as far as i remember) can handle both the Joomla QQ and normal escaped strings.

But as both should work after the changes too (until 4.0) i see no problem in changing it. The TT's need to do it anyway (atleast before 4.0) but they are not required to do it as it still works. So they get more time from us to change the strings and there is no hurry to do this for the next release as 3.7 still supports the current way? Or do i miss or missunderstand something?

@andrepereiradasilva
Copy link
Contributor Author

@zero-24 if we removed _QQ_ processing now it would be a B/C break in 3.x branch. that's why i propose to remove it completly in 4.0.

@andrepereiradasilva
Copy link
Contributor Author

andrepereiradasilva commented Dec 30, 2016

BTW checked all https://github.com/joomla-extensions ini files and didn't find a single language variable using _QQ_ (didn't checked messages and banners because i think they are dead repos)

@zero-24
Copy link
Contributor

zero-24 commented Dec 30, 2016

Correct messages and banners was prepared to be removed from core until the plt stopped that project. ;)

Ok than i got it correct. You are correct we can't stop processing the Joomla QQ

But i think we should remove the usage form the core language files too. And mark them not as requirement to change in the language translation files but as recommended :)

@mbabker
Copy link
Contributor

mbabker commented Dec 30, 2016

But i think we should remove the usage form the core language files too. And mark them not as requirement to change in the language translation files but as recommended :)

The only problem with that is it will force every string to be updated if you're using Crowdin or com_localise. So for 3.x it probably won't happen just because of the workload burden (nothing stops translation teams from optionally replacing it though), but as I hinted at above we just need to do it for 4.0 and not live another 5 years with that thing present.

@infograf768
Copy link
Member

@zero-24
Please understand that modifying so many strings will force TTs to check/modify or mark as OK (wth the QQ), all strings in core that would be modified. THis, when they use crowdin or com_localise that would mark these strings modified

@infograf768
Copy link
Member

And again, there are not 59 occurences of QQ but 190 (if we do count only the en-GB installation ini file among all installation ini). The number of QQ per string is usually multiple.
screen shot 2016-12-30 at 17 11 24

Note: while testing as there were indeed some changes in the treatment of ini strings displayed through js and for the href, I found out that our Alerts do NOT any more accept either the "_QQ_" or the \".
for example with
JLIB_HTML_PLEASE_MAKE_A_SELECTION_FROM_THE_LIST

@andrepereiradasilva
Copy link
Contributor Author

@infograf768 it's 59 language variables (sorry for the confusion) which probably correspont to your 190 _QQ_ strings.

if you are referring to https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/toolbar/button/confirm.php#L90

that should not be like that, we could use:

JText::script('JLIB_HTML_PLEASE_MAKE_A_SELECTION_FROM_THE_LIST');
// and them in js
alert(Joomla.JText._('JLIB_HTML_PLEASE_MAKE_A_SELECTION_FROM_THE_LIST'));

anyway it should work like it was, so it seems to me like a regression, but not related to this

will check this

@infograf768
Copy link
Member

anyway it should work like it was, so it seems to me like a regression, but not related to this

Unrelated indeed. That's why I made a Note.

@zero-24
Copy link
Contributor

zero-24 commented Dec 30, 2016

Is this not just a Search and replace?

The problem i see is that two weeks before 4.0 comes out we have translation teams that translate the new strings i expect that there are much strings to change (as the content is changed) than there are 59/190 strings more in that list where we know now that they needs to be changed. And this strings can be changed now without any problems so why do we not tell them (maybe in a mail too etc) and change the core to show the correct / recommended usage. That would reduce the workload before 4.0 stable. I know that they need to change / review it but what is the difference between now (small steps) or before 4.0 (one big step) ?

@mbabker
Copy link
Contributor

mbabker commented Dec 30, 2016

It is just a search and replace. But remember that every change in the core strings causes all of the translation tools (it doesn't matter what you're working with, every automated platform is going to do this) to mark a string as untranslated and force a translator to review it. So while it is indeed a simple change, it still causes a lot of extra work just to make sure that nothing changed except for "_QQ_" becoming \".

@andrepereiradasilva
Copy link
Contributor Author

so it seems that we have somewhat consensus to do the removal of _QQ_ only in for 4.0 and deprecate it in 3.x.

@PhilETaylor

This comment was marked as abuse.

@joomla-cms-bot joomla-cms-bot changed the title [RFC] Deprecate _QQ_ usage in language files [RfC] Deprecate _QQ_ usage in language files Mar 24, 2017
@nibra
Copy link
Member

nibra commented Mar 24, 2017

Is this on the roadmap already? If so, this can be closed.


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

@mbabker
Copy link
Contributor

mbabker commented Mar 24, 2017

#13451 is the final PR in the series for this item. Someone might need to take ownership since Andre has had to pull out due to other commitments.

Closing this item since there is the open PR though and the other items here have been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants