http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 claire.hernandez@biblibre.com <claire.hernandez@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |claire.hernandez@biblibre.c | |om -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #4 from Frédéric Demians <frederic@tamil.fr> ---

Is there not a risk to convert quotes with the fact that TT elements are translated (they are %s) ?

I don't think so. This patch transform only strings coming from JS. I've done a grep -R "&apos" and """ in fr-FR opac & staff templates directory, and I didn't find anything wrong. You can check yourself after applying this patch, and regenerating localized templates. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Cancel that. I'm stupid ; ) I wasn't thinking about the fact that this is a modification to the translation generator, and that I needed to regenerate my French translations before testing! -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #9 from Frédéric Demians <frederic@tamil.fr> ---

as I said on the mailing list, I am a bit worried the script might translate ' that shoudl not be replaced. Did you check the updated po files for differences? I think maybe we could start comparing a po file with not so many ' (not French ;) ) before and after the patch to find strings where this might be causing problems. I am thinking about program code that shows up in the po files by mistake.

Well, indeed, you're right to be worried. Replacing simple quote (') with ' seems not to work so well. It seems to be displayed has it is: the code ' not the '. I don't understand how it worked for Fridolyn. But the issue is here. And not only in calendar, also in biblio records merging. I will propose another patch with a test plan. Katrin, the patch doesn't touch the .po file. It modifies spefically how the JavaScript translated strings is sent back for inclusion in templates. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #12935|0 |1 is obsolete| | --- Comment #11 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 12939 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=12939&action=edit Proposed patch I leave this reworked patch here for reference. But I haven't more time to test it. It replaces ' with \' in JavaScript strings. No less no more. I can't see side effect. But someone can correct me. I solves another bug in biblio records merging, and, certainly, in other page where JavaScript code is broken due to ' presence in string between single quote like 'foo'. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Frederic, ah you are right, I made a mistake here. Your second approach limiting the change to Javascript output sounds more secure to me. Thank you! -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #12939|0 |1 is obsolete| | --- Comment #13 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 13151 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13151&action=edit [SIGNED-OFF] Bug 8942 Fix translation of JS strings containing simple/double quote With this patch, translated strings containing simple or double quotes are escaped. This prevent breaking JavaScript strings. For example: Today, translated in French is Aujourd'hui. And so this JS line: var m = _('Today'); become in French: var m = _('Aujourd'hui'); It breaks the whole JS code. With this patch: var m = _('Ajourd\'hui'); Same issue with ", and JS strings like "foo". Signed-off-by: Owen Leonard <oleonard@myacpl.org> Confirmed that the example above is handled correctly after applying the patch and generating a fresh set of French templates. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #14 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: Single quote seems to work (but see note below). Double quote does not work. If I translate: msgid "Your lists:" msgstr "Uw "lijsten:" It results in: param1 += "<optgroup label=\""+_("Uw \"lijsten:")+"\">";[% FOREACH addbarshelvesloo IN addbarshelvesloop %] (Example from opac-results.tt within document ready function section. ) OPAC displays only the word Uw. But does not show \"lijsten. NOTE: If I would use already a backslash to escape my quotes in the PO file like this: msgstr "Uw \'lijsten:" I would not like to see the backslash in the display. In other words: Could you improve the regex with lookbehind to prevent adding a backslash that is already there? Changing status to reflect need for attention. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #16 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- After reflexion, I think it is no good idea to have an automatic quote escape. For example, if you have a single quote in a double quote string (or opposite) : _("Aujourd'hui") will become _("Aujourd\'hui"), the backslash will be displayed. Also, like Marcel example, there might be problems with HTML added in JS. For example : $("#id").html("<span title=\"" + _("Dangerous") + "\" />") will generate <span title="Dangerous" /> In this case, the translation must not contain a double quotes, even escaped: $("#id").html("<span title=\"" + _("\"Danger\"") + "\" />") will generate <span title=""Danger"" />, bad HTML. I've looked at http://wiki.koha-community.org/wiki/Coding_Guidelines. In JavaScript section, double quotes are always used : _("xxx"). Looking at code, double quotes are used 2629 time and single quotes only 143 times. So if we always use double quotes, single quotes are not a problem anymore and it was the main problem (it is often used in French). I've looked at French PO, double quotes are actually never used in JavaScript translation ; it can be managed one by one. I've looked at old version of calendar, it used double quotes, that's why there was no problem with Today => Aujourd'hui. I will propose a patch for that. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #17 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 13756 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13756&action=edit Proposed patch (main) Main patch : replace _('xxx') by _("xxx"). Except when written into HTML : onclick="... _('') ...", see follow-up patch. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #18 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 13757 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13757&action=edit Always use double quotes : follow-up 1 Follow-up 1 : In some places, a JavaScript string concatenation exists into translated string : _('Are you sure you want to delete the ' + count + ' attached items? ') This does not work (see coding gidelines). This patch corrects by using 2 translations : _("Are you sure you want to delete the") + " " + count + " " + _("attached items?") -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #20 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Frederic, How do you feel about Fridolyn's counterpatches ? Just an additional thought: If we resolve all quotes now, how do we prevent them being added again the wrong way in the future? Unit test? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #22 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to comment #21)

Hi Marcel, If this rule is accepted, it would be possible to add a regexp in our qa tools in order to catch all simple-quoted strings added in the js files.

That would be good. But an automated test would still be safe too.. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #24 from Chris Cormack <chris@bigballofwax.co.nz> --- I agree +1 for a test. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #26 from Chris Cormack <chris@bigballofwax.co.nz> --- If someone signs off on Fridolyn's patches and if they work fine I'm happy to write a test in xt/ to test no we don't let single quotes sneak back in -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #13756|0 |1 is obsolete| | --- Comment #27 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 14336 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14336&action=edit [SIGNED-OFF] Bug 8942: Translation process breaks javascript Signed-off-by: Owen Leonard <oleonard@myacpl.org> I tested most scripts affected by this patch and visually verified all changes. Functionality is unaffected. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #30 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- Thanks a lot for testing Owen. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #31 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- (In reply to comment #26)

If someone signs off on Fridolyn's patches and if they work fine I'm happy to write a test in xt/ to test no we don't let single quotes sneak back in

Patches are signed-off. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14336|0 |1 is obsolete| | --- Comment #32 from Mason James <mtj@kohaaloha.com> --- Created attachment 14934 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14934&action=edit Bug 8942: Translation process breaks javascript Signed-off-by: Owen Leonard <oleonard@myacpl.org> I tested most scripts affected by this patch and visually verified all changes. Functionality is unaffected. Signed-off-by: Mason James <mtj@kohaaloha.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14338|0 |1 is obsolete| | --- Comment #34 from Mason James <mtj@kohaaloha.com> --- Created attachment 14936 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14936&action=edit Bug 8942: Translation process breaks javascript (followup 2) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Tested all changes. Functionality is unchanged. Thanks for the taking the extra effort to move alert strings into variables. Signed-off-by: Mason James <mtj@kohaaloha.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #36 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- This patch has been pushed to master. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Medium patch |Trivial patch -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #15021|0 |1 is obsolete| | --- Comment #38 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 15058 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=15058&action=edit [SIGNED-OFF] Bug 8942 : Adding a test to make sure we don't use single quotes for js Signed-off-by: Owen Leonard <oleonard@myacpl.org> Tested by adding single quotes to a translation-escaped string. Test correctly failed it. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |9555 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Fridolyn SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |oleonard@myacpl.org --- Comment #41 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- *** Bug 9152 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #43 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to comment #42)

(In reply to comment #40)

...

ping Chris, do you agree ?

Yep I do .. did you notice that now there is a review link next to each patch, I added that last weekend, you can now do inline code comments :) I hope that will be useful for doing qa.

Yes, I see it. That will be useful!

Do you want me to resubmit? Im happy if you just fix that line on signing off.

I will fix it but I wanted to have your permission first. The proposed patch for Bug 9555 fixes this test. Note that a test in koha-qa has been introduced and detect simple-quote strings. Marked as Passed QA. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #45 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- This patch has been pushed to master. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #47 from Chris Cormack <chris@bigballofwax.co.nz> --- The patches don't apply to 3.10.x or 3.8.x please rebase and submit if you wish them to be in those branches -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #49 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 17031 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17031&action=edit [3.10.x] Bug 8942: Translation process breaks javascript (followup 1) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #51 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 17033 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17033&action=edit [3.8.x] Bug 8942: Translation process breaks javascript -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 --- Comment #53 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 17035 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17035&action=edit [3.8.x] Bug 8942: Translation process breaks javascript (followup 2) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8942 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #55 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.8.x and 3.10.x I note now that we have a couple of instances of ' in the templates, will fix and push -- You are receiving this mail because: You are watching all bug changes.