Description
Frédérick Capovilla
2011-11-02 15:16:40 UTC
Comment hidden (obsolete)
Patch sponsored by the CCSR ( http://www.ccsr.qc.ca ). Created attachment 6149 [details] [review] Signed-off patch Created attachment 6150 [details] [review] Follow-up patch : Corrects other xsl files so doesn't cause problems. Added a follow-up patch : the same problem could potentially affect other xsl files according to the test suite. I added the same line in xsl files containing ":" in their translatable strings, because that's where might be added. Created attachment 6160 [details] [review] Signed-off follow-up patch QA Comment: Test plan could have been helpful. Now I added somewhere to let Koha break and your patch resolved it. These patches only add the entity definition of nbsp to xsl files. I only came across this warning: [A word of caution: some XML-related tools will try to validate the XSLT stylesheet if they see the DOCTYPE. Since the DTD subset does not include definitions for all the XSLT elements, validation errors will be reported.] Another point is that this patch adds this definition to a selected set of stylesheets. You could ask that if you want to add this definition, why not do it in all stylesheets? The current selection is based on an assumption that might perhaps not cover all cases. Although I would not oppose pushing this patch (with these notes), I am asking Ian as QAM to update the status, if he agrees too. Created attachment 6456 [details] [review] Second Follow-up : Add nbsb support to all xsl files I didn't do the change on all files because I thought it would be "overkill" to add this line to every file even if the file doesn't have translatable strings of potentially problematic strings. Here is a third patch that adds the line on the xsl files I skipped. If this patch is to be pushed, it will need to be tested thoroughly. Created attachment 6466 [details] [review] Group the 3 patches Fix a problem when translations contain various XSL files With new 3.6.1 translation files, the lake of 'nbsp' entity definition make fail t/00-valid-xml.t test. This patch applies to HEAD and 3.6.x. and must be applied to 3.6.x to hush up Jenkins. How to test: - checkout 3.6.x branch - generate French templates: cd misc/translator ./translate install fr-FR cd ../.. - run test: prove -v t/00-valid-xml.t you get failing error message - apply this patch - run test again: it passes This solution has already been implemented in UNIMARC XSLTs, and MARC21 OPAC XSLT, so implementing it for the rest of the MARC21 style sheets would be consistent. Marking as Passed QA. patch pushed, please test (hugin will be happy ;-) ) Created attachment 6641 [details] [review] Bug 7184 : Changing test to match new formatting of xsl style sheet Created attachment 6642 [details] [review] Bug 7141 : Changing test to match new formatting of xsl style sheet Created attachment 6644 [details] [review] Bug 7141 : Changing test to match new formatting of xsl style sheet http://bugs.koha-community.org/show_bug.cgi?id=7141 Signed-off-by: Liz Rea <wizzyrea@gmail.com> verified tests pass unless i've missed something, the attached patch has been pushed already Included in the 3.6 branch prior to 3.6.4. |