|
Description
Blou
2017-05-18 21:04:51 UTC
Created attachment 63550 [details] [review] Bug 18634 - Missing empty line at end of opac.pref It's a shame to have a commit line just for that, but really, that's the reason this is not translatable. Or rather, it translates but don't show up. Sponsored by : CCSR I could not reproduce on current master with de-DE and it-IT However an empty line 826 seems to be superfluous. Other *.pref-files do not have an empty line at the end. So I think it is better to just remove line 826. What do you think about? For me, I enter "paypal" in the system preferences search and get 6 entries under Payment in English. I get nothing in French. Adding the empty line fixes it. Now, is it useless and senseless? I agree. I looked at circulation.pref for comfort before commiting, and saw the "semi-emtpy" line. I looked at fr-CA-pref.po for weirdness, could not find any. The # OPAC > Payments where right in the middle, all happy and comfy. If ONE wouldn't show up, I'd get it. But all six? It really points to opac.prefs, but I have no idea why (beside this dummy end line). :( Hi Blou I just did a git pull and then wanted to apply and sign off your patch. But now it seems that already without your patch the last line 826 has vanished. Maybe it was fixed by another patch? (In reply to Marc Véron from comment #6) > Hi Blou > I just did a git pull and then wanted to apply and sign off your patch. > But now it seems that already without your patch the last line 826 has > vanished. > Maybe it was fixed by another patch? The same for me, but opac.pref was touch last time about 3 month ago (In reply to Marc Véron from comment #6) > Hi Blou > I just did a git pull and then wanted to apply and sign off your patch. > But now it seems that already without your patch the last line 826 has > vanished. > Maybe it was fixed by another patch? No this is about git and whitespace settings.. (In reply to Blou from comment #5) > For me, I enter "paypal" in the system preferences search and get 6 entries > under Payment in English. I get nothing in French. > Adding the empty line fixes it. > > Now, is it useless and senseless? I agree. I looked at circulation.pref > for comfort before commiting, and saw the "semi-emtpy" line. > > I looked at fr-CA-pref.po for weirdness, could not find any. The # OPAC > > Payments where right in the middle, all happy and comfy. > > If ONE wouldn't show up, I'd get it. But all six? It really points to > opac.prefs, but I have no idea why (beside this dummy end line). > > :( Several other .pref files do not have this empty line at the end. Just looked a bit deeper in the code: misc/translator uses YAML::Syck::LoadFile on a pref file admin/preferences first uses gettemplate on a pref file, runs template->output and feeds that into YAML::Syck::Load Seems your problem is related to the second route. In order to know what happens with your french prefs, please examine the output of $data=gettemplate(...) and then $data->output in a test script, making sure that gettemplate reads your french file. Could you add your french pref file as an attachment here too? Created attachment 63655 [details]
Sample test code
Created attachment 63656 [details]
File causing the problem
Thanks Marcel, I joined the file I have on my laptop. I had moved from the problem to other issues, but I'll be happy to do your test as soon as I find 10 minutes, sooner rather than later. (In reply to Blou from comment #14) > Thanks Marcel, > I joined the file I have on my laptop. I had moved from the problem to > other issues, but I'll be happy to do your test as soon as I find 10 > minutes, sooner rather than later. Hi. Could you also attach the actual opac.pref ? Created attachment 63660 [details] [review] Translated fr-CA opac.pref (In reply to Blou from comment #16) > Created attachment 63660 [details] [review] [review] > Translated fr-CA opac.pref Blou, When I am copying your opac.pref over my english opac.pref, and I am searching for paypal, I do find all paypal preferences, regardless of adding or removing an empty line to this file? :( of course, I sent you the one "fixed" with the extra line before translation, so I guess the resulting one doesn't cause trouble. gimme a sec (In reply to Blou from comment #18) > :( of course, I sent you the one "fixed" with the extra line before > translation, so I guess the resulting one doesn't cause trouble. I always want the files with the problems :) Created attachment 63690 [details] [review] Translated fr-CA opac.pref Version that hide 'paypal' results. (I just tested again before submitting) Can we have a test plan and screenshot please? That will ease the understanding. Created attachment 64267 [details]
French search for 'paypal'
Created attachment 64268 [details]
English search for 'paypal'
This problem has been known by Eric Bégin for many years. I guess usually we do not care, or it doesn't trigger. I've never done the deep dive like Marcel did to find the cause. All I know is that it happens in many cases for them (the users). On the CCSR's branch I just added a empty line at the bottom of EVERY .pref files, and it resolved all their missing parameters problems. As far as test case 1) translate install fr-CA 2) search for 'paypal' in the sysprefs screen. Have debugged LangInstaller a *bit*: And found that this part in the PO file seems to be the source of the problems: # OPAC > Payments msgid "opac.pref Payments" msgstr "Vie privée" # OPAC > Policy msgid "opac.pref Policy" msgstr "Politique" # OPAC > Privacy msgid "opac.pref Privacy" msgstr "Vie privée" When you install fr-CA both tabs Payments and Privacy are translated to Vie privee. In the code in LangInstaller, in the for loop around line 335 (for my $section) this causes an overwrite of the Payments hash by the Privacy hash. Please create a unique translated key for each tab, and your problem should be gone. But this does not mean by definition that we should be satisfied with this overwriting hash key problem in the code. Will give it a little try still. Created attachment 64283 [details] [review] Bug 18634: Handle colliding translation for preference sections Problem on this report was caused by translating the tabs Privacy and Payments by the same string. This caused overwriting a hash entry. This patch tests if the key already exists and if so, it merges the entries instead of overwriting the old contents. Test plan: [1] Make sure that e.g. Privacy and Payments translate to e.g Vie privee. [2] Run translate install fr-CA (or the language you altered) [3] Without this patch you should loose preferences from either Privacy or Payments. With this patch, they should be merged. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested with fr-CA. Blou, Could you test the latest patch attached? If you agree, I will remove the first one. Created attachment 64414 [details] [review] Bug 18634: Handle colliding translation for preference sections Problem on this report was caused by translating the tabs Privacy and Payments by the same string. This caused overwriting a hash entry. This patch tests if the key already exists and if so, it merges the entries instead of overwriting the old contents. Test plan: [1] Make sure that e.g. Privacy and Payments translate to e.g Vie privee. [2] Run translate install fr-CA (or the language you altered) [3] Without this patch you should loose preferences from either Privacy or Payments. With this patch, they should be merged. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested with fr-CA. Signed-off-by: Blou <philippe.blouin@inlibro.com> Reset the .po files, reproduced the problem. Applied the patch and suddenly 'paypal' appeared. Comment on attachment 63550 [details] [review] Bug 18634 - Missing empty line at end of opac.pref My patch was a solution, but not a fix to the problem. Marcel's patch is the real fix (IMO, at least :) ) Thanks Blou. Updating status. Created attachment 64843 [details] [review] Bug 18634: Handle colliding translation for preference sections Problem on this report was caused by translating the tabs Privacy and Payments by the same string. This caused overwriting a hash entry. This patch tests if the key already exists and if so, it merges the entries instead of overwriting the old contents. Test plan: [1] Make sure that e.g. Privacy and Payments translate to e.g Vie privee. [2] Run translate install fr-CA (or the language you altered) [3] Without this patch you should loose preferences from either Privacy or Payments. With this patch, they should be merged. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested with fr-CA. Signed-off-by: Blou <philippe.blouin@inlibro.com> Reset the .po files, reproduced the problem. Applied the patch and suddenly 'paypal' appeared. Created attachment 64844 [details] [review] Bug 18634: Handle colliding translation for preference sections Problem on this report was caused by translating the tabs Privacy and Payments by the same string. This caused overwriting a hash entry. This patch tests if the key already exists and if so, it merges the entries instead of overwriting the old contents. Test plan: [1] Make sure that e.g. Privacy and Payments translate to e.g Vie privee. [2] Run translate install fr-CA (or the language you altered) [3] Without this patch you should loose preferences from either Privacy or Payments. With this patch, they should be merged. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested with fr-CA. Signed-off-by: Blou <philippe.blouin@inlibro.com> Reset the .po files, reproduced the problem. Applied the patch and suddenly 'paypal' appeared. Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> There is a problem in how we handle translation here. IMO keys of a hash shouldn't be translatable. But the patch definitely fixes a problem, so passing QA. Pushed to master for 17.11, thanks to everybody involved! Pushed to 17.05.x, will be in 17.05.02 This patch has been pushed to 16.11.x and will be in 16.11.10. Pushed to 16.05.x, for 16.05.15 release |